[FFmpeg-devel] [PATCH 3/3] lavfi/overlay: support timeline through the new system.

Stefano Sabatini stefasab at gmail.com
Sat Apr 20 16:22:02 CEST 2013


On date Saturday 2013-04-20 15:17:23 +0200, Clément Bœsch encoded:
> On Sat, Apr 20, 2013 at 11:49:24AM +0200, Stefano Sabatini wrote:
> > On date Friday 2013-04-19 00:28:54 +0200, Clément Bœsch encoded:
> > > ---
> > >  doc/filters.texi         | 15 +++---------
> > >  libavfilter/vf_overlay.c | 64 +++++++++++++++++++++++++-----------------------
> > >  2 files changed, 37 insertions(+), 42 deletions(-)
> > [...]
> > 
> > I see two issues with this patch:
> > - some variables are not supported in the generic enable expression
> > - enable command is removed
> > 
> > The First problem should be mostly addressed by adding pos to the list
> > of supported variables. You may ask, why is pos relevant? Well if you
> > know the total size of a file pos can provide some hints about the
> > position of the frame in the file (so you can roughly say: enable this
> > if it is between 50% and 60% of the file).
> > 
> 
> Interesting approach, why not, added. New patch attached.
> 
> > The second problem is more serious, indeed the enable command was the
> > main reason for which I added that option.
> 

> Added another patch on top of this one to enable the enable command
> injection, please comment.

Please post as separate thread the next time.

> 
> -- 
> Clément B.

> From 7a5aa2e8bd93fa887ccb83fefb76b9eb96564e62 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
> Date: Mon, 8 Apr 2013 20:35:02 +0200
> Subject: [PATCH 2/4] lavfi: add timeline support.

nit: drop ending point

> 
> Flag added in a few simple filters. A bunch of other filters can likely
> use the feature as well.
> ---
>  Changelog                    |  1 +
>  cmdutils.c                   |  2 ++
>  doc/filters.texi             | 32 ++++++++++++++++++++++++++++
>  libavfilter/af_volume.c      |  1 +
>  libavfilter/avfilter.c       | 50 ++++++++++++++++++++++++++++++++++++++++++++
>  libavfilter/avfilter.h       | 25 +++++++++++++++++++++-
>  libavfilter/vf_boxblur.c     |  1 +
>  libavfilter/vf_colormatrix.c |  1 +
>  libavfilter/vf_cropdetect.c  |  1 +
>  libavfilter/vf_curves.c      |  1 +
>  libavfilter/vf_drawbox.c     |  1 +
>  libavfilter/vf_edgedetect.c  |  1 +
>  libavfilter/vf_gradfun.c     |  1 +
>  libavfilter/vf_histeq.c      |  1 +
>  libavfilter/vf_hqdn3d.c      |  2 +-
>  libavfilter/vf_hue.c         |  1 +
>  libavfilter/vf_lut.c         |  1 +
>  libavfilter/vf_noise.c       |  1 +
>  libavfilter/vf_pp.c          |  2 +-
>  libavfilter/vf_smartblur.c   |  1 +
>  libavfilter/vf_unsharp.c     |  2 +-
>  21 files changed, 125 insertions(+), 4 deletions(-)
> 
> diff --git a/Changelog b/Changelog
> index 7b2ae6b..dc871dc 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -28,6 +28,7 @@ version <next>:
>  - The matroska demuxer can now output proper verbatim ASS packets. It will
>    become the default at the next libavformat major bump.
>  - decent native animated GIF encoding
> +- timeline editing with filters
>  
>  
>  version 1.2:
> diff --git a/cmdutils.c b/cmdutils.c
> index cc88697..b9985d6 100644
> --- a/cmdutils.c
> +++ b/cmdutils.c
> @@ -1683,6 +1683,8 @@ static void show_help_filter(const char *name)
>      if (f->priv_class)
>          show_help_children(f->priv_class, AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_FILTERING_PARAM |
>                                            AV_OPT_FLAG_AUDIO_PARAM);
> +    if (f->flags & AVFILTER_FLAG_SUPPORT_TIMELINE)
> +        printf("This filter has support for timeline through the 'enable' option.\n");
>  #else
>      av_log(NULL, AV_LOG_ERROR, "Build without libavfilter; "
>             "can not to satisfy request\n");
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 4c702a9..b443643 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -267,6 +267,36 @@ See the ``Quoting and escaping'' section in the ffmpeg-utils manual
>  for more information about the escaping and quoting rules adopted by
>  FFmpeg.
>  
> + at chapter Timeline editing
> +
> +Some filters support a generic @option{enable} option. For the filters
> +supporting timeline editing, this option can be set to an expression which is
> +evaluated before sending a frame to the filter. If the evaluation is non-zero,
> +the filter will be enabled, otherwise the frame will be sent unchanged to the
> +next filter in the filtergraph.
> +
> +The expression accepts the following values:
> + at table @samp
> + at item t
> +timestamp expressed in seconds, NAN if the input timestamp is unknown
> +
> + at item n
> +sequential number of the input frame, starting from 0
> +
> + at item pos
> +the position in the file of the input frame, NAN if unknown

nit: alphabetical order?

So it doesn't become a mess in case we add more constants.

[...]
> From 60dbcae7054b2334b578c6d78e517ed051358840 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
> Date: Sat, 20 Apr 2013 14:11:21 +0200
> Subject: [PATCH 3/4] lavfi: add 'enable' command injection to filters
>  supporting timeline.
> 
> ---
>  doc/filters.texi       |  3 +++
>  libavfilter/avfilter.c | 59 +++++++++++++++++++++++++++++++++++++++-----------
>  2 files changed, 49 insertions(+), 13 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index b443643..e650f1d 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -287,6 +287,9 @@ sequential number of the input frame, starting from 0
>  the position in the file of the input frame, NAN if unknown
>  @end table
>  
> +Additionally, these filters support an @option{enable} command that can be used
> +to re-define the expression.
> +
>  Like any other filtering option, the @option{enable} option follows the same
>  rules.
>  
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index 0c6f1f1..06d92b2 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -366,6 +366,49 @@ int ff_poll_frame(AVFilterLink *link)
>      return min;
>  }
>  
> +static const char *const var_names[] = {   "t",   "n",   "pos",        NULL };
> +enum                                   { VAR_T, VAR_N, VAR_POS, VAR_VARS_NB };
> +
> +static int set_enable_expr(AVFilterContext *ctx, const char *expr)
> +{
> +    int ret;
> +    char *expr_dup;
> +    AVExpr *old = ctx->enable;
> +
> +    if (!(ctx->filter->flags & AVFILTER_FLAG_SUPPORT_TIMELINE)) {
> +        av_log(ctx, AV_LOG_ERROR, "Timeline ('enable' option) not supported "
> +               "with filter '%s'\n", ctx->filter->name);
> +        return AVERROR_PATCHWELCOME;
> +    }
> +
> +    expr_dup = av_strdup(expr);
> +    if (!expr_dup)
> +        return AVERROR(ENOMEM);
> +
> +    if (!ctx->var_values) {
> +        ctx->var_values = av_calloc(VAR_VARS_NB, sizeof(*ctx->var_values));
> +        if (!ctx->var_values) {
> +            av_free(expr_dup);
> +            return AVERROR(ENOMEM);
> +        }
> +    }
> +
> +    ret = av_expr_parse((AVExpr**)&ctx->enable, expr_dup, var_names,
> +                        NULL, NULL, NULL, NULL, 0, ctx->priv);
> +    if (ret < 0) {
> +        av_log(ctx->priv, AV_LOG_ERROR,
> +               "Error when evaluating the expression '%s' for enable\n",
> +               expr_dup);
> +        av_free(expr_dup);
> +        return ret;
> +    }
> +
> +    av_expr_free(old);
> +    av_free(ctx->enable_str);
> +    ctx->enable_str = expr_dup;
> +    return 0;
> +}
> +
>  void ff_update_link_current_pts(AVFilterLink *link, int64_t pts)
>  {
>      if (pts == AV_NOPTS_VALUE)
> @@ -381,6 +424,8 @@ int avfilter_process_command(AVFilterContext *filter, const char *cmd, const cha
>      if(!strcmp(cmd, "ping")){
>          av_strlcatf(res, res_len, "pong from:%s %s\n", filter->filter->name, filter->name);
>          return 0;
> +    }else if(!strcmp(cmd, "enable")) {
> +        return set_enable_expr(filter, arg);

Uhm ok although this disallows an internal command for overriding it,
not sure this is a problem and/or if there is a better approach.

>      }else if(filter->filter->process_command) {
>          return filter->filter->process_command(filter, cmd, arg, res, res_len, flags);
>      }
> @@ -634,9 +679,6 @@ void avfilter_free(AVFilterContext *filter)
>      av_free(filter);
>  }
>  
> -static const char *const var_names[] = {   "t",   "n",   "pos",        NULL };
> -enum                                   { VAR_T, VAR_N, VAR_POS, VAR_VARS_NB };
> -
>  static int process_options(AVFilterContext *ctx, AVDictionary **options,
>                             const char *args)
>  {
> @@ -707,16 +749,7 @@ static int process_options(AVFilterContext *ctx, AVDictionary **options,
>      }
>  
>      if (ctx->enable_str) {
> -        if (!(ctx->filter->flags & AVFILTER_FLAG_SUPPORT_TIMELINE)) {
> -            av_log(ctx, AV_LOG_ERROR, "Timeline ('enable' option) not supported "
> -                   "with filter '%s'\n", ctx->filter->name);
> -            return AVERROR_PATCHWELCOME;
> -        }
> -        ctx->var_values = av_calloc(VAR_VARS_NB, sizeof(*ctx->var_values));
> -        if (!ctx->var_values)
> -            return AVERROR(ENOMEM);
> -        ret = av_expr_parse((AVExpr**)&ctx->enable, ctx->enable_str, var_names,
> -                            NULL, NULL, NULL, NULL, 0, ctx->priv);
> +        ret = set_enable_expr(ctx, ctx->enable_str);
>          if (ret < 0)
>              return ret;
>      }

No more comments from me, thanks.
-- 
FFmpeg = Foolish & Fundamentalist Mega Problematic Elegant Geisha


More information about the ffmpeg-devel mailing list