diff mbox series

[FFmpeg-devel,2/4] lavfi/framesync: reorder functions to avoid a forward declaration

Message ID 20230127131639.4928-2-anton@khirnov.net
State New
Headers show
Series [FFmpeg-devel,1/4] lavfi/framesync: use a local variable to shorten code | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Anton Khirnov Jan. 27, 2023, 1:16 p.m. UTC 
---
 libavfilter/framesync.c | 144 ++++++++++++++++++++--------------------
 1 file changed, 71 insertions(+), 73 deletions(-)

Comments

Nicolas George Jan. 27, 2023, 2:47 p.m. UTC | #1 
Anton Khirnov (12023-01-27):
> ---
>  libavfilter/framesync.c | 144 ++++++++++++++++++++--------------------
>  1 file changed, 71 insertions(+), 73 deletions(-)

Getting rid of the forward declaration would be a good idea. But the
order you have put the functions is completely illogical. For example
you separated ff_framesync_preinit() and ff_framesync_init() with
functions that have nothing to do with initialization.

Moving framesync_advance() closer to its call point would probably be a
much better idea.

So: not like this.
diff mbox series

Patch

diff --git a/libavfilter/framesync.c b/libavfilter/framesync.c
index 153db4fa21..fdcc3b57c8 100644
--- a/libavfilter/framesync.c
+++ b/libavfilter/framesync.c
@@ -73,8 +73,6 @@  enum {
     STATE_EOF,
 };
 
-static int consume_from_fifos(FFFrameSync *fs);
-
 void ff_framesync_preinit(FFFrameSync *fs)
 {
     if (fs->class)
@@ -181,6 +179,77 @@  int ff_framesync_configure(FFFrameSync *fs)
     return 0;
 }
 
+static void framesync_inject_frame(FFFrameSync *fs, unsigned in, AVFrame *frame)
+{
+    int64_t pts;
+
+    av_assert0(!fs->in[in].have_next);
+    av_assert0(frame);
+    pts = av_rescale_q(frame->pts, fs->in[in].time_base, fs->time_base);
+    frame->pts = pts;
+    fs->in[in].frame_next = frame;
+    fs->in[in].pts_next   = pts;
+    fs->in[in].have_next  = 1;
+}
+
+static int64_t framesync_pts_extrapolate(FFFrameSync *fs, unsigned in,
+                                         int64_t pts)
+{
+    /* Possible enhancement: use the link's frame rate */
+    return pts + 1;
+}
+
+static void framesync_inject_status(FFFrameSync *fs, unsigned in, int status, int64_t pts)
+{
+    av_assert0(!fs->in[in].have_next);
+    pts = fs->in[in].state != STATE_RUN || fs->in[in].after == EXT_INFINITY
+        ? INT64_MAX : framesync_pts_extrapolate(fs, in, fs->in[in].pts);
+    fs->in[in].sync = 0;
+    framesync_sync_level_update(fs);
+    fs->in[in].frame_next = NULL;
+    fs->in[in].pts_next   = pts;
+    fs->in[in].have_next  = 1;
+}
+
+static int consume_from_fifos(FFFrameSync *fs)
+{
+    AVFilterContext *ctx = fs->parent;
+    AVFrame *frame = NULL;
+    int64_t pts;
+    unsigned i, nb_active, nb_miss;
+    int ret, status;
+
+    nb_active = nb_miss = 0;
+    for (i = 0; i < fs->nb_in; i++) {
+        if (fs->in[i].have_next || fs->in[i].state == STATE_EOF)
+            continue;
+        nb_active++;
+        ret = ff_inlink_consume_frame(ctx->inputs[i], &frame);
+        if (ret < 0)
+            return ret;
+        if (ret) {
+            av_assert0(frame);
+            framesync_inject_frame(fs, i, frame);
+        } else {
+            ret = ff_inlink_acknowledge_status(ctx->inputs[i], &status, &pts);
+            if (ret > 0) {
+                framesync_inject_status(fs, i, status, pts);
+            } else if (!ret) {
+                nb_miss++;
+            }
+        }
+    }
+    if (nb_miss) {
+        if (nb_miss == nb_active && !ff_outlink_frame_wanted(ctx->outputs[0]))
+            return FFERROR_NOT_READY;
+        for (i = 0; i < fs->nb_in; i++)
+            if (!fs->in[i].have_next && fs->in[i].state != STATE_EOF)
+                ff_inlink_request_frame(ctx->inputs[i]);
+        return 0;
+    }
+    return 1;
+}
+
 static int framesync_advance(FFFrameSync *fs)
 {
     unsigned i;
@@ -231,38 +300,6 @@  static int framesync_advance(FFFrameSync *fs)
     return 0;
 }
 
-static int64_t framesync_pts_extrapolate(FFFrameSync *fs, unsigned in,
-                                         int64_t pts)
-{
-    /* Possible enhancement: use the link's frame rate */
-    return pts + 1;
-}
-
-static void framesync_inject_frame(FFFrameSync *fs, unsigned in, AVFrame *frame)
-{
-    int64_t pts;
-
-    av_assert0(!fs->in[in].have_next);
-    av_assert0(frame);
-    pts = av_rescale_q(frame->pts, fs->in[in].time_base, fs->time_base);
-    frame->pts = pts;
-    fs->in[in].frame_next = frame;
-    fs->in[in].pts_next   = pts;
-    fs->in[in].have_next  = 1;
-}
-
-static void framesync_inject_status(FFFrameSync *fs, unsigned in, int status, int64_t pts)
-{
-    av_assert0(!fs->in[in].have_next);
-    pts = fs->in[in].state != STATE_RUN || fs->in[in].after == EXT_INFINITY
-        ? INT64_MAX : framesync_pts_extrapolate(fs, in, fs->in[in].pts);
-    fs->in[in].sync = 0;
-    framesync_sync_level_update(fs);
-    fs->in[in].frame_next = NULL;
-    fs->in[in].pts_next   = pts;
-    fs->in[in].have_next  = 1;
-}
-
 int ff_framesync_get_frame(FFFrameSync *fs, unsigned in, AVFrame **rframe,
                             unsigned get)
 {
@@ -312,45 +349,6 @@  void ff_framesync_uninit(FFFrameSync *fs)
     av_freep(&fs->in);
 }
 
-static int consume_from_fifos(FFFrameSync *fs)
-{
-    AVFilterContext *ctx = fs->parent;
-    AVFrame *frame = NULL;
-    int64_t pts;
-    unsigned i, nb_active, nb_miss;
-    int ret, status;
-
-    nb_active = nb_miss = 0;
-    for (i = 0; i < fs->nb_in; i++) {
-        if (fs->in[i].have_next || fs->in[i].state == STATE_EOF)
-            continue;
-        nb_active++;
-        ret = ff_inlink_consume_frame(ctx->inputs[i], &frame);
-        if (ret < 0)
-            return ret;
-        if (ret) {
-            av_assert0(frame);
-            framesync_inject_frame(fs, i, frame);
-        } else {
-            ret = ff_inlink_acknowledge_status(ctx->inputs[i], &status, &pts);
-            if (ret > 0) {
-                framesync_inject_status(fs, i, status, pts);
-            } else if (!ret) {
-                nb_miss++;
-            }
-        }
-    }
-    if (nb_miss) {
-        if (nb_miss == nb_active && !ff_outlink_frame_wanted(ctx->outputs[0]))
-            return FFERROR_NOT_READY;
-        for (i = 0; i < fs->nb_in; i++)
-            if (!fs->in[i].have_next && fs->in[i].state != STATE_EOF)
-                ff_inlink_request_frame(ctx->inputs[i]);
-        return 0;
-    }
-    return 1;
-}
-
 int ff_framesync_activate(FFFrameSync *fs)
 {
     int ret;