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

Nicolas George george at nsup.org
Sat Dec 31 12:41:06 EET 2016


Le decadi 10 nivôse, an CCXXV, Michael Niedermayer a écrit :
> // 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

That makes sense. Changed. The new code looks like this:

    dstctx->is_disabled = !ff_inlink_evaluate_timeline_at_frame(link, frame);

And the doxy:

/**
 * Evaluate the timeline expression of the link for the time and properties
 * of the frame.
 * @return  >0 if enabled, 0 if disabled
 * @note  It does not update link->dst->is_disabled.
 */
int ff_inlink_evaluate_timeline_at_frame(AVFilterLink *link, const AVFrame *frame);

> > /**
> >  * 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"

I considered this, but I decided against it for the combination of
several minor reasons:

- a return code is more easily lost in a refactoring;

- the function is still needed for the framework (note that it already
  exists, this patch only makes it visible outside avfilter.c);

- the function will be needed for filters that react to external or
  asynchronous events;

- having several mechanisms to do approximatively the same thing is not
  the best idea;

- it does not allow to set a priority (I have not yet documented the
  priorities, but the idea is that processing a frame is more urgent
  than forwarding a request).

> iam tempted to suggest to call this
> ff_inlink_request_frame()
> 
> it would be the better name if ff_request_frame() didnt exist.

I tried to make ff_filter_frame() work for both cases, but it proved too
inconvenient. I renamed it ff_inlink_request_frame(), I do not think the
confusion can cause actual problems. At worst, someone will write
ff_request_frame(), get an assert failure when testing and fix it, the
same could happen as well with the other name.

The code changes are still minor, I will wait a bit before sending the
new series in case there are remarks about patches 12, 13, 14, 16, 17 or
more about the others.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161231/6156ea30/attachment.sig>


More information about the ffmpeg-devel mailing list