[FFmpeg-devel] [PATCH] lavfi: add asendcmd and sendcmd filters
Stefano Sabatini
stefasab at gmail.com
Fri Sep 21 12:37:36 CEST 2012
On date Friday 2012-09-07 11:32:51 +0200, Nicolas George encoded:
> Le nonidi 19 fructidor, an CCXX, Stefano Sabatini a écrit :
> > diff --git a/doc/filters.texi b/doc/filters.texi
> > index 712936d..fcee490 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 @code{sendcmd} but for audio.
>
> I liked the previous version better: "for audio" seems to imply that the
> filter cares about the type of data it sees.
Me too, changed again.
> > +
> > +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.
>
> This paragraph was not updated to correspond to the interval logic.
Fixed.
> > +
> > +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};
> > + 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} is optional and specifies the optional list of argument for
> > +the given @var{COMMAND}.
> > +
> > +Between one interval specification and another, whitespaces, or
> > +sequences of characters starting with @code{#} until the end of line,
> > +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 and hue commands in a file.
> > + at example
> > +# show text in the interval 5-10
> > +5.0-10.0 [enter] drawtext reinit 'fontfile=FreeSerif.ttf:text=hello world',
> > + [leave] drawtext reinit 'fontfile=FreeSerif.ttf:text=';
> > +
> > +# desaturate the image in the interval 15-20
> > +15.0-20.0 [enter] hue reinit s=0,
> > + [enter] drawtext reinit 'fontfile=FreeSerif.ttf:text=nocolor',
> > + [leave] hue reinit s=1,
> > + [leave] drawtext reinit 'fontfile=FreeSerif.ttf:text=color';
> > + at end example
> > +
> > +A filtergraph allowing to read and process the above command list
> > +stored in a file @file{test.cmd}, can be specified with:
> > + at example
> > +sendcmd=f=test.cmd,drawtext=fontfile=FreeSerif.ttf:text='',hue
> > + at end example
> > + at end itemize
> > +
> > @section asetpts, setpts
> >
> > Change the PTS (presentation timestamp) of the input frames.
> > diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> > index 29b5f0e..0d815f9 100644
> > --- a/libavfilter/Makefile
> > +++ b/libavfilter/Makefile
> > @@ -53,6 +53,7 @@ OBJS-$(CONFIG_AMERGE_FILTER) += af_amerge.o
> > OBJS-$(CONFIG_AMIX_FILTER) += af_amix.o
> > OBJS-$(CONFIG_ANULL_FILTER) += af_anull.o
> > OBJS-$(CONFIG_ARESAMPLE_FILTER) += af_aresample.o
> > +OBJS-$(CONFIG_ASENDCMD_FILTER) += f_sendcmd.o
> > OBJS-$(CONFIG_ASETNSAMPLES_FILTER) += af_asetnsamples.o
> > OBJS-$(CONFIG_ASETPTS_FILTER) += f_setpts.o
> > OBJS-$(CONFIG_ASETTB_FILTER) += f_settb.o
> > @@ -121,6 +122,7 @@ OBJS-$(CONFIG_PIXDESCTEST_FILTER) += vf_pixdesctest.o
> > OBJS-$(CONFIG_REMOVELOGO_FILTER) += bbox.o lswsutils.o lavfutils.o vf_removelogo.o
> > OBJS-$(CONFIG_SCALE_FILTER) += vf_scale.o
> > OBJS-$(CONFIG_SELECT_FILTER) += vf_select.o
> > +OBJS-$(CONFIG_SENDCMD_FILTER) += f_sendcmd.o
> > OBJS-$(CONFIG_SETDAR_FILTER) += vf_aspect.o
> > OBJS-$(CONFIG_SETFIELD_FILTER) += vf_setfield.o
> > OBJS-$(CONFIG_SETPTS_FILTER) += f_setpts.o
> > diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> > index 6842ec9..b28c024 100644
> > --- a/libavfilter/allfilters.c
> > +++ b/libavfilter/allfilters.c
> > @@ -42,6 +42,7 @@ void avfilter_register_all(void)
> > REGISTER_FILTER (AMIX, amix, af);
> > REGISTER_FILTER (ANULL, anull, af);
> > REGISTER_FILTER (ARESAMPLE, aresample, af);
> > + REGISTER_FILTER (ASENDCMD, asendcmd, af);
> > REGISTER_FILTER (ASETNSAMPLES, asetnsamples, af);
> > REGISTER_FILTER (ASETPTS, asetpts, af);
> > REGISTER_FILTER (ASETTB, asettb, af);
> > @@ -113,6 +114,7 @@ void avfilter_register_all(void)
> > REGISTER_FILTER (REMOVELOGO, removelogo, vf);
> > REGISTER_FILTER (SCALE, scale, vf);
> > REGISTER_FILTER (SELECT, select, vf);
> > + REGISTER_FILTER (SENDCMD, sendcmd, vf);
> > REGISTER_FILTER (SETDAR, setdar, vf);
> > REGISTER_FILTER (SETFIELD, setfield, vf);
> > REGISTER_FILTER (SETPTS, setpts, vf);
> > diff --git a/libavfilter/f_sendcmd.c b/libavfilter/f_sendcmd.c
> > new file mode 100644
> > index 0000000..8fa3de9
> > --- /dev/null
> > +++ b/libavfilter/f_sendcmd.c
> > @@ -0,0 +1,589 @@
> > +/*
> > + * Copyright (c) 2012 Stefano Sabatini
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > + */
> > +
> > +/**
> > + * @file
> > + * send commands filter
> > + */
> > +
> > +#include "libavutil/avstring.h"
> > +#include "libavutil/file.h"
> > +#include "libavutil/opt.h"
> > +#include "libavutil/parseutils.h"
> > +#include "avfilter.h"
> > +#include "internal.h"
> > +#include "avfiltergraph.h"
> > +#include "audio.h"
> > +#include "video.h"
> > +
> > +#define COMMAND_FLAG_ENTER 1
> > +#define COMMAND_FLAG_LEAVE 2
> > +
> > +static inline char *av_make_command_flags_str(char *buf, size_t buf_size, int flags)
> > +{
> > + const char *strs[] = { "enter", "leave" };
> > + int len, i, j;
> > + char *buf0 = buf;
> > +
> > + for (i = 0; i < FF_ARRAY_ELEMS(strs); i++) {
> > + if (flags & 1<<i) {
> > + if (buf != buf0) {
> > + *(buf++) = '+';
> > + buf_size--;
> > + }
> > + for (j = 0, len = strlen("enter"); j < len && j < buf_size; j++)
> > + buf[j] = strs[i][j];
> > + buf += j;
> > + buf_size -= j;
> > + }
> > + }
> > +
> > + return buf0;
> > +}
>
> You could use bprint for that, that would probably be simpler.
I did that to avoid to pass the ABPrint buffer as argument, but
changed.
>
> > +
> > +#define flags2str(flags) \
> > + av_make_command_flags_str((char[32]){0}, 32, flags)
> > +
> > +typedef struct {
> > + int flags;
> > + char *target, *command, *arg;
> > + int index;
> > +} Command;
> > +
> > +typedef struct {
> > + int64_t start_ts; ///< start timestamp expressed as microseconds units
> > + int64_t end_ts; ///< end timestamp expressed as microseconds units
> > + int index; ///< unique index for these interval commands
> > + Command *commands;
> > + int nb_commands;
>
> > + int enabled;
>
> "inside" may be more descriptive.
I prefer to keep this (interval.inside is not very meaningful), added
a comment.
> > +} Interval;
> > +
> > +typedef struct {
> > + const AVClass *class;
> > + Interval *intervals;
> > + int nb_intervals;
> > +
> > + char *commands_filename;
> > + char *commands_str;
> > +
> > + uint8_t *commands_file_buf;
> > + size_t commands_file_bufsize;
> > +} SendCmdContext;
> > +
> > +#define OFFSET(x) offsetof(SendCmdContext, x)
> > +
> > +static const AVOption sendcmd_options[]= {
> > + { "commands", "set commands", OFFSET(commands_str), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0 },
> > + { "c", "set commands", OFFSET(commands_str), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0 },
> > + { "filename", "set commands file", OFFSET(commands_filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0 },
> > + { "f", "set commands file", OFFSET(commands_filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0 },
> > + {NULL},
> > +};
> > +
> > +AVFILTER_DEFINE_CLASS(sendcmd);
> > +
> > +#define SPACES " \f\t\n\r"
> > +
> > +static void skip_comments(const char **buf)
> > +{
> > + int len;
> > +
>
> > + while (1) {
> > + if (!**buf)
> > + break;
>
> while (**buf) ?
Yes, simplified.
>
> > +
> > + /* skip leading spaces */
>
> > + len = strspn(*buf, SPACES);
> > + if (len)
> > + *buf += len;
>
> If len == 0, then *buf += len is a nop: you can write that as *buf += strspn
> directly.
Changed.
> > + if (**buf != '#')
> > + break;
> > +
> > + (*buf)++;
> > +
> > + /* skip comment until the end of line */
> > + *buf += strcspn(*buf, "\n");
> > + if (**buf)
> > + (*buf)++;
> > + }
> > +}
> > +
> > +#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);
>
> If **buf is null, strspn will return 0, so the test is unnecessary. There
> are a lot of similar constructs that can be simplified that way.
Changed.
> > +
> > + /* 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;
>
> With that code, "[enter+enter]" will be parsed as
> COMMAND_FLAG_ENTER+COMMAND_FLAG_ENTER = 1+1 = 2. Using |= instead of +=
> seems more logical.
Good catch, fixed.
> > + else {
> > + char flag_buf[64];
> > + av_strlcpy(flag_buf, *buf, sizeof(flag_buf));
> > + av_log(log_ctx, AV_LOG_ERROR,
> > + "Unknown flag '%s' in in interval #%d, command #%d\n",
> > + flag_buf, interval_count, cmd_count);
> > + return AVERROR(EINVAL);
> > + }
>
> I am not sure what the strlcpy is for. If you want to print only up to 63
> chars, you can write "%63s".
I don't like literals when handling strings, so I try to avoid them
whenever I can.
>
> > + *buf += len;
> > + if (**buf == ']')
> > + break;
> > + if (**buf != '+') {
> > + av_log(log_ctx, AV_LOG_ERROR,
> > + "Invalid flags char '%c' in interval #%d, command #%d\n",
> > + **buf, interval_count, cmd_count);
> > + return AVERROR(EINVAL);
> > + }
> > + if (**buf)
> > + (*buf)++;
> > + }
> > +
> > + if (**buf != ']') {
> > + av_log(log_ctx, AV_LOG_ERROR,
> > + "Missing flag terminator or extraneous data found at the end of flags "
> > + "in interval #%d, command #%d\n", interval_count, cmd_count);
> > + return AVERROR(EINVAL);
> > + }
> > + (*buf)++; /* skip "]" */
> > + } else {
> > + cmd->flags = COMMAND_FLAG_ENTER;
> > + }
> > +
> > + if (**buf)
> > + *buf += strspn(*buf, SPACES);
> > + cmd->target = av_get_token(buf, COMMAND_DELIMS);
> > + if (!cmd->target || !cmd->target[0]) {
> > + av_log(log_ctx, AV_LOG_ERROR,
> > + "No target specified in in interval #%d, command #%d\n",
> > + interval_count, cmd_count);
> > + ret = AVERROR(EINVAL);
> > + goto fail;
> > + }
> > +
> > + if (**buf)
> > + *buf += strspn(*buf, SPACES);
> > + cmd->command = av_get_token(buf, COMMAND_DELIMS);
> > + if (!cmd->command || !cmd->command[0]) {
> > + av_log(log_ctx, AV_LOG_ERROR,
> > + "No command specified in in interval #%d, command #%d\n",
> > + interval_count, cmd_count);
> > + ret = AVERROR(EINVAL);
> > + goto fail;
> > + }
> > +
> > + if (**buf)
> > + *buf += strspn(*buf, SPACES);
> > + cmd->arg = av_get_token(buf, COMMAND_DELIMS);
> > +
> > + return 1;
> > +
> > +fail:
> > + av_freep(&cmd->target);
> > + av_freep(&cmd->command);
> > + av_freep(&cmd->arg);
> > + return ret;
> > +}
> > +
> > +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;
>
> NULL?
Fixed.
> > + *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 */
> > + 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));
> > + 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);
> > + av_log(log_ctx, AV_LOG_ERROR,
> > + "Command was parsed as: flags:[%s] target:%s command:%s arg:%s\n",
> > + flags2str(cmd.flags), cmd.target, cmd.command, cmd.arg);
> > + 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) {
> > + 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) {
> > + av_log(log_ctx, AV_LOG_ERROR,
> > + "Invalid end time specification '%s' in interval #%d\n",
> > + end, interval_count);
> > + goto end;
> > + }
> > + } else {
> > + interval->end_ts = INT64_MAX;
> > + }
> > + if (interval->end_ts < interval->start_ts) {
> > + av_log(log_ctx, AV_LOG_ERROR,
> > + "Invalid end time '%s' in interval #%d: "
> > + "cannot be lesser than start time '%s'\n",
> > + end, interval_count, start);
> > + ret = AVERROR(EINVAL);
> > + goto end;
> > + }
> > + } else {
> > + av_log(log_ctx, AV_LOG_ERROR,
> > + "No interval specified for interval #%d\n", interval_count);
> > + ret = AVERROR(EINVAL);
> > + goto end;
> > + }
> > +
> > + /* parse commands */
> > + ret = parse_commands(&interval->commands, &interval->nb_commands,
> > + interval_count, buf, log_ctx);
> > +
> > +end:
> > + av_free(intervalstr);
> > + return ret;
> > +}
> > +
> > +static int parse_intervals(Interval **intervals, int *nb_intervals,
> > + const char *buf, void *log_ctx)
> > +{
> > + int interval_count = 0;
> > + int ret, n = 0;
> > +
> > + *intervals = NULL;
> > + *nb_intervals = 0;
> > +
> > + while (1) {
> > + Interval interval;
> > +
> > + skip_comments(&buf);
> > + if (!(*buf))
> > + break;
> > +
> > + if ((ret = parse_interval(&interval, interval_count, &buf, log_ctx)) < 0)
> > + return ret;
> > +
> > + if (*buf)
> > + buf += strspn(buf, SPACES);
> > + if (*buf != ';') {
> > + av_log(log_ctx, AV_LOG_ERROR,
> > + "Missing terminator or extraneous data found at the end of interval #%d\n",
> > + interval_count);
> > + return AVERROR(EINVAL);
> > + }
> > + buf++; /* skip ';' */
> > + interval_count++;
> > +
> > + /* (re)allocate commands array if required */
> > + if (*nb_intervals == n) {
> > + n = FFMAX(16, 2*n); /* first allocation = 16, or double the number */
> > + *intervals = av_realloc_f(*intervals, n, 2*sizeof(Interval));
> > + if (!*intervals) {
> > + av_log(log_ctx, AV_LOG_ERROR,
> > + "Could not (re)allocate intervals array\n");
> > + return AVERROR(ENOMEM);
> > + }
> > + }
> > +
> > + (*intervals)[(*nb_intervals)++] = interval;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int cmp_intervals(const void *a, const void *b)
> > +{
> > + const Interval *i1 = a;
> > + const Interval *i2 = b;
>
> > + int ret;
> > +
> > + ret = i1->start_ts - i2->start_ts;
> > + return ret == 0 ? i1->index - i2->index : ret;
>
> start_ts is an int64_t, the subtraction could overflow.
Fixed.
>
> > +}
> > +
> > +static av_cold int init(AVFilterContext *ctx, const char *args)
> > +{
> > + SendCmdContext *sendcmd = ctx->priv;
> > + int ret, i, j;
> > + char *buf;
> > +
> > + sendcmd->class = &sendcmd_class;
> > + av_opt_set_defaults(sendcmd);
> > +
> > + if ((ret = av_set_options_string(sendcmd, args, "=", ":")) < 0)
> > + return ret;
> > +
> > + if (sendcmd->commands_filename && sendcmd->commands_str) {
> > + av_log(ctx, AV_LOG_ERROR,
> > + "Only one of the filename or commands options must be specified\n");
> > + return AVERROR(EINVAL);
> > + }
> > +
>
> > + if (sendcmd->commands_filename) {
> > + ret = av_file_map(sendcmd->commands_filename,
> > + &sendcmd->commands_file_buf,
> > + &sendcmd->commands_file_bufsize, 0, ctx);
> > + if (ret < 0)
> > + return ret;
> > + buf = sendcmd->commands_file_buf;
>
> Beware: sendcmd->commands_file_buf is not necessarily 0-terminated. It will
> not be if the file size happens to be a multiple of 4096, and in that case
> the parsing code will likely segfault.
OK, changed (we may consider to extend the av_file_* API to ease
handling this situation.
> > + } else {
> > + buf = sendcmd->commands_str;
> > + }
> > +
> > + if ((ret = parse_intervals(&sendcmd->intervals, &sendcmd->nb_intervals,
> > + buf, ctx)) < 0)
> > + return ret;
> > +
> > + qsort(sendcmd->intervals, sendcmd->nb_intervals, sizeof(Interval), cmp_intervals);
> > +
> > + av_log(ctx, AV_LOG_DEBUG, "Parsed commands:\n");
> > + for (i = 0; i < sendcmd->nb_intervals; i++) {
> > + Interval *interval = &sendcmd->intervals[i];
> > + av_log(ctx, AV_LOG_VERBOSE, "start_time:%f end_time:%f index:%d\n",
> > + (double)interval->start_ts/1000000, (double)interval->end_ts/1000000, interval->index);
> > + for (j = 0; j < interval->nb_commands; j++) {
> > + Command *cmd = &interval->commands[j];
> > + av_log(ctx, AV_LOG_VERBOSE,
> > + " [%s] target:%s command:%s arg:%s index:%d\n",
> > + flags2str(cmd->flags), cmd->target, cmd->command, cmd->arg, cmd->index);
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void av_cold uninit(AVFilterContext *ctx)
> > +{
> > + SendCmdContext *sendcmd = ctx->priv;
> > + int i, j;
> > +
> > + av_opt_free(sendcmd);
> > +
> > + for (i = 0; i < sendcmd->nb_intervals; i++) {
> > + Interval *interval = &sendcmd->intervals[i];
> > + for (j = 0; j < interval->nb_commands; j++) {
> > + Command *cmd = &interval->commands[j];
> > + av_free(cmd->target);
> > + av_free(cmd->command);
> > + av_free(cmd->arg);
> > + }
> > + av_free(interval->commands);
> > + }
> > + av_freep(&sendcmd->intervals);
> > +
> > + av_file_unmap(sendcmd->commands_file_buf,
> > + sendcmd->commands_file_bufsize);
> > +}
> > +
> > +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};
>
> There is already AV_TIME_BASE_Q.
On the other hand that could be redefined, changed anyway.
> > +
> > + 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;
> > + }
>
> I would suggest to take start_ts inclusively and end_ts exclusively:
> ts >= interval->start_ts && ts < interval->end_ts
Makes sense.
>
> > + if (interval->enabled && (ts < interval->start_ts || ts > interval->end_ts)) {
> > + flags += COMMAND_FLAG_LEAVE;
> > + interval->enabled = 0;
> > + }
>
> A macro is_inside_interval(interval, ts) would make the block easier to
> understand, I believe.
OK.
>
> > +
> > + 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);
> > +
> > + for (j = 0; flags && j < interval->nb_commands; j++) {
> > + Command *cmd = &interval->commands[j];
> > + char buf[1024];
> > +
> > + if (cmd->flags & flags) {
> > + av_log(ctx, AV_LOG_VERBOSE,
> > + "Processing command #%d target:%s command:%s arg:%s\n",
> > + cmd->index, cmd->target, cmd->command, cmd->arg);
> > + ret = avfilter_graph_send_command(inlink->graph,
> > + cmd->target, cmd->command, cmd->arg,
> > + buf, sizeof(buf),
> > + AVFILTER_CMD_FLAG_ONE);
> > + av_log(ctx, AV_LOG_VERBOSE,
> > + "Command reply for command #%d: ret:%s res:%s\n",
> > + cmd->index, av_err2str(ret), buf);
> > + }
> > + }
> > + }
> > + }
> > +
> > +end:
> > + /* give the reference away, do not store in cur_buf */
> > + inlink->cur_buf = NULL;
> > +
> > + switch (inlink->type) {
> > + case AVMEDIA_TYPE_VIDEO: return ff_start_frame (inlink->dst->outputs[0], ref);
> > + case AVMEDIA_TYPE_AUDIO: return ff_filter_samples(inlink->dst->outputs[0], ref);
> > + }
> > + return AVERROR(ENOSYS);
> > +}
> > +
> > +#if CONFIG_SENDCMD_FILTER
> > +
> > +AVFilter avfilter_vf_sendcmd = {
> > + .name = "sendcmd",
> > + .description = NULL_IF_CONFIG_SMALL("Send commands to filters."),
> > +
> > + .init = init,
> > + .uninit = uninit,
> > + .priv_size = sizeof(SendCmdContext),
> > +
> > + .inputs = (const AVFilterPad[]) {
> > + {
> > + .name = "default",
> > + .type = AVMEDIA_TYPE_VIDEO,
> > + .get_video_buffer = ff_null_get_video_buffer,
> > + .start_frame = process_frame,
> > + .end_frame = ff_null_end_frame,
> > + },
> > + { .name = NULL }
> > + },
> > + .outputs = (const AVFilterPad[]) {
> > + {
> > + .name = "default",
> > + .type = AVMEDIA_TYPE_VIDEO,
> > + },
> > + { .name = NULL }
> > + },
> > +};
> > +
> > +#endif
> > +
> > +#if CONFIG_ASENDCMD_FILTER
> > +
> > +AVFilter avfilter_af_asendcmd = {
> > + .name = "asendcmd",
> > + .description = NULL_IF_CONFIG_SMALL("Send commands to filters."),
> > +
> > + .init = init,
> > + .uninit = uninit,
> > + .priv_size = sizeof(SendCmdContext),
> > +
> > + .inputs = (const AVFilterPad[]) {
> > + {
> > + .name = "default",
> > + .type = AVMEDIA_TYPE_AUDIO,
> > + .get_audio_buffer = ff_null_get_audio_buffer,
> > + .filter_samples = process_frame,
> > + },
> > + { .name = NULL }
> > + },
> > + .outputs = (const AVFilterPad[]) {
> > + {
> > + .name = "default",
> > + .type = AVMEDIA_TYPE_AUDIO,
> > + },
> > + { .name = NULL }
> > + },
> > +};
> > +
> > +#endif
>
> Sorry for the delay. Thanks for the great work.
Thanks for the review.
I noticed there are some cases which needs to be addressed better
(e.g. ;;), will fix and push the patch in a few days if I read no more
comments.
--
FFmpeg = Fast and Fostering Merciless Portentous Earthshaking Gadget
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavfi-add-asendcmd-and-sendcmd-filters.patch
Type: text/x-diff
Size: 26178 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120921/a08d9657/attachment.bin>
More information about the ffmpeg-devel
mailing list