[FFmpeg-devel] [PATCH] Command passing interface for libavfilter

Stefano Sabatini stefano.sabatini-lala at poste.it
Mon Aug 29 16:57:10 CEST 2011


On date Monday 2011-08-29 00:45:13 +0200, Michael Niedermayer encoded:
> Hi
> 
> the 5 patches below allow changing drawtext parameters at runtime
> with ffmpeg. And add a interface to pass commands to filters and
> their responses back. As well as queing commands for specific times
> 
> I will apply this soon if i see no objections
> Also cosmetic cleanup is very welcome as commits on top of mine after
> i push it

Style fixes (if(foo){} -> if (foo) {}; foo= bar; -> foo = bar;) are
welcome although I won't insist on it, but will improve your karma.

I already noticed that I find disturbing/distracting mixed style when
reading the code on which I work. If you won't, I'll fix it later on
the part of code which I often read.

> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> I have often repented speaking, but never of holding my tongue.
> -- Xenocrates

> From 074cda2ba4c10e39b7230a9030475c230a8336f8 Mon Sep 17 00:00:00 2001
> From: Michael Niedermayer <michaelni at gmx.at>
> Date: Sun, 28 Aug 2011 20:46:31 +0200
> Subject: [PATCH 1/5] avfilter: Add command passing support
> 
> Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> ---
>  libavfilter/avfilter.c      |   12 ++++++++++++
>  libavfilter/avfilter.h      |   22 +++++++++++++++++++++-
>  libavfilter/avfiltergraph.c |   30 ++++++++++++++++++++++++++++++
>  libavfilter/avfiltergraph.h |   20 ++++++++++++++++++++
>  4 files changed, 83 insertions(+), 1 deletions(-)
> 
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index 23bb26c..5c293dd 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -26,6 +26,7 @@
>  #include "libavutil/audioconvert.h"
>  #include "libavutil/imgutils.h"
>  #include "libavutil/avassert.h"
> +#include "libavutil/avstring.h"
>  #include "avfilter.h"
>  #include "internal.h"
>  
> @@ -616,6 +617,17 @@ void avfilter_draw_slice(AVFilterLink *link, int y, int h, int slice_dir)
>      draw_slice(link, y, h, slice_dir);
>  }
>  
> +int avfilter_command(AVFilterContext *filter, const char *cmd, const char *arg, char *ret, int ret_len, int flags)

"avfilter_process_command" looks less confusing to me, basically
"command" is asking a filter to process a command with given
arguments.

"command" alone doesn't tell if the function is "sending" or
"processing" a command.

Nit: ret     -> res
     ret_len -> res_len 

for avoiding ret/res conflicts in the code (ret is usually the return code)

> +{
> +    if(!strcmp(cmd, "ping")){
> +        av_strlcatf(ret, ret_len, "pong from:%s %s\n", filter->filter->name, filter->name);
> +        return 0;
> +    }else if(filter->filter->command) {
> +        return filter->filter->command(filter, cmd, arg, ret, ret_len, flags);
> +    }
> +    return AVERROR(ENOSYS);
> +}
> +
>  void avfilter_filter_samples(AVFilterLink *link, AVFilterBufferRef *samplesref)
>  {
>      void (*filter_samples)(AVFilterLink *, AVFilterBufferRef *);
> diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
> index db6ff6d..4a8c436 100644
> --- a/libavfilter/avfilter.h
> +++ b/libavfilter/avfilter.h
> @@ -29,7 +29,7 @@
>  #include "libavutil/rational.h"
>  
>  #define LIBAVFILTER_VERSION_MAJOR  2
> -#define LIBAVFILTER_VERSION_MINOR 35
> +#define LIBAVFILTER_VERSION_MINOR 36
>  #define LIBAVFILTER_VERSION_MICRO  0
>  
>  #define LIBAVFILTER_VERSION_INT AV_VERSION_INT(LIBAVFILTER_VERSION_MAJOR, \
> @@ -552,6 +552,20 @@ typedef struct AVFilter {
>       * NULL_IF_CONFIG_SMALL() macro to define it.
>       */
>      const char *description;
> +    
> +    /**
> +     * Send a command to this filter instance.

What about: make the filter instance process/execute a command.

> +     *
> +     * @param cmd    the command to sent, for handling simplicity all commands must be alphanummeric only

typos: "to sent", alphanummeric

> +     * @param arg    the argument for the command
> +     * @param ret    An array where the filter(s) can return a response. This must not change when the command is not supported.
> +     * @param flags  if AVFILTER_CMD_FLAG_FAST is set and the command would be
> +     *               timeconsuming then a filter should treat it like an unsupported command
> +     * 
> +     * @returns >=0 on success otherwise an error code.
> +     *          AVERROR(ENOSYS) on unsupported commands
> +     */

> +    int (*command)(AVFilterContext *, const char *cmd, const char *arg, char *ret, int ret_len, int flags);

ditto: process_command should be less confusing

>  } AVFilter;
>  
>  /** An instance of a filter */
> @@ -792,6 +806,12 @@ void avfilter_end_frame(AVFilterLink *link);
>  void avfilter_draw_slice(AVFilterLink *link, int y, int h, int slice_dir);
>  

>  /**
> + * Send a command to a single filter

Ditto.

> + * It is recommanded to use avfilter_graph_send_command()
> + */
> +int avfilter_command(AVFilterContext *filter, const char *cmd, const char *arg, char *ret, int ret_len, int flags);
> +
> +/**
>   * Send a buffer of audio samples to the next filter.
>   *
>   * @param link       the output link over which the audio samples are being sent
> diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
> index 8756e42..486b0df 100644
> --- a/libavfilter/avfiltergraph.c
> +++ b/libavfilter/avfiltergraph.c
> @@ -253,3 +253,33 @@ int avfilter_graph_config(AVFilterGraph *graphctx, void *log_ctx)
>  
>      return 0;
>  }
> +

> +int avfilter_graph_send_command(AVFilterGraph *graph, const char *target, const char *cmd, const char *arg, char *ret, int ret_len, int flags)

Ditto: please s/ret/res/

> +{
> +    int i, r=AVERROR(ENOSYS);
> +
> +    if(!graph)
> +        return r;
> +
> +    if((flags & AVFILTER_CMD_FLAG_ONE) && !(flags & AVFILTER_CMD_FLAG_FAST)){
> +        r=avfilter_graph_send_command(graph, target, cmd, arg, ret, ret_len, flags | AVFILTER_CMD_FLAG_FAST);
> +        if(r != AVERROR(ENOSYS))
> +            return r;
> +    }
> +
> +    if(ret_len && ret)
> +        ret[0]= 0;
> +
> +    for (i = 0; i < graph->filter_count; i++) {
> +        AVFilterContext *filter = graph->filters[i];
> +        if(!strcmp(target, "all") || !strcmp(target, filter->name) || !strcmp(target, filter->filter->name)){
> +            r= avfilter_command(filter, cmd, arg, ret, ret_len, flags);
> +            if(r!=AVERROR(ENOSYS)){
> +                if((flags & AVFILTER_CMD_FLAG_ONE) || r<0)
> +                    return r;
> +            }
> +        }
> +    }
> +
> +    return r;
> +}
> diff --git a/libavfilter/avfiltergraph.h b/libavfilter/avfiltergraph.h
> index f4c88bc..e668104 100644
> --- a/libavfilter/avfiltergraph.h
> +++ b/libavfilter/avfiltergraph.h
> @@ -136,4 +136,24 @@ int avfilter_graph_parse(AVFilterGraph *graph, const char *filters,
>                           AVFilterInOut **inputs, AVFilterInOut **outputs,
>                           void *log_ctx);
>  
> +#define AVFILTER_CMD_FLAG_ONE   1 ///< Stop once a filter understood the command (for target=all for example), fast filters are favored automatically
> +#define AVFILTER_CMD_FLAG_FAST  2 ///< Only execute command when its fast (like a video out that supports contrast adjustment in hw)
> +
> +/**
> + * Send a command to one or more filter instances.
> + *
> + * @param graph  the filter graph
> + * @param target the filter(s) to which the command should be sent
> + *               "all" sends to all filters
> + *               otherwise it can be a filter or filter instance name
> + *               which will send the command to all matching filters.

> + * @param cmd    the command to sent, for handling simplicity all commands must be alphanummeric only

typo: "to be sent", or "to send", alphanummeric

> + * @param arg    the argument for the command

> + * @param ret    An array where the filter(s) can return a response.

a buffer with size ret_size where the filter(s) can write a response message / return a response

> + *
> + * @returns >=0 on success otherwise an error code.
> + *              AVERROR(ENOSYS) on unsupported commands
> + */
> +int avfilter_graph_send_command(AVFilterGraph *graph, const char *target, const char *cmd, const char *arg, char *ret, int ret_len, int flags);

Same consideration on ret->res

> +
>  #endif /* AVFILTER_AVFILTERGRAPH_H */
> -- 
> 1.7.4.1
> 

> From 0b5e374f779af982c86696d0c621a6d0a8ea9701 Mon Sep 17 00:00:00 2001
> From: Michael Niedermayer <michaelni at gmx.at>
> Date: Sun, 28 Aug 2011 20:47:06 +0200
> Subject: [PATCH 2/5] ffmpeg: Support passing commands to filters at runtime
> 
> Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> ---
>  ffmpeg.c |   17 +++++++++++++++++
>  1 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/ffmpeg.c b/ffmpeg.c
> index 95b3252..e9dbc66 100644
> --- a/ffmpeg.c
> +++ b/ffmpeg.c
> @@ -2681,6 +2681,22 @@ static int transcode(AVFormatContext **output_files,
>                      do_pkt_dump = 1;
>                  av_log_set_level(AV_LOG_DEBUG);
>              }

> +            if (key == 'c' || key == 'C'){
> +                char ret[4096], target[64], cmd[256], arg[256]={0};
> +                fprintf(stderr, "\nEnter command: <target> <command>[ <argument>]\n");
> +                if(scanf("%4095[^\n\r]%*c", ret) == 1 && sscanf(ret, "%63[^ ] %255[^ ] %255[^\n]", target, cmd, arg) >= 2){
> +                    for(i=0;i<nb_ostreams;i++) {
> +                        int r;
> +                        ost = ost_table[i];
> +                        if(ost->graph){
> +                            r= avfilter_graph_send_command(ost->graph, target, cmd, arg, ret, sizeof(ret), key == 'c' ? AVFILTER_CMD_FLAG_ONE : 0);
> +                            fprintf(stderr, "Command reply for %d: %d, %s\n", i, r, ret);
> +                        }
> +                    }
> +                }else{
> +                    fprintf(stderr, "Parse error\n");
> +                }

is this really useful for testing? What's the behavior if you have
many output streams? In this case I suppose the selected graph will be
randomic. Maybe ffplay is a better place for testing this feature.

> +            }
>              if (key == 'd' || key == 'D'){
>                  int debug=0;
>                  if(key == 'D') {
> @@ -2705,6 +2721,7 @@ static int transcode(AVFormatContext **output_files,
>                                  "?      show this help\n"
>                                  "+      increase verbosity\n"
>                                  "-      decrease verbosity\n"
> +                                "c      Send command to filtergraph\n"
>                                  "D      cycle through available debug modes\n"
>                                  "h      dump packets/hex press to cycle through the 3 states\n"
>                                  "q      quit\n"
> -- 
> 1.7.4.1
> 

> From 3c042c804a4478035986ce635cf9d32e14388261 Mon Sep 17 00:00:00 2001
> From: Michael Niedermayer <michaelni at gmx.at>
> Date: Sun, 28 Aug 2011 20:47:33 +0200
> Subject: [PATCH 3/5] drawtext: Support changing parameters through reinit command at runtime.
> 
> Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> ---
>  libavfilter/vf_drawtext.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
> index 7d8372b..a4359bc 100644
> --- a/libavfilter/vf_drawtext.c
> +++ b/libavfilter/vf_drawtext.c
> @@ -401,6 +401,16 @@ static int config_input(AVFilterLink *inlink)
>      return 0;
>  }
>  
> +static int command(AVFilterContext *ctx, const char *cmd, const char *arg, char *ret, int ret_len, int flags)
> +{
> +    if(!strcmp(cmd, "reinit")){
> +        uninit(ctx);
> +        return init(ctx, arg, NULL);
> +    }
> +
> +    return AVERROR(ENOSYS);
> +}
> +
>  #define GET_BITMAP_VAL(r, c)                                            \
>      bitmap->pixel_mode == FT_PIXEL_MODE_MONO ?                          \
>          (bitmap->buffer[(r) * bitmap->pitch + ((c)>>3)] & (0x80 >> ((c)&7))) * 255 : \
> @@ -707,4 +717,5 @@ AVFilter avfilter_vf_drawtext = {
>      .outputs   = (AVFilterPad[]) {{ .name             = "default",
>                                      .type             = AVMEDIA_TYPE_VIDEO, },
>                                    { .name = NULL}},
> +    .command   = command,
>  };
> -- 
> 1.7.4.1

OK, but I still miss which is the point of this. 


> From dbce6e78ed13e52fc117187e2ef85f4f144503e0 Mon Sep 17 00:00:00 2001
> From: Michael Niedermayer <michaelni at gmx.at>
> Date: Mon, 29 Aug 2011 00:06:16 +0200
> Subject: [PATCH 4/5] avfilter: Add avfilter_graph_que_command()
> 
> Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> ---
>  libavfilter/avfilter.c      |   18 ++++++++++++++++++
>  libavfilter/avfilter.h      |    4 +++-
>  libavfilter/avfiltergraph.c |   25 +++++++++++++++++++++++++
>  libavfilter/avfiltergraph.h |   18 ++++++++++++++++++
>  libavfilter/internal.h      |    7 +++++++
>  5 files changed, 71 insertions(+), 1 deletions(-)
> 
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index 5c293dd..9432519 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -45,6 +45,15 @@ const char *avfilter_license(void)
>      return LICENSE_PREFIX FFMPEG_LICENSE + sizeof(LICENSE_PREFIX) - 1;
>  }
>  

> +static void command_que_pop(AVFilterContext *filter)

Nit: give this to a non-native speaker and it will wonder what's this
que. Thus I'd prefer "queue".

> +{
> +    AVFilterCommand *c= filter->command_que;
> +    av_freep(&c->arg);
> +    av_freep(&c->command);
> +    filter->command_que= c->next;
> +    av_free(c);
> +}
> +

>  AVFilterBufferRef *avfilter_ref_buffer(AVFilterBufferRef *ref, int pmask)
>  {
>      AVFilterBufferRef *ret = av_malloc(sizeof(AVFilterBufferRef));
> @@ -534,6 +543,7 @@ void avfilter_start_frame(AVFilterLink *link, AVFilterBufferRef *picref)
>      void (*start_frame)(AVFilterLink *, AVFilterBufferRef *);
>      AVFilterPad *dst = link->dstpad;
>      int perms = picref->perms;
> +    AVFilterCommand *cmd= link->dst->command_que;
>  
>      FF_DPRINTF_START(NULL, start_frame); ff_dlog_link(NULL, link, 0); av_dlog(NULL, " "); ff_dlog_ref(NULL, picref, 1);
>  
> @@ -556,6 +566,11 @@ void avfilter_start_frame(AVFilterLink *link, AVFilterBufferRef *picref)
>      else
>          link->cur_buf = picref;
>  
> +    if(cmd && cmd->time <= picref->pts * av_q2d(link->time_base)){
> +        avfilter_command(link->dst, cmd->command, cmd->arg, 0, 0, cmd->flags);
> +        command_que_pop(link->dst);
> +    }
> +
>      start_frame(link, link->cur_buf);
>  }
>  
> @@ -815,6 +830,9 @@ void avfilter_free(AVFilterContext *filter)
>      av_freep(&filter->inputs);
>      av_freep(&filter->outputs);
>      av_freep(&filter->priv);
> +    while(filter->command_que){
> +        command_que_pop(filter);
> +    }
>      av_free(filter);
>  }
>  
> diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
> index 4a8c436..a4b9638 100644
> --- a/libavfilter/avfilter.h
> +++ b/libavfilter/avfilter.h
> @@ -29,7 +29,7 @@
>  #include "libavutil/rational.h"
>  
>  #define LIBAVFILTER_VERSION_MAJOR  2
> -#define LIBAVFILTER_VERSION_MINOR 36
> +#define LIBAVFILTER_VERSION_MINOR 37
>  #define LIBAVFILTER_VERSION_MICRO  0
>  
>  #define LIBAVFILTER_VERSION_INT AV_VERSION_INT(LIBAVFILTER_VERSION_MAJOR, \
> @@ -585,6 +585,8 @@ struct AVFilterContext {
>      AVFilterLink **outputs;         ///< array of pointers to output links
>  
>      void *priv;                     ///< private data for use by the filter
> +
> +    struct AVFilterCommand *command_que;

Nit: que -> queue

>  };
>  
>  enum AVFilterPacking {
> diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
> index 486b0df..8d443d4 100644
> --- a/libavfilter/avfiltergraph.c
> +++ b/libavfilter/avfiltergraph.c
> @@ -283,3 +283,28 @@ int avfilter_graph_send_command(AVFilterGraph *graph, const char *target, const
>  
>      return r;
>  }
> +
> +int avfilter_graph_que_command(AVFilterGraph *graph, const char *target, const char *command, const char *arg, int flags, double ts)

Nit: que -> queue

> +{
> +    int i;
> +
> +    if(!graph)
> +        return 0;
> +
> +    for (i = 0; i < graph->filter_count; i++) {
> +        AVFilterContext *filter = graph->filters[i];
> +        if(filter && (!strcmp(target, "all") || !strcmp(target, filter->name) || !strcmp(target, filter->filter->name))){
> +            AVFilterCommand **que = &filter->command_que;
> +            while(*que) que = &(*que)->next;
> +            *que= av_mallocz(sizeof(AVFilterCommand));
> +            (*que)->command = av_strdup(command);
> +            (*que)->arg     = av_strdup(arg);
> +            (*que)->time    = ts;
> +            (*que)->flags   = flags;
> +            if(flags & AVFILTER_CMD_FLAG_ONE)
> +                return 0;
> +        }
> +    }
> +
> +    return 0;
> +}
> diff --git a/libavfilter/avfiltergraph.h b/libavfilter/avfiltergraph.h
> index e668104..134345a 100644
> --- a/libavfilter/avfiltergraph.h
> +++ b/libavfilter/avfiltergraph.h
> @@ -156,4 +156,22 @@ int avfilter_graph_parse(AVFilterGraph *graph, const char *filters,
>   */
>  int avfilter_graph_send_command(AVFilterGraph *graph, const char *target, const char *cmd, const char *arg, char *ret, int ret_len, int flags);
>  
> +/**
> + * Que a command for one or more filter instances.
> + *
> + * @param graph  the filter graph
> + * @param target the filter(s) to which the command should be sent
> + *               "all" sends to all filters
> + *               otherwise it can be a filter or filter instance name
> + *               which will send the command to all matching filters.

> + * @param cmd    the command to sent, for handling simplicity all commands must be alphanummeric only

Same typos as before

> + * @param arg    the argument for the command
> + * @param ts     time at which the command should be sent to the filter
> + *
> + * @note As this executes commands after this function returns, no return code
> + *       from the filter is provided, also AVFILTER_CMD_FLAG_ONE is not supported.
> + */
> +int avfilter_graph_que_command(AVFilterGraph *graph, const char *target, const char *cmd, const char *arg, int flags, double ts);

So the main difference with *_send_command() is that this is meant to
be queued in the target filter(s), and executed when the time is ripe.
Do you have a specific use case for it?

> +
> +
>  #endif /* AVFILTER_AVFILTERGRAPH_H */
> diff --git a/libavfilter/internal.h b/libavfilter/internal.h
> index b9613d5..bb9d0e5 100644
> --- a/libavfilter/internal.h
> +++ b/libavfilter/internal.h
> @@ -33,6 +33,13 @@ typedef struct AVFilterPool {
>      int count;
>  } AVFilterPool;
>  
> +typedef struct AVFilterCommand {
> +    double time;
> +    char *command, *arg;
> +    int flags;
> +    struct AVFilterCommand *next;
> +} AVFilterCommand;
> +
>  /**
>   * Check for the validity of graph.
>   *
> -- 
> 1.7.4.1
> 

> From d0283827f376feee5abd42fa8df8cb7609f6e626 Mon Sep 17 00:00:00 2001
> From: Michael Niedermayer <michaelni at gmx.at>
> Date: Mon, 29 Aug 2011 00:07:30 +0200
> Subject: [PATCH 5/5] ffmpeg: Support queing filter commands for later times
> 
> Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> ---
>  ffmpeg.c |   13 +++++++++----
>  1 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/ffmpeg.c b/ffmpeg.c
> index e9dbc66..0f8de4e 100644
> --- a/ffmpeg.c
> +++ b/ffmpeg.c
> @@ -2683,14 +2683,19 @@ static int transcode(AVFormatContext **output_files,
>              }
>              if (key == 'c' || key == 'C'){
>                  char ret[4096], target[64], cmd[256], arg[256]={0};
> -                fprintf(stderr, "\nEnter command: <target> <command>[ <argument>]\n");
> -                if(scanf("%4095[^\n\r]%*c", ret) == 1 && sscanf(ret, "%63[^ ] %255[^ ] %255[^\n]", target, cmd, arg) >= 2){
> +                double ts;
> +                fprintf(stderr, "\nEnter command: <target> <time> <command>[ <argument>]\n");
> +                if(scanf("%4095[^\n\r]%*c", ret) == 1 && sscanf(ret, "%63[^ ] %lf %255[^ ] %255[^\n]", target, &ts, cmd, arg) >= 3){
>                      for(i=0;i<nb_ostreams;i++) {
>                          int r;
>                          ost = ost_table[i];
>                          if(ost->graph){
> -                            r= avfilter_graph_send_command(ost->graph, target, cmd, arg, ret, sizeof(ret), key == 'c' ? AVFILTER_CMD_FLAG_ONE : 0);
> -                            fprintf(stderr, "Command reply for %d: %d, %s\n", i, r, ret);
> +                            if(ts<0){
> +                                r= avfilter_graph_send_command(ost->graph, target, cmd, arg, ret, sizeof(ret), key == 'c' ? AVFILTER_CMD_FLAG_ONE : 0);
> +                                fprintf(stderr, "Command reply for %d: %d, %s\n", i, r, ret);
> +                            }else{
> +                                r= avfilter_graph_que_command(ost->graph, target, cmd, arg, 0, ts);
> +                            }

Same consideration, seems clumsy, maybe we should read from a file,
but I won't object if you consider this patch useful for testing.
-- 
FFmpeg = Fundamentalist Fiendish Monstrous Picky Elfic God


More information about the ffmpeg-devel mailing list