[FFmpeg-devel] [PATCH v4 1/3] avcodec/sanm: ignore unknown codecs in FOBJs

[FFmpeg-devel] [PATCH v4 1/3] avcodec/sanm: ignore unknown codecs in FOBJs

[Manuel Lauss]

Don't error out, just ignore unknown codec numbers and pretend
decode succeeded.  This is useful for older LucasArts titles
which stack a lot of different FOBJs with different codecs into
a single frame.

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
---
v4: no changes
v3: no changes
v2: Mark frame as corrupt, suggested by Marton Balint
 libavcodec/sanm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
index c30095ed32..a4f0a28c7c 100644
--- a/libavcodec/sanm.c
+++ b/libavcodec/sanm.c
@@ -1274,7 +1274,8 @@ static int process_frame_obj(SANMVideoContext *ctx)
         return old_codec48(ctx, w, h);
     default:
         avpriv_request_sample(ctx->avctx, "Subcodec %d", codec);
-        return AVERROR_PATCHWELCOME;
+        ctx->frame->flags |= AV_FRAME_FLAG_CORRUPT;
+        return 0;
     }
 }
 
-- 
2.48.1

_______________________________________________
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 v4 2/3] avcodec/sanm: fobj left/top are signed

[Manuel Lauss]

The left and top parameters of an FOBJ are signed values.

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
---
v4: revert v3, it arose due to a misunderstanding
v3: change the bytestream accessor to signed too
v2: no changes
 libavcodec/sanm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
index a4f0a28c7c..71dbac4320 100644
--- a/libavcodec/sanm.c
+++ b/libavcodec/sanm.c
@@ -1238,8 +1238,8 @@ static int old_codec48(SANMVideoContext *ctx, int width, int height)
 static int process_frame_obj(SANMVideoContext *ctx)
 {
     uint16_t codec = bytestream2_get_le16u(&ctx->gb);
-    uint16_t left  = bytestream2_get_le16u(&ctx->gb);
-    uint16_t top   = bytestream2_get_le16u(&ctx->gb);
+    int16_t  left  = bytestream2_get_le16u(&ctx->gb);
+    int16_t  top   = bytestream2_get_le16u(&ctx->gb);
     uint16_t w     = bytestream2_get_le16u(&ctx->gb);
     uint16_t h     = bytestream2_get_le16u(&ctx->gb);
 
-- 
2.48.1

_______________________________________________
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 v4 2/3] avcodec/sanm: fobj left/top are signed

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1424 bytes --]

Hi Manuel

On Tue, Mar 04, 2025 at 06:07:18PM +0100, Manuel Lauss wrote:
> The left and top parameters of an FOBJ are signed values.
> 
> Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
> ---
> v4: revert v3, it arose due to a misunderstanding
> v3: change the bytestream accessor to signed too
> v2: no changes
>  libavcodec/sanm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
> index a4f0a28c7c..71dbac4320 100644
> --- a/libavcodec/sanm.c
> +++ b/libavcodec/sanm.c
> @@ -1238,8 +1238,8 @@ static int old_codec48(SANMVideoContext *ctx, int width, int height)
>  static int process_frame_obj(SANMVideoContext *ctx)
>  {
>      uint16_t codec = bytestream2_get_le16u(&ctx->gb);
> -    uint16_t left  = bytestream2_get_le16u(&ctx->gb);
> -    uint16_t top   = bytestream2_get_le16u(&ctx->gb);
> +    int16_t  left  = bytestream2_get_le16u(&ctx->gb);
> +    int16_t  top   = bytestream2_get_le16u(&ctx->gb);
>      uint16_t w     = bytestream2_get_le16u(&ctx->gb);
>      uint16_t h     = bytestream2_get_le16u(&ctx->gb);

Does the following code also handle all error conditions that
negative left/top could now trigger ?

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: 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 v4 2/3] avcodec/sanm: fobj left/top are signed

[Manuel Lauss]

Hi Michael,

On Sat, Mar 8, 2025 at 8:11 PM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> Hi Manuel
>
> On Tue, Mar 04, 2025 at 06:07:18PM +0100, Manuel Lauss wrote:
> > The left and top parameters of an FOBJ are signed values.
> >
> > Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
> > ---
> > v4: revert v3, it arose due to a misunderstanding
> > v3: change the bytestream accessor to signed too
> > v2: no changes
> >  libavcodec/sanm.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
> > index a4f0a28c7c..71dbac4320 100644
> > --- a/libavcodec/sanm.c
> > +++ b/libavcodec/sanm.c
> > @@ -1238,8 +1238,8 @@ static int old_codec48(SANMVideoContext *ctx, int width, int height)
> >  static int process_frame_obj(SANMVideoContext *ctx)
> >  {
> >      uint16_t codec = bytestream2_get_le16u(&ctx->gb);
> > -    uint16_t left  = bytestream2_get_le16u(&ctx->gb);
> > -    uint16_t top   = bytestream2_get_le16u(&ctx->gb);
> > +    int16_t  left  = bytestream2_get_le16u(&ctx->gb);
> > +    int16_t  top   = bytestream2_get_le16u(&ctx->gb);
> >      uint16_t w     = bytestream2_get_le16u(&ctx->gb);
> >      uint16_t h     = bytestream2_get_le16u(&ctx->gb);
>
> Does the following code also handle all error conditions that
> negative left/top could now trigger ?

For the LucasArts titles that sanm.c currently supports well,
no negative values are ever encountered.
I let ffplay run through maybe 1/3 of the Rebel Assault 1 videos,
which are the only ones that make use of negative values, but
didn't encounter any crashes; mostly because the codecs it
uses aren't supported by ffmpeg/sanm (yet).

Manuel
_______________________________________________
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 v4 3/3] avcodec/sanm: add smush codec23 decoder

[Manuel Lauss]

This codec alternatingly skips and changes existing pixels.
A second 16bit parameter in the FOBJ header indicates how to do
the pixel changes: either by specifying a LUT in the codec datastream
or by adding a constant value to the pixel.
For ANIMv1 (Rebel Assault 1) the first 8bit parameter is used as
a constant offset to add to the existing pixel value.

Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
---
v4: also support c23 on ANIMv0/1 (Rebel Assault 1, e.g LVL11/L11PLAY.ANM water)
v3: updates due to changes in patch 2
v2: no changes.

Videos showing the before/after state (Rebel Assault II LEV09/09PLAY.SAN file)
http://mlau.at/ffmpeg_c23_before.mp4
http://mlau.at/ffmpeg_c23_after.mp4
notice the blue transparent forcefields.

 libavcodec/sanm.c | 66 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 64 insertions(+), 2 deletions(-)

diff --git a/libavcodec/sanm.c b/libavcodec/sanm.c
index 71dbac4320..534ec7aee6 100644
--- a/libavcodec/sanm.c
+++ b/libavcodec/sanm.c
@@ -292,6 +292,7 @@ typedef struct SANMVideoContext {
     int8_t p4x4glyphs[NGLYPHS][16];
     int8_t p8x8glyphs[NGLYPHS][64];
     uint8_t c47itbl[0x10000];
+    uint8_t c23lut[256];
 } SANMVideoContext;
 
 typedef struct SANMFrameHeader {
@@ -555,6 +556,62 @@ static int rle_decode(SANMVideoContext *ctx, uint8_t *dst, const int out_size)
     return 0;
 }
 
+static int old_codec23(SANMVideoContext *ctx, int top, int left, int width,
+                       int height, uint8_t param, uint16_t param2)
+{
+    const uint32_t maxpxo = ctx->width * ctx->pitch;
+    uint8_t *dst, lut[256], c;
+    int i, j, k, pc, sk;
+    int32_t pxoff;
+
+    if (ctx->subversion < 2) {
+        /* Rebel Assault 1: constant offset + 0xd0 */
+        for (i = 0; i < 256; i++)
+            lut[i] = (i + param + 0xd0) & 0xff;
+    } else if (param2 == 256) {
+        if (bytestream2_get_bytes_left(&ctx->gb) < 256)
+            return AVERROR_INVALIDDATA;
+        bytestream2_get_bufferu(&ctx->gb, ctx->c23lut, 256);
+    } else if (param2 < 256) {
+        for (i = 0; i < 256; i++)
+            lut[i] = (i + param2) & 0xff;
+    } else {
+        memcpy(lut, ctx->c23lut, 256);
+    }
+    if (bytestream2_get_bytes_left(&ctx->gb) < 1)
+        return 0;  /* some c23 frames just set up the LUT */
+
+    dst = (uint8_t *)ctx->frm0;
+    for (i = 0; i < height; i++) {
+        if (bytestream2_get_bytes_left(&ctx->gb) < 2)
+            return 0;
+        pxoff = left + ((top + i) * ctx->pitch);
+        k = bytestream2_get_le16u(&ctx->gb);
+        sk = 1;
+        pc = 0;
+        while (k > 0 && pc <= width) {
+            if (bytestream2_get_bytes_left(&ctx->gb) < 1)
+                return AVERROR_INVALIDDATA;
+            j = bytestream2_get_byteu(&ctx->gb);
+            if (sk) {
+                pxoff += j;
+                pc += j;
+            } else {
+                while (j--) {
+                    if (pxoff >=0 && pxoff < maxpxo) {
+                        c = *(dst + pxoff);
+                        *(dst + pxoff) = lut[c];
+                    }
+                    pxoff++;
+                    pc++;
+                }
+            }
+            sk ^= 1;
+        }
+    }
+    return 0;
+}
+
 static int old_codec1(SANMVideoContext *ctx, int top,
                       int left, int width, int height)
 {
@@ -1237,11 +1294,15 @@ static int old_codec48(SANMVideoContext *ctx, int width, int height)
 
 static int process_frame_obj(SANMVideoContext *ctx)
 {
-    uint16_t codec = bytestream2_get_le16u(&ctx->gb);
+    uint16_t parm2;
+    uint8_t  codec = bytestream2_get_byteu(&ctx->gb);
+    uint8_t  param = bytestream2_get_byteu(&ctx->gb);
     int16_t  left  = bytestream2_get_le16u(&ctx->gb);
     int16_t  top   = bytestream2_get_le16u(&ctx->gb);
     uint16_t w     = bytestream2_get_le16u(&ctx->gb);
     uint16_t h     = bytestream2_get_le16u(&ctx->gb);
+    bytestream2_skip(&ctx->gb, 2);
+    parm2 = bytestream2_get_le16u(&ctx->gb);
 
     if (!w || !h) {
         av_log(ctx->avctx, AV_LOG_ERROR, "Dimensions are invalid.\n");
@@ -1260,12 +1321,13 @@ static int process_frame_obj(SANMVideoContext *ctx)
             return AVERROR(ENOMEM);
         }
     }
-    bytestream2_skip(&ctx->gb, 4);
 
     switch (codec) {
     case 1:
     case 3:
         return old_codec1(ctx, top, left, w, h);
+    case 23:
+        return old_codec23(ctx, top, left, w, h, param, parm2);
     case 37:
         return old_codec37(ctx, top, left, w, h);
     case 47:
-- 
2.48.1

_______________________________________________
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 v4 1/3] avcodec/sanm: ignore unknown codecs in FOBJs

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 718 bytes --]

Hi Manuel

On Tue, Mar 04, 2025 at 06:07:17PM +0100, Manuel Lauss wrote:
> Don't error out, just ignore unknown codec numbers and pretend
> decode succeeded.  This is useful for older LucasArts titles
> which stack a lot of different FOBJs with different codecs into
> a single frame.
> 
> Signed-off-by: Manuel Lauss <manuel.lauss@gmail.com>
> ---
> v4: no changes
> v3: no changes
> v2: Mark frame as corrupt, suggested by Marton Balint
>  libavcodec/sanm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

will apply

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: 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".