On Tue, Nov 26, 2024 at 11:56 AM Anton Khirnov <[email protected]> wrote:
> Quoting Ramiro Polla (2024-10-22 11:25:58)
> > On Mon, Oct 21, 2024 at 1:41 AM Michael Niedermayer
> > <[email protected]> wrote:
> > > On Thu, Oct 17, 2024 at 01:00:12PM +0200, Ramiro Polla wrote:
> > > > Consider APPx fields that are too short to contain an id field (32-bit)
> > > > as stubs, and silently ignore them.
> > > >
> > > > This has been seen in the MJPEG output from some webcams (such as the
> > > > Logitech C270 and C920) and the JPEG images embedded in DNG images
> > > > from the Pentax K-1 camera.
> > > > ---
> > > >  libavcodec/mjpegdec.c | 20 +++++++++++---------
> > > >  1 file changed, 11 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
> > > > index a775fdca30..3cd9904595 100644
> > > > --- a/libavcodec/mjpegdec.c
> > > > +++ b/libavcodec/mjpegdec.c
> > > > @@ -1856,20 +1856,22 @@ static int mjpeg_decode_app(MJpegDecodeContext 
> > > > *s)
> > > >      int len, id, i;
> > > >
> > > >      len = get_bits(&s->gb, 16);
> > > > -    if (len < 6) {
> > > > -        if (s->bayer) {
> > > > -            // Pentax K-1 (digital camera) JPEG images embedded in DNG 
> > > > images contain unknown APP0 markers
> > > > -            av_log(s->avctx, AV_LOG_WARNING, "skipping APPx 
> > > > (len=%"PRId32") for bayer-encoded image\n", len);
> > > > -            skip_bits(&s->gb, len);
> > > > -            return 0;
> > > > -        } else
> > > > -            return AVERROR_INVALIDDATA;
> > > > +    if (len < 2)
> > > > +        return AVERROR_INVALIDDATA;
> > > > +    len -= 2;
> > > > +
> > > > +    if (len < 4) {
> > > > +        /* Silently ignore APPx stubs */
> > > > +        if (show_bits(&s->gb, 8 * len) == 0)
> > > > +            goto out;
> > > > +        return AVERROR_INVALIDDATA;
> > >
> > > this silently errors on the ones it doesnt ignore, it was more informative
> > > before
> >
> > It still prints the error message after returning AVERROR_INVALIDDATA
> > from mjpeg_decode_app():
> > [mjpeg @ 0x7fc4a0002dc0] unable to decode APP fields: Invalid data
> > found when processing input
> >
> > I could also add another verbose log before "goto out" to
> > not-so-silently ignore the APPx stubs if you want.
>
> The best approach IMO is to check for AV_EF_EXPLODE and error out if
> it's set.

New patch attached. It errors out if AV_EF_EXPLODE is set, otherwise
it does an av_log(AV_LOG_VERBOSE) and skips the stub.
From 770ffdd3c8c613db34be70640351e15e7d023cc3 Mon Sep 17 00:00:00 2001
From: Ramiro Polla <[email protected]>
Date: Tue, 1 Oct 2024 20:50:05 +0200
Subject: [PATCH v2 2/2] avcodec/mjpegdec: ignore APPx stubs unless
 AV_EF_EXPLODE is set

Consider APPx fields that are too short to contain an id field (32-bit)
as stubs, and ignore them if AV_EF_EXPLODE is not set.

This has been seen in the MJPEG output from some webcams (such as the
Logitech C270 and C920) and the JPEG images embedded in DNG images
from the Pentax K-1 camera.
---
 libavcodec/mjpegdec.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 76d70a0307..a9cc4c2d52 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -1860,20 +1860,22 @@ static int mjpeg_decode_app(MJpegDecodeContext *s)
     int len, id, i;
 
     len = get_bits(&s->gb, 16);
-    if (len < 6) {
-        if (s->bayer) {
-            // Pentax K-1 (digital camera) JPEG images embedded in DNG images contain unknown APP0 markers
-            av_log(s->avctx, AV_LOG_WARNING, "skipping APPx (len=%"PRId32") for bayer-encoded image\n", len);
-            skip_bits(&s->gb, len);
-            return 0;
-        } else
+    if (len < 2)
+        return AVERROR_INVALIDDATA;
+    len -= 2;
+
+    if (len < 4) {
+        if (s->avctx->err_recognition & AV_EF_EXPLODE)
             return AVERROR_INVALIDDATA;
+        av_log(s->avctx, AV_LOG_VERBOSE, "skipping APPx stub (len=%" PRId32 ")\n", len);
+        goto out;
     }
+
     if (8 * len > get_bits_left(&s->gb))
         return AVERROR_INVALIDDATA;
 
     id   = get_bits_long(&s->gb, 32);
-    len -= 6;
+    len -= 4;
 
     if (s->avctx->debug & FF_DEBUG_STARTCODE)
         av_log(s->avctx, AV_LOG_DEBUG, "APPx (%s / %8X) len=%d\n",
-- 
2.39.5

_______________________________________________
ffmpeg-devel mailing list
[email protected]
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
[email protected] with subject "unsubscribe".

Reply via email to