[FFmpeg-devel] [PATCH] lavfi/setfield: add support to named options and introspection

[Clément Bœsch]

On Sat, Dec 08, 2012 at 05:36:05PM +0100, Stefano Sabatini wrote:
> TODO: bump micro
> ---
>  doc/filters.texi          |    5 ++++-
>  libavfilter/vf_setfield.c |   46 +++++++++++++++++++++------------------------
>  2 files changed, 25 insertions(+), 26 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 1a2efcd..271fe10 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -3632,7 +3632,10 @@ output frames. It does not change the input frame, but only sets the
>  corresponding property, which affects how the frame is treated by
>  following filters (e.g. @code{fieldorder} or @code{yadif}).
>  
> -It accepts a string parameter, which can assume the following values:
> +This filter accepts a single option @option{mode}, which can be
> +specified either by setting @code{mode=VALUE} either setting the
> +value alone. Available values are:
> +
>  @table @samp
>  @item auto
>  Keep the same field property.
> diff --git a/libavfilter/vf_setfield.c b/libavfilter/vf_setfield.c
> index 026b965..8b5546f 100644
> --- a/libavfilter/vf_setfield.c
> +++ b/libavfilter/vf_setfield.c
> @@ -23,6 +23,7 @@
>   * set field order
>   */
>  
> +#include "libavutil/opt.h"
>  #include "avfilter.h"
>  #include "internal.h"
>  #include "video.h"
> @@ -35,39 +36,33 @@ enum SetFieldMode {
>  };
>  
>  typedef struct {
> +    const AVClass *class;
>      enum SetFieldMode mode;
>  } SetFieldContext;
>  
> +#define OFFSET(x) offsetof(SetFieldContext, x)
> +#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
> +
> +static const AVOption setfield_options[] = {
> +    {"mode", "select interlace mode", OFFSET(mode), AV_OPT_TYPE_INT, {.i64=MODE_AUTO}, -1, MODE_PROG, FLAGS, "mode"},
> +    {"auto", "keep the same input field",  0, AV_OPT_TYPE_CONST, {.i64=MODE_AUTO}, INT_MIN, INT_MAX, FLAGS, "mode"},
> +    {"bff",  "mark as bottom-field-first", 0, AV_OPT_TYPE_CONST, {.i64=MODE_BFF},  INT_MIN, INT_MAX, FLAGS, "mode"},
> +    {"tff",  "mark as top-field-first",    0, AV_OPT_TYPE_CONST, {.i64=MODE_TFF},  INT_MIN, INT_MAX, FLAGS, "mode"},
> +    {"prog", "mark as progressive",        0, AV_OPT_TYPE_CONST, {.i64=MODE_PROG}, INT_MIN, INT_MAX, FLAGS, "mode"},
> +    {NULL}
> +};
> +
> +AVFILTER_DEFINE_CLASS(setfield);
> +
>  static av_cold int init(AVFilterContext *ctx, const char *args)
>  {
>      SetFieldContext *setfield = ctx->priv;
> +    static const char *shorthand[] = { "mode", NULL };
>  
> -    setfield->mode = MODE_AUTO;
> -
> -    if (args) {
> -        char c;
> -        if (sscanf(args, "%d%c", &setfield->mode, &c) != 1) {
> -            if      (!strcmp("tff",  args)) setfield->mode = MODE_TFF;
> -            else if (!strcmp("bff",  args)) setfield->mode = MODE_BFF;
> -            else if (!strcmp("prog", args)) setfield->mode = MODE_PROG;
> -            else if (!strcmp("auto", args)) setfield->mode = MODE_AUTO;
> -            else {
> -                av_log(ctx, AV_LOG_ERROR, "Invalid argument '%s'\n", args);
> -                return AVERROR(EINVAL);
> -            }
> -        } else {
> -            if (setfield->mode < -1 || setfield->mode > 1) {
> -                av_log(ctx, AV_LOG_ERROR,
> -                       "Provided integer value %d must be included between -1 and +1\n",
> -                       setfield->mode);
> -                return AVERROR(EINVAL);
> -            }
> -            av_log(ctx, AV_LOG_WARNING,
> -                   "Using -1/0/1 is deprecated, use auto/tff/bff/prog\n");
> -        }
> -    }
> +    setfield->class = &setfield_class;
> +    av_opt_set_defaults(setfield);
>  
> -    return 0;
> +    return av_opt_set_from_string(setfield, args, shorthand, "=", ":");
>  }
>  
>  static int filter_frame(AVFilterLink *inlink, AVFilterBufferRef *picref)
> @@ -109,4 +104,5 @@ AVFilter avfilter_vf_setfield = {
>      .priv_size = sizeof(SetFieldContext),
>      .inputs    = setfield_inputs,
>      .outputs   = setfield_outputs,
> +    .priv_class = &setfield_class,
>  };

LGTM but we should decide what to do about av_opt_free(): call it all the
time (to prevent any future leaks), or just when necessary (only needed
with strings in option, right?)

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121208/a47b0687/attachment.asc>