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
> > > <stebb...@jetheaddev.com> wrote:
> > > > From: Nicolas George <nicolas.geo...@normalesup.org>
> > > >
> > > > (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.
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
