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

[Anssi Hannula]

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

>> 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.


>             if (!st->r_frame_rate.num){
>                 if(    st->codec->time_base.den * (int64_t)st->time_base.num
>                     <= st->codec->time_base.num * st->codec->ticks_per_frame * (int64_t)st->time_base.den){
>                     st->r_frame_rate.num = st->codec->time_base.den;
>                     st->r_frame_rate.den = st->codec->time_base.num * st->codec->ticks_per_frame;
>                 }else{
>                     st->r_frame_rate.num = st->time_base.den;
>                     st->r_frame_rate.den = st->time_base.num;
>                 }
>             }

-- 
Anssi Hannula