[FFmpeg-devel] [PATCH] avformat: Fix probing on some JPEGs

Niki Bowe nbowe at google.com
Sat Sep 7 03:00:14 EEST 2019


As it turns out the last patch still lets a lot of jpegs get misidentified.

This new version avoids the problem by checking for jpeg magic at the start.

I also added a FATE test, and attached the jpeg for the test.



On Tue, Aug 27, 2019 at 6:30 PM Niki Bowe <nbowe at google.com> wrote:

>
>
> On Sun, Aug 25, 2019 at 11:39 AM Michael Niedermayer
> <michael at niedermayer.cc> wrote:
>
>> On Fri, Aug 23, 2019 at 04:03:10PM -0700, Niki Bowe wrote:
>> > On Thu, Aug 22, 2019 at 2:30 AM Paul B Mahol <onemda at gmail.com> wrote:
>> >
>> > > On Thu, Aug 22, 2019 at 11:19 AM Carl Eugen Hoyos <ceffmpeg at gmail.com
>> >
>> > > wrote:
>> > >
>> > > > Am Mi., 21. Aug. 2019 um 23:05 Uhr schrieb Niki Bowe
>> > > > <nbowe-at-google.com at ffmpeg.org>:
>> > > > >
>> > > > > On Mon, Aug 19, 2019 at 7:22 PM Carl Eugen Hoyos <
>> ceffmpeg at gmail.com>
>> > > > wrote:
>> > > > >
>> > > > > >
>> > > > > > This score would mean that mjpeg can never be detected.
>> > > > > > I suspect you have to reduce one of the demuxers to "- 1".
>> > > > > >
>> > > > > >
>> > > > > Thanks Carl.
>> > > > > Attached patch to reduce mpeg probe by -1, which also fixes the
>> issue.
>> > > >
>> > > > Sorry, I misread the original report, it looked to me as if mJpeg
>> was
>> > > > the culprit.
>> > > >
>> > > > Imo, the mpeg probing should be fixed (return a smaller value) for
>> your
>> > > > sample
>> > > > by detecting that it is not mpeg, not by returning a smaller value
>> for
>> > > > all samples.
>> > > >
>> > >
>> > > 1000% agree.
>> > >
>> > >
>> > It didn't return a smaller value for all samples, only the "invalid VDR
>> > files and short PES streams" case.
>> > Most mpegps files still return 26 immediately, because they have pack
>> > headers.
>> >
>> > However, here is another patch where I try to limit it to only changing
>> > score for these jpegs.
>> > I noticed that these jpegs had a lot of 0x00000100 sequences, which
>> matches
>> > mpeg picture header start code. I added another heuristic which matches
>> > these jpegs, but not any mpegps files I could find.
>> >
>> > Alternatively I could make reduce score if it doesn't start with a start
>> > code? At the moment its happy to search until it finds start codes.
>> >
>> >
>> > Is everyone really sure the best approach is to modify mpegps_probe for
>> > this?
>> > The mpegps_probe function returns 25 in many instances where it may not
>> be
>> > mpegps. It does only minimal structural checking, and allows invalid
>> data
>> > to still classify as mpegps.
>> > jpeg probing returns 25 in some cases where it is almost certainly a
>> jpeg
>> > (Has to go through multiple tags to get to SOS, many of which early out
>> if
>> > they find invalid data).
>> > Note that 25 is still treated as "low confidence" for jpeg. It logs
>> "Format
>> > jpeg_pipe detected only with low score of 25, misdetection possible!"
>> for
>> > these jpegs.
>> > So I still think a score of 25 is too low for these jpegs, and that a
>> > better fix would be to return 26 for jpeg_pipe and mjpeg if it makes it
>> > past multiple tags to SOS.
>>
>> jpegs can be in other container formats its not jpeg in that case but the
>> other container format
>>
> about this patch
>> it breaks this:
>>
>> ./ffplay tickets//3327/issue3327-libc-2.17.so
>>
>> https://trac.ffmpeg.org/raw-attachment/ticket/3327/issue3327-libc-2.17.so
>>
>> which is detected as mpeg after the patch.
>> really nothing should be detected as mpeg after this that was not before
>> the idea IIUC is make something that was detected as mpeg to be not
>> anymore
>>
>>
> Thanks Michael. Good find.
> Attached patch which only applies the extra sanity checks to the "Invalid
> VDR files and short PES streams" path.
>
>
>
>> Thanks
>>
>> [...]
>>
>>
>> --
>> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>> Awnsering whenever a program halts or runs forever is
>> On a turing machine, in general impossible (turings halting problem).
>> On any real computer, always possible as a real computer has a finite
>> number
>> of states N, and will either halt in less than N cycles or never halt.
>> _______________________________________________
>> 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".
>
>
>
> --
>
> Nikolas Bowe |  SWE |  nbowe at google.com |  408-565-5137
>


-- 

Nikolas Bowe |  SWE |  nbowe at google.com |  408-565-5137
-------------- next part --------------
A non-text attachment was scrubbed...
Name: jpeg_with_mpeg_start_codes
Type: application/octet-stream
Size: 9440 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190906/253831e3/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Fix-JPEGs-being-misidentified-as-mpeg.patch
Type: text/x-patch
Size: 1964 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190906/253831e3/attachment.bin>


More information about the ffmpeg-devel mailing list