On Thu, 2015-10-29 at 20:12 +0100, Anton Khirnov wrote: > Quoting John Stebbins (2015-10-29 19:59:44) > > On Thu, 2015-10-29 at 19:47 +0100, Anton Khirnov wrote: > > > Quoting John Stebbins (2015-10-28 17:48:17) > > > > From: Nicolas George <[email protected]> > > > > > > > > (cherry picked from ffmpeg commit > > > > 7b42036b3b23c85f473bf9369e37fa8da22eaf93) > > > > --- > > > > libavfilter/avfilter.c | 2 ++ > > > > libavfilter/avfilter.h | 12 ++++++++++++ > > > > 2 files changed, 14 insertions(+) > > > > > > > > diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c > > > > index 64b2645..cd98d16 100644 > > > > --- a/libavfilter/avfilter.c > > > > +++ b/libavfilter/avfilter.c > > > > @@ -195,6 +195,8 @@ int avfilter_config_links(AVFilterContext > > > > *filter) > > > > link->src->inputs[0] > > > > ->sample_aspect_ratio : > > > > (AVRational){1,1}; > > > > > > > > if (link->src->nb_inputs) { > > > > + if (!link->frame_rate.num && !link > > > > ->frame_rate.den) > > > > + link->frame_rate = link->src->inputs[0] > > > > ->frame_rate; > > > > if (!link->w) > > > > link->w = link->src->inputs[0]->w; > > > > if (!link->h) > > > > diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h > > > > index 9dbfeea..b7b97ce 100644 > > > > --- a/libavfilter/avfilter.h > > > > +++ b/libavfilter/avfilter.h > > > > @@ -375,6 +375,18 @@ struct AVFilterLink { > > > > AVLINK_STARTINIT, ///< started, but incomplete > > > > AVLINK_INIT ///< complete > > > > } init_state; > > > > + > > > > + /** > > > > + * Frame rate of the stream on the link, or 1/0 if unknown; > > > > + * if left to 0/0, will be automatically be copied from the > > > > first input > > > > + * of the source filter if it exists. > > > > + * > > > > + * Sources should set it to the best estimation of the real > > > > frame rate. > > > > + * Filters should update it if necessary depending on their > > > > function. > > > > + * Sinks can use it to set a default output frame rate. > > > > + * It is similar to the avg_frame_rate field in AVStream. > > > > + */ > > > > > > Please no! No guesses or estimates, we already have plenty of this > > > crap > > > in lavf and we want to get rid of it, not add more. VFR streams > > > with > > > no > > > such thing as "real frame rate" exist. > > > > > > So IMO this should be: > > > - if AND ONLY if the stream is known to be of some specific > > > constant > > > framerate, this field should be set to that number > > > - in ALL OTHER CASES, this field should be set to 0/0 (or whatever > > > other > > > invalid value) and treated as VFR. > > > > An actual framerate has to be written at the muxing stage by a > > transcoder such as HandBrake. We can't know how the filter chain > > will > > affect our nominal framerate without passing the nominal value to > > the > > input of the filter chain and seeing what comes out the other side. > > So > > although nominal framerates are a PITA, they are a requirement we > > can't > > ignore. > > > > Although HandBrake generates output that is technically variable > > framerate by default (i.e. vfr flags are set in h.264 headers), it > > is > > often the case that the measured framerate is constant throughout > > the > > stream. There is just no way to know this in advance with certain > > source types (e.g. DVD). > > > > We could document this as you prefer as a means of discouraging this > > kind of usage. But this is a real world case that is going to > > happen. > > If you really need some kind of a framerate, you could e.g. pass two > dummy frames through the filtergraph and check the pts difference on > the > output. Would that be a lot more work for you? > > I recall you complaining about the lavf timestamps mess yourself. And > one of the main reasons it is so horrible is precisely that it somehow > makes up some guesses or estimates and then yet more guesses on top of > that until we have no clue what are real data and what is made up. I > would really like to avoid that in any new code, if reasonably > possible. >
In theory, HandBrake could calculate the average framerate over the entire video (or keep track of the framerate of the majority of the video) and update muxer header information before finalizing the file. Headers for many formats are not actually written till muxing is complete anyway. But this would rarely result in a different value than what we estimate up front. We initially sample the source at various locations to collect statistics about several properties of the source prior to transcoding. So our estimates are pretty accurate. Probably more accurate that running the first few frames through the filter chain. I see your point about the timestamp mess. But I think this case is (or should be) a little different. There are no filters that directly use this frame_rate information yet. It is informational and not used to calculate timestamps. The only case I can think of where a filter might want to use the input link's frame_rate is if the filter needs to insert extra frames or duplicate frames *and* the filter is not required to generate constant framerate output. In which case it's going to need to know *something* about the nominal input frame_rate in order to do it's function. Such a filter would have to fail initialization if given an unknown frame_rate. _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
