On Wed, Oct 28, 2015 at 5:24 AM, John Stebbins <[email protected]> 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
sure but we're going to edit this patch, so the hash will be even more
meanlingless
>
>>> ---
>>> 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.
sure
>>> + * 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 r_frae_rate field in AVStream.
>>
>> that field has been absent for a long time, I suppose it refers to
>> avg_frame_rate now
>
>
> It's an old comment. This code was introduced in 2012.
yes, it doesn't make sense to commit code and then apply patch on top
it for typos and whatnot -- let's modify the patch directly and avoid
commit history noise
>>> + */
>>> + AVRational frame_rate;
>>> };
>>
>> question, would it be simpler or is it possible at all to avoid adding
>> a new field and use time_base to calculate framerate when needed
>> instead?
>>
>> thanks
>
>
> No. time_base != 1 / frame_rate. For example, the time_base in HandBrake
> is 1/90000.
> This is what we initialize the time_base to in buffersrc. You need a
> time_base such as
> this to represent variable frame rate timestamps for frames. The filter
> chain can modify
> the time_base to make it == 1 / frame_rate in special cases (e.g. vf_fps),
> but in general
> they are completely different things.
yes i keep forgetting about vfr (self prevervaiton?)
thanks for clearing that out
--
Vittorio
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
