Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] WIP: avcodec/sanm: updates (PR #20350)
@ 2025-08-27  8:27 Manuel Lauss via ffmpeg-devel
  0 siblings, 0 replies; only message in thread
From: Manuel Lauss via ffmpeg-devel @ 2025-08-27  8:27 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Manuel Lauss

PR #20350 opened by Manuel Lauss (mlauss2)
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20350
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20350.patch

Next try.

I've tried to improve the codec1/4/21/23/31 implementations to try as best as possible to not write outside the defined buffer and deal with garbage input as much as possible,
 and avoid wrapping the image around when it should scroll outside the visible area.

Some other low-hanging fruit is stacked on top:
- fix condec48 playback as far as possible after the mv check was introduced.  some legitimate videos have half-block sizes (esp. Mysteries of the Sith) and they are encoded with vectors that reach up to 1 blockrow outside their height.  Some tiny artifacts remain, i'm trying to fix this later.
- codec47 has a 4-byte codebook in the header, read it into context instead of redirecting the stream pointer every time it needs to be read.
- add header size plausibility checks to the block codecs37/47/48.
- other non-functional changes.
- revert a check that makes sure the size+dimension of a frame object is inside the defined buffer extents. This is used by Rebel Assault 1 ANIM videos to scroll "sprites" in and out of the visible area.  the improvements to codec1..31 however now (hopefully) catch this.

Feedback very much appreciated!


>From 6d1fc89ac380f64110438e93ed947679eafca5aa Mon Sep 17 00:00:00 2001
From: Manuel Lauss <manuel.lauss@gmail.com>
Date: Mon, 25 Aug 2025 11:13:24 +0200
Subject: [PATCH 01/11] avcodec/sanm: codec23 improvements

- don't draw outside the buffers
- don't wrap around when coordinates go over the edge

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
---
 libavcodec/sanm.c | 52 +++++++++++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
index 7c41d622eb..d810ff7e6e 100644
--- a/libavcodec/sanm.c
+++ b/libavcodec/sanm.c
@@ -766,10 +766,9 @@ static int rle_decode(SANMVideoContext *ctx, GetByteContext *gb, uint8_t *dst, c
 static int old_codec23(SANMVideoContext *ctx, GetByteContext *gb, int top, int left,
                        int width, int height, uint8_t param, uint16_t param2)
 {
-    const uint32_t maxpxo = ctx->height * ctx->pitch;
-    uint8_t *dst, lut[256], c;
-    int i, j, k, pc, sk;
-    int32_t pxoff;
+    const uint16_t mx = ctx->width, my = ctx->height, p = ctx->pitch;
+    uint8_t c, lut[256], *dst = (uint8_t *)ctx->fbuf;
+    int sk, i, j, ls, pc, y;
 
     if (ctx->subversion < 2) {
         /* Rebel Assault 1: constant offset + 0xd0 */
@@ -788,30 +787,39 @@ static int old_codec23(SANMVideoContext *ctx, GetByteContext *gb, int top, int l
     if (bytestream2_get_bytes_left(gb) < 1)
         return 0;  /* some c23 frames just set up the LUT */
 
-    dst = (uint8_t *)ctx->fbuf;
-    for (i = 0; i < height; i++) {
-        if (bytestream2_get_bytes_left(gb) < 2)
-            return 0;
-        pxoff = left + ((top + i) * ctx->pitch);
-        k = bytestream2_get_le16u(gb);
-        sk = 1;
-        pc = 0;
-        while (k > 0 && pc <= width) {
-            if (bytestream2_get_bytes_left(gb) < 1)
+    if (((top + height) < 0) || (top >= my) || (left + width < 0) || (left >= mx))
+        return 0;
+
+    if (top < 0) {
+        y = -top;
+        while (y-- && bytestream2_get_bytes_left(gb) > 1) {
+            ls = bytestream2_get_le16u(gb);
+            if (bytestream2_get_bytes_left(gb) < ls)
                 return AVERROR_INVALIDDATA;
+            bytestream2_skip(gb, ls);
+        }
+        height += top;
+        top = 0;
+    }
+
+    y = top;
+    for (; (bytestream2_get_bytes_left(gb) > 1) && (height > 0) && (y < my); height--, y++) {
+        ls = bytestream2_get_le16u(gb);
+        sk = 1;
+        pc = left;
+        while ((bytestream2_get_bytes_left(gb) > 0) && (ls > 0) && (pc <= (width + left))) {
             j = bytestream2_get_byteu(gb);
-            if (sk) {
-                pxoff += j;
-                pc += j;
-            } else {
+            ls--;
+            if (!sk) {
                 while (j--) {
-                    if (pxoff >=0 && pxoff < maxpxo) {
-                        c = *(dst + pxoff);
-                        *(dst + pxoff) = lut[c];
+                    if ((pc >= 0) && (pc < mx)) {
+                        c = *(dst + (y * p) + pc);
+                        *(dst + (y * p) + pc) = lut[c];
                     }
-                    pxoff++;
                     pc++;
                 }
+            } else {
+                    pc += j;
             }
             sk ^= 1;
         }
-- 
2.49.1


>From febd43c099852fc2e766e0cb978b916653f49b07 Mon Sep 17 00:00:00 2001
From: Manuel Lauss <manuel.lauss@gmail.com>
Date: Mon, 25 Aug 2025 13:33:57 +0200
Subject: [PATCH 02/11] avcodec/sanm: codec21 improvements

- don't draw outside the buffers
- don't wrap around when coordinates go over the edge

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
---
 libavcodec/sanm.c | 46 +++++++++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
index d810ff7e6e..7f6cfc0c42 100644
--- a/libavcodec/sanm.c
+++ b/libavcodec/sanm.c
@@ -830,39 +830,39 @@ static int old_codec23(SANMVideoContext *ctx, GetByteContext *gb, int top, int l
 static int old_codec21(SANMVideoContext *ctx, GetByteContext *gb, int top, int left,
                        int width, int height)
 {
-    const uint32_t maxpxo = ctx->height * ctx->pitch;
+    const uint16_t mx = ctx->width, my = ctx->height, p = ctx->pitch;
     uint8_t *dst = (uint8_t *)ctx->fbuf, c;
-    int i, j, k, pc, sk, pxoff;
+    int j, y, pc, sk, ls;
 
-    dst = (uint8_t *)ctx->fbuf;
-    for (i = 0; i < height; i++) {
-        if (bytestream2_get_bytes_left(gb) < 2)
-            return 0;
-        pxoff = left + ((top + i) * ctx->pitch);
-        k = bytestream2_get_le16u(gb);
+    if (((top + height) < 0) || (top >= my) || (left + width < 0) || (left >= mx))
+        return 0;
+
+    y = top;
+    for (; (bytestream2_get_bytes_left(gb) > 2) && (height > 0) && (y < my); height--, y++) {
+        ls = bytestream2_get_le16u(gb);
+        if (y < 0) {
+            if (ls >= bytestream2_get_bytes_left(gb))
+                return 0;
+            bytestream2_skip(gb, ls);
+            continue;
+        }
         sk = 1;
-        pc = 0;
-        while (k > 0 && pc <= width) {
-            if (bytestream2_get_bytes_left(gb) < 2)
-                return AVERROR_INVALIDDATA;
+        pc = left;
+        while ((bytestream2_get_bytes_left(gb) > 1) && (ls > 1) && (pc <= (width + left))) {
             j = bytestream2_get_le16u(gb);
-            k -= 2;
+            ls -= 2;
             if (sk) {
-                pxoff += j;
                 pc += j;
             } else {
-                if (bytestream2_get_bytes_left(gb) < (j + 1))
-                    return AVERROR_INVALIDDATA;
-                do {
+                while ((bytestream2_get_bytes_left(gb) > 0) && (ls > 0) && (j >= 0)) {
                     c = bytestream2_get_byteu(gb);
-                    if (pxoff >=0 && pxoff < maxpxo) {
-                        *(dst + pxoff) = c;
+                    if ((pc >= 0) && (pc < mx)) {
+                        *(dst + (y * p) + pc) = c;
                     }
-                    pxoff++;
-                    pc++;
+                    ls--;
                     j--;
-                    k--;
-                } while (j > -1);
+                    pc++;
+                }
             }
             sk ^= 1;
         }
-- 
2.49.1


>From bd58a50d4f739649967206e3ed059fdfb98938e1 Mon Sep 17 00:00:00 2001
From: Manuel Lauss <manuel.lauss@gmail.com>
Date: Tue, 26 Aug 2025 17:51:46 +0200
Subject: [PATCH 03/11] avcodec/sanm: codec1 improvements

- don't draw outside the buffers
- don't wrap around when coordinates go over the edge
  this is especially noticeable in the e.g. O1OPEN.ANM, C1C3PO.ANM
  RA1 files with planets wrapping around.

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
---
 libavcodec/sanm.c | 65 +++++++++++++++++++++++++++++------------------
 1 file changed, 40 insertions(+), 25 deletions(-)

diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
index 7f6cfc0c42..2d8fe8f9b8 100644
--- a/libavcodec/sanm.c
+++ b/libavcodec/sanm.c
@@ -873,45 +873,60 @@ static int old_codec21(SANMVideoContext *ctx, GetByteContext *gb, int top, int l
 static int old_codec1(SANMVideoContext *ctx, GetByteContext *gb, int top,
                       int left, int width, int height, int opaque)
 {
-    int i, j, len, flag, code, val, end, pxoff;
-    const int maxpxo = ctx->height * ctx->pitch;
-    uint8_t *dst = (uint8_t *)ctx->fbuf;
+    const uint16_t mx = ctx->width, my = ctx->height, p = ctx->pitch;
+    uint8_t *dst = (uint8_t *)ctx->fbuf, code, c;
+    int j, x, y, flag, dlen;
 
-    for (i = 0; i < height; i++) {
-        if (bytestream2_get_bytes_left(gb) < 2)
-            return AVERROR_INVALIDDATA;
+    if (((top + height) < 0) || (top >= my) || (left + width < 0) || (left >= mx))
+            return 0;
 
-        len = bytestream2_get_le16u(gb);
-        end = bytestream2_tell(gb) + len;
-
-        pxoff = left + ((top + i) * ctx->pitch);
-        while (bytestream2_tell(gb) < end) {
-            if (bytestream2_get_bytes_left(gb) < 2)
+    if (top < 0) {
+        y = -top;
+        while (y-- && bytestream2_get_bytes_left(gb) > 1) {
+            dlen = bytestream2_get_le16u(gb);
+            if (bytestream2_get_bytes_left(gb) <= dlen)
                 return AVERROR_INVALIDDATA;
+            bytestream2_skip(gb, dlen);
+        }
+        height += top;
+        top = 0;
+    }
 
+    y = top;
+    for (; (bytestream2_get_bytes_left(gb) > 1) && (height > 0) && (y < my); height--, y++) {
+        dlen = bytestream2_get_le16u(gb);
+        x = left;
+        while (bytestream2_get_bytes_left(gb) > 1 && dlen) {
             code = bytestream2_get_byteu(gb);
+            dlen--;
             flag = code & 1;
             code = (code >> 1) + 1;
             if (flag) {
-                val = bytestream2_get_byteu(gb);
-                if (val || opaque) {
-                    for (j = 0; j < code; j++) {
-                        if (pxoff >= 0 && pxoff < maxpxo)
-                            *(dst + pxoff) = val;
-                        pxoff++;
-                    }
-                } else {
-                    pxoff += code;
+                c = bytestream2_get_byteu(gb);
+                dlen--;
+                if (x < 0) {
+                    int dff = FFMIN(-x, code);
+                    code -= dff;
+                    x += dff;
                 }
+                if (x + code >= mx)
+                    code = mx - x;
+                if (code < 1)
+                    continue;
+                for (j = 0; (j < code) && (c || opaque); j++) {
+                    *(dst + (y * p) + x + j) = c;
+                }
+                x += code;
             } else {
                 if (bytestream2_get_bytes_left(gb) < code)
                     return AVERROR_INVALIDDATA;
                 for (j = 0; j < code; j++) {
-                    val = bytestream2_get_byteu(gb);
-                    if ((pxoff >= 0) && (pxoff < maxpxo) && (val || opaque))
-                        *(dst + pxoff) = val;
-                    pxoff++;
+                    c = bytestream2_get_byteu(gb);
+                    if ((x >= 0) && (x < mx) && (c || opaque))
+                        *(dst + (y * p) + x) = c;
+                    x++;
                 }
+                dlen -= code;
             }
         }
     }
-- 
2.49.1


>From 57f287f8525836dc2db30b1ab3f926d19d548afd Mon Sep 17 00:00:00 2001
From: Manuel Lauss <manuel.lauss@gmail.com>
Date: Tue, 26 Aug 2025 20:25:53 +0200
Subject: [PATCH 04/11] avcodec/sanm: codec31 improvements

- don't draw outside the buffers
- don't wrap around when coordinates go over the edge
  this is especially noticeable in the e.g. O1OPEN.ANM, C1C3PO.ANM
  RA1 files with planets wrapping around.

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
---
 libavcodec/sanm.c | 69 ++++++++++++++++++++++++++---------------------
 1 file changed, 38 insertions(+), 31 deletions(-)

diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
index 2d8fe8f9b8..61797bf4de 100644
--- a/libavcodec/sanm.c
+++ b/libavcodec/sanm.c
@@ -938,51 +938,58 @@ static int old_codec1(SANMVideoContext *ctx, GetByteContext *gb, int top,
 static int old_codec31(SANMVideoContext *ctx, GetByteContext *gb, int top,
                        int left, int width, int height, int p1, int opaque)
 {
-    int i, j, len, flag, code, val, end, pxoff;
-    const int maxpxo = ctx->height * ctx->pitch;
-    uint8_t *dst = (uint8_t *)ctx->fbuf;
+    const uint16_t mx = ctx->width, my = ctx->height, p = ctx->pitch;
+    uint8_t *dst = (uint8_t *)ctx->fbuf, c;
+    int j, x, y, flag, dlen, code;
 
-    for (i = 0; i < height; i++) {
-        if (bytestream2_get_bytes_left(gb) < 2)
-            return AVERROR_INVALIDDATA;
+    if (((top + height) < 0) || (top >= my) || (left + width < 0) || (left >= mx))
+            return 0;
 
-        len = bytestream2_get_le16u(gb);
-        end = bytestream2_tell(gb) + len;
-
-        pxoff = left + ((top + i) * ctx->pitch);
-        while (bytestream2_tell(gb) < end) {
-            if (bytestream2_get_bytes_left(gb) < 2)
+    if (top < 0) {
+        y = -top;
+        while (y-- && bytestream2_get_bytes_left(gb) > 1) {
+            dlen = bytestream2_get_le16u(gb);
+            if (bytestream2_get_bytes_left(gb) <= dlen)
                 return AVERROR_INVALIDDATA;
+            bytestream2_skip(gb, dlen);
+        }
+        height += top;
+        top = 0;
+    }
 
+    y = top;
+    for (; (bytestream2_get_bytes_left(gb) > 1) && (height > 0) && (y < my); height--, y++) {
+        dlen = bytestream2_get_le16u(gb);
+        x = left;
+        while (bytestream2_get_bytes_left(gb) > 1 && dlen) {
             code = bytestream2_get_byteu(gb);
+            dlen--;
             flag = code & 1;
             code = (code >> 1) + 1;
             if (flag) {
-                val = bytestream2_get_byteu(gb);
-                for (j = 0; j < code; j++) {
-                    if ((0 != (val & 0xf)) || opaque) {
-                        if (pxoff >= 0 && pxoff < maxpxo)
-                            *(dst + pxoff) = p1 + (val & 0xf);
-                    }
-                    pxoff++;
-                    if ((0 != (val >> 4)) || opaque) {
-                        if (pxoff >= 0 && pxoff < maxpxo)
-                            *(dst + pxoff) = p1 + (val >> 4);
-                    }
-                    pxoff++;
+                c = bytestream2_get_byteu(gb);
+                dlen--;
+                for (j = 0; (j < code); j++) {
+                    if ((opaque || (c & 0xf)) && (x >= 0) && (x < mx))
+                        *(dst + (y * p) + x) = p1 + (c & 0xf);
+                    x++;
+                    if ((opaque || (c >> 4)) && (x >= 0) && (x < mx))
+                        *(dst + (y * p) + x) = p1 + (c >> 4);
+                    x++;
                 }
             } else {
                 if (bytestream2_get_bytes_left(gb) < code)
                     return AVERROR_INVALIDDATA;
                 for (j = 0; j < code; j++) {
-                    val = bytestream2_get_byteu(gb);
-                    if ((pxoff >= 0) && (pxoff < maxpxo) && ((0 != (val & 0xf)) || opaque))
-                        *(dst + pxoff) = p1 + (val & 0xf);
-                    pxoff++;
-                    if ((pxoff >= 0) && (pxoff < maxpxo) && ((0 != (val >> 4)) || opaque))
-                        *(dst + pxoff) = p1 + (val >> 4);
-                    pxoff++;
+                    c = bytestream2_get_byteu(gb);
+                    if ((opaque || (c & 0xf)) && (x >= 0) && (x < mx))
+                        *(dst + (y * p) + x) = p1 + (c & 0xf);
+                    x++;
+                    if ((opaque || (c >> 4)) && (x >= 0) && (x < mx))
+                        *(dst + (y * p) + x) = p1 + (c >> 4);
+                    x++;
                 }
+                dlen -= code;
             }
         }
     }
-- 
2.49.1


>From 84e44dbbda41a2a53ecd05905288022cf51d95d5 Mon Sep 17 00:00:00 2001
From: Manuel Lauss <manuel.lauss@gmail.com>
Date: Tue, 26 Aug 2025 20:39:20 +0200
Subject: [PATCH 05/11] avcodec/sanm: codec4 improvements

- don't draw outside the buffers
- don't wrap around when coordinates go over the edge

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
---
 libavcodec/sanm.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
index 61797bf4de..b559466a09 100644
--- a/libavcodec/sanm.c
+++ b/libavcodec/sanm.c
@@ -674,11 +674,9 @@ static av_cold int decode_end(AVCodecContext *avctx)
 static int old_codec4(SANMVideoContext *ctx, GetByteContext *gb, int top, int left,
                       int w, int h, uint8_t param, uint16_t param2, int codec)
 {
-    const uint16_t p = ctx->pitch;
-    const uint32_t maxpxo = ctx->height * p;
+    const uint16_t mx = ctx->width, my = ctx->height, p = ctx->pitch;
     uint8_t mask, bits, idx, *gs, *dst = (uint8_t *)ctx->fbuf;
-    int i, j, k, l, bit, ret;
-    int32_t pxoff, pxo2;
+    int i, j, k, l, bit, ret, x, y;
 
     if (ctx->c4param != param) {
         if (codec > 32)
@@ -698,8 +696,9 @@ static int old_codec4(SANMVideoContext *ctx, GetByteContext *gb, int top, int le
 
     for (j = 0; j < w; j += 4) {
         mask = bits = 0;
+        x = left + j;
         for (i = 0; i < h; i += 4) {
-            pxoff = j + left + ((top + i) * p);
+            y = top + i;
             if (param2 > 0) {
                 if (bits == 0) {
                     if (bytestream2_get_bytes_left(gb) < 1)
@@ -719,18 +718,15 @@ static int old_codec4(SANMVideoContext *ctx, GetByteContext *gb, int top, int le
             idx = bytestream2_get_byteu(gb);
             if ((bit == 0) && (idx == 0x80) && (codec != 5))
                 continue;
-
+            if ((y >= my) || ((y + 4) < 0) || ((x + 4) < 0) || (x >= mx))
+                continue;
             gs = &(ctx->c4tbl[bit][idx][0]);
-            pxo2 = pxoff;
             for (k = 0; k < 4; k++) {
-                for (l = 0; l < 4; l++) {
-                    if (pxo2 >= 0 && pxo2 < maxpxo) {
-                        *(dst + pxo2) = *gs;
-                    }
-                    gs++;
-                    pxo2++;
+                for (l = 0; l < 4; l++, gs++) {
+                    const int yo = y + k, xo = x + l;
+                    if ((yo >= 0) && (yo < my) && (xo >= 0) && (xo < mx))
+                        *(dst + yo * p + xo) = *gs;
                 }
-                pxo2 = pxo2 - 4 + p;
             }
         }
     }
-- 
2.49.1


>From 484426cdb05ea7358b12856e02a14665c4a8c623 Mon Sep 17 00:00:00 2001
From: Manuel Lauss <manuel.lauss@gmail.com>
Date: Tue, 26 Aug 2025 21:50:56 +0200
Subject: [PATCH 06/11] avcodec/sanm: partially fix codec48

The mv check introduced with d5bdb0b705ce broke MotS videos:
- their height (300 lines) is 37,5 blocks; unfortunately the videos try to
  access up to 1 block more.
  Extend the mv check to the aligned_height, which fixes most artifacts.
- don't return an error when an mv is invalid; rather skip the (subblock).
  Gets rid of almost all artifacts.

Some artifacts still remain, esp in space scenes where the original
encoder apparently fetched black pixels from outside of the aligned
height.  An increase of the buffer size by 8 lines will fix that later.

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
---
 libavcodec/sanm.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
index b559466a09..7c4211ac53 100644
--- a/libavcodec/sanm.c
+++ b/libavcodec/sanm.c
@@ -1456,7 +1456,7 @@ static void c48_4to8(uint8_t *dst, const uint8_t *src, const uint16_t w)
     }
 }
 
-static int check_mv(int x, int y, const uint16_t w, int h, int blocksize, int mvofs) {
+static int c48_invalid_mv(int x, int y, const uint16_t w, int h, int blocksize, int mvofs) {
     if (mvofs < -x + -y*w)
         return AVERROR_INVALIDDATA;
 
@@ -1492,8 +1492,8 @@ static int codec48_block(SANMVideoContext *ctx, uint8_t *dst, uint8_t *db, int x
         if (bytestream2_get_bytes_left(&ctx->gb) < 2)
             return 1;
         mvofs =  bytestream2_get_le16(&ctx->gb);
-        if (check_mv(x, y, w, h, 8, mvofs))
-            return 1;
+        if (c48_invalid_mv(x, y, w, ctx->aligned_height, 8, mvofs))
+            break;
         for (i = 0; i < 8; i++) {
             ofs = w * i;
             for (k = 0; k < 8; k++)
@@ -1521,8 +1521,8 @@ static int codec48_block(SANMVideoContext *ctx, uint8_t *dst, uint8_t *db, int x
             for (k = 0; k < 8; k += 4) {
                 opc =  bytestream2_get_byteu(&ctx->gb);
                 mvofs = c37_mv[opc * 2] + (c37_mv[opc * 2 + 1] * w);
-                if (check_mv(x+k, y+i, w, h, 4, mvofs))
-                    return 1;
+                if (c48_invalid_mv(x+k, y+i, w, ctx->aligned_height, 4, mvofs))
+                    continue;
                 for (j = 0; j < 4; j++) {
                     ofs = (w * (j + i)) + k;
                     for (l = 0; l < 4; l++)
@@ -1537,8 +1537,8 @@ static int codec48_block(SANMVideoContext *ctx, uint8_t *dst, uint8_t *db, int x
         for (i = 0; i < 8; i += 4) {
             for (k = 0; k < 8; k += 4) {
                 mvofs = bytestream2_get_le16(&ctx->gb);
-                if (check_mv(x+k, y+i, w, h, 4, mvofs))
-                    return 1;
+                if (c48_invalid_mv(x+k, y+i, w, ctx->aligned_height, 4, mvofs))
+                    continue;
                 for (j = 0; j < 4; j++) {
                     ofs = (w * (j + i)) + k;
                     for (l = 0; l < 4; l++)
@@ -1561,8 +1561,8 @@ static int codec48_block(SANMVideoContext *ctx, uint8_t *dst, uint8_t *db, int x
                 ofs = (w * i) + j;
                 opc = bytestream2_get_byteu(&ctx->gb);
                 mvofs = c37_mv[opc * 2] + (c37_mv[opc * 2 + 1] * w);
-                if (check_mv(x+j, y+i, w, h, 2, mvofs))
-                    return 1;
+                if (c48_invalid_mv(x+j, y+i, w, ctx->aligned_height, 2, mvofs))
+                    continue;
                 for (l = 0; l < 2; l++) {
                     *(dst + ofs + l + 0) = *(db + ofs + l + 0 + mvofs);
                     *(dst + ofs + l + w) = *(db + ofs + l + w + mvofs);
@@ -1577,8 +1577,8 @@ static int codec48_block(SANMVideoContext *ctx, uint8_t *dst, uint8_t *db, int x
             for (j = 0; j < 8; j += 2) {
                 ofs = w * i + j;
                 mvofs = bytestream2_get_le16(&ctx->gb);
-                if (check_mv(x+j, y+i, w, h, 2, mvofs))
-                    return 1;
+                if (c48_invalid_mv(x+j, y+i, w, ctx->aligned_height, 2, mvofs))
+                    continue;
                 for (l = 0; l < 2; l++) {
                     *(dst + ofs + l + 0) = *(db + ofs + l + 0 + mvofs);
                     *(dst + ofs + l + w) = *(db + ofs + l + w + mvofs);
@@ -1597,8 +1597,8 @@ static int codec48_block(SANMVideoContext *ctx, uint8_t *dst, uint8_t *db, int x
         break;
     default:    // copy 8x8 block from prev, c37_mv from source
         mvofs = c37_mv[opc * 2] + (c37_mv[opc * 2 + 1] * w);
-        if (check_mv(x, y, w, h, 8, mvofs))
-            return 1;
+        if (c48_invalid_mv(x, y, w, ctx->aligned_height, 8, mvofs))
+            break;
         for (i = 0; i < 8; i++) {
             ofs = i * w;
             for (l = 0; l < 8; l++)
-- 
2.49.1


>From da3c734ee6bf304907722d9baeecd0bf4a31f991 Mon Sep 17 00:00:00 2001
From: Manuel Lauss <manuel.lauss@gmail.com>
Date: Tue, 26 Aug 2025 22:11:44 +0200
Subject: [PATCH 07/11] avcodec/sanm: codec47: read the small codebook

codec47 carries a 4-byte small codebook in its header. Read those
4 bytes into context member instead of awkwardly redirecting the
bytestream pointer every time it needs to be accessed.

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
---
 libavcodec/sanm.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
index 7c4211ac53..ef83d2e672 100644
--- a/libavcodec/sanm.c
+++ b/libavcodec/sanm.c
@@ -296,6 +296,7 @@ typedef struct SANMVideoContext {
     uint8_t c23lut[256];
     uint8_t c4tbl[2][256][16];
     uint16_t c4param;
+    uint8_t c47cb[4];
 } SANMVideoContext;
 
 typedef struct SANMFrameHeader {
@@ -1214,7 +1215,7 @@ static int old_codec37(SANMVideoContext *ctx, int width, int height)
 }
 
 static int process_block(SANMVideoContext *ctx, uint8_t *dst, uint8_t *prev1,
-                         uint8_t *prev2, int stride, int tbl, int size)
+                         uint8_t *prev2, int stride, int size)
 {
     int code, k, t;
     uint8_t colors[2];
@@ -1236,18 +1237,18 @@ static int process_block(SANMVideoContext *ctx, uint8_t *dst, uint8_t *prev1,
                 dst[1 + stride] = bytestream2_get_byteu(&ctx->gb);
             } else {
                 size >>= 1;
-                if (process_block(ctx, dst, prev1, prev2, stride, tbl, size))
+                if (process_block(ctx, dst, prev1, prev2, stride, size))
                     return AVERROR_INVALIDDATA;
                 if (process_block(ctx, dst + size, prev1 + size, prev2 + size,
-                                  stride, tbl, size))
+                                  stride, size))
                     return AVERROR_INVALIDDATA;
                 dst   += size * stride;
                 prev1 += size * stride;
                 prev2 += size * stride;
-                if (process_block(ctx, dst, prev1, prev2, stride, tbl, size))
+                if (process_block(ctx, dst, prev1, prev2, stride, size))
                     return AVERROR_INVALIDDATA;
                 if (process_block(ctx, dst + size, prev1 + size, prev2 + size,
-                                  stride, tbl, size))
+                                  stride, size))
                     return AVERROR_INVALIDDATA;
             }
             break;
@@ -1276,12 +1277,8 @@ static int process_block(SANMVideoContext *ctx, uint8_t *dst, uint8_t *prev1,
                 memcpy(dst + k * stride, prev1 + k * stride, size);
             break;
         default:
-            k = bytestream2_tell(&ctx->gb);
-            bytestream2_seek(&ctx->gb, tbl + (code & 7), SEEK_SET);
-            t = bytestream2_get_byte(&ctx->gb);
-            bytestream2_seek(&ctx->gb, k, SEEK_SET);
             for (k = 0; k < size; k++)
-                memset(dst + k * stride, t, size);
+                memset(dst + k * stride, ctx->c47cb[code & 3], size);
         }
     } else {
         int mx = motion_vectors[code][0];
@@ -1361,13 +1358,13 @@ static int old_codec47(SANMVideoContext *ctx, int width, int height)
     uint8_t *prev1 = (uint8_t *)ctx->frm1;
     uint8_t *prev2 = (uint8_t *)ctx->frm2;
     uint8_t auxcol[2];
-    int tbl_pos = bytestream2_tell(&ctx->gb);
     int seq     = bytestream2_get_le16(&ctx->gb);
     int compr   = bytestream2_get_byte(&ctx->gb);
     int new_rot = bytestream2_get_byte(&ctx->gb);
     int skip    = bytestream2_get_byte(&ctx->gb);
 
-    bytestream2_skip(&ctx->gb, 7);
+    bytestream2_skip(&ctx->gb, 3);
+    bytestream2_get_bufferu(&ctx->gb, ctx->c47cb, 4);
     auxcol[0] = bytestream2_get_byteu(&ctx->gb);
     auxcol[1] = bytestream2_get_byteu(&ctx->gb);
     decoded_size = bytestream2_get_le32(&ctx->gb);
@@ -1407,8 +1404,7 @@ static int old_codec47(SANMVideoContext *ctx, int width, int height)
         if (seq == ctx->prev_seq + 1) {
             for (j = 0; j < height; j += 8) {
                 for (i = 0; i < width; i += 8)
-                    if (process_block(ctx, dst + i, prev1 + i, prev2 + i, stride,
-                                      tbl_pos + 8, 8))
+                    if (process_block(ctx, dst + i, prev1 + i, prev2 + i, stride, 8))
                         return AVERROR_INVALIDDATA;
                 dst   += stride * 8;
                 prev1 += stride * 8;
-- 
2.49.1


>From 67bea0d4a8cca0c6e475b11e66cd8bd7c36072a1 Mon Sep 17 00:00:00 2001
From: Manuel Lauss <manuel.lauss@gmail.com>
Date: Tue, 26 Aug 2025 22:24:08 +0200
Subject: [PATCH 08/11] avcodec/sanm: codec37/47/48 size checks

Add more size checks to old_codec37/47/48, esp. the headers.

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
---
 libavcodec/sanm.c | 49 +++++++++++++++++++++++++++++------------------
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
index ef83d2e672..16c2d86109 100644
--- a/libavcodec/sanm.c
+++ b/libavcodec/sanm.c
@@ -1056,14 +1056,18 @@ static int old_codec37(SANMVideoContext *ctx, int width, int height)
     ptrdiff_t stride = ctx->pitch;
     uint8_t *dst, *prev;
     int skip_run = 0;
-    int compr = bytestream2_get_byte(&ctx->gb);
-    int mvoff = bytestream2_get_byte(&ctx->gb);
-    int seq   = bytestream2_get_le16(&ctx->gb);
-    uint32_t decoded_size = bytestream2_get_le32(&ctx->gb);
+
+    if (bytestream2_get_bytes_left(&ctx->gb) < 16) // header size
+        return AVERROR_INVALIDDATA;
+
+    int compr = bytestream2_get_byteu(&ctx->gb);
+    int mvoff = bytestream2_get_byteu(&ctx->gb);
+    int seq   = bytestream2_get_le16u(&ctx->gb);
+    uint32_t decoded_size = bytestream2_get_le32u(&ctx->gb);
     int flags;
 
     bytestream2_skip(&ctx->gb, 4);
-    flags = bytestream2_get_byte(&ctx->gb);
+    flags = bytestream2_get_byteu(&ctx->gb);
     bytestream2_skip(&ctx->gb, 3);
 
     if (decoded_size > ctx->height * stride) {
@@ -1087,10 +1091,9 @@ static int old_codec37(SANMVideoContext *ctx, int width, int height)
 
     switch (compr) {
     case 0:
-        for (i = 0; i < height; i++) {
-            bytestream2_get_buffer(&ctx->gb, dst, width);
-            dst += stride;
-        }
+        if (bytestream2_get_bytes_left(&ctx->gb) < width * height)
+            return AVERROR_INVALIDDATA;
+        bytestream2_get_bufferu(&ctx->gb, dst, width * height);
         memset(ctx->frm2, 0, ctx->height * stride);
         break;
     case 1:
@@ -1358,16 +1361,20 @@ static int old_codec47(SANMVideoContext *ctx, int width, int height)
     uint8_t *prev1 = (uint8_t *)ctx->frm1;
     uint8_t *prev2 = (uint8_t *)ctx->frm2;
     uint8_t auxcol[2];
-    int seq     = bytestream2_get_le16(&ctx->gb);
-    int compr   = bytestream2_get_byte(&ctx->gb);
-    int new_rot = bytestream2_get_byte(&ctx->gb);
-    int skip    = bytestream2_get_byte(&ctx->gb);
+
+    if (bytestream2_get_bytes_left(&ctx->gb) < 26) // header size
+         return AVERROR_INVALIDDATA;
+
+    int seq     = bytestream2_get_le16u(&ctx->gb);
+    int compr   = bytestream2_get_byteu(&ctx->gb);
+    int new_rot = bytestream2_get_byteu(&ctx->gb);
+    int skip    = bytestream2_get_byteu(&ctx->gb);
 
     bytestream2_skip(&ctx->gb, 3);
     bytestream2_get_bufferu(&ctx->gb, ctx->c47cb, 4);
     auxcol[0] = bytestream2_get_byteu(&ctx->gb);
     auxcol[1] = bytestream2_get_byteu(&ctx->gb);
-    decoded_size = bytestream2_get_le32(&ctx->gb);
+    decoded_size = bytestream2_get_le32u(&ctx->gb);
     bytestream2_skip(&ctx->gb, 8);
 
     if (decoded_size > ctx->height * stride) {
@@ -1608,12 +1615,16 @@ static int codec48_block(SANMVideoContext *ctx, uint8_t *dst, uint8_t *db, int x
 static int old_codec48(SANMVideoContext *ctx, int width, int height)
 {
     uint8_t *dst, *prev;
-    int compr = bytestream2_get_byte(&ctx->gb);
-    int mvidx = bytestream2_get_byte(&ctx->gb);
-    int seq   = bytestream2_get_le16(&ctx->gb);
-    uint32_t decoded_size = bytestream2_get_le32(&ctx->gb);
     int i, j, flags;
 
+    if (bytestream2_get_bytes_left(&ctx->gb) < 16) // header size
+        return AVERROR_INVALIDDATA;
+
+    int compr = bytestream2_get_byteu(&ctx->gb);
+    int mvidx = bytestream2_get_byteu(&ctx->gb);
+    int seq   = bytestream2_get_le16u(&ctx->gb);
+    uint32_t decoded_size = bytestream2_get_le32u(&ctx->gb);
+
     // all codec48 videos use 1, but just to be safe...
     if (mvidx != 1) {
         av_log(ctx->avctx, AV_LOG_ERROR, "Invalid motion base value %d.\n", mvidx);
@@ -1621,7 +1632,7 @@ static int old_codec48(SANMVideoContext *ctx, int width, int height)
     }
 
     bytestream2_skip(&ctx->gb, 4);
-    flags = bytestream2_get_byte(&ctx->gb);
+    flags = bytestream2_get_byteu(&ctx->gb);
     bytestream2_skip(&ctx->gb, 3);
 
     if (flags & 8) {
-- 
2.49.1


>From 436d21e77a83c34f0360ca417b4c6ca3899e9279 Mon Sep 17 00:00:00 2001
From: Manuel Lauss <manuel.lauss@gmail.com>
Date: Tue, 26 Aug 2025 22:26:34 +0200
Subject: [PATCH 09/11] avcodec/sanm: rename process_block to codec47_block

the new name better indicates where it belongs to.

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
---
 libavcodec/sanm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
index 16c2d86109..81f9787b07 100644
--- a/libavcodec/sanm.c
+++ b/libavcodec/sanm.c
@@ -1217,7 +1217,7 @@ static int old_codec37(SANMVideoContext *ctx, int width, int height)
     return 0;
 }
 
-static int process_block(SANMVideoContext *ctx, uint8_t *dst, uint8_t *prev1,
+static int codec47_block(SANMVideoContext *ctx, uint8_t *dst, uint8_t *prev1,
                          uint8_t *prev2, int stride, int size)
 {
     int code, k, t;
@@ -1240,17 +1240,17 @@ static int process_block(SANMVideoContext *ctx, uint8_t *dst, uint8_t *prev1,
                 dst[1 + stride] = bytestream2_get_byteu(&ctx->gb);
             } else {
                 size >>= 1;
-                if (process_block(ctx, dst, prev1, prev2, stride, size))
+                if (codec47_block(ctx, dst, prev1, prev2, stride, size))
                     return AVERROR_INVALIDDATA;
-                if (process_block(ctx, dst + size, prev1 + size, prev2 + size,
+                if (codec47_block(ctx, dst + size, prev1 + size, prev2 + size,
                                   stride, size))
                     return AVERROR_INVALIDDATA;
                 dst   += size * stride;
                 prev1 += size * stride;
                 prev2 += size * stride;
-                if (process_block(ctx, dst, prev1, prev2, stride, size))
+                if (codec47_block(ctx, dst, prev1, prev2, stride, size))
                     return AVERROR_INVALIDDATA;
-                if (process_block(ctx, dst + size, prev1 + size, prev2 + size,
+                if (codec47_block(ctx, dst + size, prev1 + size, prev2 + size,
                                   stride, size))
                     return AVERROR_INVALIDDATA;
             }
@@ -1411,7 +1411,7 @@ static int old_codec47(SANMVideoContext *ctx, int width, int height)
         if (seq == ctx->prev_seq + 1) {
             for (j = 0; j < height; j += 8) {
                 for (i = 0; i < width; i += 8)
-                    if (process_block(ctx, dst + i, prev1 + i, prev2 + i, stride, 8))
+                    if (codec47_block(ctx, dst + i, prev1 + i, prev2 + i, stride, 8))
                         return AVERROR_INVALIDDATA;
                 dst   += stride * 8;
                 prev1 += stride * 8;
-- 
2.49.1


>From 0ef60e34486bda6890dc0271e2c24cb3c57410ee Mon Sep 17 00:00:00 2001
From: Manuel Lauss <manuel.lauss@gmail.com>
Date: Tue, 26 Aug 2025 22:29:04 +0200
Subject: [PATCH 10/11] avcodec/sanm: reset rotate_code every iteration

and eliminate the explicit reset in the other decoders that
don't need it.

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
---
 libavcodec/sanm.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
index 81f9787b07..1bdbca3aa3 100644
--- a/libavcodec/sanm.c
+++ b/libavcodec/sanm.c
@@ -927,7 +927,6 @@ static int old_codec1(SANMVideoContext *ctx, GetByteContext *gb, int top,
             }
         }
     }
-    ctx->rotate_code = 0;
 
     return 0;
 }
@@ -990,7 +989,6 @@ static int old_codec31(SANMVideoContext *ctx, GetByteContext *gb, int top,
             }
         }
     }
-    ctx->rotate_code = 0;
 
     return 0;
 }
@@ -1075,8 +1073,6 @@ static int old_codec37(SANMVideoContext *ctx, int width, int height)
         av_log(ctx->avctx, AV_LOG_WARNING, "Decoded size is too large.\n");
     }
 
-    ctx->rotate_code = 0;
-
     if (((seq & 1) || !(flags & 1)) && (compr && compr != 2)) {
         FFSWAP(uint16_t*, ctx->frm0, ctx->frm2);
     }
@@ -1436,8 +1432,7 @@ static int old_codec47(SANMVideoContext *ctx, int width, int height)
     }
     if (seq == ctx->prev_seq + 1)
         ctx->rotate_code = new_rot;
-    else
-        ctx->rotate_code = 0;
+
     ctx->prev_seq = seq;
 
     return 0;
@@ -1691,7 +1686,7 @@ static int old_codec48(SANMVideoContext *ctx, int width, int height)
                                       "Subcodec 48 compression %d", compr);
         return AVERROR_PATCHWELCOME;
     }
-    ctx->rotate_code = 1;    // swap frm[0] and frm[2]
+    ctx->rotate_code = 1;    // swap frm0 and frm2
     ctx->prev_seq = seq;
     return 0;
 }
@@ -2496,7 +2491,7 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *frame,
     }
     if (ctx->rotate_code)
         rotate_bufs(ctx, ctx->rotate_code);
-
+    ctx->rotate_code = 0;
     return pkt->size;
 }
 
-- 
2.49.1


>From 33d7d6cf050e9d8c7acdafaa7fd9b14e622813fd Mon Sep 17 00:00:00 2001
From: Manuel Lauss <manuel.lauss@gmail.com>
Date: Tue, 26 Aug 2025 22:52:26 +0200
Subject: [PATCH 11/11] Revert "avcodec/sanm: Check w,h,left,top"

This reverts commit 134fbfd1dcb59441e38d870ddd231772f4e8e127.

As it breaks valid uses of this in Rebel Assault 1 videos.
---
 libavcodec/sanm.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
index 1bdbca3aa3..3c012a87bf 100644
--- a/libavcodec/sanm.c
+++ b/libavcodec/sanm.c
@@ -1793,11 +1793,6 @@ static int process_frame_obj(SANMVideoContext *ctx, GetByteContext *gb,
             memset(ctx->fbuf, 0, ctx->frm0_size);
     }
 
-    if (w + FFMAX(left, 0) > ctx->avctx->width || h + FFMAX(top, 0) > ctx->avctx->height) {
-        avpriv_request_sample(ctx->avctx, "overly large frame\n");
-        return AVERROR_PATCHWELCOME;
-    }
-
     switch (codec) {
     case 1:
     case 3:
-- 
2.49.1

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2025-08-27  8:27 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-08-27  8:27 [FFmpeg-devel] [PATCH] WIP: avcodec/sanm: updates (PR #20350) Manuel Lauss via ffmpeg-devel

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git