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.

> 
> Also, this needs a bump+APIchanges (disregard this comment if this
> happens in some later patch I didn't see yet).
> 

Yes, I was going to do this and then forgot.  Thanks for the reminder.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to