[FFmpeg-devel] [PATCH] Command passing interface for libavfilter
Michael Niedermayer
michaelni at gmx.at
Mon Aug 29 17:49:47 CEST 2011
On Mon, Aug 29, 2011 at 04:57:10PM +0200, Stefano Sabatini wrote:
> 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.
yes, if you could improve the whitespace consistency that would be
great. To me i dont really notice its being inconsistent at that
detail level without conciously looking for it.
I fixed some i stubmled across though
>
> > --
> > 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.
ok
>
> Nit: ret -> res
> ret_len -> res_len
>
> for avoiding ret/res conflicts in the code (ret is usually the return code)
ok
>
> > +{
> > + 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.
ok
>
> > + *
> > + * @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
ok
>
> > } 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.
changed too
>
> > + * 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
ok
>
> > + *
> > + * @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
i think ive changed them all
>
> > +
> > #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.
the intent is to allow an application to control the filtergraph of
ffmpeg through stdin to add text to live video in realtime.
[...]
> > 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".
ok
>
> > +{
> > + 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
ok
>
> > };
> >
> > 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
ok
>
> > +{
> > + 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?
like said, ffmpeg being controled through stdin by another application
the texts should be shown at the correct times which makes this
neccessary
Will apply attached patches soon if i hear no more comments
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-avfilter-Add-command-passing-support.patch
Type: text/x-patch
Size: 6503 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110829/584d2b54/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-ffmpeg-Support-passing-commands-to-filters-at-runtim.patch
Type: text/x-patch
Size: 2223 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110829/584d2b54/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-drawtext-Support-changing-parameters-through-reinit-.patch
Type: text/x-patch
Size: 1446 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110829/584d2b54/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-avfilter-Add-avfilter_graph_que_command.patch
Type: text/x-patch
Size: 5926 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110829/584d2b54/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-ffmpeg-Support-queing-filter-commands-for-later-time.patch
Type: text/x-patch
Size: 2091 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110829/584d2b54/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110829/584d2b54/attachment.asc>
More information about the ffmpeg-devel
mailing list