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

[Michael Niedermayer]

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
2. the first method should only be used if the resulting frame rate is within
   sane limits ( < 1000fps maybe)
3. if the header values are reliable tb_unreliable() should say so

2. is a one line change

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

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- 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/20110129/7b8a04d8/attachment.pgp>