[FFmpeg-devel] [PATCH] lavfi/fps: rescale input PTS immediately.

[Nicolas George]

Keeping it in its original time base leads to more complex code
and systematic rounding errors.

Also change default rounding direction: rounding down is more
logical, rounding up was a workaround for the systematic errors.

Fix trac ticket #3329 and part of #2674.

Signed-off-by: Nicolas George <george at nsup.org>
---
 libavfilter/vf_fps.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)


The 24->48 part of #2674 is due to the lack of frame duration or EOF PTS.
I intend to propose an API that will allow, amongst other things, EOF PTS.

This code also has a lot of room for improvement. Amongst other things:

* Use AV_OPT_TYPE_DURATION for start_time and get rid of floating-point
  arithmetic.

* Keep the first frame separately from the FIFO.


diff --git a/libavfilter/vf_fps.c b/libavfilter/vf_fps.c
index e6266cc..c835e7b 100644
--- a/libavfilter/vf_fps.c
+++ b/libavfilter/vf_fps.c
@@ -65,7 +65,7 @@ typedef struct FPSContext {
 static const AVOption fps_options[] = {
     { "fps", "A string describing desired output framerate", OFFSET(framerate), AV_OPT_TYPE_VIDEO_RATE, { .str = "25" }, .flags = V|F },
     { "start_time", "Assume the first PTS should be this value.", OFFSET(start_time), AV_OPT_TYPE_DOUBLE, { .dbl = DBL_MAX}, -DBL_MAX, DBL_MAX, V },
-    { "round", "set rounding method for timestamps", OFFSET(rounding), AV_OPT_TYPE_INT, { .i64 = AV_ROUND_NEAR_INF }, 0, 5, V|F, "round" },
+    { "round", "set rounding method for timestamps", OFFSET(rounding), AV_OPT_TYPE_INT, { .i64 = AV_ROUND_DOWN }, 0, 5, V|F, "round" },
     { "zero", "round towards 0",      OFFSET(rounding), AV_OPT_TYPE_CONST, { .i64 = AV_ROUND_ZERO     }, 0, 5, V|F, "round" },
     { "inf",  "round away from 0",    OFFSET(rounding), AV_OPT_TYPE_CONST, { .i64 = AV_ROUND_INF      }, 0, 5, V|F, "round" },
     { "down", "round towards -infty", OFFSET(rounding), AV_OPT_TYPE_CONST, { .i64 = AV_ROUND_DOWN     }, 0, 5, V|F, "round" },
@@ -141,8 +141,7 @@ 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;
+            buf->pts = s->first_pts + s->frames_out;
 
             if ((ret = ff_filter_frame(outlink, buf)) < 0)
                 return ret;
@@ -177,6 +176,10 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *buf)
     int64_t delta;
     int i, ret;
 
+    if (buf->pts != AV_NOPTS_VALUE)
+        buf->pts = av_rescale_q_rnd(buf->pts, inlink->time_base,
+                                    outlink->time_base, s->rounding);
+
     s->frames_in++;
     /* discard frames until we get the first timestamp */
     if (s->pts == AV_NOPTS_VALUE) {
@@ -189,10 +192,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *buf)
                 double first_pts = s->start_time * AV_TIME_BASE;
                 first_pts = FFMIN(FFMAX(first_pts, INT64_MIN), INT64_MAX);
                 s->first_pts = s->pts = av_rescale_q(first_pts, AV_TIME_BASE_Q,
-                                                     inlink->time_base);
-                av_log(ctx, AV_LOG_VERBOSE, "Set first pts to (in:%"PRId64" out:%"PRId64")\n",
-                       s->first_pts, av_rescale_q(first_pts, AV_TIME_BASE_Q,
-                                                  outlink->time_base));
+                                                     outlink->time_base);
+                av_log(ctx, AV_LOG_VERBOSE, "Set first pts to %"PRId64"\n",
+                       s->first_pts);
             } else {
                 s->first_pts = s->pts = buf->pts;
             }
@@ -211,8 +213,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *buf)
     }
 
     /* number of output frames */
-    delta = av_rescale_q_rnd(buf->pts - s->pts, inlink->time_base,
-                             outlink->time_base, s->rounding);
+    delta = buf->pts - s->pts;
 
     if (delta < 1) {
         /* drop the frame and everything buffered except the first */
@@ -254,8 +255,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *buf)
             s->dup++;
         }
 
-        buf_out->pts = av_rescale_q(s->first_pts, inlink->time_base,
-                                    outlink->time_base) + s->frames_out;
+        buf_out->pts = s->first_pts + s->frames_out;
 
         if ((ret = ff_filter_frame(outlink, buf_out)) < 0) {
             av_frame_free(&buf);
@@ -267,7 +267,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *buf)
     flush_fifo(s->fifo);
 
     ret = write_to_fifo(s->fifo, buf);
-    s->pts = s->first_pts + av_rescale_q(s->frames_out, outlink->time_base, inlink->time_base);
+    s->pts = s->first_pts + s->frames_out;
 
     return ret;
 }
-- 
1.8.5.2