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

[Matthieu Bouron]

On Wed, Apr 01, 2020 at 06:55:42PM +0200, Carl Eugen Hoyos wrote:
> Am Mi., 1. Apr. 2020 um 18:35 Uhr schrieb Matthieu Bouron
> <matthieu.bouron at gmail.com>:
> >
> > On Wed, Apr 01, 2020 at 06:29:03PM +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
> > >
> > > The sample works fine here with FFmpeg 4.2: Is there a regression?
> >
> > This sample does not work on FFmpeg 4.2 and master if you set a big
> > enought formatprobesize (which matches the file size of the sample):
> 
> Inlined is a patch that fixes detection but I wonder if this shouldn't be
> detected as mjpeg instead
> 
> Carl Eugen
> 
> diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c
> index 40f3e3d499..4e63b3494e 100644
> --- a/libavformat/img2dec.c
> +++ b/libavformat/img2dec.c
> @@ -737,20 +737,22 @@ static int j2k_probe(const AVProbeData *p)
>  static int jpeg_probe(const AVProbeData *p)
>  {
>      const uint8_t *b = p->buf;
> -    int i, state = SOI;
> +    int i, state = SOI, MPF = 0;
> 
>      if (AV_RB16(b) != 0xFFD8 ||
>          AV_RB32(b) == 0xFFD8FFF7)
>      return 0;
> 
>      b += 2;
> -    for (i = 0; i < p->buf_size - 3; i++) {
> +    for (i = 0; i < p->buf_size - 8; i++) {
>          int c;
>          if (b[i] != 0xFF)
>              continue;
>          c = b[i + 1];
>          switch (c) {
>          case SOI:
> +            if (state == EOI && MPF)
> +                return AVPROBE_SCORE_EXTENSION + 1;

Your patch might work but the code is trying to find JPEG markers in the
MPF metadata until we reach the first EOI in the MPF JPEG thumbnails, even
if we already found the SOS and EOI markes of the main JPEG chunk. This
logic does not seem right to me.

Is there a specific reason why we couldn't end the probe when we reach the
EOI marker in the SOS state (ie: what my patch is doing) ?

>              return 0;
>          case SOF0:
>          case SOF1:
> @@ -775,10 +777,12 @@ static int jpeg_probe(const AVProbeData *p)
>                  return 0;
>              state = EOI;
>              break;
> +        case APP2:
> +            if (AV_RB24(&b[i + 4]) == AV_RB24("MPF"))
> +                MPF = 1;
>          case DQT:
>          case APP0:
>          case APP1:
> -        case APP2:
>          case APP3:
>          case APP4:
>          case APP5:
> _______________________________________________
> 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.