[FFmpeg-devel] [PATCH 1/2] lavc/riffenc: Fix msrle support on Windows 95

[FFmpeg-devel] [PATCH 1/2] lavc/riffenc: Fix msrle support on Windows 95

[Tomas Härdin]

[-- Attachment #1: Type: text/plain, Size: 66 bytes --]

This is important for the GoldSrc community among others.

/Tomas

[-- Attachment #2: 0001-lavc-riffenc-Fix-msrle-support-on-Windows-95.patch --]
[-- Type: text/x-patch, Size: 1522 bytes --]

From c5c2d535b3e5dc94b063e13359e475e9ce18e1b6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se>
Date: Thu, 8 Jun 2023 11:55:28 +0200
Subject: [PATCH 1/2] lavc/riffenc: Fix msrle support on Windows 95

---
 libavformat/riffenc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libavformat/riffenc.c b/libavformat/riffenc.c
index 179b0f12cb..3325419b94 100644
--- a/libavformat/riffenc.c
+++ b/libavformat/riffenc.c
@@ -239,14 +239,16 @@ void ff_put_bmp_header(AVIOContext *pb, AVCodecParameters *par,
     /* depth */
     avio_wl16(pb, par->bits_per_coded_sample ? par->bits_per_coded_sample : 24);
     /* compression type */
-    avio_wl32(pb, par->codec_tag);
+    // MSRLE compatibility with Media Player 3.1 and Windows 95
+    avio_wl32(pb, par->codec_id == AV_CODEC_ID_MSRLE ? 1 : par->codec_tag);
     avio_wl32(pb, (par->width * par->height * (par->bits_per_coded_sample ? par->bits_per_coded_sample : 24)+7) / 8);
     avio_wl32(pb, 0);
     avio_wl32(pb, 0);
     /* Number of color indices in the color table that are used.
      * A value of 0 means 2^biBitCount indices, but this doesn't work
      * with Windows Media Player and files containing xxpc chunks. */
-    avio_wl32(pb, pal_avi ? 1 << par->bits_per_coded_sample : 0);
+    // MSRLE on Windows 95 requires a zero here
+    avio_wl32(pb, pal_avi && par->codec_id != AV_CODEC_ID_MSRLE ? 1 << par->bits_per_coded_sample : 0);
     avio_wl32(pb, 0);
 
     if (!ignore_extradata) {
-- 
2.30.2


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/2] lavc/msrleenc: Add msrle encoder

[Tomas Härdin]

[-- Attachment #1: Type: text/plain, Size: 125 bytes --]

Please check that I got the palette handling correct. Else this
producing output of similar size to the VfW encoder.

/Tomas

[-- Attachment #2: 0002-lavc-msrleenc-Add-msrle-encoder.patch --]
[-- Type: text/x-patch, Size: 16353 bytes --]

From ab9bb1aca7ddda8f4788b0a63460470fce021e72 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se>
Date: Thu, 8 Jun 2023 11:57:53 +0200
Subject: [PATCH 2/2] lavc/msrleenc: Add msrle encoder

Keyframes are marked automagically
---
 MAINTAINERS                        |   1 +
 doc/encoders.texi                  |  14 ++
 libavcodec/Makefile                |   1 +
 libavcodec/allcodecs.c             |   1 +
 libavcodec/msrleenc.c              | 303 +++++++++++++++++++++++++++++
 tests/fate/vcodec.mak              |   3 +
 tests/ref/vsynth/vsynth1-msrle     |   4 +
 tests/ref/vsynth/vsynth2-msrle     |   4 +
 tests/ref/vsynth/vsynth3-msrle     |   4 +
 tests/ref/vsynth/vsynth_lena-msrle |   4 +
 10 files changed, 339 insertions(+)
 create mode 100644 libavcodec/msrleenc.c
 create mode 100644 tests/ref/vsynth/vsynth1-msrle
 create mode 100644 tests/ref/vsynth/vsynth2-msrle
 create mode 100644 tests/ref/vsynth/vsynth3-msrle
 create mode 100644 tests/ref/vsynth/vsynth_lena-msrle

diff --git a/MAINTAINERS b/MAINTAINERS
index 07852486e4..3584a68442 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -210,6 +210,7 @@ Codecs:
   mqc*                                  Nicolas Bertrand
   msmpeg4.c, msmpeg4data.h              Michael Niedermayer
   msrle.c                               Mike Melanson
+  msrleenc.c                            Tomas Härdin
   msvideo1.c                            Mike Melanson
   nuv.c                                 Reimar Doeffinger
   nvdec*, nvenc*                        Timo Rothenpieler
diff --git a/doc/encoders.texi b/doc/encoders.texi
index 20cb8a1421..25d6b7f09e 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -3061,6 +3061,20 @@ Video encoders can take input in either of nv12 or yuv420p form
 (some encoders support both, some support only either - in practice,
 nv12 is the safer choice, especially among HW encoders).
 
+@section Microsoft RLE
+
+Microsoft RLE aka MSRLE encoder.
+Only 8-bit palette mode supported.
+Compatible with Windows 3.1 and Windows 95.
+
+@subsection Options
+
+@table @option
+@item g @var{integer}
+Keyframe interval.
+A keyframe is inserted at least every @code{-g} frames, sometimes sooner.
+@end table
+
 @section mpeg2
 
 MPEG-2 video encoder.
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 87a8b90037..2c88dd65d5 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -552,6 +552,7 @@ OBJS-$(CONFIG_MSA1_DECODER)            += mss3.o
 OBJS-$(CONFIG_MSCC_DECODER)            += mscc.o
 OBJS-$(CONFIG_MSNSIREN_DECODER)        += siren.o
 OBJS-$(CONFIG_MSP2_DECODER)            += msp2dec.o
+OBJS-$(CONFIG_MSRLE_ENCODER)           += msrleenc.o
 OBJS-$(CONFIG_MSRLE_DECODER)           += msrle.o msrledec.o
 OBJS-$(CONFIG_MSS1_DECODER)            += mss1.o mss12.o
 OBJS-$(CONFIG_MSS2_DECODER)            += mss2.o mss12.o mss2dsp.o wmv2data.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index a98c300da4..5d4889b968 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -228,6 +228,7 @@ extern const FFCodec ff_msmpeg4v3_encoder;
 extern const FFCodec ff_msmpeg4v3_decoder;
 extern const FFCodec ff_msmpeg4_crystalhd_decoder;
 extern const FFCodec ff_msp2_decoder;
+extern const FFCodec ff_msrle_encoder;
 extern const FFCodec ff_msrle_decoder;
 extern const FFCodec ff_mss1_decoder;
 extern const FFCodec ff_mss2_decoder;
diff --git a/libavcodec/msrleenc.c b/libavcodec/msrleenc.c
new file mode 100644
index 0000000000..17d40cbd6a
--- /dev/null
+++ b/libavcodec/msrleenc.c
@@ -0,0 +1,303 @@
+/*
+ * Copyright (c) 2023 Tomas Härdin
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * MSRLE encoder
+ * @see https://wiki.multimedia.cx/index.php?title=Microsoft_RLE
+ */
+
+// TODO: pal4 mode?
+
+#include "bytestream.h"
+#include "codec_internal.h"
+#include "encode.h"
+
+typedef struct MSRLEContext {
+    const AVClass *class;
+    int curframe;
+    AVFrame *last_frame;
+} MSRLEContext;
+
+static av_cold int msrle_encode_init(AVCodecContext *avctx)
+{
+    avctx->bits_per_coded_sample = 8;
+    return 0;
+}
+
+static void write_run(AVCodecContext *avctx, uint8_t **data, int len, int value)
+{
+    // we're allowed to write odd runs
+    while (len >= 255) {
+        bytestream_put_byte(data, 255);
+        bytestream_put_byte(data, value);
+        len -= 255;
+    }
+    if (len >= 1) {
+        // this is wasteful when len == 1 and sometimes when len == 2
+        // but sometimes we have no choice. also write_absolute()
+        // relies on this
+        bytestream_put_byte(data, len);
+        bytestream_put_byte(data, value);
+    }
+}
+
+static void write_absolute(AVCodecContext *avctx, uint8_t **data, uint8_t *line, int len)
+{
+    // writing 255 would be wasteful here due to the padding requirement
+    while (len >= 254) {
+        bytestream_put_byte(data, 0);
+        bytestream_put_byte(data, 254);
+        bytestream_put_buffer(data, line, 254);
+        line += 254;
+        len -= 254;
+    }
+    if (len == 1) {
+        // it's less wasteful to write single pixels as runs
+        // not to mention that absolute mode requires >= 3 pixels
+        write_run(avctx, data, 1, line[0]);
+    } else if (len == 2) {
+        write_run(avctx, data, 1, line[0]);
+        write_run(avctx, data, 1, line[1]);
+    } else if (len > 0) {
+        bytestream_put_byte(data, 0);
+        bytestream_put_byte(data, len);
+        bytestream_put_buffer(data, line, len);
+        if (len & 1)
+            bytestream_put_byte(data, 0);
+    }
+}
+
+static void write_delta(AVCodecContext *avctx, uint8_t **data, int delta)
+{
+    // we let the yskip logic handle the case where we want to delta
+    // to following lines. it's not perfect but it's easier than finding
+    // the optimal combination of end-of-lines and deltas to reach any
+    // following position including places where dx < 0
+    while (delta >= 255) {
+        bytestream_put_byte(data, 0);
+        bytestream_put_byte(data, 2);
+        bytestream_put_byte(data, 255);
+        bytestream_put_byte(data, 0);
+        delta -= 255;
+    }
+    if (delta > 0) {
+        bytestream_put_byte(data, 0);
+        bytestream_put_byte(data, 2);
+        bytestream_put_byte(data, delta);
+        bytestream_put_byte(data, 0);
+    }
+}
+
+static void write_yskip(AVCodecContext *avctx, uint8_t **data, int yskip)
+{
+    if (yskip < 4)
+        return;
+    // we have yskip*2 nul bytess
+    *data -= 2*yskip;
+    // the end-of-line counts as one skip
+    yskip--;
+    while (yskip >= 255) {
+        bytestream_put_byte(data, 0);
+        bytestream_put_byte(data, 2);
+        bytestream_put_byte(data, 0);
+        bytestream_put_byte(data, 255);
+        yskip -= 255;
+    }
+    if (yskip > 0) {
+        bytestream_put_byte(data, 0);
+        bytestream_put_byte(data, 2);
+        bytestream_put_byte(data, 0);
+        bytestream_put_byte(data, yskip);
+    }
+    bytestream_put_be16(data, 0x0000);
+}
+
+// used both to encode lines in keyframes and to encode lines between deltas
+static void encode_line(AVCodecContext *avctx, uint8_t **data, uint8_t *line, int length)
+{
+    int run = 0, last = -1, absstart = 0;
+    if (length == 0)
+        return;
+    for (int x = 0; x < length; x++) {
+        if (last == line[x]) {
+            run++;
+            if (run == 3)
+                write_absolute(avctx, data, &line[absstart], x - absstart - 2);
+        } else {
+            if (run >= 3) {
+                write_run(avctx, data, run, last);
+                absstart = x;
+            }
+            run = 1;
+        }
+        last = line[x];
+    }
+    if (run >= 3)
+        write_run(avctx, data, run, last);
+    else
+        write_absolute(avctx, data, &line[absstart], length - absstart);
+}
+
+static int encode(AVCodecContext *avctx, AVPacket *pkt,
+                  const AVFrame *pict, int keyframe, int *got_keyframe)
+{
+    MSRLEContext *s = avctx->priv_data;
+    uint8_t *data = pkt->data;
+    
+    /*  Compare the current frame to the last frame, or code the entire frame
+        if keyframe != 0. We're continually outputting pairs of bytes:
+
+            00 00           end of line
+            00 01           end of bitmap
+            00 02 dx dy     delta. move pointer to x+dx, y+dy
+            00 ll dd dd ..  absolute (verbatim) mode. ll >= 3
+            rr dd           run. rr >= 1
+
+        For keyframes we only have absolute mode and runs at our disposal, and
+        we are not allowed to end a line early. If this happens when keyframe == 0
+        then *got_keyframe is set to 1 and s->curframe is reset.
+    */
+    *got_keyframe = 1;  // set to zero whenever we use a feature that makes this a not-keyframe
+    
+    if (keyframe) {
+        for (int y = avctx->height-1; y >= 0; y--) {
+            uint8_t *line = &pict->data[0][y*pict->linesize[0]];
+            encode_line(avctx, &data, line, avctx->width);
+            bytestream_put_be16(&data, 0x0000); // end of line
+        }
+    } else {
+        // compare to previous frame
+        int yskip = 0; // we can encode large skips using deltas
+        for (int y = avctx->height-1; y >= 0; y--) {
+            uint8_t *line = &pict->data[0][y*pict->linesize[0]];
+            uint8_t *prev = &s->last_frame->data[0][y*s->last_frame->linesize[0]];
+            // we need at least 5 pixels in a row for a delta to be worthwhile
+            int delta = 0, linestart = 0, encoded = 0;
+            for (int x = 0; x < avctx->width; x++) {
+                if (line[x] == prev[x]) {
+                    delta++;
+                    if (delta == 5) {
+                        int len = x - linestart - 4;
+                        if (len > 0) {
+                            write_yskip(avctx, &data, yskip);
+                            yskip = 0;
+                            encode_line(avctx, &data, &line[linestart], len);
+                            encoded = 1;
+                        }
+                        linestart = -1;
+                    }
+                } else {
+                    if (delta >= 5) {
+                        write_yskip(avctx, &data, yskip);
+                        yskip = 0;
+                        write_delta(avctx, &data, delta);
+                        *got_keyframe = 0;
+                        encoded = 1;
+                    }
+                    delta = 0;
+                    if (linestart == -1)
+                        linestart = x;
+                }
+            }
+            if (delta < 5) {
+                write_yskip(avctx, &data, yskip);
+                yskip = 0;
+                encode_line(avctx, &data, &line[linestart], avctx->width - linestart);
+                encoded  = 1;
+            } else
+                *got_keyframe = 0;
+            bytestream_put_be16(&data, 0x0000); // end of line
+            if (!encoded)
+                yskip++;
+            else
+                yskip = 0;
+        }
+        write_yskip(avctx, &data, yskip);
+    }
+    bytestream_put_be16(&data, 0x0001); // end of bitmap
+    pkt->size = data - pkt->data;
+    return 0;\r}
+
+static int msrle_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
+                              const AVFrame *pict, int *got_packet)
+{
+    MSRLEContext *s = avctx->priv_data;
+    int ret, got_keyframe;
+
+    if ((ret = ff_alloc_packet(avctx, pkt, (
+                avctx->width*2 /* worst case = rle every pixel */ + 2 /*end of line */
+            ) * avctx->height + 2 /* end of bitmap */ + AV_INPUT_BUFFER_MIN_SIZE)))
+        return ret;
+
+    if (pict->data[1]) {
+        uint8_t *side_data = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE);
+        memcpy(side_data, pict->data[1], AVPALETTE_SIZE);
+    }
+
+    if ((ret = encode(avctx, pkt, pict, s->curframe == 0, &got_keyframe)))
+        return ret;
+
+    if (got_keyframe) {
+        pkt->flags |= AV_PKT_FLAG_KEY;
+        s->curframe = 0;
+    }
+    if (++s->curframe >= avctx->gop_size)
+        s->curframe = 0;
+    *got_packet = 1;
+
+    if (!s->last_frame)
+        s->last_frame = av_frame_alloc();
+    else
+        av_frame_unref(s->last_frame);
+
+    av_frame_ref(s->last_frame, pict);
+    return 0;
+}
+
+static int msrle_encode_close(AVCodecContext *avctx)
+{
+    MSRLEContext *s = avctx->priv_data;
+    av_frame_free(&s->last_frame);
+    return 0;
+}
+
+static const AVClass msrle_class = {
+    .class_name = "Microsoft RLE encoder",
+    .item_name  = av_default_item_name,
+    .version    = LIBAVUTIL_VERSION_INT,
+};
+
+const FFCodec ff_msrle_encoder = {
+    .p.name         = "msrle",
+    CODEC_LONG_NAME("Microsoft RLE"),
+    .p.type         = AVMEDIA_TYPE_VIDEO,
+    .p.id           = AV_CODEC_ID_MSRLE,
+    .p.capabilities = AV_CODEC_CAP_DR1,
+    .priv_data_size = sizeof(MSRLEContext),
+    .init           = msrle_encode_init,
+    FF_CODEC_ENCODE_CB(msrle_encode_frame),
+    .close          = msrle_encode_close,
+    .p.pix_fmts     = (const enum AVPixelFormat[]){
+        AV_PIX_FMT_PAL8, AV_PIX_FMT_NONE
+    },
+    .p.priv_class   = &msrle_class,
+    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
+};
diff --git a/tests/fate/vcodec.mak b/tests/fate/vcodec.mak
index fbee264a9d..ef8904613a 100644
--- a/tests/fate/vcodec.mak
+++ b/tests/fate/vcodec.mak
@@ -338,6 +338,9 @@ fate-vsynth%-msmpeg4:            ENCOPTS = -qscale 10
 FATE_VCODEC-$(call ENCDEC, MSMPEG4V2, AVI) += msmpeg4v2
 fate-vsynth%-msmpeg4v2:          ENCOPTS = -qscale 10
 
+FATE_VCODEC_SCALE-$(call ENCDEC, MSRLE, AVI) += msrle
+fate-vsynth%-msrle:              CODEC   = msrle
+
 FATE_VCODEC_SCALE-$(call ENCDEC, PNG, AVI) += mpng
 fate-vsynth%-mpng:               CODEC   = png
 
diff --git a/tests/ref/vsynth/vsynth1-msrle b/tests/ref/vsynth/vsynth1-msrle
new file mode 100644
index 0000000000..6174ff57f8
--- /dev/null
+++ b/tests/ref/vsynth/vsynth1-msrle
@@ -0,0 +1,4 @@
+b19bc15e2c5866f3c7942aad47ce0261 *tests/data/fate/vsynth1-msrle.avi
+5216296 tests/data/fate/vsynth1-msrle.avi
+f142ee03bf9f37bb2e1902fe32366bbf *tests/data/fate/vsynth1-msrle.out.rawvideo
+stddev:    8.69 PSNR: 29.34 MAXDIFF:   64 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth2-msrle b/tests/ref/vsynth/vsynth2-msrle
new file mode 100644
index 0000000000..dd579b6ee9
--- /dev/null
+++ b/tests/ref/vsynth/vsynth2-msrle
@@ -0,0 +1,4 @@
+850744d6d38ab09adb2fbd685d5df740 *tests/data/fate/vsynth2-msrle.avi
+4556642 tests/data/fate/vsynth2-msrle.avi
+df26a524cad8ebf0cf5ba4376c5f115f *tests/data/fate/vsynth2-msrle.out.rawvideo
+stddev:    7.57 PSNR: 30.54 MAXDIFF:   35 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth3-msrle b/tests/ref/vsynth/vsynth3-msrle
new file mode 100644
index 0000000000..9cc92be036
--- /dev/null
+++ b/tests/ref/vsynth/vsynth3-msrle
@@ -0,0 +1,4 @@
+ee8f4d86f117d69919be69fbc976981a *tests/data/fate/vsynth3-msrle.avi
+72866 tests/data/fate/vsynth3-msrle.avi
+fa6042492a3116c1ae9a32b487caa677 *tests/data/fate/vsynth3-msrle.out.rawvideo
+stddev:    8.88 PSNR: 29.16 MAXDIFF:   51 bytes:    86700/    86700
diff --git a/tests/ref/vsynth/vsynth_lena-msrle b/tests/ref/vsynth/vsynth_lena-msrle
new file mode 100644
index 0000000000..67226f05a2
--- /dev/null
+++ b/tests/ref/vsynth/vsynth_lena-msrle
@@ -0,0 +1,4 @@
+9654924690cbaf6348ea798e442e819c *tests/data/fate/vsynth_lena-msrle.avi
+4671320 tests/data/fate/vsynth_lena-msrle.avi
+db453693ceae6f65c173dd716ee2662e *tests/data/fate/vsynth_lena-msrle.out.rawvideo
+stddev:    8.07 PSNR: 29.99 MAXDIFF:   32 bytes:  7603200/  7603200
-- 
2.30.2


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] lavc/riffenc: Fix msrle support on Windows 95

[Paul B Mahol]

But does this break it on other platforms?
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] lavc/riffenc: Fix msrle support on Windows 95

[Tomas Härdin]

tor 2023-06-08 klockan 13:27 +0200 skrev Paul B Mahol:
> But does this break it on other platforms?

That is a good question. What other RIFF decoders are there for which
MSRLE support is important and depends on behavior different from VfW?
mpv and vlc work just fine.

/Tomas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/2] lavc/msrleenc: Add msrle encoder

[Andreas Rheinhardt]

Tomas Härdin:
> +typedef struct MSRLEContext {
> +    const AVClass *class;
> +    int curframe;
> +    AVFrame *last_frame;
> +} MSRLEContext;
> +
> +static av_cold int msrle_encode_init(AVCodecContext *avctx)
> +{
> +    avctx->bits_per_coded_sample = 8;
> +    return 0;
> +}
> +
> +static void write_run(AVCodecContext *avctx, uint8_t **data, int len, int value)
> +{
> +    // we're allowed to write odd runs
> +    while (len >= 255) {
> +        bytestream_put_byte(data, 255);
> +        bytestream_put_byte(data, value);
> +        len -= 255;
> +    }
> +    if (len >= 1) {
> +        // this is wasteful when len == 1 and sometimes when len == 2
> +        // but sometimes we have no choice. also write_absolute()
> +        // relies on this
> +        bytestream_put_byte(data, len);
> +        bytestream_put_byte(data, value);
> +    }
> +}
> +
> +static void write_absolute(AVCodecContext *avctx, uint8_t **data, uint8_t *line, int len)
> +{
> +    // writing 255 would be wasteful here due to the padding requirement
> +    while (len >= 254) {
> +        bytestream_put_byte(data, 0);
> +        bytestream_put_byte(data, 254);
> +        bytestream_put_buffer(data, line, 254);
> +        line += 254;
> +        len -= 254;
> +    }
> +    if (len == 1) {
> +        // it's less wasteful to write single pixels as runs
> +        // not to mention that absolute mode requires >= 3 pixels
> +        write_run(avctx, data, 1, line[0]);
> +    } else if (len == 2) {
> +        write_run(avctx, data, 1, line[0]);
> +        write_run(avctx, data, 1, line[1]);
> +    } else if (len > 0) {
> +        bytestream_put_byte(data, 0);
> +        bytestream_put_byte(data, len);
> +        bytestream_put_buffer(data, line, len);
> +        if (len & 1)
> +            bytestream_put_byte(data, 0);
> +    }
> +}
> +
> +static void write_delta(AVCodecContext *avctx, uint8_t **data, int delta)
> +{
> +    // we let the yskip logic handle the case where we want to delta
> +    // to following lines. it's not perfect but it's easier than finding
> +    // the optimal combination of end-of-lines and deltas to reach any
> +    // following position including places where dx < 0
> +    while (delta >= 255) {
> +        bytestream_put_byte(data, 0);
> +        bytestream_put_byte(data, 2);
> +        bytestream_put_byte(data, 255);
> +        bytestream_put_byte(data, 0);
> +        delta -= 255;
> +    }
> +    if (delta > 0) {
> +        bytestream_put_byte(data, 0);
> +        bytestream_put_byte(data, 2);
> +        bytestream_put_byte(data, delta);
> +        bytestream_put_byte(data, 0);
> +    }
> +}
> +
> +static void write_yskip(AVCodecContext *avctx, uint8_t **data, int yskip)
> +{
> +    if (yskip < 4)
> +        return;
> +    // we have yskip*2 nul bytess
> +    *data -= 2*yskip;
> +    // the end-of-line counts as one skip
> +    yskip--;
> +    while (yskip >= 255) {
> +        bytestream_put_byte(data, 0);
> +        bytestream_put_byte(data, 2);
> +        bytestream_put_byte(data, 0);
> +        bytestream_put_byte(data, 255);
> +        yskip -= 255;
> +    }
> +    if (yskip > 0) {
> +        bytestream_put_byte(data, 0);
> +        bytestream_put_byte(data, 2);
> +        bytestream_put_byte(data, 0);
> +        bytestream_put_byte(data, yskip);
> +    }
> +    bytestream_put_be16(data, 0x0000);
> +}
> +
> +// used both to encode lines in keyframes and to encode lines between deltas
> +static void encode_line(AVCodecContext *avctx, uint8_t **data, uint8_t *line, int length)
> +{
> +    int run = 0, last = -1, absstart = 0;
> +    if (length == 0)
> +        return;
> +    for (int x = 0; x < length; x++) {
> +        if (last == line[x]) {
> +            run++;
> +            if (run == 3)
> +                write_absolute(avctx, data, &line[absstart], x - absstart - 2);
> +        } else {
> +            if (run >= 3) {
> +                write_run(avctx, data, run, last);
> +                absstart = x;
> +            }
> +            run = 1;
> +        }
> +        last = line[x];
> +    }
> +    if (run >= 3)
> +        write_run(avctx, data, run, last);
> +    else
> +        write_absolute(avctx, data, &line[absstart], length - absstart);
> +}
> +
> +static int encode(AVCodecContext *avctx, AVPacket *pkt,
> +                  const AVFrame *pict, int keyframe, int *got_keyframe)
> +{
> +    MSRLEContext *s = avctx->priv_data;
> +    uint8_t *data = pkt->data;
> +    
> +    /*  Compare the current frame to the last frame, or code the entire frame
> +        if keyframe != 0. We're continually outputting pairs of bytes:
> +
> +            00 00           end of line
> +            00 01           end of bitmap
> +            00 02 dx dy     delta. move pointer to x+dx, y+dy
> +            00 ll dd dd ..  absolute (verbatim) mode. ll >= 3
> +            rr dd           run. rr >= 1
> +
> +        For keyframes we only have absolute mode and runs at our disposal, and
> +        we are not allowed to end a line early. If this happens when keyframe == 0
> +        then *got_keyframe is set to 1 and s->curframe is reset.
> +    */
> +    *got_keyframe = 1;  // set to zero whenever we use a feature that makes this a not-keyframe
> +    
> +    if (keyframe) {
> +        for (int y = avctx->height-1; y >= 0; y--) {
> +            uint8_t *line = &pict->data[0][y*pict->linesize[0]];
> +            encode_line(avctx, &data, line, avctx->width);
> +            bytestream_put_be16(&data, 0x0000); // end of line
> +        }
> +    } else {
> +        // compare to previous frame
> +        int yskip = 0; // we can encode large skips using deltas
> +        for (int y = avctx->height-1; y >= 0; y--) {
> +            uint8_t *line = &pict->data[0][y*pict->linesize[0]];
> +            uint8_t *prev = &s->last_frame->data[0][y*s->last_frame->linesize[0]];

Should be const.

> +            // we need at least 5 pixels in a row for a delta to be worthwhile
> +            int delta = 0, linestart = 0, encoded = 0;
> +            for (int x = 0; x < avctx->width; x++) {
> +                if (line[x] == prev[x]) {
> +                    delta++;
> +                    if (delta == 5) {
> +                        int len = x - linestart - 4;
> +                        if (len > 0) {
> +                            write_yskip(avctx, &data, yskip);
> +                            yskip = 0;
> +                            encode_line(avctx, &data, &line[linestart], len);
> +                            encoded = 1;
> +                        }
> +                        linestart = -1;
> +                    }
> +                } else {
> +                    if (delta >= 5) {
> +                        write_yskip(avctx, &data, yskip);
> +                        yskip = 0;
> +                        write_delta(avctx, &data, delta);
> +                        *got_keyframe = 0;
> +                        encoded = 1;
> +                    }
> +                    delta = 0;
> +                    if (linestart == -1)
> +                        linestart = x;
> +                }
> +            }
> +            if (delta < 5) {
> +                write_yskip(avctx, &data, yskip);
> +                yskip = 0;
> +                encode_line(avctx, &data, &line[linestart], avctx->width - linestart);
> +                encoded  = 1;
> +            } else
> +                *got_keyframe = 0;
> +            bytestream_put_be16(&data, 0x0000); // end of line
> +            if (!encoded)
> +                yskip++;
> +            else
> +                yskip = 0;
> +        }
> +        write_yskip(avctx, &data, yskip);
> +    }
> +    bytestream_put_be16(&data, 0x0001); // end of bitmap
> +    pkt->size = data - pkt->data;
> +    return 0;
> }
> +
> +static int msrle_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
> +                              const AVFrame *pict, int *got_packet)
> +{
> +    MSRLEContext *s = avctx->priv_data;
> +    int ret, got_keyframe;
> +
> +    if ((ret = ff_alloc_packet(avctx, pkt, (
> +                avctx->width*2 /* worst case = rle every pixel */ + 2 /*end of line */
> +            ) * avctx->height + 2 /* end of bitmap */ + AV_INPUT_BUFFER_MIN_SIZE)))
> +        return ret;
> +
> +    if (pict->data[1]) {
> +        uint8_t *side_data = av_packet_new_side_data(pkt, AV_PKT_DATA_PALETTE, AVPALETTE_SIZE);
> +        memcpy(side_data, pict->data[1], AVPALETTE_SIZE);
> +    }
> +
> +    if ((ret = encode(avctx, pkt, pict, s->curframe == 0, &got_keyframe)))
> +        return ret;
> +
> +    if (got_keyframe) {
> +        pkt->flags |= AV_PKT_FLAG_KEY;
> +        s->curframe = 0;
> +    }
> +    if (++s->curframe >= avctx->gop_size)
> +        s->curframe = 0;
> +    *got_packet = 1;
> +
> +    if (!s->last_frame)
> +        s->last_frame = av_frame_alloc();
> +    else
> +        av_frame_unref(s->last_frame);
> +
> +    av_frame_ref(s->last_frame, pict);

Wouldn't it be simpler to allocate this frame during init and then use
av_frame_replace() here?
Apart from that: You need to check the av_frame_ref().

> +    return 0;
> +}
> +
> +static int msrle_encode_close(AVCodecContext *avctx)
> +{
> +    MSRLEContext *s = avctx->priv_data;
> +    av_frame_free(&s->last_frame);
> +    return 0;
> +}
> +
> +static const AVClass msrle_class = {
> +    .class_name = "Microsoft RLE encoder",
> +    .item_name  = av_default_item_name,
> +    .version    = LIBAVUTIL_VERSION_INT,
> +};

An AVClass is pointless without options, so you should just remove it
(and the AVClass* from the context).

> +
> +const FFCodec ff_msrle_encoder = {
> +    .p.name         = "msrle",
> +    CODEC_LONG_NAME("Microsoft RLE"),
> +    .p.type         = AVMEDIA_TYPE_VIDEO,
> +    .p.id           = AV_CODEC_ID_MSRLE,
> +    .p.capabilities = AV_CODEC_CAP_DR1,
> +    .priv_data_size = sizeof(MSRLEContext),
> +    .init           = msrle_encode_init,
> +    FF_CODEC_ENCODE_CB(msrle_encode_frame),
> +    .close          = msrle_encode_close,
> +    .p.pix_fmts     = (const enum AVPixelFormat[]){
> +        AV_PIX_FMT_PAL8, AV_PIX_FMT_NONE
> +    },
> +    .p.priv_class   = &msrle_class,
> +    .caps_internal  = FF_CODEC_CAP_INIT_CLEANUP,
> +};

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/3] Add Changelog entry

[Tomas Härdin]

[-- Attachment #1: Type: text/plain, Size: 41 bytes --]

Bumping with a forgotten Changelog entry

[-- Attachment #2: 0003-Add-Changelog-entry.patch --]
[-- Type: text/x-patch, Size: 561 bytes --]

From 33045f32da50390e6d58c34e95b3d344a53b6968 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se>
Date: Fri, 16 Jun 2023 15:19:24 +0200
Subject: [PATCH 3/3] Add Changelog entry

---
 Changelog | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Changelog b/Changelog
index d51e03b8eb..2d6b63bfa2 100644
--- a/Changelog
+++ b/Changelog
@@ -16,6 +16,7 @@ version <next>:
 - nlmeans_vulkan filter
 - RivaTuner video decoder
 - xfade_vulkan filter
+- Microsoft RLE video encoder
 
 version 6.0:
 - Radiance HDR image support
-- 
2.30.2


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] lavc/riffenc: Fix msrle support on Windows 95

[Tomas Härdin]

Will push later today

/Tomas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] lavc/riffenc: Fix msrle support on Windows 95

[Leo Izen]

On 6/8/23 07:44, Tomas Härdin wrote:
> tor 2023-06-08 klockan 13:27 +0200 skrev Paul B Mahol:
>> But does this break it on other platforms?
> 
> That is a good question. What other RIFF decoders are there for which
> MSRLE support is important and depends on behavior different from VfW?
> mpv and vlc work just fine.
> 
> /Tomas
> 

mpv's RIFF decoder is really avformat, so this doesn't say much. VLC has 
its own though, iirc.

- Leo Izen

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] lavc/riffenc: Fix msrle support on Windows 95

[Tomas Härdin]

mån 2023-06-19 klockan 11:42 -0400 skrev Leo Izen:
> On 6/8/23 07:44, Tomas Härdin wrote:
> > tor 2023-06-08 klockan 13:27 +0200 skrev Paul B Mahol:
> > > But does this break it on other platforms?
> > 
> > That is a good question. What other RIFF decoders are there for
> > which
> > MSRLE support is important and depends on behavior different from
> > VfW?
> > mpv and vlc work just fine.
> > 
> > /Tomas
> > 
> 
> mpv's RIFF decoder is really avformat, so this doesn't say much. VLC
> has 
> its own though, iirc.

Yeah. Also we'd be hard pressed to find an msrle decoder that isn't
either ffmpeg, vlc or VfW

It might be necessary to do this same thing for more codecs. We only
tested msrle which was painful enough. I wouldn't be surprised if say
msvideo1 requires similar hacks. This patch is conservative until we
learn why VfW needs this.

/Tomas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/3] Add Changelog entry

[Tomas Härdin]

Squashed with patch 2/2, trailing whitespaces fixed and pushed

/Tomas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".