[FFmpeg-devel] [PATCH] vf_fade: use options framework

[Mark Himsley]

On 09/11/11 01:01, Stefano Sabatini wrote:
> On date Saturday 2011-11-05 02:00:22 +0000, Mark Himsley encoded:
>> use options framework
>>
>> --
>> Mark
>
>> diff --git a/doc/filters.texi b/doc/filters.texi
>> index 0da5702..a6110e4 100644
>> --- a/doc/filters.texi
>> +++ b/doc/filters.texi
>> @@ -1078,7 +1078,7 @@ For more information about libfreetype, check:
>>   Apply fade-in/out effect to input video.
>>
>>   It accepts the parameters:
>> - at var{type}:@var{start_frame}:@var{nb_frames}
>> + at var{type}:@var{start_frame}:@var{nb_frames}[:@var{options}]
>>
>>   @var{type} specifies if the effect type, can be either "in" for
>>   fade-in, or "out" for a fade-out effect.
>> @@ -1091,6 +1091,22 @@ effect has to last. At the end of the fade-in effect the output video
>>   will have the same intensity as the input video, at the end of the
>>   fade-out transition the output video will be completely black.
>>
>> +This source accepts an optional sequence of @var{key}=@var{value} pairs,
>> +separated by ":". The description of the accepted options follows.
>> +
>> + at table @option
>> +
>> + at item type, t
>> +See @var{type}
>> +
>> + at item in_point, i
>> +See @var{start_frame}
>> +
>> + at item duration, d
>> +See @var{nb_frames}
>
> Changed this to start_frame/nb_frames for overall consistency.

Fine. I see now that the other occurrences of duration are in seconds 
and nb_frames differentiates from that. Does it need the "nb_" prefix?

>> + at end table
>> +
>>   A few usage examples follow, usable too as test scenarios.
>>   @example
>>   # fade in first 30 frames of video
>> diff --git a/libavfilter/vf_fade.c b/libavfilter/vf_fade.c
>> index 5032019..6f750b4 100644
>> --- a/libavfilter/vf_fade.c
>> +++ b/libavfilter/vf_fade.c
>> @@ -25,48 +25,130 @@
>>    * based heavily on vf_negate.c by Bobby Bingham
>>    */
>>
>> +#include "libavutil/avstring.h"
>> +#include "libavutil/eval.h"
>> +#include "libavutil/opt.h"
>>   #include "libavutil/pixdesc.h"
>>   #include "avfilter.h"
>> +#include "drawutils.h"
>>   #include "internal.h"
>>
>>   typedef struct {
>> +    const AVClass *class;
>>       int factor, fade_per_frame;
>>       unsigned int frame_index, start_frame, stop_frame;
>>       int hsub, vsub, bpp;
>>       unsigned int black_level, black_level_scaled;
>> +
>> +    char *type, *start_expr, *count_expr;
>>   } FadeContext;
>>
>> +#define OFFSET(x) offsetof(FadeContext, x)
>> +
>> +static const AVOption fade_options[] = {
>> +    { "type",     "set the fade direction",                     OFFSET(type),       AV_OPT_TYPE_STRING, {.str = "in" }, CHAR_MIN, CHAR_MAX },
>> +    { "t",        "set the fade direction",                     OFFSET(type),       AV_OPT_TYPE_STRING, {.str = "in" }, CHAR_MIN, CHAR_MAX },
>> +    { "in_point", "set expression of frame to start fading",    OFFSET(start_expr), AV_OPT_TYPE_STRING, {.str = "0"  }, CHAR_MIN, CHAR_MAX },
>> +    { "i",        "set expression of frame to start fading",    OFFSET(start_expr), AV_OPT_TYPE_STRING, {.str = "0"  }, CHAR_MIN, CHAR_MAX },
>> +    { "duration", "set expression for fade duration in frames", OFFSET(count_expr), AV_OPT_TYPE_STRING, {.str = "25" }, CHAR_MIN, CHAR_MAX },
>> +    { "d",        "set expression for fade duration in frames", OFFSET(count_expr), AV_OPT_TYPE_STRING, {.str = "25" }, CHAR_MIN, CHAR_MAX },
>> +    {NULL},
>> +};
>> +
>> +static const char *fade_get_name(void *ctx)
>> +{
>> +    return "fade";
>> +}
>> +
>> +static const AVClass fade_class = {
>> +    "FadeContext",
>> +    fade_get_name,
>> +    fade_options
>> +};
>> +
>>   static av_cold int init(AVFilterContext *ctx, const char *args, void *opaque)
>>   {
>>       FadeContext *fade = ctx->priv;
>>       unsigned int nb_frames;
>> -    char in_out[4];
>> +    double res;
>> +    int ret = 0;
>> +    char *args1, *expr, *bufptr = NULL;
>>
>> -    if (!args ||
>> -        sscanf(args, " %3[^:]:%u:%u", in_out,&fade->start_frame,&nb_frames) != 3) {
>> -        av_log(ctx, AV_LOG_ERROR,
>> -               "Expected 3 arguments '(in|out):#:#':'%s'\n", args);
>> -        return AVERROR(EINVAL);
>> +    fade->class =&fade_class;
>> +    av_opt_set_defaults(fade);
>> +
>> +    if (!(args1 = av_strdup(args))) {
>> +        ret = AVERROR(ENOMEM);
>> +        goto end;
>> +    }
>> +
>> +    if (expr = av_strtok(args1, ":",&bufptr)) {
>> +        if (!(fade->type = av_strdup(expr))) {
>> +            ret = AVERROR(ENOMEM);
>> +            goto end;
>> +        }
>> +    }
>> +    if (expr = av_strtok(NULL, ":",&bufptr)) {
>> +        if (!(fade->start_expr = av_strdup(expr))) {
>> +            ret = AVERROR(ENOMEM);
>> +            goto end;
>> +        }
>> +    }
>> +    if (expr = av_strtok(NULL, ":",&bufptr)) {
>> +        if (!(fade->count_expr = av_strdup(expr))) {
>> +            ret = AVERROR(ENOMEM);
>> +            goto end;
>> +        }
>>       }
>>
>> -    nb_frames = nb_frames ? nb_frames : 1;
>> +    if (bufptr&&  (ret = av_set_options_string(fade, bufptr, "=", ":"))<  0)
>> +        goto end;
>> +
>
>> +    if ((ret = av_expr_parse_and_eval(&res, (expr = fade->start_expr), NULL, NULL,
>> +                                      NULL, NULL, NULL, NULL, NULL, 0, ctx))<  0)
>> +        goto fail;
>> +    fade->start_frame = res;
>> +    if ((ret = av_expr_parse_and_eval(&res, (expr = fade->count_expr), NULL, NULL,
>> +                                      NULL, NULL, NULL, NULL, NULL, 0, ctx))<  0)
>> +        goto fail;
>> +    nb_frames = res;
>
> I'm not sure about this, do you have a specific use case for it? I'd
> rather go with plain ints, and avoid all the evaluation stuff (plus
> range checks and rounding).

I am planning further patches to add, for example, "TB" to the 
expression variables to enable expressing in point and duration in seconds.

But have ints if you want, and we can argue about converting to an 
expression when I submit the TB patch later ;-)


-- 
Mark