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

Tue Feb 15 04:31:22 CET 2011

On Fri, Feb 04, 2011 at 11:54:42AM +1100, Peter Ross wrote:
> On Thu, Feb 03, 2011 at 11:15:33AM +0100, Michael Niedermayer wrote:
> > On Thu, Feb 03, 2011 at 01:37:38AM +1100, Peter Ross wrote:
> > > 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
> > 
> > where can i find these files ?
> > They dont seem to be in http://samples.mplayerhq.hu/allsamples.txt
> 
> Files here:
> upload.mplayerhq.hu /incoming/wtv-incorrect-r_frame_rate

should be fixed in ffmpeg at videolan

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

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- 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/20110215/5d7c5f2a/attachment.pgp>