[FFmpeg-devel] [PATCH 0/2] increase fps detection accuracy in wtv demuxer

[Peter Ross]

On Sat, Jan 29, 2011 at 08:21:23PM +0100, Michael Niedermayer wrote:
> On Fri, Jan 28, 2011 at 02:53:12PM +1100, Peter Ross wrote:
> > On Thu, Jan 27, 2011 at 09:25:12AM -0500, Ronald S. Bultje wrote:
> > > Hi,
> > >
> > > 2011/1/27 M?ns Rullg?rd <mans at mansr.com>:
> > > > Peter Ross <pross at xvid.org> writes:
> > > >> On Wed, Jan 26, 2011 at 12:52:28PM -0500, Ronald S. Bultje wrote:
> > > >>> On Tue, Jan 25, 2011 at 5:40 AM, Peter Ross <pross at xvid.org> wrote:
> > > >>> > [initial stab at git-send-email]
> > > >>> >
> > > >>> > FFmpeg currently treats the fps information within MPEG2 and H264
> > > >>> > video stream headers as unreliable, because many encoders put bogus
> > > >>> > data in the headers. For streams containg these codecs, libavformat
> > > >>> > chooses to caculate the fps using dts deltas.
> > > >>> >
> > > >>> > In WTV files, the pts values are often sparse, and/or shaky for the
> > > >>> > first few seconds of video. This results in an incorrect fps value to
> > > >>> > be caculated. My solution is to introduce CODEC_FLAG2_TIMEBASE_RELIABLE,
> > > >>> > and flag streams as having reliable header information.
> > > >>> >
> > > >>> > Patches threaded below. Other ideas welcome..
> > > >>>
> > > >>> Disregard my other email, let's discuss this here. Shaky timestamps =
> > > >>> unreliable timestamp information. Are you saying the base is OK but
> > > >>> the timestamps themselves are not? I'm unconfomfortable with adding
> > > >>> hacks to circumvent other hacks that should fix bugs but don't.
> > > >>
> > > >> Hi Ronald, the symptom is an incorrect AVStream.r_frame_rate value, often in
> > > >> the kilo-frames-per-second range.
> > > >>
> > > >> As bit of background, the wtv file format sometimes provides PTS values.
> > > >> Many packets output by the demuxer have no PTS.
> > > >
> > > > How does it end up with that ridiculous value in the first place? ?IMO
> > > > that should be fixed, not patched up afterwards.
> >
> > Agree.
> >
> > > I'm all with this, try not having that value to begin with, and then
> > > maybe we don't need this hack. AV_NOPTS_VALUE is meant for this, and
> > > whatever tb_reliable() does should take that into account.
> >
> > av_stream_info() guesses the r_frame_rate value by calculating the gcd of
> > the dts of the first n packets. there are a couple of special rules in
> > the caclulation
> >
> > 1.  relative dts values fed into the gcd calc. they are offset by 'AVStream.info.last_dts'.
> >     last_dts is set to the dts of the second packet having dts > 0
> >     (see libabformat/utils.c lines 2348 ~ 2360 ).
> >
> > 2.  the first four dts values are not fed into the gcd, to avoid jitter.
> >
> > A ridiculous r_frame_rate value occurs when the dts values do not have
> > a common divisor. Lets look at the dts values for some problematic files.
> > Importantly, the dts values are computed; wtv demuxer only supplies pts.
>
> That is not correct
>
> There are several ways by which av_find_stream_info() can calculate the real
> base frame rate (r_frame_rate)
> One method is what you describe and it is used when
>  * there _IS_ a common divisor (2 in this case)
>  * the demuxer did not set a r_frame_rate explixitly
>  * there have been at least 15 frames
>  * tb_unreliable() is true
>
> if any of above are false and
>  * the demuxer did not set a r_frame_rate explixitly
>  * there have been at least 1 frame
>  * tb_unreliable() is true
>
> Then a more robust method is used that though only considers some frame rates
> (which is also part of why its more robust)
>
> if that still didnt set r_frame_rate (aka tb_unreliable() is false or previous
> code failed)
> then the header value / timebase is used for r_frame_rate
>
>
> I think there are a few bugs.
> 1. There is something wrong with the timestamps or start_time or code messing
>    with them

start_time is not set, because it is unknown to the demuxer.

I suspect the the problem is caused by the last_dts var.

last_dts is determined to be the second packet with dts > 0. The first
 four dts > 0 packets are discarded from the gcd calculation, for
reasons of jitter.

But what if the there is jitter in the first four packets?

In both my sample file, the choice of last_dts results in gcd(...)=2.
Choosing the fourth packet with dts>0 as last_dts fixes the problem.

e.g.
Gilmore Girls_Net 5_2011_01_08_16_10_53.wtv, stream #1, dts values
(one packet per line):
 0
 400000 <-- last_dts
 800000
 35232214
 35632214  <-- first dts fed into gcd
 36032214
 36432214

> 2. the first method should only be used if the resulting frame rate is within
>    sane limits ( < 1000fps maybe)

Ok.

> 3. if the header values are reliable tb_unreliable() should say so

The demuxer supplies DVB MPEG2VIDEO and H264. The headers are reliable,
but tb_unreliable() considers any any stream containing MPEG2VIDEO or H264
to be unreliable.

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110203/47e1b346/attachment.pgp>