On Wed, 2015-10-28 at 08:14 -0700, John Stebbins wrote: > On Wed, 2015-10-28 at 11:31 +0100, Luca Barbato wrote: > > On 28/10/15 05:24, John Stebbins wrote: > > > On 10/27/2015 07:51 PM, Vittorio Giovara wrote: > > > > On Tue, Oct 27, 2015 at 9:17 PM, John Stebbins > > > > <[email protected]> wrote: > > > > > From: Nicolas George <[email protected]> > > > > > > > > > > (cherry picked from commit > > > > > 7b42036b3b23c85f473bf9369e37fa8da22eaf93) > > > > (this comment is valid for the next patches too) there is no > > > > such > > > > hash > > > > in the libav tree, you should either mention its a ffmpeg hash > > > > or > > > > just > > > > remove it since the commit name is unchanged. > > > > > > I can amend the comments, but most people know where these are > > > being > > > cherry picked from > > > > > > > > --- > > > > > 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..0b670e0 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; > > > > would 0/1 be safer? also where is it initialized? > > > > > > This comment seems to be misleading. Any filter that > > > doesn't explicitly override frame_rate passes input frame_rate > > > along > > > to the next stage in the filter chain. If it's not set in > > > buffersrc, > > > it's > > > value is 0/0 and that just gets passed through the chain (until > > > some > > > filter sets the frame_rate or it reaches buffersink). > > > > > > IMO the whole initial bit about "unknown" value should just > > > be omitted. > > > > Since the set got scattered I couldn't follow and I assumed that > > would > > be implemented as documented. > > > > If is not please get an updated set and then lets think again what > > that > > should be. > > > > Ideally makes sense to build a filter chain that preserves the frame > > rate and forwards it if it is known (since then some rate controls > > could > > use it), but also would be beneficial forward the information that > > the > > data doesn't have a frame rate, even a nominal one. > > > > > > I will address current comments and post the patches again. > But I'm not quite sure what you are asking for. The code as it stands > forwards whatever value frame_rate has through the filter chain. > frame_rate is initialized to 0/0 when the AVFilterLink is av_mallocz'd > and it can be initialized by buffersrc which sets a default value of > 0/0. > > So if you do not explicitly set frame_rate in buffersrc and no filter > converts to a specific frame_rate (e.g. vf_fps), you will get 0/0 at > the > end of the filter chain. There are filters (interlace, framepack, and > yadif) that multiply input frame_rate.num or frame_rate.den by 2, but > if > input frame_rate is 0/0, it will still be 0/0 at the end of the filter > chain. > > Maybe you are just asking to keep the comment, but correct it to say > "or > 0/0 if unknown"? That would make sense. >
Sorry, I'm being dense. I see what you mean. The comment currently implies that if you explicitly set frame_rate to 1/0, nobody in the filter chain will modify it. The code obviously does not do this and I see no attempt in the ffmpeg code to try to preserve 1/0 in any way. In addition, there are few filters or sources where you can explicitly set the frame_rate, and all of these with the exception of buffersrc require a valid framerate (i.e. they will not accept 1/0). It seems easier to me to define 0/0 as the "unknown" value because this *is* preserved by the code as it currently stands. There is no filter aside from filters that convert to a known constant frame_rate that will modify a value of 0/0. _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
