[FFmpeg-devel] [PATCH 06/17] lavfi: add ff_inlink_process_timeline().

[Michael Niedermayer]

On Fri, Dec 30, 2016 at 12:37:41PM +0100, Nicolas George wrote:
> Le nonidi 9 nivôse, an CCXXV, Michael Niedermayer a écrit :
> > maybe the enabledness value should be returned by the function (too)
> 
> It does not seem very useful at first glance, but I may be missing
> something.

// do some statistics, whatever
...
if (ff_inlink_evaluate_timeline_at_frame()) {
    process frame
}
pass output on


If we imagine a filter that processes a series of frames, lets say
for motion estimation or deinterlacing then the point at which it
becomes enabled may need some processing (or storage) to have occured
on past frames

i find it cleaner to have the effect of a function be its return
value instead of it primarly writing a value into some structure field



> 
> > how does this work with multiple inlinks ?
> > (dstctx / dstctx->enable / dstctx->is_disabled would be the same)
> 
> Currently, if I read the code correctly, only filters with a single
> input support timeline enable/disable.
> 
> In fact, I do not see how timeline could make sense for a filter with
> several inputs. "Disabling" a filter means replacing it by the
> pass-through filter "null", but it has a single input and output.
> 

> > Also whats your oppinion about calling this
> > ff_inlink_evaluate_timeline_at_frame
> > or
> > ff_inlink_evaluate_timeline_at
> > ?
> 
> I have no strong feeling about it one way or the other, I just find the
> symmetry ff_inlink_process_timeline() / ff_inlink_process_commands()
> nice.

i think i didnt explain why i suggested this.
to me "process" sounds a bit unspecified and unclear. as in
what does processing a timeline mean ? processing the whole timeline?
what would that do ?
while "evaluate_timeline_at" avoids this ambiguity.
Iam not attached to the specific words, maybe theres a better term

ff_inlink_is_enabled_at() maybe


> 
> I have addressed your remarks about the documentation in my work tree. I
> do not think it is worth sending the whole series again just for that,
> but here are the docs in case they make understanding the rest easier.
> It repeats a bit what is explained in the new doxy for activate().
> 
> Note that I also changed ff_inlink_acknowledge_status() to return the
> link's timestamp, currently ignored but will be useful soon (vf_fps
> typically), and it avoids filters accessing the link directly (better
> for threading later).
> 
> Also note that they are referring to the activate callback before it is
> introduced. I can move the "lavfi: add AVFilter.activate" before if it
> is a concern.
> 
> /**
>  * Mark a filter ready and schedule it for activation.
>  *
>  * This is automatically done when something happens to the filter (queued
>  * frame, status change, request on output).
>  * Filters implementing the activate callback can call it directly to
>  * perform one more round of processing later.
>  * It is also useful for filters reacting to external or asynchronous
>  * events.
>  */
> void ff_filter_set_ready(AVFilterContext *filter, unsigned priority);

if the only use of this (from a filter) is to call it to get activate()
re-called, just an idea but
what would be your oppinion on using a return code from activate() ?
E"iam not done, call me again later"

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161230/32b87748/attachment.sig>