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

Submitted by Thierry Foucu on Sept. 11, 2017, 9 p.m.

Details

Message ID 20170911210016.69852-2-tfoucu@gmail.com
State New
Headers show

Commit Message

Thierry Foucu Sept. 11, 2017, 9 p.m.
Fix ticket #2674
Tested with examples from ticket 2674.
---
 libavfilter/vf_fps.c        | 40 +++++++++++++++++++++++++++++++++++-----
 tests/ref/fate/filter-fps   |  6 ++++++
 tests/ref/fate/filter-fps-r |  4 ++++
 tests/ref/fate/m4v-cfr      |  1 -
 4 files changed, 45 insertions(+), 6 deletions(-)

Comments

Thomas Mundt Sept. 12, 2017, 12:01 a.m.
Hi Thierry,

2017-09-11 23:00 GMT+02:00 Thierry Foucu <tfoucu@gmail.com>:

> Fix ticket #2674
> Tested with examples from ticket 2674.
> ---
>  libavfilter/vf_fps.c        | 40 +++++++++++++++++++++++++++++++++++-----
>  tests/ref/fate/filter-fps   |  6 ++++++
>  tests/ref/fate/filter-fps-r |  4 ++++
>  tests/ref/fate/m4v-cfr      |  1 -
>  4 files changed, 45 insertions(+), 6 deletions(-)
>
> diff --git a/libavfilter/vf_fps.c b/libavfilter/vf_fps.c
> index 20ccd797d1..c0d68d8972 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,41 @@ 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++;
>
Duplicated frames are reported to verbose log by the dup variable.
You need to insert something like if(j) s->dup++ here.
Otherwise the log becomes wrong, as in the 48fps example:
Parsed_fps_0 @ 0x34ea4a0] 24 frames in, 48 frames out; 0 frames dropped,
23 frames duplicated.

Anyway, the conversion results are accurate now for any sample I tested.
Thanks.

Patch hide | download patch | download mbox

diff --git a/libavfilter/vf_fps.c b/libavfilter/vf_fps.c
index 20ccd797d1..c0d68d8972 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,41 @@  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 {
+                    /* for delta less or equal to 0, we should drop the frame,
+                     * otherwise, we will have one or more extra frames */
+                    av_frame_free(&buf);
+                    s->drop++;
+                }
+            }
         }
         return 0;
     }
diff --git a/tests/ref/fate/filter-fps b/tests/ref/fate/filter-fps
index 55712cfb1c..242fb04e85 100644
--- a/tests/ref/fate/filter-fps
+++ b/tests/ref/fate/filter-fps
@@ -85,3 +85,9 @@ 
 0,         79,         79,        1,    30576, 0xa2fcd06f
 0,         80,         80,        1,    30576, 0xa2fcd06f
 0,         81,         81,        1,    30576, 0xd4150aad
+0,         82,         82,        1,    30576, 0xd4150aad
+0,         83,         83,        1,    30576, 0xd4150aad
+0,         84,         84,        1,    30576, 0xd4150aad
+0,         85,         85,        1,    30576, 0xd4150aad
+0,         86,         86,        1,    30576, 0xd4150aad
+0,         87,         87,        1,    30576, 0xd4150aad
diff --git a/tests/ref/fate/filter-fps-r b/tests/ref/fate/filter-fps-r
index 826b1ed6c6..c1bc7d1547 100644
--- a/tests/ref/fate/filter-fps-r
+++ b/tests/ref/fate/filter-fps-r
@@ -72,3 +72,7 @@ 
 0,         79,         79,        1,    30576, 0xa2fcd06f
 0,         80,         80,        1,    30576, 0xa2fcd06f
 0,         82,         82,        1,    30576, 0xd4150aad
+0,         83,         83,        1,    30576, 0xd4150aad
+0,         84,         84,        1,    30576, 0xd4150aad
+0,         85,         85,        1,    30576, 0xd4150aad
+0,         86,         86,        1,    30576, 0xd4150aad
diff --git a/tests/ref/fate/m4v-cfr b/tests/ref/fate/m4v-cfr
index 4eee84d01b..e2d02032fe 100644
--- a/tests/ref/fate/m4v-cfr
+++ b/tests/ref/fate/m4v-cfr
@@ -44,4 +44,3 @@ 
 0,         38,         38,        1,   115200, 0xf30825d5
 0,         39,         39,        1,   115200, 0xe3c944a1
 0,         40,         40,        1,   115200, 0x8fec4420
-0,         41,         41,        1,   115200, 0x9381fdab