[FFmpeg-devel] [PATCH] Revert "matroskadec: don't set codec timebase."

[Michael Niedermayer]

On Mon, Apr 16, 2012 at 11:02:53AM -0700, Dale Curtis wrote:
> On Sat, Apr 14, 2012 at 1:17 AM, Michael Niedermayer <michaelni at gmx.at>wrote:
> 
> > On Fri, Apr 13, 2012 at 05:10:49PM -0700, Dale Curtis wrote:
> > > On Fri, Apr 13, 2012 at 5:39 AM, Michael Niedermayer <michaelni at gmx.at
> > >wrote:
> > >
> > > > On Thu, Apr 12, 2012 at 05:26:23PM -0700, dalecurtis at chromium.orgwrote:
> > > > > From: Dale Curtis <dalecurtis at chromium.org>
> > > > >
> > > > > This commit introduced a regression in the amount of upfront
> > > > > data required to identify and parse a webm container.  Most
> > > > > visible under constrained networks: http://goo.gl/isfLc
> > > >
> > > > Setting r_frame_rate to avg_frame_rate should fix this
> > > >
> > >
> > >
> > > Can you elaborate on this? To be more clear, the time regression is in
> > > avformat_find_stream_info which occurs before either of those values are
> > > usable.
> >
> > yes, the code in avformat_find_stream_info() that causes the delay
> > is calculating r_frame_rate. If you set r_frame_rate=avg_frame_rate
> > in the demuxer, the code will belive thats a sufficient estimate of
> > r_frame_rate and use it without the delay causing computation.
> > Similarly if you set the timebase to a value that is a beliveable
> > 1/frame rate value and the heuristic has no reason to distrust this
> > value then the code will use that instead of collecting 20-
> > 40 frames and inspecting their timestamps to find the r_frame_rate.
> > its no magic, the code just tries its best to provide the user with
> > the requested information (fps_probe_size > 0 indicates the user wants
> > this information)
> >
> >
> > >
> > > After discussion on libav, I found that setting fps_probe_size = 0
> > resolves
> > > the time regression in avformat_find_stream_info as well.
> >
> > sure, that disables the computation of r_frame_rate
> > and i have to note btw, the commit that caused the regression:
> > commit c98c1f434eed06390f4990dd23f7ec15dbe53703
> > originates from libav.
> >
> >
> > >
> > > To note, the issue does not show up in master ffmpeg/libav though because
> > > w/o Chrome's patch to add incremental parsing of matroska clusters
> > > find_stream_info takes forever parsing the whole first cluster anyways.
> > > Some numbers:
> > >
> > > find_stream_info w/o incremental parsing and w/o default_duration:
> > 21054ms
> > > (inline with avplay/ffplay)
> > > find_stream_info w/ incremental parsing and w/o default_duration: 6683ms
> > > find_stream_info w/ incremental parsing and w/ default_duration: 1026ms
> >
> > with your incremental patch and without setting r_frame_rate:
> > real    0m1.086s
> > find_stream_info reads a bit over 3mb
> >
> > with your incremental patch and with setting r_frame_rate:
> > real    0m0.535s
> > find_stream_info reads a bit less than 400kb
> >
> > thats with a random mkv file i had laying around
> > (nausicaa_oldx264_hi10p_greenish_sample.mkv)
> > and ill admit thats locally from my harddisk so its a bit faster than
> > over a normal network but the issue is still vissible
> 
> 
> Thanks for the explanation! To be clear you're talking about making the
> patch below instead? Testing using our constrained network tools shows
> similar results to the numbers you collected above.
> 
> - dale
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index e922c6e..f9865d1 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -1587,7 +1587,7 @@ static int matroska_read_header(AVFormatContext *s)
>              if (st->codec->codec_id != CODEC_ID_H264)
>              st->need_parsing = AVSTREAM_PARSE_HEADERS;
>              if (track->default_duration)
> -                st->avg_frame_rate =
> av_d2q(1000000000.0/track->default_duration, INT_MAX);
> +                st->r_frame_rate = st->avg_frame_rate =
> av_d2q(1000000000.0/track->default_duration, INT_MAX);
>          } else if (track->type == MATROSKA_TRACK_TYPE_AUDIO) {

thats not looking like git master but, yes thats approximately
the idea. the INT_MAX will probably need some adjustment though

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

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120417/7803c41f/attachment.asc>