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

[Dale Curtis]

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) {
             st->codec->codec_type = AVMEDIA_TYPE_AUDIO;
             st->codec->sample_rate = track->audio.out_samplerate;