[FFmpeg-devel] [PATCH 2/2] avcodec/mjpegdec: silently ignore APPx stubs

[Michael Niedermayer]

On Tue, Oct 22, 2024 at 11:25:58AM +0200, Ramiro Polla wrote:
> On Mon, Oct 21, 2024 at 1:41 AM Michael Niedermayer
> <michael at niedermayer.cc> 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.

Personally i would like to see maybe some DEBUG level information
on mystery/not understood APP*

not sure this is within the scope of this patch ...

thx

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

If you drop bombs on a foreign country and kill a hundred thousand
innocent people, expect your government to call the consequence
"unprovoked inhuman terrorist attacks" and use it to justify dropping
more bombs and killing more people. The technology changed, the idea is old.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20241023/77826a41/attachment.sig>