[FFmpeg-devel] [PATCH 1/4] lavfi/framesync: use a local variable to shorten code

[FFmpeg-devel] [PATCH 1/4] lavfi/framesync: use a local variable to shorten code

[Anton Khirnov]

---
 libavfilter/framesync.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/libavfilter/framesync.c b/libavfilter/framesync.c
index ee91e4cf68..153db4fa21 100644
--- a/libavfilter/framesync.c
+++ b/libavfilter/framesync.c
@@ -201,24 +201,23 @@ static int framesync_advance(FFFrameSync *fs)
             break;
         }
         for (i = 0; i < fs->nb_in; i++) {
-            if (fs->in[i].pts_next == pts ||
-                (fs->in[i].ts_mode == TS_NEAREST &&
-                 fs->in[i].have_next &&
-                 fs->in[i].pts_next != INT64_MAX && fs->in[i].pts != AV_NOPTS_VALUE &&
-                 fs->in[i].pts_next - pts < pts - fs->in[i].pts) ||
-                (fs->in[i].before == EXT_INFINITY &&
-                 fs->in[i].state == STATE_BOF)) {
-                av_frame_free(&fs->in[i].frame);
-                fs->in[i].frame      = fs->in[i].frame_next;
-                fs->in[i].pts        = fs->in[i].pts_next;
-                fs->in[i].frame_next = NULL;
-                fs->in[i].pts_next   = AV_NOPTS_VALUE;
-                fs->in[i].have_next  = 0;
-                fs->in[i].state      = fs->in[i].frame ? STATE_RUN : STATE_EOF;
-                if (fs->in[i].sync == fs->sync_level && fs->in[i].frame)
+            FFFrameSyncIn * const in = &fs->in[i];
+
+            if (in->pts_next == pts ||
+                (in->ts_mode == TS_NEAREST && in->have_next             &&
+                 in->pts_next != INT64_MAX && in->pts != AV_NOPTS_VALUE &&
+                 in->pts_next - pts < pts - in->pts) ||
+                (in->before == EXT_INFINITY && in->state == STATE_BOF)) {
+                av_frame_free(&in->frame);
+                in->frame      = in->frame_next;
+                in->pts        = in->pts_next;
+                in->frame_next = NULL;
+                in->pts_next   = AV_NOPTS_VALUE;
+                in->have_next  = 0;
+                in->state      = in->frame ? STATE_RUN : STATE_EOF;
+                if (in->sync == fs->sync_level && in->frame)
                     fs->frame_ready = 1;
-                if (fs->in[i].state == STATE_EOF &&
-                    fs->in[i].after == EXT_STOP)
+                if (in->state == STATE_EOF && in->after == EXT_STOP)
                     framesync_eof(fs);
             }
         }
-- 
2.35.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[Anton Khirnov]

---
 libavfilter/framesync.c | 144 ++++++++++++++++++++--------------------
 1 file changed, 71 insertions(+), 73 deletions(-)

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

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/4] lavfi/framesync: add syncing via external timestamp map

[Anton Khirnov]

Useful when there is some external process that determines canonical
frame synchronization. E.g. the framerate conversion code in ffmpeg CLI.
---
 doc/filters.texi        |   6 ++
 libavfilter/framesync.c | 121 ++++++++++++++++++++++++++++++++++++++--
 libavfilter/framesync.h |  11 ++++
 3 files changed, 132 insertions(+), 6 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index be70a2396b..2fc50f3a91 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -363,6 +363,12 @@ primary input frame.
 Frame from secondary input with the absolute nearest timestamp to the primary
 input frame.
 @end table
+
+@item ts_map
+Specify an explicit timestamp map. The string should be composed of lines, one
+per each output frame. The line should contain whitespace-separated times in
+microseconds, one for every input. Frames with these timestamps will be matched
+together to produces output events.
 @end table
 
 @c man end OPTIONS FOR FILTERS WITH SEVERAL INPUTS
diff --git a/libavfilter/framesync.c b/libavfilter/framesync.c
index fdcc3b57c8..b52cf318c0 100644
--- a/libavfilter/framesync.c
+++ b/libavfilter/framesync.c
@@ -49,6 +49,7 @@ static const AVOption framesync_options[] = {
             0, AV_OPT_TYPE_CONST, { .i64 = TS_DEFAULT }, .flags = FLAGS, "ts_sync_mode" },
         { "nearest", "Frame from secondary input with the absolute nearest timestamp to the primary input frame",
             0, AV_OPT_TYPE_CONST, { .i64 = TS_NEAREST }, .flags = FLAGS, "ts_sync_mode" },
+    { "ts_map", "Timestamp map", OFFSET(ts_map_str), AV_OPT_TYPE_STRING, .flags = FLAGS },
     { NULL }
 };
 static const AVClass framesync_class = {
@@ -129,10 +130,78 @@ static void framesync_sync_level_update(FFFrameSync *fs)
         framesync_eof(fs);
 }
 
+static int ts_map_parse(FFFrameSync *fs, const char *ts_map_str)
+{
+    while (*ts_map_str) {
+        int64_t *dst;
+
+        ts_map_str += strspn(ts_map_str, " \t\r\n");
+
+        // skip comments
+        if (*ts_map_str == '#' || !*ts_map_str)
+            goto skip_line;
+
+        dst = av_fast_realloc(fs->ts_map, &fs->ts_map_allocated,
+                              sizeof(*fs->ts_map) * fs->nb_in * (fs->nb_ts_map + 1));
+        if (!dst)
+            return AVERROR(ENOMEM);
+
+        fs->ts_map = dst;
+        dst += fs->nb_in * fs->nb_ts_map;
+        fs->nb_ts_map++;
+
+        // read a timestamp for each input
+        for (int i = 0; i < fs->nb_in; i++) {
+            char *p;
+            dst[i] = strtol(ts_map_str, &p, 0);
+            if (p == ts_map_str) {
+                av_log(fs, AV_LOG_ERROR,
+                       "Invalid number in timestamp map on line %zu: %s\n",
+                       fs->nb_ts_map - 1, ts_map_str);
+                return AVERROR_INVALIDDATA;
+            }
+            ts_map_str = p;
+
+            if (fs->nb_ts_map > 1 && dst[i - (int)fs->nb_in] > dst[i]) {
+                av_log(fs, AV_LOG_ERROR,
+                       "Timestamp map for input %d, frame %zu goes backwards\n",
+                       i, fs->nb_ts_map - 1);
+                return AVERROR_INVALIDDATA;
+            }
+
+            ts_map_str += strspn(p, " \t");
+        }
+
+        // skip everything after the needed timestamp
+skip_line:
+        ts_map_str = strchr(ts_map_str, '\n');
+        if (!ts_map_str)
+            break;
+    }
+
+    return 0;
+}
+
 int ff_framesync_configure(FFFrameSync *fs)
 {
     unsigned i;
 
+    if (fs->ts_map_str) {
+        int ret;
+
+        if (fs->opt_ts_sync_mode != TS_DEFAULT) {
+            av_log(fs, AV_LOG_ERROR,
+                   "ts_sync_mode must be set to default when a map is used\n");
+            return AVERROR(EINVAL);
+        }
+
+        ret = ts_map_parse(fs, fs->ts_map_str);
+        if (ret < 0) {
+            av_log(fs, AV_LOG_ERROR, "Error reading the explicit timestamp map\n");
+            return ret;
+        }
+    }
+
     if (!fs->opt_repeatlast || fs->opt_eof_action == EOF_ACTION_PASS) {
         fs->opt_repeatlast = 0;
         fs->opt_eof_action = EOF_ACTION_PASS;
@@ -250,17 +319,55 @@ static int consume_from_fifos(FFFrameSync *fs)
     return 1;
 }
 
+static void frame_advance(FFFrameSyncIn *in)
+{
+    av_frame_free(&in->frame);
+    in->frame      = in->frame_next;
+    in->pts        = in->pts_next;
+    in->frame_next = NULL;
+    in->pts_next   = AV_NOPTS_VALUE;
+    in->have_next  = 0;
+}
+
 static int framesync_advance(FFFrameSync *fs)
 {
     unsigned i;
     int64_t pts;
     int ret;
 
+    if (fs->ts_map && fs->nb_events >= fs->nb_ts_map) {
+        framesync_eof(fs);
+        return 0;
+    }
+
     while (!(fs->frame_ready || fs->eof)) {
         ret = consume_from_fifos(fs);
         if (ret <= 0)
             return ret;
 
+        if (fs->ts_map) {
+            fs->frame_ready = 1;
+            for (i = 0; i < fs->nb_in; i++) {
+                FFFrameSyncIn * const in = &fs->in[i];
+                int64_t next_ts = av_rescale_q(fs->ts_map[fs->nb_events * fs->nb_in + i],
+                                               AV_TIME_BASE_Q, fs->time_base);
+                uint64_t delta_cur  = in->frame      ? FFABS(in->pts      - next_ts) : UINT64_MAX;
+                uint64_t delta_next = in->frame_next ? FFABS(in->pts_next - next_ts) : UINT64_MAX;
+
+                if (!in->frame ||
+                    (in->frame_next && delta_next < delta_cur)) {
+                    frame_advance(in);
+                    fs->frame_ready = 0;
+                    in->state       = in->frame ? STATE_RUN : STATE_EOF;
+                    if (in->state == STATE_EOF) {
+                        av_log(fs, AV_LOG_WARNING,
+                               "Input stream %d ended before the timestamp map did\n", i);
+                        framesync_eof(fs);
+                    }
+                }
+            }
+            pts = fs->in[0].pts;
+        } else {
         pts = INT64_MAX;
         for (i = 0; i < fs->nb_in; i++)
             if (fs->in[i].have_next && fs->in[i].pts_next < pts)
@@ -277,12 +384,7 @@ static int framesync_advance(FFFrameSync *fs)
                  in->pts_next != INT64_MAX && in->pts != AV_NOPTS_VALUE &&
                  in->pts_next - pts < pts - in->pts) ||
                 (in->before == EXT_INFINITY && in->state == STATE_BOF)) {
-                av_frame_free(&in->frame);
-                in->frame      = in->frame_next;
-                in->pts        = in->pts_next;
-                in->frame_next = NULL;
-                in->pts_next   = AV_NOPTS_VALUE;
-                in->have_next  = 0;
+                frame_advance(in);
                 in->state      = in->frame ? STATE_RUN : STATE_EOF;
                 if (in->sync == fs->sync_level && in->frame)
                     fs->frame_ready = 1;
@@ -295,6 +397,7 @@ static int framesync_advance(FFFrameSync *fs)
                 if ((fs->in[i].state == STATE_BOF &&
                      fs->in[i].before == EXT_STOP))
                     fs->frame_ready = 0;
+        }
         fs->pts = pts;
     }
     return 0;
@@ -347,6 +450,11 @@ void ff_framesync_uninit(FFFrameSync *fs)
     }
 
     av_freep(&fs->in);
+
+    av_freep(&fs->ts_map_str);
+    av_freep(&fs->ts_map);
+    fs->nb_ts_map        = 0;
+    fs->ts_map_allocated = 0;
 }
 
 int ff_framesync_activate(FFFrameSync *fs)
@@ -359,6 +467,7 @@ int ff_framesync_activate(FFFrameSync *fs)
     if (fs->eof || !fs->frame_ready)
         return 0;
     ret = fs->on_event(fs);
+    fs->nb_events++;
     if (ret < 0)
         return ret;
     fs->frame_ready = 0;
diff --git a/libavfilter/framesync.h b/libavfilter/framesync.h
index 233f50a0eb..979f54e16e 100644
--- a/libavfilter/framesync.h
+++ b/libavfilter/framesync.h
@@ -188,6 +188,11 @@ typedef struct FFFrameSync {
      */
     int64_t pts;
 
+    /**
+     * Number of times on_event() was called.
+     */
+    uint64_t nb_events;
+
     /**
      * Callback called when a frame event is ready
      */
@@ -229,6 +234,12 @@ typedef struct FFFrameSync {
     int opt_eof_action;
     int opt_ts_sync_mode;
 
+    char *ts_map_str;
+
+    // explicit frame map
+    int64_t        *ts_map;
+    size_t       nb_ts_map;
+    unsigned int    ts_map_allocated;
 } FFFrameSync;
 
 /**
-- 
2.35.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 4/4] lavfi/framesync: reindent after previous commit

[Anton Khirnov]

---
 libavfilter/framesync.c | 56 ++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/libavfilter/framesync.c b/libavfilter/framesync.c
index b52cf318c0..9986b04e48 100644
--- a/libavfilter/framesync.c
+++ b/libavfilter/framesync.c
@@ -368,35 +368,35 @@ static int framesync_advance(FFFrameSync *fs)
             }
             pts = fs->in[0].pts;
         } else {
-        pts = INT64_MAX;
-        for (i = 0; i < fs->nb_in; i++)
-            if (fs->in[i].have_next && fs->in[i].pts_next < pts)
-                pts = fs->in[i].pts_next;
-        if (pts == INT64_MAX) {
-            framesync_eof(fs);
-            break;
-        }
-        for (i = 0; i < fs->nb_in; i++) {
-            FFFrameSyncIn * const in = &fs->in[i];
-
-            if (in->pts_next == pts ||
-                (in->ts_mode == TS_NEAREST && in->have_next             &&
-                 in->pts_next != INT64_MAX && in->pts != AV_NOPTS_VALUE &&
-                 in->pts_next - pts < pts - in->pts) ||
-                (in->before == EXT_INFINITY && in->state == STATE_BOF)) {
-                frame_advance(in);
-                in->state      = in->frame ? STATE_RUN : STATE_EOF;
-                if (in->sync == fs->sync_level && in->frame)
-                    fs->frame_ready = 1;
-                if (in->state == STATE_EOF && in->after == EXT_STOP)
-                    framesync_eof(fs);
-            }
-        }
-        if (fs->frame_ready)
+            pts = INT64_MAX;
             for (i = 0; i < fs->nb_in; i++)
-                if ((fs->in[i].state == STATE_BOF &&
-                     fs->in[i].before == EXT_STOP))
-                    fs->frame_ready = 0;
+                if (fs->in[i].have_next && fs->in[i].pts_next < pts)
+                    pts = fs->in[i].pts_next;
+            if (pts == INT64_MAX) {
+                framesync_eof(fs);
+                break;
+            }
+            for (i = 0; i < fs->nb_in; i++) {
+                FFFrameSyncIn * const in = &fs->in[i];
+
+                if (in->pts_next == pts ||
+                    (in->ts_mode == TS_NEAREST && in->have_next             &&
+                     in->pts_next != INT64_MAX && in->pts != AV_NOPTS_VALUE &&
+                     in->pts_next - pts < pts - in->pts) ||
+                    (in->before == EXT_INFINITY && in->state == STATE_BOF)) {
+                    frame_advance(in);
+                    in->state      = in->frame ? STATE_RUN : STATE_EOF;
+                    if (in->sync == fs->sync_level && in->frame)
+                        fs->frame_ready = 1;
+                    if (in->state == STATE_EOF && in->after == EXT_STOP)
+                        framesync_eof(fs);
+                }
+            }
+            if (fs->frame_ready)
+                for (i = 0; i < fs->nb_in; i++)
+                    if ((fs->in[i].state == STATE_BOF &&
+                         fs->in[i].before == EXT_STOP))
+                        fs->frame_ready = 0;
         }
         fs->pts = pts;
     }
-- 
2.35.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/4] lavfi/framesync: add syncing via external timestamp map

[Paul B Mahol]

On 1/27/23, Anton Khirnov <anton@khirnov.net> wrote:
> Useful when there is some external process that determines canonical
> frame synchronization. E.g. the framerate conversion code in ffmpeg CLI.
> ---
>  doc/filters.texi        |   6 ++
>  libavfilter/framesync.c | 121 ++++++++++++++++++++++++++++++++++++++--
>  libavfilter/framesync.h |  11 ++++
>  3 files changed, 132 insertions(+), 6 deletions(-)

Looks like hack to fix some specific nonsense.
How are timestamps supposed to be generated?

The ts_map is unlimited in length?

Very fragile.

>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index be70a2396b..2fc50f3a91 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -363,6 +363,12 @@ primary input frame.
>  Frame from secondary input with the absolute nearest timestamp to the
> primary
>  input frame.
>  @end table
> +
> +@item ts_map
> +Specify an explicit timestamp map. The string should be composed of lines,
> one
> +per each output frame. The line should contain whitespace-separated times
> in
> +microseconds, one for every input. Frames with these timestamps will be
> matched
> +together to produces output events.
>  @end table
>
>  @c man end OPTIONS FOR FILTERS WITH SEVERAL INPUTS
> diff --git a/libavfilter/framesync.c b/libavfilter/framesync.c
> index fdcc3b57c8..b52cf318c0 100644
> --- a/libavfilter/framesync.c
> +++ b/libavfilter/framesync.c
> @@ -49,6 +49,7 @@ static const AVOption framesync_options[] = {
>              0, AV_OPT_TYPE_CONST, { .i64 = TS_DEFAULT }, .flags = FLAGS,
> "ts_sync_mode" },
>          { "nearest", "Frame from secondary input with the absolute nearest
> timestamp to the primary input frame",
>              0, AV_OPT_TYPE_CONST, { .i64 = TS_NEAREST }, .flags = FLAGS,
> "ts_sync_mode" },
> +    { "ts_map", "Timestamp map", OFFSET(ts_map_str), AV_OPT_TYPE_STRING,
> .flags = FLAGS },
>      { NULL }
>  };
>  static const AVClass framesync_class = {
> @@ -129,10 +130,78 @@ static void framesync_sync_level_update(FFFrameSync
> *fs)
>          framesync_eof(fs);
>  }
>
> +static int ts_map_parse(FFFrameSync *fs, const char *ts_map_str)
> +{
> +    while (*ts_map_str) {
> +        int64_t *dst;
> +
> +        ts_map_str += strspn(ts_map_str, " \t\r\n");
> +
> +        // skip comments
> +        if (*ts_map_str == '#' || !*ts_map_str)
> +            goto skip_line;
> +
> +        dst = av_fast_realloc(fs->ts_map, &fs->ts_map_allocated,
> +                              sizeof(*fs->ts_map) * fs->nb_in *
> (fs->nb_ts_map + 1));
> +        if (!dst)
> +            return AVERROR(ENOMEM);
> +
> +        fs->ts_map = dst;
> +        dst += fs->nb_in * fs->nb_ts_map;
> +        fs->nb_ts_map++;
> +
> +        // read a timestamp for each input
> +        for (int i = 0; i < fs->nb_in; i++) {
> +            char *p;
> +            dst[i] = strtol(ts_map_str, &p, 0);
> +            if (p == ts_map_str) {
> +                av_log(fs, AV_LOG_ERROR,
> +                       "Invalid number in timestamp map on line %zu:
> %s\n",
> +                       fs->nb_ts_map - 1, ts_map_str);
> +                return AVERROR_INVALIDDATA;
> +            }
> +            ts_map_str = p;
> +
> +            if (fs->nb_ts_map > 1 && dst[i - (int)fs->nb_in] > dst[i]) {
> +                av_log(fs, AV_LOG_ERROR,
> +                       "Timestamp map for input %d, frame %zu goes
> backwards\n",
> +                       i, fs->nb_ts_map - 1);
> +                return AVERROR_INVALIDDATA;
> +            }
> +
> +            ts_map_str += strspn(p, " \t");
> +        }
> +
> +        // skip everything after the needed timestamp
> +skip_line:
> +        ts_map_str = strchr(ts_map_str, '\n');
> +        if (!ts_map_str)
> +            break;
> +    }
> +
> +    return 0;
> +}
> +
>  int ff_framesync_configure(FFFrameSync *fs)
>  {
>      unsigned i;
>
> +    if (fs->ts_map_str) {
> +        int ret;
> +
> +        if (fs->opt_ts_sync_mode != TS_DEFAULT) {
> +            av_log(fs, AV_LOG_ERROR,
> +                   "ts_sync_mode must be set to default when a map is
> used\n");
> +            return AVERROR(EINVAL);
> +        }
> +
> +        ret = ts_map_parse(fs, fs->ts_map_str);
> +        if (ret < 0) {
> +            av_log(fs, AV_LOG_ERROR, "Error reading the explicit timestamp
> map\n");
> +            return ret;
> +        }
> +    }
> +
>      if (!fs->opt_repeatlast || fs->opt_eof_action == EOF_ACTION_PASS) {
>          fs->opt_repeatlast = 0;
>          fs->opt_eof_action = EOF_ACTION_PASS;
> @@ -250,17 +319,55 @@ static int consume_from_fifos(FFFrameSync *fs)
>      return 1;
>  }
>
> +static void frame_advance(FFFrameSyncIn *in)
> +{
> +    av_frame_free(&in->frame);
> +    in->frame      = in->frame_next;
> +    in->pts        = in->pts_next;
> +    in->frame_next = NULL;
> +    in->pts_next   = AV_NOPTS_VALUE;
> +    in->have_next  = 0;
> +}
> +
>  static int framesync_advance(FFFrameSync *fs)
>  {
>      unsigned i;
>      int64_t pts;
>      int ret;
>
> +    if (fs->ts_map && fs->nb_events >= fs->nb_ts_map) {
> +        framesync_eof(fs);
> +        return 0;
> +    }
> +
>      while (!(fs->frame_ready || fs->eof)) {
>          ret = consume_from_fifos(fs);
>          if (ret <= 0)
>              return ret;
>
> +        if (fs->ts_map) {
> +            fs->frame_ready = 1;
> +            for (i = 0; i < fs->nb_in; i++) {
> +                FFFrameSyncIn * const in = &fs->in[i];
> +                int64_t next_ts = av_rescale_q(fs->ts_map[fs->nb_events *
> fs->nb_in + i],
> +                                               AV_TIME_BASE_Q,
> fs->time_base);
> +                uint64_t delta_cur  = in->frame      ? FFABS(in->pts      -
> next_ts) : UINT64_MAX;
> +                uint64_t delta_next = in->frame_next ? FFABS(in->pts_next -
> next_ts) : UINT64_MAX;
> +
> +                if (!in->frame ||
> +                    (in->frame_next && delta_next < delta_cur)) {
> +                    frame_advance(in);
> +                    fs->frame_ready = 0;
> +                    in->state       = in->frame ? STATE_RUN : STATE_EOF;
> +                    if (in->state == STATE_EOF) {
> +                        av_log(fs, AV_LOG_WARNING,
> +                               "Input stream %d ended before the timestamp
> map did\n", i);
> +                        framesync_eof(fs);
> +                    }
> +                }
> +            }
> +            pts = fs->in[0].pts;
> +        } else {
>          pts = INT64_MAX;
>          for (i = 0; i < fs->nb_in; i++)
>              if (fs->in[i].have_next && fs->in[i].pts_next < pts)
> @@ -277,12 +384,7 @@ static int framesync_advance(FFFrameSync *fs)
>                   in->pts_next != INT64_MAX && in->pts != AV_NOPTS_VALUE &&
>                   in->pts_next - pts < pts - in->pts) ||
>                  (in->before == EXT_INFINITY && in->state == STATE_BOF)) {
> -                av_frame_free(&in->frame);
> -                in->frame      = in->frame_next;
> -                in->pts        = in->pts_next;
> -                in->frame_next = NULL;
> -                in->pts_next   = AV_NOPTS_VALUE;
> -                in->have_next  = 0;
> +                frame_advance(in);
>                  in->state      = in->frame ? STATE_RUN : STATE_EOF;
>                  if (in->sync == fs->sync_level && in->frame)
>                      fs->frame_ready = 1;
> @@ -295,6 +397,7 @@ static int framesync_advance(FFFrameSync *fs)
>                  if ((fs->in[i].state == STATE_BOF &&
>                       fs->in[i].before == EXT_STOP))
>                      fs->frame_ready = 0;
> +        }
>          fs->pts = pts;
>      }
>      return 0;
> @@ -347,6 +450,11 @@ void ff_framesync_uninit(FFFrameSync *fs)
>      }
>
>      av_freep(&fs->in);
> +
> +    av_freep(&fs->ts_map_str);
> +    av_freep(&fs->ts_map);
> +    fs->nb_ts_map        = 0;
> +    fs->ts_map_allocated = 0;
>  }
>
>  int ff_framesync_activate(FFFrameSync *fs)
> @@ -359,6 +467,7 @@ int ff_framesync_activate(FFFrameSync *fs)
>      if (fs->eof || !fs->frame_ready)
>          return 0;
>      ret = fs->on_event(fs);
> +    fs->nb_events++;
>      if (ret < 0)
>          return ret;
>      fs->frame_ready = 0;
> diff --git a/libavfilter/framesync.h b/libavfilter/framesync.h
> index 233f50a0eb..979f54e16e 100644
> --- a/libavfilter/framesync.h
> +++ b/libavfilter/framesync.h
> @@ -188,6 +188,11 @@ typedef struct FFFrameSync {
>       */
>      int64_t pts;
>
> +    /**
> +     * Number of times on_event() was called.
> +     */
> +    uint64_t nb_events;
> +
>      /**
>       * Callback called when a frame event is ready
>       */
> @@ -229,6 +234,12 @@ typedef struct FFFrameSync {
>      int opt_eof_action;
>      int opt_ts_sync_mode;
>
> +    char *ts_map_str;
> +
> +    // explicit frame map
> +    int64_t        *ts_map;
> +    size_t       nb_ts_map;
> +    unsigned int    ts_map_allocated;
>  } FFFrameSync;
>
>  /**
> --
> 2.35.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/4] lavfi/framesync: use a local variable to shorten code

[Nicolas George]

Anton Khirnov (12023-01-27):
> ---
>  libavfilter/framesync.c | 33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/libavfilter/framesync.c b/libavfilter/framesync.c
> index ee91e4cf68..153db4fa21 100644
> --- a/libavfilter/framesync.c
> +++ b/libavfilter/framesync.c
> @@ -201,24 +201,23 @@ static int framesync_advance(FFFrameSync *fs)
>              break;
>          }
>          for (i = 0; i < fs->nb_in; i++) {
> -            if (fs->in[i].pts_next == pts ||
> -                (fs->in[i].ts_mode == TS_NEAREST &&
> -                 fs->in[i].have_next &&
> -                 fs->in[i].pts_next != INT64_MAX && fs->in[i].pts != AV_NOPTS_VALUE &&
> -                 fs->in[i].pts_next - pts < pts - fs->in[i].pts) ||
> -                (fs->in[i].before == EXT_INFINITY &&
> -                 fs->in[i].state == STATE_BOF)) {
> -                av_frame_free(&fs->in[i].frame);
> -                fs->in[i].frame      = fs->in[i].frame_next;
> -                fs->in[i].pts        = fs->in[i].pts_next;
> -                fs->in[i].frame_next = NULL;
> -                fs->in[i].pts_next   = AV_NOPTS_VALUE;
> -                fs->in[i].have_next  = 0;
> -                fs->in[i].state      = fs->in[i].frame ? STATE_RUN : STATE_EOF;
> -                if (fs->in[i].sync == fs->sync_level && fs->in[i].frame)

> +            FFFrameSyncIn * const in = &fs->in[i];

Get rid of the const, since the rest of this code does not use anything
similar and the benefit is dubious.

> +
> +            if (in->pts_next == pts ||
> +                (in->ts_mode == TS_NEAREST && in->have_next             &&
> +                 in->pts_next != INT64_MAX && in->pts != AV_NOPTS_VALUE &&
> +                 in->pts_next - pts < pts - in->pts) ||
> +                (in->before == EXT_INFINITY && in->state == STATE_BOF)) {
> +                av_frame_free(&in->frame);
> +                in->frame      = in->frame_next;
> +                in->pts        = in->pts_next;
> +                in->frame_next = NULL;
> +                in->pts_next   = AV_NOPTS_VALUE;
> +                in->have_next  = 0;
> +                in->state      = in->frame ? STATE_RUN : STATE_EOF;
> +                if (in->sync == fs->sync_level && in->frame)
>                      fs->frame_ready = 1;
> -                if (fs->in[i].state == STATE_EOF &&
> -                    fs->in[i].after == EXT_STOP)
> +                if (in->state == STATE_EOF && in->after == EXT_STOP)
>                      framesync_eof(fs);
>              }
>          }

I do not like this change. In fact, the more I think about it the more I
remember that I specifically considered using intermediate pointers for
inputs and deciding against: too much extra noise if done everywhere,
too little benefit for the inconsistency if done only where there are
many.

So: thanks but no.

-- 
  Nicolas George
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

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

[Nicolas George]

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.

-- 
  Nicolas George
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/4] lavfi/framesync: add syncing via external timestamp map

[Nicolas George]

Anton Khirnov (12023-01-27):
> Useful when there is some external process that determines canonical
> frame synchronization. E.g. the framerate conversion code in ffmpeg CLI.
> ---
>  doc/filters.texi        |   6 ++
>  libavfilter/framesync.c | 121 ++++++++++++++++++++++++++++++++++++++--
>  libavfilter/framesync.h |  11 ++++
>  3 files changed, 132 insertions(+), 6 deletions(-)

I agree with Paul, this looks like an instance of a XY problem
<https://en.wikipedia.org/wiki/XY_problem>.

framesync generates output based on its input. Therefore to force
timestamps on output frames you need to force timestamps on input
frames.

And serializing timestamps in decimal in a giant string with newlines in
it: definitely no.

-- 
  Nicolas George
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/4] lavfi/framesync: add syncing via external timestamp map

[Anton Khirnov]

Quoting Nicolas George (2023-01-27 15:53:42)
> framesync generates output based on its input. Therefore to force
> timestamps on output frames you need to force timestamps on input
> frames.

This is not forcing timestamps on output frames. This is solving the
general problem where the correct matching of input frames is determined
by some external logic. The specific case that is of interest to me is
where this logic is the ffmpeg CLI framerate conversion, which allows
framesync to accurately match videos processed through it. But I can
imagine other cases where this would be useful.

> And serializing timestamps in decimal in a giant string with newlines in
> it: definitely no.

Why not?
'No' with no reasoning and no suggested alternative is not much of an
argument.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/4] lavfi/framesync: add syncing via external timestamp map

[Nicolas George]

Anton Khirnov (12023-01-27):
> This is not forcing timestamps on output frames. This is solving the
> general problem where the correct matching of input frames is determined
> by some external logic. The specific case that is of interest to me is
> where this logic is the ffmpeg CLI framerate conversion, which allows
> framesync to accurately match videos processed through it. But I can
> imagine other cases where this would be useful.

You are explaining nothing that was not already present in the commit
message, and my interpretation is still: you are engaged in a wrong
solution and are trying to make it work, i.e. XY problem
<https://en.wikipedia.org/wiki/XY_problem>.

Just force the timestamps to the input of framesync filters and you will
get what you want on the output.

> > And serializing timestamps in decimal in a giant string with newlines in
                                  1            2     3           4
> > it: definitely no.
> Why not?
> 'No' with no reasoning and no suggested alternative is not much of an
> argument.

There were four arguments.

-- 
  Nicolas George
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".