[FFmpeg-devel] [PATCH] lavf: inspect more frames when the time base is unreliable

[Måns Rullgård]

Anssi Hannula <anssi.hannula at iki.fi> writes:

> On 29.01.2011 15:41, M?ns Rullg?rd wrote:
>> Anssi Hannula <anssi.hannula at iki.fi> writes:
>> 
>>> On 29.01.2011 06:45, M?ns Rullg?rd wrote:
>>>> Anssi Hannula <anssi.hannula at iki.fi> writes:
>>>>
>>>>> 23.976 fps H.264 Matroska files are usually detected as 24 fps
>>>>> (r_frame_rate) by av_find_stream_info(). Fix that by inspecting the
>>>>> timestamps of 30 frames instead of just 20 for video codecs with
>>>>> unreliable time base.
>>>>
>>>> In my experience most matroska files have correct timing information
>>>> in the headers.  It sounds me more like the "clever" estimation code
>>>> is hallucinating inaccuracies where none exist.
>>>
>>> Yes, the timing information is correct in the headers.
>>>
>>> The timing estimation code is triggered simply because codec_id ==
>>> CODEC_ID_H264,
>> 
>> So remove that ugly hack, and all will be fine.
>
> Isn't the hack there for an actual reason?
>
> For the record, here's the commit that added it:
> http://git.ffmpeg.org/?p=ffmpeg.git;a=commit;h=7f123e7f8ad2cfd4ab7e2fbd83e17d0a8b436785

That commit message isn't exactly informative, though it hints at
something to do with telecine.  Telecined video can be easily detected
by parsing a couple of frame headers looking for a repeat first field
flag.  There is no need to play guessing games with timestamps.

>>> and the timings simply can't be estimated correctly in just 20
>>> frames by the current algorithm with only millisecond-precision
>>> timestamps.
>> 
>> Nor should they.  The frame duration from the header should be used.
>
> Well, I see that for normal non-MPEG2/non-H264 files we use the
> framerate calculated from the codec time base (as below). Do you think
> using that would be ok for H.264 as well (at least if the first 5 or so
> timestamps seem to match that value, assuming H264 is considered
> unreliable for a reason), or if we should add some new case for using
> the format headers instead.
>
> For the files in question the codec time base shows an fps of 24/1.001
> exactly.

So the container header is correct, the elementary stream header is
correct, and lavf still manages to arrive at a wrong value.  Well done!

-- 
M?ns Rullg?rd
mans at mansr.com