[FFmpeg-devel] [PATCH] vf_framestep: add blend parameter for motion blur effect

[Moritz Barsnick]

On Wed, May 31, 2017 at 13:31:14 +0100, Matthias C. M. Troffaes wrote:
> @@ -2,6 +2,7 @@ Entries are sorted chronologically from oldest to youngest within each release,
>  releases are sorted from youngest to oldest.
>  
>  version <next>:
> +- framestep filter: add blend parameter for motion blur effect
>  - deflicker video filter

Did you read the text up there? You need to add your change to the
bottom of the "<next>:" list.

> +    int frame_blend;    ///< how many frames to blend on each step
[...]
>  static const AVOption framestep_options[] = {
>      { "step", "set frame step",  OFFSET(frame_step), AV_OPT_TYPE_INT, {.i64=1}, 1, INT_MAX, FLAGS},
> +    { "blend", "number of frames to blend per step",  OFFSET(frame_blend), AV_OPT_TYPE_INT, {.i64=1}, 1, 65535, FLAGS},

Your maximum is too high to be assigned to an int, unless you properly
checked that any overflow still results in correct behaviour. The upper
limit probably needs to be "INT_MAX" (which would be 32767, but you can
use the constant).


>      AVFilterContext *ctx = outlink->src;
> -    FrameStepContext *framestep = ctx->priv;
> -    AVFilterLink *inlink = ctx->inputs[0];
> +    const FrameStepContext *s = ctx->priv;
> +    const AVFilterLink *inlink = ctx->inputs[0];
>  
>      outlink->frame_rate =
> -        av_div_q(inlink->frame_rate, (AVRational){framestep->frame_step, 1});
> +        av_div_q(inlink->frame_rate, (AVRational){s->frame_step, 1});
>  
>      av_log(ctx, AV_LOG_VERBOSE, "step:%d frame_rate:%d/%d(%f) -> frame_rate:%d/%d(%f)\n",
> -           framestep->frame_step,
> +           s->frame_step,
>             inlink->frame_rate.num, inlink->frame_rate.den, av_q2d(inlink->frame_rate),
>             outlink->frame_rate.num, outlink->frame_rate.den, av_q2d(outlink->frame_rate));
> +
>      return 0;
>  }

Isn't this just a rename and const-ification? This probably doesn't
belong into this patch (perhaps a separate one, if at all.) And it adds
an empty line, which certainly doesn't belong into a functional patch.

Moritz