[FFmpeg-devel] [PATCH] lavfi/fade: Add ability to do video fade based on time

Clément Bœsch ubitux at gmail.com
Fri Apr 19 01:59:16 CEST 2013


On Tue, Apr 16, 2013 at 10:20:13PM -0400, Andy n Deanna wrote:
> 
> Attached is a resubmit of a patch to allow video fade filters based
> on time.  Note that I had previously neglected to subscribe to the
> ffmpeg-devel list, so if you have commented before I may have missed
> it.
> 
> Previous email conversation is also attached.
> 
> Thanks,
> Andy

> From 641c0ea90a83223c5d49228bf82e1c54018c98fc Mon Sep 17 00:00:00 2001
> From: Andy Martin <amartin at ubuntu.(none)>
> Date: Tue, 16 Apr 2013 21:35:02 -0400
> Subject: [PATCH] lavfi/fade: Added ability to do video fade based on
>  timestamp
> 
> ---
>  doc/filters.texi      |   18 +++++++++++
>  libavfilter/vf_fade.c |   84 +++++++++++++++++++++++++++++++++++++++----------
>  2 files changed, 85 insertions(+), 17 deletions(-)
> 
[...]
>  static av_cold int init(AVFilterContext *ctx)
> @@ -68,18 +69,17 @@ static av_cold int init(AVFilterContext *ctx)
>      FadeContext *fade = ctx->priv;
>  
>      fade->fade_per_frame = (1 << 16) / fade->nb_frames;
> -    if (fade->type == FADE_IN) {
> -        fade->factor = 0;
> -    } else if (fade->type == FADE_OUT) {
> -        fade->fade_per_frame = -fade->fade_per_frame;
> -        fade->factor = (1 << 16);
> +    fade->fade_state = VF_FADE_WAITING;
> +    
> +    if (fade->duration != 0.0) {
> +        // If duration (seconds) is non-zero, assume that we are not fading based on frames
> +        fade->nb_frames = 0; // Mostly to clean up logging
>      }
> -    fade->stop_frame = fade->start_frame + fade->nb_frames;
> -
> +     
>      av_log(ctx, AV_LOG_VERBOSE,
> -           "type:%s start_frame:%d nb_frames:%d alpha:%d\n",
> -           fade->type == FADE_IN ? "in" : "out", fade->start_frame,
> -           fade->nb_frames, fade->alpha);
> +           "type:%s start_frame:%d nb_frames:%d start_time:%f duration:%f alpha:%d\n",
> +           fade->type == FADE_IN ? "in" : "out", fade->start_frame, fade->nb_frames, 
> +           fade->start_time, fade->duration,fade->alpha);

It would be nice to have some conditional logging: you print either
start_time or start_frame, and nb_frames or duration. I don't consider
this blocking though, so feel free to ignore.

>      return 0;
>  }
>  
> @@ -153,7 +153,53 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
>      FadeContext *fade = inlink->dst->priv;
>      uint8_t *p;
>      int i, j, plane;
> -
> +    double frame_timestamp = frame->pts == AV_NOPTS_VALUE ? -1 : frame->pts * av_q2d(inlink->time_base);
> +    
> +    // Calculate Fade assuming this is a Fade In
> +    if (fade->fade_state == VF_FADE_WAITING) {
> +            fade->factor=0;
> +            if ((frame_timestamp > fade->start_time) && (fade->frame_index >= fade->start_frame)) {
> +                // Time to start fading
> +                fade->fade_state = VF_FADE_FADING;
> +                
> +                // Save start time in case we are starting based on frames and fading based on time
> +                if ((fade->start_time == 0) && (fade->start_frame != 0)) {
> +                    fade->start_time = frame_timestamp;
> +                }
> +                
> +                // Save start frame in case we are starting based on time and fading based on frames
> +                if ((fade->start_time != 0) && (fade->start_frame == 0)) {
> +                    fade->start_frame = fade->frame_index;
> +                }
> +            }

broken indentation.

[...]
> @@ -214,6 +256,14 @@ static const AVOption fade_options[] = {
>                                                      OFFSET(nb_frames),   AV_OPT_TYPE_INT, { .i64 = 25 }, 0, INT_MAX, FLAGS },
>      { "n",           "Number of frames to which the effect should be applied.",
>                                                      OFFSET(nb_frames),   AV_OPT_TYPE_INT, { .i64 = 25 }, 0, INT_MAX, FLAGS },

> +    { "start_time",  "Number of seconds of the beginning of the effect.",    
> +                                                    OFFSET(start_time),   AV_OPT_TYPE_DOUBLE, {.dbl = 0.   }, 0, 7*24*60*60,FLAGS },

Here and in the following, you can use AV_OPT_TYPE_DURATION.

> +    { "st",          "Number of seconds of the beginning of the effect.",    
> +                                                    OFFSET(start_time),   AV_OPT_TYPE_DOUBLE, {.dbl = 0.   }, 0, 7*24*60*60,FLAGS },
> +    { "duration",    "Duration of the effect in seconds.", 
> +                                                    OFFSET(duration),     AV_OPT_TYPE_DOUBLE, {.dbl = 0.   }, 0, 24*60*60,  FLAGS },
> +    { "d",            "Duration of the effect in seconds.", 

                       ^ off-by-one space align.

> +                                                    OFFSET(duration),     AV_OPT_TYPE_DOUBLE, {.dbl = 0.   }, 0, 24*60*60,  FLAGS },
>      { "alpha",       "fade alpha if it is available on the input", OFFSET(alpha),       AV_OPT_TYPE_INT, {.i64 = 0    }, 0,       1, FLAGS },
>      { NULL },


The patch looks mostly good from a technical point of view. So I'd like to
apply this when you have addressed the above comments.

Also, please remove all the trailing whitespaces you have all over your
patch.

Note that it could be relevant to update the FATE test to make use of
these new options.

Thanks for the patch, and sorry for the delayed answer. I'm looking
forward the next patch.

-- 
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/20130419/1ed32df3/attachment.asc>


More information about the ffmpeg-devel mailing list