[FFmpeg-devel,05/17] lavfi: add ff_inlink_process_commands().

Signed-off-by: Nicolas George <george@nsup.org>
---
 libavfilter/avfilter.c | 24 +++++++++++++++---------
 libavfilter/filters.h  |  6 ++++++
 2 files changed, 21 insertions(+), 9 deletions(-)


Changes in this commit: rename ff_link -> ff_inlink and move to filters.h.

On Thu, Dec 29, 2016 at 03:33:51PM +0100, Nicolas George wrote:
> Signed-off-by: Nicolas George <george@nsup.org>
> ---
>  libavfilter/avfilter.c | 24 +++++++++++++++---------
>  libavfilter/filters.h  |  6 ++++++
>  2 files changed, 21 insertions(+), 9 deletions(-)
> 
> 
> Changes in this commit: rename ff_link -> ff_inlink and move to filters.h.
> 
> 
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index 2c41ea8c22..2fe8b980e0 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -1099,7 +1099,6 @@ static int ff_filter_frame_framed(AVFilterLink *link, AVFrame *frame)
>      AVFilterContext *dstctx = link->dst;
>      AVFilterPad *dst = link->dstpad;
>      int ret;
> -    AVFilterCommand *cmd= link->dst->command_queue;
>      int64_t pts;
>  
>      if (!(filter_frame = dst->filter_frame))
> @@ -1111,14 +1110,7 @@ static int ff_filter_frame_framed(AVFilterLink *link, AVFrame *frame)
>              goto fail;
>      }
>  
> -    while(cmd && cmd->time <= frame->pts * av_q2d(link->time_base)){
> -        av_log(link->dst, AV_LOG_DEBUG,
> -               "Processing command time:%f command:%s arg:%s\n",
> -               cmd->time, cmd->command, cmd->arg);
> -        avfilter_process_command(link->dst, cmd->command, cmd->arg, 0, 0, cmd->flags);
> -        ff_command_queue_pop(link->dst);
> -        cmd= link->dst->command_queue;
> -    }
> +    ff_inlink_process_commands(link, frame);
>  
>      pts = frame->pts;
>      if (dstctx->enable_str) {
> @@ -1565,6 +1557,20 @@ int ff_inlink_make_frame_writable(AVFilterLink *link, AVFrame **rframe)
>      return 0;
>  }
>  
> +void ff_inlink_process_commands(AVFilterLink *link, const AVFrame *frame)
> +{
> +    AVFilterCommand *cmd = link->dst->command_queue;
> +
> +    while(cmd && cmd->time <= frame->pts * av_q2d(link->time_base)){
> +        av_log(link->dst, AV_LOG_DEBUG,
> +               "Processing command time:%f command:%s arg:%s\n",
> +               cmd->time, cmd->command, cmd->arg);
> +        avfilter_process_command(link->dst, cmd->command, cmd->arg, 0, 0, cmd->flags);
> +        ff_command_queue_pop(link->dst);
> +        cmd= link->dst->command_queue;
> +    }
> +}
> +
>  const AVClass *avfilter_get_class(void)
>  {
>      return &avfilter_class;
> diff --git a/libavfilter/filters.h b/libavfilter/filters.h
> index 543f4df680..efbef2918d 100644
> --- a/libavfilter/filters.h
> +++ b/libavfilter/filters.h
> @@ -33,6 +33,12 @@
>  void ff_filter_set_ready(AVFilterContext *filter, unsigned priority);
>  
>  /**
> + * Process the commands queued in the link up to the time of the frame.
> + * Commands will trigger the process_command() callback.
> + */
> +void ff_inlink_process_commands(AVFilterLink *link, const AVFrame *frame);

avfilter_process_command() has a return code, should this be checked
or passed on somehow ?

gut feeling but, if the only thing from the frame that is used is the
pts, it may be more flexible to pass the pts instead of the frame.


[...]

Le nonidi 9 nivôse, an CCXXV, Michael Niedermayer a écrit :
> avfilter_process_command() has a return code, should this be checked
> or passed on somehow ?

Currently, the return code of avfilter_process_command() is ignored
here. Since this is a refactoring commit, I do not want to introduce
functional changes in it. I added a return code to
ff_inlink_process_commands() for future convenience, but for now it is
always 0 and ignored by the caller.

Using the return codes can be added later; I expect the number of call
sites will stay very small for a long time.

Actually, I am not even sure what to do with the return code. Discard
the corresponding frame? Fail immediately? Print a warning? Queue the
return code in the link? The semantic has not been well defined, and it
is quite orthogonal to the changes I am working on.

> gut feeling but, if the only thing from the frame that is used is the
> pts, it may be more flexible to pass the pts instead of the frame.

I wanted to keep the symmetry with ff_inlink_process_timeline(), and
that one already uses another field of the frame as a variable for the
expression, and could use more.

As long as the functions are ff_* instead of av_* and there are only a
few call sites, changing one way or the other is very easy, so I propose
to leave it that way for now.

Regards,