[FFmpeg-devel] [PATCH] lavfi: add asendcmd and sendcmd filters

Stefano Sabatini stefasab at gmail.com
Mon Sep 3 16:04:30 CEST 2012


On date Monday 2012-09-03 13:32:04 +0200, Clément Bœsch encoded:
> On Mon, Sep 03, 2012 at 12:28:11PM +0200, Stefano Sabatini wrote:
> [...]
> > > Even if "filter class" and "specific filter instance" sounds more correct
> > > from a developer perspective, would it make more sense to say that it's
> > > just the filter name?
> > > 
> > 
> > > And by the way, how do you identify the different target instances of the
> > > same filter? ("volume", "volume1", "volume2", ...?)
> > 
> > Different filters have different instance name (parsed_...), it is not
> > still possible to specify the name of an instance in the filter graph.
> > 
> > > > + at var{time} specify the time when the filter command is sent, expressed
> > > > +as a time duration.
> > > > +
> > > 
> > > Note: as I said on IRC, I really think this should allow frame accurate
> > > timing range at some point.
> > 
> > Mixing times and frames is really hard if not impossible.
> >  
> 
> :(

When I was working on an extended fade filter I tried to mix frame
count and time interval, and it was a mess. For example:
#100-100

depending on the timestamps the interval could be negative (end <
start). I also tried to handle special interval like:
10+#100

(starting from 10 and continuing for 100 frames).

Dealing with frame-count interval e.g. #100-#200 should be feasible,
but again that won't work with seeking, since the frame counter will
always increase.

> 
> [...]
> > From 56bef733a712a8fbca47109ff2183ad3c680a131 Mon Sep 17 00:00:00 2001
> > From: Stefano Sabatini <stefasab at gmail.com>
> > Date: Mon, 13 Aug 2012 20:13:26 +0200
> > Subject: [PATCH] lavfi: add asendcmd and sendcmd filters
> > 
> > ---
> >  doc/filters.texi         |  124 ++++++++++
> >  libavfilter/Makefile     |    2 +
> >  libavfilter/allfilters.c |    2 +
> >  libavfilter/f_sendcmd.c  |  578 ++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 706 insertions(+), 0 deletions(-)
> >  create mode 100644 libavfilter/f_sendcmd.c
> > 
> > diff --git a/doc/filters.texi b/doc/filters.texi
> > index 712936d..ceef7c2 100644
> > --- a/doc/filters.texi
> > +++ b/doc/filters.texi
> > @@ -4176,6 +4176,130 @@ tools.
> >  
> >  Below is a description of the currently available multimedia filters.
> >  
> > + at section asendcmd, sendcmd
> > +
> > +Send commands to filters in the filtergraph.
> > +
> > +These filters read commands to be sent to other filters in the
> > +filtergraph.
> > +
> > + at code{sendcmd} must be inserted between two video filters.
> > + at code{asendcmd} works the same way as sendcmd but for audio.
> > +
> 
> nit+: @code{sendcmd}

Fixed.

> > +The specification of commands can be specified in the filter arguments
> > +with the @var{commands} option, or in a file specified with the
> > + at var{filename} option.
> > +
> > +Commands are sent the first time when a frame with time greater or
> > +equal to the specified command time is processed by the filter.
> > +
> > +These filters accept the following options:
> > + at table @option
> > + at item commands, c
> > +Set the commands to be read and sent to the other filters.
> > + at item filename, f
> > +Set the filename of the commands to be read and sent to the other
> > +filters.
> > + at end table
> > +
> > + at subsection Commands syntax
> > +
> > +A commands description consists of a sequence of interval
> > +specifications, comprising a list of commands to be specified at
> > +particular events relating to that interval.
> > +
> > +An interval is specified by the following syntax:
> > + at example
> > + at var{START}[- at var{END} @var{COMMANDS};
> 
> Syntax error: ']' expected around "[- at var{END} @var{COMMANDS};"

Fixed.

> 
> > + at end example
> > +
> > +The time interval is specified by the @var{START} and @var{END} times.
> > + at var{END} is optional and defaults to the maximum time.
> > +
> > + at var{COMMANDS} consists of a sequence of one or more command
> > +descriptions, separated by ",", relating to that interval.
> > +The syntax of a command is given by:
> > + at example
> > +[@var{FLAGS}] @var{TARGET} @var{COMMAND} @var{ARG}
> > + at end example
> > +
> > + at var{FLAGS} is optional and specifies the type of events relating to
> > +the time interval which enable to send the specified command, and must
> > +be a sequence of identifier flag separated by "+" and enclosed between
> > +"[" and "]".
> > +
> > +The following flags are recognized:
> > + at table @option
> > + at item enter
> > +The command is sent when the current frame timestamp enters the
> > +specified interval. In other words, the command is sent when the
> > +previous frame timestamp was not in the given interval, and the
> > +current is.
> > +
> > + at item leave
> > +The command is sent when the current frame timestamp leaves the
> > +specified interval. In other words, the command is sent when the
> > +previous frame timestamp was in the given interval, and the
> > +current is not.
> > + at end table
> > +
> > +If @var{FLAGS} is not specified, a default value of @code{[enter]} is
> > +assumed.
> > +
> > + at var{TARGET} specifies the target of the command, usually the name of
> > +the filter class or a specific filter instance.
> > +
> > + at var{COMMAND} specifies the name of the command for the @var{TARGET}
> > +filter instance.
> > +
> > + at var{ARG} specifies the optional list of argument for the given
> > + at var{COMMAND}.
> > +
> 
> Maybe mention that this one isn't mandatory? (according to my
> understanding of the BNF below)

Yes.

> > +Between one interval specification and another, whitespaces, or
> > +sequence of characters until the end of line starting with @code{#},
> > +are ignored and can be used to annotate comments.
> > +
> > +A simplified BNF description of the commands specification syntax
> > +follows:
> > + at example
> > + at var{COMMAND_FLAG}  ::= "enter" | "leave"
> > + at var{COMMAND_FLAGS} ::= @var{COMMAND_FLAG} [+ at var{COMMAND_FLAG}]
> > + at var{COMMAND}       ::= ["[" @var{COMMAND_FLAGS} "]"] @var{TARGET} @var{COMMAND} [@var{ARG}]
> > + at var{COMMANDS}      ::= @var{COMMAND} [, at var{COMMANDS}]
> > + at var{INTERVAL}      ::= @var{START}[- at var{END}] @var{COMMANDS}
> > + at var{INTERVALS}     ::= @var{INTERVAL}[;@var{INTERVALS}]
> > + at end example
> > +
> > + at subsection Examples
> > +
> > + at itemize
> > + at item
> > +Specify audio tempo change at second 4:
> > + at example
> > +asendcmd=c='4.0 atempo tempo 1.5',atempo
> > + at end example
> > +
> > + at item
> > +Specify a list of drawtext commands in a file.
> > + at example
> > +# show text in the interval 5-10
> > +5.0-10.0 [enter] drawtext reinit
> > +                 fontfile=FreeSerif.ttf:x=(w-text_w)/2:y=(h-text_h-line_h)/2:text="hello world",
> > +         [leave] drawtext reinit fontfile=FreeSerif.ttf:draw=0;
> > +
> > +# desaturate the image in the interval 15-20
> > +15.0-20.0-8.0 [enter] hue reinit s=0,
> > +              [enter] drawtext reinit "fontsize=25:fontfile=FreeSerif.ttf:text='no color'",
> > +              [leave] hue reinit s=1,
> > +              [enter] drawtext reinit "fontfile=FreeSerif.ttf:text='color'";
> > + at end example
> > +

> > +The file containing the list of commands can be specified with:
> > + at example
> > +sendcmd=f=test.cmd,drawtext=fontfile=FreeSerif.ttf:text='hello world 0',hue
> > + at end example
> 

> Here we need a dummy arg line for drawtext, where "hello world 0" won't
> ever appear, right?

Yes, we may need to extend drawtext, so people can start (ab)using
sendcmd to implement a new subtitles format.

> 
> [...]
> > +#define COMMAND_DELIMS " \f\t\n\r,;"
> > +
> > +static int parse_command(Command *cmd, int cmd_count, int interval_count,
> > +                         const char **buf, void *log_ctx)
> > +{
> > +    int ret;
> > +
> > +    memset(cmd, 0, sizeof(Command));
> > +    cmd->index = cmd_count;
> > +
> > +    /* format: [FLAGS] target command arg */
> > +    if (**buf)
> > +        *buf += strspn(*buf, SPACES);
> > +
> > +    /* parse flags */
> > +    if (**buf == '[') {
> > +        (*buf)++; /* skip "[" */
> > +
> > +        while (**buf) {
> > +            int len = strcspn(*buf, "+]");
> > +
> > +            if      (!strncmp(*buf, "enter", strlen("enter"))) cmd->flags += COMMAND_FLAG_ENTER;
> > +            else if (!strncmp(*buf, "leave", strlen("leave"))) cmd->flags += COMMAND_FLAG_LEAVE;
> 
> Why += and not |= ?

I think they are equivalent, aren't they? And since I'm using '+' as
separator I wanted to keep lexycal consistency.

> [...]
> > +static int parse_commands(Command **cmds, int *nb_cmds, int interval_count,
> > +                          const char **buf, void *log_ctx)
> > +{
> > +    int cmd_count = 0;
> > +    int ret, n = 0;
> > +
> > +    *cmds = 0;
> > +    *nb_cmds = 0;
> > +
> > +    while (1) {
> > +        Command cmd;
> > +
> > +        if ((ret = parse_command(&cmd, cmd_count, interval_count, buf, log_ctx)) < 0)
> > +            return ret;
> > +        cmd_count++;
> > +
> > +        /* (re)allocate commands array if required*/
> 
> nit++: "required */"

Fixed.

> > +        if (*nb_cmds == n) {
> > +            n = FFMAX(16, 2*n); /* first allocation = 16, or double the number */
> > +            *cmds = av_realloc_f(*cmds, n, 2*sizeof(Command));
> 
> nit: maybe use av_fast_realloc()? (see in lavf/subtitles.c for instance)

What would be the advantage? (considering this is init code, it won't
make much difference). I'll do if it doesn't involve code
complications. 

> > +            if (!*cmds) {
> > +                av_log(log_ctx, AV_LOG_ERROR,
> > +                       "Could not (re)allocate command array\n");
> > +                return AVERROR(ENOMEM);
> > +            }
> > +        }
> > +
> > +        (*cmds)[(*nb_cmds)++] = cmd;
> > +
> > +        if (**buf)
> > +            *buf += strspn(*buf, SPACES);
> > +        if (**buf != ';' && **buf != ',') {
> > +            av_log(log_ctx, AV_LOG_ERROR,
> > +                   "Missing separator or extraneous data found at the end of "
> > +                   "interval #%d, in command #%d\n",
> > +                   interval_count, cmd_count);
> > +            return AVERROR(EINVAL);
> > +        }
> > +        if (**buf == ';')
> > +            break;
> > +        if (**buf == ',')
> > +            (*buf)++;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +#define DELIMS " \f\t\n\r,;"
> > +
> > +static int parse_interval(Interval *interval, int interval_count,
> > +                          const char **buf, void *log_ctx)
> > +{
> > +    char *intervalstr;
> > +    int ret;
> > +
> > +    if (!**buf)
> > +        return 0;
> > +
> > +    /* reset data */
> > +    memset(interval, 0, sizeof(Interval));
> > +    interval->index = interval_count;
> > +
> > +    /* format: INTERVAL COMMANDS */
> > +
> > +    /* parse interval */
> > +    if (**buf)
> > +        *buf += strspn(*buf, SPACES);
> > +    intervalstr = av_get_token(buf, DELIMS);
> > +    if (intervalstr) {
> > +        char *start, *end;
> > +
> > +        start = av_strtok(intervalstr, "-", &end);
> > +        if ((ret = av_parse_time(&(interval->start_ts), start, 1)) < 0) {
> 
> the ( ) look uneeded

Fixed.

> > +            av_log(log_ctx, AV_LOG_ERROR,
> > +                   "Invalid start time specification '%s' in interval #%d\n",
> > +                   start, interval_count);
> > +            goto end;
> > +        }
> > +
> > +        if (end) {
> > +            if ((ret = av_parse_time(&(interval->end_ts), end, 1)) < 0) {
> 
> ditto

Fixed.
 
> [...]
> > +static int process_frame(AVFilterLink *inlink, AVFilterBufferRef *ref)
> > +{
> > +    AVFilterContext *ctx = inlink->dst;
> > +    SendCmdContext *sendcmd = ctx->priv;
> > +    int64_t ts;
> > +    int i, j, ret;
> > +    AVRational tb = {1, 1000000};
> > +
> > +    if (ref->pts == AV_NOPTS_VALUE)
> > +        goto end;
> > +
> > +    ts = av_rescale_q(ref->pts, inlink->time_base, tb);
> > +
> > +    for (i = 0; i < sendcmd->nb_intervals; i++) {
> > +        Interval *interval = &sendcmd->intervals[i];
> > +        int flags = 0;
> > +
> > +        if (!interval->enabled && ts >= interval->start_ts && ts <= interval->end_ts) {
> > +            flags += COMMAND_FLAG_ENTER;
> > +            interval->enabled = 1;
> > +        }
> > +        if (interval->enabled && (ts < interval->start_ts || ts > interval->end_ts)) {
> > +            flags += COMMAND_FLAG_LEAVE;
> > +            interval->enabled = 0;
> > +        }
> > +
> 
> ditto += vs |=

As above.

> > +        if (flags) {
> > +            av_log(ctx, AV_LOG_VERBOSE,
> > +                   "[%s] interval #%d start_ts:%f end_ts:%f ts:%f\n",
> > +                   flags2str(flags), interval->index,
> > +                   (double)interval->start_ts/1000000, (double)interval->end_ts/1000000,
> > +                   (double)ts/1000000);
> > +
> 
> no av_ts2str or related?

I'm lazy.

...

Updated patch with some logic fixes, I'll try to address the remarks I
didn't already address. Also it seems there are some problems with
av_get_token() which I need to fix.
-- 
FFmpeg = Furious Fascinating Mortal Puritan EnGine
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-lavfi-add-asendcmd-and-sendcmd-filters.patch
Type: text/x-diff
Size: 25095 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120903/2a3412e0/attachment.bin>


More information about the ffmpeg-devel mailing list