[FFmpeg-devel] [PATCH] vf_fps: when reading EOF, using current_pts to duplicate the last frame if needed.

[Thomas Mundt]

Hi Thierry,

2017-09-08 19:03 GMT+02:00 Thierry Foucu <tfoucu at gmail.com>:

> ---
>  libavfilter/vf_fps.c        | 42 ++++++++++++++++++++++++++++++
> +++++++-----
>  tests/ref/fate/filter-fps   |  6 ++++++
>  tests/ref/fate/filter-fps-r |  4 ++++
>  3 files changed, 47 insertions(+), 5 deletions(-)
>
> diff --git a/libavfilter/vf_fps.c b/libavfilter/vf_fps.c
> index 20ccd797d1..e450723173 100644
> --- a/libavfilter/vf_fps.c
> +++ b/libavfilter/vf_fps.c
> @@ -34,6 +34,8 @@
>  #include "libavutil/opt.h"
>  #include "libavutil/parseutils.h"
>
> +#define FF_INTERNAL_FIELDS 1
> +#include "framequeue.h"
>  #include "avfilter.h"
>  #include "internal.h"
>  #include "video.h"
> @@ -137,13 +139,43 @@ static int request_frame(AVFilterLink *outlink)
>              AVFrame *buf;
>
>              av_fifo_generic_read(s->fifo, &buf, sizeof(buf), NULL);
> -            buf->pts = av_rescale_q(s->first_pts,
> ctx->inputs[0]->time_base,
> -                                    outlink->time_base) + s->frames_out;
> +            if (av_fifo_size(s->fifo)) {
> +                buf->pts = av_rescale_q(s->first_pts,
> ctx->inputs[0]->time_base,
> +                                        outlink->time_base) +
> s->frames_out;
>
> -            if ((ret = ff_filter_frame(outlink, buf)) < 0)
> -                return ret;
> +                if ((ret = ff_filter_frame(outlink, buf)) < 0)
> +                    return ret;
>
> -            s->frames_out++;
> +                s->frames_out++;
> +            } else {
> +                /* This is the last frame, we may have to duplicate it to
> match
> +                 * the last frame duration */
> +                int j;
> +                int delta = av_rescale_q_rnd(ctx->inputs[0]->current_pts
> - s->first_pts,
> +                                             ctx->inputs[0]->time_base,
> +                                             outlink->time_base,
> s->rounding) - s->frames_out ;
> +                if (delta > 0 ) {
> +                    for (j = 0; j < delta; j++) {
> +                        AVFrame *dup = av_frame_clone(buf);
> +
> +                        av_log(ctx, AV_LOG_DEBUG, "Duplicating frame.\n");
> +                        dup->pts = av_rescale_q(s->first_pts,
> ctx->inputs[0]->time_base,
> +                                                outlink->time_base) +
> s->frames_out;
> +
> +                        if ((ret = ff_filter_frame(outlink, dup)) < 0)
> +                            return ret;
> +
> +                        s->frames_out++;
> +                    }
> +                } else {
> +                    buf->pts = av_rescale_q(s->first_pts,
> ctx->inputs[0]->time_base,
> +                                            outlink->time_base) +
> s->frames_out;
> +
> +                    if ((ret = ff_filter_frame(outlink, buf)) < 0)
> +                        return ret;
> +                    s->frames_out++;
> +                }
> +            }
>          }
>          return 0;
>      }
>

Your patch only resolves wrong framerate conversion durations for output
fps > input fps. I think the EOF misbehaviour should be solved for any
conversion.
I wrote a similar patch some months ago:
http://ffmpeg.org/pipermail/ffmpeg-devel/2017-March/209085.html
It was rejected because of the use of pkt_duration, like your first patch,
and the dupliction of non-trivial code. Also it used average frame duration
when pkt_duration was not available.
Now, after the patches Nicolas wrote and pushed the last weeks, it should
be possible to find a general solution.
Check the example at ticket #2674 for testing several framerate conversions.