[FFmpeg-devel] [PATCH 2/3] lavfi: timeline support.

Stefano Sabatini stefasab at gmail.com
Sat Apr 20 11:40:39 CEST 2013


Nit+++++: lavfi: add timeline support

On date Friday 2013-04-19 00:28:53 +0200, Clément Bœsch encoded:
> 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             | 29 ++++++++++++++++++++++++++
>  libavfilter/af_volume.c      |  1 +
>  libavfilter/avfilter.c       | 48 ++++++++++++++++++++++++++++++++++++++++++++
>  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, 120 insertions(+), 4 deletions(-)
> 
> diff --git a/Changelog b/Changelog
> index 6f895be..8840e5a 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -27,6 +27,7 @@ version <next>:
>  - colorchannelmixer filter
>  - The matroska demuxer can now output proper verbatim ASS packets. It will
>    become the default at the next libavformat major bump.
> +- 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 43d7368..f905c33 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -267,6 +267,33 @@ 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 different
> +from 0, the filter will be enabled, otherwise the frame will be sent unchanged

nit: different from zero -> non-zero?

seems less clumsy

> +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 end table
> +
> +Like any other filtering option, the @option{enable} option follows the same
> +rules.
> +
> +For example, to enable a denoiser filter (@ref{hqdn3d}) from 10 seconds to 3
> +minutes, and a @ref{curves} filter starting at 3 seconds:
> + at example
> +hqdn3d = enable='between(t,10,3*60)',
> +curves = enable='gte(t,3)' : preset=cross_process
> + at end example

I wonder how we could allow to read intervals from a file.

Second note: how do you think it would be possible to implement a
generic process_command() callback?

> +
>  @c man end FILTERGRAPH DESCRIPTION
>  
>  @chapter Audio Filters
> @@ -2386,6 +2413,7 @@ indicates never reset and return the largest area encountered during
>  playback.
>  @end table
>  
> + at anchor{curves}
>  @section curves
>  
>  Apply color adjustments using curves.
> @@ -3972,6 +4000,7 @@ ffplay -i input -vf histogram
>  
>  @end itemize
>  
> + at anchor{hqdn3d}
>  @section hqdn3d
>  
>  High precision/quality 3d denoise filter. This filter aims to reduce
> diff --git a/libavfilter/af_volume.c b/libavfilter/af_volume.c
> index 7d2b9ba..a55e1a3 100644
> --- a/libavfilter/af_volume.c
> +++ b/libavfilter/af_volume.c
> @@ -296,4 +296,5 @@ AVFilter avfilter_af_volume = {
>      .init           = init,
>      .inputs         = avfilter_af_volume_inputs,
>      .outputs        = avfilter_af_volume_outputs,
> +    .flags          = AVFILTER_FLAG_SUPPORT_TIMELINE,
>  };
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index 43340d1..e1bee2a 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -23,6 +23,7 @@
>  #include "libavutil/avstring.h"
>  #include "libavutil/channel_layout.h"
>  #include "libavutil/common.h"
> +#include "libavutil/eval.h"
>  #include "libavutil/imgutils.h"
>  #include "libavutil/opt.h"
>  #include "libavutil/pixdesc.h"
> @@ -483,12 +484,20 @@ static const AVClass *filter_child_class_next(const AVClass *prev)
>      return NULL;
>  }
>  
> +#define OFFSET(x) offsetof(AVFilterContext, x)
> +#define FLAGS AV_OPT_FLAG_FILTERING_PARAM
> +static const AVOption filters_common_options[] = {
> +    { "enable", "set enable expression", OFFSET(enable_str), AV_OPT_TYPE_STRING, {.str=NULL}, .flags = FLAGS },
> +    { NULL }
> +};
> +

>  static const AVClass avfilter_class = {
>      .class_name = "AVFilter",
>      .item_name  = default_filter_name,
>      .version    = LIBAVUTIL_VERSION_INT,
>      .category   = AV_CLASS_CATEGORY_FILTER,
>      .child_next = filter_child_next,
> +    .option     = filters_common_options,
>      .child_class_next = filter_child_class_next,
>  };

I'm happy we have AVFILTER_DEFINE_CLASS.

>  
> @@ -618,9 +627,16 @@ void avfilter_free(AVFilterContext *filter)
>      while(filter->command_queue){
>          ff_command_queue_pop(filter);
>      }
> +    av_opt_free(filter);
> +    av_expr_free(filter->enable);
> +    filter->enable = NULL;
> +    av_freep(&filter->var_values);
>      av_free(filter);
>  }
>  
> +static const char *const var_names[] = {   "t",   "n",        NULL };

> +enum                                   { VAR_T, VAR_N, VAR_VARS_NB };
> +
>  static int process_options(AVFilterContext *ctx, AVDictionary **options,
>                             const char *args)
>  {
> @@ -630,6 +646,8 @@ static int process_options(AVFilterContext *ctx, AVDictionary **options,
>      const char *key;
>      int offset= -1;
>  
> +    av_opt_set_defaults(ctx);
> +
>      if (!args)
>          return 0;
>  
> @@ -665,6 +683,12 @@ static int process_options(AVFilterContext *ctx, AVDictionary **options,
>          }
>  
>          av_log(ctx, AV_LOG_DEBUG, "Setting '%s' to value '%s'\n", key, value);
> +
> +        if (av_opt_find(ctx, key, NULL, 0, 0)) {
> +            ret = av_opt_set(ctx, key, value, 0);
> +            if (ret < 0)
> +                return ret;
> +        } else {
>          av_dict_set(options, key, value, 0);
>          if ((ret = av_opt_set(ctx->priv, key, value, 0)) < 0) {
>              if (!av_opt_find(ctx->priv, key, NULL, 0, AV_OPT_SEARCH_CHILDREN | AV_OPT_SEARCH_FAKE_OBJ)) {
> @@ -675,11 +699,27 @@ static int process_options(AVFilterContext *ctx, AVDictionary **options,
>              return ret;
>              }
>          }
> +        }
>  
>          av_free(value);
>          av_free(parsed_key);
>          count++;
>      }
> +
> +    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);
> +        if (ret < 0)
> +            return ret;
> +    }
>      return count;
>  }
>  
> @@ -852,6 +892,7 @@ static int default_filter_frame(AVFilterLink *link, AVFrame *frame)
>  static int ff_filter_frame_framed(AVFilterLink *link, AVFrame *frame)
>  {
>      int (*filter_frame)(AVFilterLink *, AVFrame *);
> +    AVFilterContext *dstctx = link->dst;
>      AVFilterPad *dst = link->dstpad;
>      AVFrame *out;
>      int ret;
> @@ -914,6 +955,13 @@ static int ff_filter_frame_framed(AVFilterLink *link, AVFrame *frame)
>      }
>  
>      pts = out->pts;
> +    if (dstctx->enable_str) {
> +        dstctx->var_values[VAR_N] = link->frame_count;
> +        dstctx->var_values[VAR_T] = pts == AV_NOPTS_VALUE ? NAN : pts * av_q2d(link->time_base);
> +        if (!av_expr_eval(dstctx->enable, dstctx->var_values, NULL))
> +            filter_frame = dst->passthrough_filter_frame ? dst->passthrough_filter_frame
> +                                                         : default_filter_frame;
> +    }
>      ret = filter_frame(link, out);
>      link->frame_count++;
>      link->frame_requested = 0;
> diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
> index 38bc5ee..7df42f7 100644
> --- a/libavfilter/avfilter.h
> +++ b/libavfilter/avfilter.h
> @@ -385,6 +385,19 @@ struct AVFilterPad {
>      int needs_fifo;
>  
>      int needs_writable;
> +
> +    /**
> +     * Passthrough filtering callback.
> +     *

> +     * If a filter support timeline editing (AVFILTER_FLAG_SUPPORT_TIMELINE)

supportS

(in case the AVFILTER_FLAG_SUPPORT_TIMELINE is enabled)

> +     * then it can implement a custom passthrough callback to update its local
> +     * context (for example to keep a frame reference, or simply send the
> +     * filter to a custom outlink). The filter is obviously not supposed to do

s/obviously//

also maybe state it more strongly: The filter must not do any change...

> +     * any change to the frame in this callback.
> +     *

> +     * Input pads only.

Flying saucepans only.

Context and verb missing.

> +     */
> +    int (*passthrough_filter_frame)(AVFilterLink *link, AVFrame *frame);
>  };
>  #endif
>  
> @@ -428,6 +441,12 @@ enum AVMediaType avfilter_pad_get_type(const AVFilterPad *pads, int pad_idx);
>   * the options supplied to it.
>   */
>  #define AVFILTER_FLAG_DYNAMIC_OUTPUTS       (1 << 1)
> +/**

nit++: missing separating empty line?

> + * Some filters support a generic "enable" expression option that can be used
> + * to enable or disable a filter in the timeline. Filters supporting this
> + * option have this flag set.
> + */
> +#define AVFILTER_FLAG_SUPPORT_TIMELINE      (1 << 16)
[...]

LGTM otherwise, but give some time to Nicolas to reply.

PS Please bump old patch threads so it's easier to follow them and the
reviewer doesn't need to chase around patches with similar names
amongst several thousands of other mails.
-- 
FFmpeg = Fancy and Frightening Mastering Portentous Elected Gladiator


More information about the ffmpeg-devel mailing list