diff mbox

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

Message ID 20170908170320.31831-1-tfoucu@gmail.com
State Superseded
Headers show

Commit Message

Thierry Foucu Sept. 8, 2017, 5:03 p.m. UTC 
---
 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(-)

Comments

Thomas Mundt Sept. 8, 2017, 11:33 p.m. UTC | #1 
Hi Thierry,

2017-09-08 19:03 GMT+02:00 Thierry Foucu <tfoucu@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.
Thierry Foucu Sept. 11, 2017, 9 p.m. UTC | #2 
Please find attached new patch. I tried it against ticket 2674 and it seems to work
Here are the output:

For the up-sample, getting 48 frames out, instead of 47
------------------------------
./ffmpeg -loglevel verbose -i 24fps.avi -vf fps=48 -y 48fps.avi
ffmpeg version N-87234-g9a32769f5e Copyright (c) 2000-2017 the FFmpeg developers
  built with gcc 4.8 (Ubuntu 4.8.4-2ubuntu1~14.04.3)
  configuration: 
  libavutil      55. 74.100 / 55. 74.100
  libavcodec     57.105.100 / 57.105.100
  libavformat    57. 81.100 / 57. 81.100
  libavdevice    57.  8.100 / 57.  8.100
  libavfilter     6.103.100 /  6.103.100
  libswscale      4.  7.103 /  4.  7.103
  libswresample   2.  8.100 /  2.  8.100
Input #0, avi, from '24fps.avi':
  Metadata:
    encoder         : Lavf57.81.100
  Duration: 00:00:01.00, start: 0.000000, bitrate: 368 kb/s
    Stream #0:0: Video: mpeg4 (Simple Profile), 1 reference frame (FMP4 / 0x34504D46), yuv420p(left), 320x240 [SAR 1:1 DAR 4:3], 24 fps, 24 tbr, 24 tbn, 24 tbc
Stream mapping:
  Stream #0:0 -> #0:0 (mpeg4 (native) -> mpeg4 (native))
Press [q] to stop, [?] for help
[Parsed_fps_0 @ 0x34ea4a0] fps=48/1
[graph 0 input from stream 0:0 @ 0x365ca20] w:320 h:240 pixfmt:yuv420p tb:1/24 fr:24/1 sar:1/1 sws_param:flags=2
Output #0, avi, to '48fps.avi':
  Metadata:
    ISFT            : Lavf57.81.100
    Stream #0:0: Video: mpeg4, 1 reference frame (FMP4 / 0x34504D46), yuv420p(left), 320x240 [SAR 1:1 DAR 4:3], q=2-31, 200 kb/s, 48 fps, 48 tbn, 48 tbc
    Metadata:
      encoder         : Lavc57.105.100 mpeg4
    Side data:
      cpb: bitrate max/min/avg: 0/0/200000 buffer size: 0 vbv_delay: -1
No more output streams to write to, finishing.
frame=   48 fps=0.0 q=2.0 Lsize=      67kB time=00:00:01.00 bitrate= 552.9kbits/s speed=28.2x    
video:61kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: 11.158647%
Input file #0 (24fps.avi):
  Input stream #0:0 (video): 24 packets read (39693 bytes); 24 frames decoded; 
  Total: 24 packets (39693 bytes) demuxed
Output file #0 (48fps.avi):
  Output stream #0:0 (video): 48 frames encoded; 48 packets muxed (62176 bytes); 
  Total: 48 packets (62176 bytes) muxed
[Parsed_fps_0 @ 0x34ea4a0] 24 frames in, 48 frames out; 0 frames dropped, 23 frames duplicated.
------------------------------


For down-sample, getting 12 frames out instead of 13
-----------------------------
./ffmpeg -loglevel verbose -i 24fps.avi -vf fps=12 -y 12fps.avi
ffmpeg version N-87234-g9a32769f5e Copyright (c) 2000-2017 the FFmpeg developers
  built with gcc 4.8 (Ubuntu 4.8.4-2ubuntu1~14.04.3)
  configuration: 
  libavutil      55. 74.100 / 55. 74.100
  libavcodec     57.105.100 / 57.105.100
  libavformat    57. 81.100 / 57. 81.100
  libavdevice    57.  8.100 / 57.  8.100
  libavfilter     6.103.100 /  6.103.100
  libswscale      4.  7.103 /  4.  7.103
  libswresample   2.  8.100 /  2.  8.100
Input #0, avi, from '24fps.avi':
  Metadata:
    encoder         : Lavf57.81.100
  Duration: 00:00:01.00, start: 0.000000, bitrate: 368 kb/s
    Stream #0:0: Video: mpeg4 (Simple Profile), 1 reference frame (FMP4 / 0x34504D46), yuv420p(left), 320x240 [SAR 1:1 DAR 4:3], 24 fps, 24 tbr, 24 tbn, 24 tbc
Stream mapping:
  Stream #0:0 -> #0:0 (mpeg4 (native) -> mpeg4 (native))
Press [q] to stop, [?] for help
[Parsed_fps_0 @ 0x304e4a0] fps=12/1
[graph 0 input from stream 0:0 @ 0x31c0aa0] w:320 h:240 pixfmt:yuv420p tb:1/24 fr:24/1 sar:1/1 sws_param:flags=2
Output #0, avi, to '12fps.avi':
  Metadata:
    ISFT            : Lavf57.81.100
    Stream #0:0: Video: mpeg4, 1 reference frame (FMP4 / 0x34504D46), yuv420p(left), 320x240 [SAR 1:1 DAR 4:3], q=2-31, 200 kb/s, 12 fps, 12 tbn, 12 tbc
    Metadata:
      encoder         : Lavc57.105.100 mpeg4
    Side data:
      cpb: bitrate max/min/avg: 0/0/200000 buffer size: 0 vbv_delay: -1
No more output streams to write to, finishing.
frame=   12 fps=0.0 q=2.0 Lsize=      26kB time=00:00:01.00 bitrate= 211.3kbits/s speed=39.8x    
video:20kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: 29.731853%
Input file #0 (24fps.avi):
  Input stream #0:0 (video): 24 packets read (39693 bytes); 24 frames decoded; 
  Total: 24 packets (39693 bytes) demuxed
Output file #0 (12fps.avi):
  Output stream #0:0 (video): 12 frames encoded; 12 packets muxed (20362 bytes); 
  Total: 12 packets (20362 bytes) muxed
[Parsed_fps_0 @ 0x304e4a0] 24 frames in, 12 frames out; 12 frames dropped, 0 frames duplicated.
Thierry Foucu Sept. 11, 2017, 9:03 p.m. UTC | #3 
See new patch.


Getting 12 frame out when down-sample (instead of 13)
===================================
I tried it against ticket 2674 and here is the output:
ffmpeg -loglevel verbose -i 24fps.avi -vf fps=12 -y 12fps.avi
ffmpeg version N-87234-g9a32769f5e Copyright (c) 2000-2017 the FFmpeg
developers
  built with gcc 4.8 (Ubuntu 4.8.4-2ubuntu1~14.04.3)
  configuration:
  libavutil      55. 74.100 / 55. 74.100
  libavcodec     57.105.100 / 57.105.100
  libavformat    57. 81.100 / 57. 81.100
  libavdevice    57.  8.100 / 57.  8.100
  libavfilter     6.103.100 /  6.103.100
  libswscale      4.  7.103 /  4.  7.103
  libswresample   2.  8.100 /  2.  8.100
Input #0, avi, from '24fps.avi':
  Metadata:
    encoder         : Lavf57.81.100
  Duration: 00:00:01.00, start: 0.000000, bitrate: 368 kb/s
    Stream #0:0: Video: mpeg4 (Simple Profile), 1 reference frame (FMP4 /
0x34504D46), yuv420p(left), 320x240 [SAR 1:1 DAR 4:3], 24 fps, 24 tbr, 24
tbn, 24 tbc
Stream mapping:
  Stream #0:0 -> #0:0 (mpeg4 (native) -> mpeg4 (native))
Press [q] to stop, [?] for help
[Parsed_fps_0 @ 0x304e4a0] fps=12/1
[graph 0 input from stream 0:0 @ 0x31c0aa0] w:320 h:240 pixfmt:yuv420p
tb:1/24 fr:24/1 sar:1/1 sws_param:flags=2
Output #0, avi, to '12fps.avi':
  Metadata:
    ISFT            : Lavf57.81.100
    Stream #0:0: Video: mpeg4, 1 reference frame (FMP4 / 0x34504D46),
yuv420p(left), 320x240 [SAR 1:1 DAR 4:3], q=2-31, 200 kb/s, 12 fps, 12 tbn,
12 tbc
    Metadata:
      encoder         : Lavc57.105.100 mpeg4
    Side data:
      cpb: bitrate max/min/avg: 0/0/200000 buffer size: 0 vbv_delay: -1
No more output streams to write to, finishing.
frame=   12 fps=0.0 q=2.0 Lsize=      26kB time=00:00:01.00 bitrate=
211.3kbits/s speed=39.8x
video:20kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB
muxing overhead: 29.731853%
Input file #0 (24fps.avi):
  Input stream #0:0 (video): 24 packets read (39693 bytes); 24 frames
decoded;
  Total: 24 packets (39693 bytes) demuxed
Output file #0 (12fps.avi):
  Output stream #0:0 (video): 12 frames encoded; 12 packets muxed (20362
bytes);
  Total: 12 packets (20362 bytes) muxed
[Parsed_fps_0 @ 0x304e4a0] 24 frames in, 12 frames out; 12 frames dropped,
0 frames duplicated.


getting 48 frames out instead of 47
=====================================================
ffmpeg -loglevel verbose -i 24fps.avi -vf fps=48 -y 48fps.avi
ffmpeg version N-87234-g9a32769f5e Copyright (c) 2000-2017 the FFmpeg
developers
  built with gcc 4.8 (Ubuntu 4.8.4-2ubuntu1~14.04.3)
  configuration:
  libavutil      55. 74.100 / 55. 74.100
  libavcodec     57.105.100 / 57.105.100
  libavformat    57. 81.100 / 57. 81.100
  libavdevice    57.  8.100 / 57.  8.100
  libavfilter     6.103.100 /  6.103.100
  libswscale      4.  7.103 /  4.  7.103
  libswresample   2.  8.100 /  2.  8.100
Input #0, avi, from '24fps.avi':
  Metadata:
    encoder         : Lavf57.81.100
  Duration: 00:00:01.00, start: 0.000000, bitrate: 368 kb/s
    Stream #0:0: Video: mpeg4 (Simple Profile), 1 reference frame (FMP4 /
0x34504D46), yuv420p(left), 320x240 [SAR 1:1 DAR 4:3], 24 fps, 24 tbr, 24
tbn, 24 tbc
Stream mapping:
  Stream #0:0 -> #0:0 (mpeg4 (native) -> mpeg4 (native))
Press [q] to stop, [?] for help
[Parsed_fps_0 @ 0x34ea4a0] fps=48/1
[graph 0 input from stream 0:0 @ 0x365ca20] w:320 h:240 pixfmt:yuv420p
tb:1/24 fr:24/1 sar:1/1 sws_param:flags=2
Output #0, avi, to '48fps.avi':
  Metadata:
    ISFT            : Lavf57.81.100
    Stream #0:0: Video: mpeg4, 1 reference frame (FMP4 / 0x34504D46),
yuv420p(left), 320x240 [SAR 1:1 DAR 4:3], q=2-31, 200 kb/s, 48 fps, 48 tbn,
48 tbc
    Metadata:
      encoder         : Lavc57.105.100 mpeg4
    Side data:
      cpb: bitrate max/min/avg: 0/0/200000 buffer size: 0 vbv_delay: -1
No more output streams to write to, finishing.
frame=   48 fps=0.0 q=2.0 Lsize=      67kB time=00:00:01.00 bitrate=
552.9kbits/s speed=28.2x
video:61kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB
muxing overhead: 11.158647%
Input file #0 (24fps.avi):
  Input stream #0:0 (video): 24 packets read (39693 bytes); 24 frames
decoded;
  Total: 24 packets (39693 bytes) demuxed
Output file #0 (48fps.avi):
  Output stream #0:0 (video): 48 frames encoded; 48 packets muxed (62176
bytes);
  Total: 48 packets (62176 bytes) muxed
[Parsed_fps_0 @ 0x34ea4a0] 24 frames in, 48 frames out; 0 frames dropped,
23 frames duplicated.


On Fri, Sep 8, 2017 at 4:33 PM, Thomas Mundt <tmundt75@gmail.com> wrote:

> Hi Thierry,
>
> 2017-09-08 19:03 GMT+02:00 Thierry Foucu <tfoucu@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.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
diff mbox

Patch

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;
     }
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