[FFmpeg-devel] [PATCH] lavf: accept time base from untrusted codecs if it matches timings

[Michael Niedermayer]

On Tue, Feb 15, 2011 at 07:40:30AM +0200, Anssi Hannula wrote:
> On 15.02.2011 01:54, Michael Niedermayer wrote:
> > On Mon, Feb 14, 2011 at 05:00:25PM +0200, Anssi Hannula wrote:
> >> On 14.02.2011 14:24, Michael Niedermayer wrote:
> >>> On Sun, Feb 13, 2011 at 12:52:35PM +0200, Anssi Hannula wrote:
> >>>> On 06.02.2011 08:32, Anssi Hannula wrote:
> >>>>> Anssi Hannula wrote:
> >>>>>> Here's a new patch that checks the timestamps of the first 4 frames
> >>>>>> (using the same method which is used in the guess-framerate code) and
> >>>>>> uses codec time base for r_frame_rate if the timestamps fit to it.
> >>>>>>
> >>>>>> I tested also with several other files, including H.264 PAFF, MVE
> >>>>>> (ipmovie.c), and spotted no regressions.
> >>>>>>
> >>>>>> What do you think?
> >>>>>
> >>>>> Ping. Also, I noticed an extra added newline in the patch, now removed.
> >>>>
> >>>> This patch is indeed not enough. The h264 decoder may not discover the
> >>>> time base immediately from the first few frames, so st->codec->time_base
> >>>> may still be stream time base. So the code will notice from the first 4
> >>>> frames that the stream time base can accurately represent timestamps
> >>>> (which is always true), and sets codec_tb_matches_dts = 1.
> >>>> Since the mpegts timebase is 1/90000 (i.e. too fine to be fps),
> >>>> tb_unreliable() says (correctly) false despite codec_tb_matches_dts==1,
> >>>> so the detection code continues to inspect frames.
> >>>>
> >>>> When the h264 decoder finally sets the codec timebase (e.g. 1/25), it is
> >>>> assumed correct due to codec_tb_matches_dts==1, even if it is not able
> >>>> accurately represent timestamps at all (e.g. 1/50 intervals).
> >>>>
> >>>> At least this sample shows the above issue:
> >>>>
> >>>>
> >>>> So some checks need to be added to the patch to guard against above. Or
> >>>> use some entirely different/better approach :)
> >>>>
> >>>> I'd really appreciate someone more experienced looking at this issue,
> >>>> but I'll take a further look at this myself later as well.
> >>>
> >>> Could you explain elaborately what issue you are trying to fix?
> >>
> >> OK, here's a recap of this thread :)
> >>
> >> - normally lavf assumes that the (1 / codec->time_base /
> >>   codec->ticks_per_frame) is the fps (r_frame_rate), unless it would be
> >>   too high to be represented by the st->time_base, in which case
> >>   (1 / st->time_base) is taken instead (utils.c 2431-2440)
> >>
> >> - if ((1/5) > codec->time_base >= (1/101)) is false, the
> >>   codec->time_base is considered unreliable and instead a custom
> >>   fps probing code is used (tb_unreliable()) which reads the timestamps
> >>   of the first 20 packets
> >>
> >> - some H.264 (and I guess MPEG2) streams have a specific interlacing
> >>   mode that causes there to be 2x more packets (as output from the
> >>   demuxer) than codec->time_base and codec->ticks_per_frame would
> >>   indicate (well, I guess technically they are correct, if the packets
> >>   actually contain half-frames due to interlacing), e.g. [1]
> >>
> >> - due to the above, H264 and MPEG2 are always assumed to have unreliable
> >>   timebase and the fps probing code is always used
> >>
> >> - mkv tracks have generally millisecond precision for timestamps
> >>
> >> - 23.976fps therefore requires a pattern of 41ms and 42ms frames, that
> >>   add up to 1.001s in 24 frames
> >>
> >> - the fps probing code doesn't detect the difference between 24fps and
> >>   23.976fps from just 20 frames, it would need more than that (25-30)
> >>   => 23.976fps files are wrongly detected as 24fps
> >>
> >> - 23.976fps files are progressive (unless insane), so the codec timebase
> >>   as got from the decoder would actually be reliable and show an exact
> >>   rate of 24/1.001.
> >>
> >> - mkv tracks also contain a default_duration field that contains the
> >>   length of frames in nanosecond precision. In the file I checked it
> >>   was accurate to within 1.5ns (some rounding issue I guess), which
> >>   corresponds to an error of about 0.000001 fps.
> >>
> > 
> >> So the issue is that 23.976 h264 mkv files are detected as having wrong
> >> r_frame_rate of 24.
> > 
> > i need a sample to look into this
> > (and sorry if a url was posted and i missed it)
>

> Well, here's one:
> http://onse.fi/files/h264-23.976-detected-as-24.mkv

this has been fixed a while ago

if you have other files that failed and still fail, iam interrsted in
them


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

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111006/7ab2a6e0/attachment.asc>