[FFmpeg-devel] [PATCH] lavfi/select: factorize options definition between select and aselect

[Clément Bœsch]

On Mon, Apr 15, 2013 at 09:03:05PM +0200, Stefano Sabatini wrote:
> ---
>  libavfilter/f_select.c |   35 ++++++++++++++++-------------------
>  1 file changed, 16 insertions(+), 19 deletions(-)
> 
> diff --git a/libavfilter/f_select.c b/libavfilter/f_select.c
> index 5888f97..a142679 100644
> --- a/libavfilter/f_select.c
> +++ b/libavfilter/f_select.c
> @@ -141,6 +141,20 @@ typedef struct {
>      int nb_outputs;
>  } SelectContext;
>  
> +#define OFFSET(x) offsetof(SelectContext, x)
> +#define DEFINE_OPTIONS(filt_name, filt_type)                                               \
> +static const AVOption filt_name##_options[] = {                                            \
> +    { "expr", "set an expression to use for selecting frames", OFFSET(expr_str), AV_OPT_TYPE_STRING, { .str = "1" }, \
> +      .flags=AV_OPT_FLAG_##filt_type##_PARAM|AV_OPT_FLAG_FILTERING_PARAM }, \
> +    { "e",    "set an expression to use for selecting frames", OFFSET(expr_str), AV_OPT_TYPE_STRING, { .str = "1" }, \
> +      .flags=AV_OPT_FLAG_##filt_type##_PARAM|AV_OPT_FLAG_FILTERING_PARAM }, \
> +    { "outputs", "set the number of outputs", OFFSET(nb_outputs), AV_OPT_TYPE_INT, {.i64 = 1}, 1, INT_MAX, \
> +      .flags=AV_OPT_FLAG_##filt_type##_PARAM|AV_OPT_FLAG_FILTERING_PARAM }, \
> +    { "n",       "set the number of outputs", OFFSET(nb_outputs), AV_OPT_TYPE_INT, {.i64 = 1}, 1, INT_MAX, \
> +      .flags=AV_OPT_FLAG_##filt_type##_PARAM|AV_OPT_FLAG_FILTERING_PARAM }, \
> +    { NULL }                                                            \
> +}
> +
>  static int request_frame(AVFilterLink *outlink);
>  
>  static av_cold int init(AVFilterContext *ctx)
> @@ -412,15 +426,7 @@ static int query_formats(AVFilterContext *ctx)
>  
>  #if CONFIG_ASELECT_FILTER
>  
> -#define OFFSET(x) offsetof(SelectContext, x)
> -#define AFLAGS AV_OPT_FLAG_FILTERING_PARAM | AV_OPT_FLAG_AUDIO_PARAM
> -static const AVOption aselect_options[] = {
> -    { "expr", "An expression to use for selecting frames", OFFSET(expr_str), AV_OPT_TYPE_STRING, { .str = "1" }, .flags = AFLAGS },
> -    { "e",    "An expression to use for selecting frames", OFFSET(expr_str), AV_OPT_TYPE_STRING, { .str = "1" }, .flags = AFLAGS },
> -    { "outputs", "set the number of outputs", OFFSET(nb_outputs), AV_OPT_TYPE_INT, {.i64 = 1}, 1, INT_MAX, AFLAGS },
> -    { "n",       "set the number of outputs", OFFSET(nb_outputs), AV_OPT_TYPE_INT, {.i64 = 1}, 1, INT_MAX, AFLAGS },
> -    { NULL },
> -};
> +DEFINE_OPTIONS(aselect, AUDIO);
>  AVFILTER_DEFINE_CLASS(aselect);
>  
>  static av_cold int aselect_init(AVFilterContext *ctx)
> @@ -464,16 +470,7 @@ AVFilter avfilter_af_aselect = {
>  
>  #if CONFIG_SELECT_FILTER
>  
> -#define OFFSET(x) offsetof(SelectContext, x)
> -#define FLAGS AV_OPT_FLAG_FILTERING_PARAM | AV_OPT_FLAG_VIDEO_PARAM
> -static const AVOption select_options[] = {
> -    { "expr", "An expression to use for selecting frames", OFFSET(expr_str), AV_OPT_TYPE_STRING, { .str = "1" }, .flags = FLAGS },
> -    { "e",    "An expression to use for selecting frames", OFFSET(expr_str), AV_OPT_TYPE_STRING, { .str = "1" }, .flags = FLAGS },
> -    { "outputs", "set the number of outputs", OFFSET(nb_outputs), AV_OPT_TYPE_INT, {.i64 = 1}, 1, INT_MAX, FLAGS },
> -    { "n",       "set the number of outputs", OFFSET(nb_outputs), AV_OPT_TYPE_INT, {.i64 = 1}, 1, INT_MAX, FLAGS },
> -    { NULL },
> -};
> -
> +DEFINE_OPTIONS(select, VIDEO);
>  AVFILTER_DEFINE_CLASS(select);
>  

I would use:

    #define FLAGS AV_OPT_FLAG_VIDEO_PARAM|AV_OPT_FLAG_FILTERING_PARAM
    DEFINE_OPTIONS(select, FLAGS);

    #define AFLAGS AV_OPT_FLAG_AUDIO_PARAM|AV_OPT_FLAG_FILTERING_PARAM
    DEFINE_OPTIONS(aselect, AFLAGS);

...and simplify the macro to make it more readable... but consider it as a
mater of preferences.

Patch is OK

-- 
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/20130416/ae0fcffd/attachment.asc>