[FFmpeg-devel] [PATCH] avformat/img2dec: return immediately in jpeg_probe() when EOI is found

[Matthieu Bouron]

On Wed, Apr 01, 2020 at 06:19:56PM +0200, Carl Eugen Hoyos wrote:
> Am Mi., 1. Apr. 2020 um 18:01 Uhr schrieb Matthieu Bouron
> <matthieu.bouron at gmail.com>:
> >
> > Fixes probing of JPEG files containing MPF metadata appended at the end
> > of the file.
> >
> > The MPF metadata chunk can contains multiple JPEG images (thumbnails)
> > which makes the jpeg_probe fails (return 0) because it finds a SOI
> > marker after EOI.
> > ---
> >
> > This patch fixes probing of JPEG files containing MPF metadata [1] appended
> > at the end of the file.
> >
> > Such files can be produced by GoPro camera (which produces JPEG files
> > with MPF metadata).
> >
> > You can find a sample here:
> > https://0x5c.me/gopro_jpg_mpf_probe_fail
> >
> > To reproduce the issue using ffmpeg master:
> > wget https://0x5c.me/gopro_jpg_mpf_probe_fail
> > ./ffmpeg -formatprobesize 5000000 -i gopro_jpg_mpf_probe_fail
> >
> > I removed intentionally the jpeg extension from the filename so the
> > image2 demuxer is not used.
> >
> > Current ffmpeg master won't still be able to decode this file because of a
> > "regression" introduced by ec3d8a0e6945fe015d16cd98a1e7dbb4be815c15 on
> > the mjpeg_parser.
> > The jpeg_pipe demuxer outputs partial chunks of the jpeg file that the
> > mjpeg decoder can't handle (it needs the whole data). Before
> > ec3d8a0e6945fe015d16cd98a1e7dbb4be815c15, the mjpeg_parser was
> > outputting a complete frame.
> >
> > If the parser is correct as is, another way to fix this issue whould be
> > to add AVSTREAM_PARSE_HEADERS to the need_parsing flags from the
> > jpeg_pipe demuxer, ie:
> >
> > --- a/libavformat/img2dec.c
> > +++ b/libavformat/img2dec.c
> > @@ -209,7 +209,7 @@ int ff_img_read_header(AVFormatContext *s1)
> >          s->is_pipe = 0;
> >      else {
> >          s->is_pipe       = 1;
> > -        st->need_parsing = AVSTREAM_PARSE_FULL;
> > +        st->need_parsing = AVSTREAM_PARSE_FULL | AVSTREAM_PARSE_HEADERS;
> >
> > Settings AVSTREAM_PARSE_HEADERS makes avformat requests complete frames
> > from the parser in libavformat/utils.c
> >
> > What do you think ?
> 
> Why is your sample not detected by the mjpeg demuxer?

I don't know yet, it looks like nb_invalid is incremented in the default
case of mjpeg_probe(). I will investigate why.

This shouldn't change the fact that the jpeg_probe() should be able to
recognize this kind of files.

> 
> I suspect your patch breaks the mjpeg demuxer.

Why would it break the mjpeg demuxer ?

> 
> > [1] https://exiftool.org/TagNames/MPF.html
> >
> > ---
> >  libavformat/img2dec.c | 6 +-----
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c
> > index 93cd51c1932..decd8023e02 100644
> > --- a/libavformat/img2dec.c
> > +++ b/libavformat/img2dec.c
> > @@ -773,9 +773,7 @@ static int jpeg_probe(const AVProbeData *p)
> >          case EOI:
> >              if (state != SOS)
> >                  return 0;
> > -            state = EOI;
> > -            break;
> > -        case DHT:
> > +            return AVPROBE_SCORE_EXTENSION + 1;
> 
> I don't think this is correct.

Why ? Isn't the data JPEG if we find an EOI marker after having found a
SOS marker ?

> 
> >          case DQT:
> >          case APP0:
> >          case APP1:
> > @@ -803,8 +801,6 @@ static int jpeg_probe(const AVProbeData *p)
> >          }
> >      }
> >
> 
> > -    if (state == EOI)
> > -        return AVPROBE_SCORE_EXTENSION + 1;
> 
> >      if (state == SOS)
> >          return AVPROBE_SCORE_EXTENSION / 2;
> >      return AVPROBE_SCORE_EXTENSION / 8;
> 
> Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".

-- 
Matthieu B.