Am Mi., 1. Apr. 2020 um 21:39 Uhr schrieb Matthieu Bouron <matthieu.bou...@gmail.com>: > > On Wed, Apr 01, 2020 at 09:10:19PM +0200, Carl Eugen Hoyos wrote: > > Am Mi., 1. Apr. 2020 um 21:07 Uhr schrieb Matthieu Bouron > > <matthieu.bou...@gmail.com>: > > > > > > On Wed, Apr 01, 2020 at 07:59:46PM +0200, Carl Eugen Hoyos wrote: > > > > Am Mi., 1. Apr. 2020 um 19:54 Uhr schrieb Matthieu Bouron > > > > <matthieu.bou...@gmail.com>: > > > > > > > > > > On Wed, Apr 01, 2020 at 07:30:55PM +0200, Carl Eugen Hoyos wrote: > > > > > > Am Mi., 1. Apr. 2020 um 19:15 Uhr schrieb Matthieu Bouron > > > > > > <matthieu.bou...@gmail.com>: > > > > > > > > > > > > > > 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.bou...@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.bou...@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 > > > > > > > > > > > > You could test it. > > > > > > > > > > > > > but the code is trying to find JPEG markers in the > > > > > > > MPF metadata until we reach the first EOI in the MPF JPEG > > > > > > > thumbnails, > > > > > > > > > > > > > > > > I meant SOI sorry. > > > > > > > > > > > No, and I am quite sure that I tested this sufficiently. > > > > > > > > > > My point is, we are trying to parse way more data than necessary (the > > > > > data > > > > > we parse might not be jpeg at this point): the code looks for a > > > > > marker and > > > > > handles it, if the marker is unknown it repeats this operation for the > > > > > next byte until it reaches the end of the buffer or it finds SOI in > > > > > the > > > > > EOI+MPF state (with your patch). > > > > > > > > > > Why cant' we end the processing at EOI ? If there is a particular > > > > > reason > > > > > for that ? > > > > > > > > You claim (that's how I understood it) was that a valid MPF jpg > > > > contains a > > > > second (smaller) jpeg immediately after the big jpeg (that contains > > > > an additional jpg in its APP2 data). I test if this is the case. > > > > If you tell me a valid MPF jpg can contain any data after EOI, my > > > > test if of course not correct. > > > > > > The MPF metadata contained in the APP2 segment can reference multiple > > > images. For each image, an offset and a size is given. > > > The multiple images are stored after the main JPEG chunk. So the EOI of > > > the main JPEG chunk might be followed by the SOI of the first referenced > > > image (and this is the case in the sample I provided). So your patch do > > > not introduce any unnecessary processing if there is no padding between > > > EOI and SOI. > > > > > > I wrongly remembered how MPF worked and assumed there was MPF data between > > > the SOI and EOI. Last time I worked on this was two years ago and I am > > > trying to upstream the patches I made back then. > > > > > > Sorry to ask again but, why can't we stop parsing at EOI in the SOS state > > > ? > > > > Sorry for the repeated answer: It would break mjpeg detection. > > > > We can of course stop at EOI for MPF but if every valid MPF file either ends > > at EOI or contains another jpeg image, we are still good. > > Ok, I think I understand why it would break the mjpeg detection now. This > is why jpeg_probe() returns 0 if it finds SOI to give a chance to the > mjpeg probe to find something (and because the mjpeg demuxer is supposed > to handle multiple JPEG chunks). Is this correct ?
Yes. Carl Eugen _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
