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

Anssi Hannula anssi.hannula
Mon Feb 14 16:00:25 CET 2011


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.

[1]
http://samples.ffmpeg.org/V-codecs/h264/hdtv-interlaced/sky_720p_test_why-cant-i-overwrite.ts

-- 
Anssi Hannula



More information about the ffmpeg-devel mailing list