* [FFmpeg-devel] [PATCH 2/4] lavfi/framesync: reorder functions to avoid a forward declaration
2023-01-27 13:16 [FFmpeg-devel] [PATCH 1/4] lavfi/framesync: use a local variable to shorten code Anton Khirnov
@ 2023-01-27 13:16 ` Anton Khirnov
2023-01-27 14:47 ` Nicolas George
2023-01-27 13:16 ` [FFmpeg-devel] [PATCH 3/4] lavfi/framesync: add syncing via external timestamp map Anton Khirnov
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Anton Khirnov @ 2023-01-27 13:16 UTC (permalink / raw)
To: ffmpeg-devel
---
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".
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [FFmpeg-devel] [PATCH 2/4] lavfi/framesync: reorder functions to avoid a forward declaration
2023-01-27 13:16 ` [FFmpeg-devel] [PATCH 2/4] lavfi/framesync: reorder functions to avoid a forward declaration Anton Khirnov
@ 2023-01-27 14:47 ` Nicolas George
0 siblings, 0 replies; 10+ messages in thread
From: Nicolas George @ 2023-01-27 14:47 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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".
^ permalink raw reply [flat|nested] 10+ messages in thread
* [FFmpeg-devel] [PATCH 3/4] lavfi/framesync: add syncing via external timestamp map
2023-01-27 13:16 [FFmpeg-devel] [PATCH 1/4] lavfi/framesync: use a local variable to shorten code Anton Khirnov
2023-01-27 13:16 ` [FFmpeg-devel] [PATCH 2/4] lavfi/framesync: reorder functions to avoid a forward declaration Anton Khirnov
@ 2023-01-27 13:16 ` Anton Khirnov
2023-01-27 13:22 ` Paul B Mahol
2023-01-27 14:53 ` Nicolas George
2023-01-27 13:16 ` [FFmpeg-devel] [PATCH 4/4] lavfi/framesync: reindent after previous commit Anton Khirnov
2023-01-27 14:39 ` [FFmpeg-devel] [PATCH 1/4] lavfi/framesync: use a local variable to shorten code Nicolas George
3 siblings, 2 replies; 10+ messages in thread
From: Anton Khirnov @ 2023-01-27 13:16 UTC (permalink / raw)
To: ffmpeg-devel
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".
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [FFmpeg-devel] [PATCH 3/4] lavfi/framesync: add syncing via external timestamp map
2023-01-27 13:16 ` [FFmpeg-devel] [PATCH 3/4] lavfi/framesync: add syncing via external timestamp map Anton Khirnov
@ 2023-01-27 13:22 ` Paul B Mahol
2023-01-27 14:53 ` Nicolas George
1 sibling, 0 replies; 10+ messages in thread
From: Paul B Mahol @ 2023-01-27 13:22 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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".
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [FFmpeg-devel] [PATCH 3/4] lavfi/framesync: add syncing via external timestamp map
2023-01-27 13:16 ` [FFmpeg-devel] [PATCH 3/4] lavfi/framesync: add syncing via external timestamp map Anton Khirnov
2023-01-27 13:22 ` Paul B Mahol
@ 2023-01-27 14:53 ` Nicolas George
2023-01-27 16:45 ` Anton Khirnov
1 sibling, 1 reply; 10+ messages in thread
From: Nicolas George @ 2023-01-27 14:53 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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".
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [FFmpeg-devel] [PATCH 3/4] lavfi/framesync: add syncing via external timestamp map
2023-01-27 14:53 ` Nicolas George
@ 2023-01-27 16:45 ` Anton Khirnov
2023-01-30 11:01 ` Nicolas George
0 siblings, 1 reply; 10+ messages in thread
From: Anton Khirnov @ 2023-01-27 16:45 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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".
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [FFmpeg-devel] [PATCH 3/4] lavfi/framesync: add syncing via external timestamp map
2023-01-27 16:45 ` Anton Khirnov
@ 2023-01-30 11:01 ` Nicolas George
0 siblings, 0 replies; 10+ messages in thread
From: Nicolas George @ 2023-01-30 11:01 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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".
^ permalink raw reply [flat|nested] 10+ messages in thread
* [FFmpeg-devel] [PATCH 4/4] lavfi/framesync: reindent after previous commit
2023-01-27 13:16 [FFmpeg-devel] [PATCH 1/4] lavfi/framesync: use a local variable to shorten code Anton Khirnov
2023-01-27 13:16 ` [FFmpeg-devel] [PATCH 2/4] lavfi/framesync: reorder functions to avoid a forward declaration Anton Khirnov
2023-01-27 13:16 ` [FFmpeg-devel] [PATCH 3/4] lavfi/framesync: add syncing via external timestamp map Anton Khirnov
@ 2023-01-27 13:16 ` Anton Khirnov
2023-01-27 14:39 ` [FFmpeg-devel] [PATCH 1/4] lavfi/framesync: use a local variable to shorten code Nicolas George
3 siblings, 0 replies; 10+ messages in thread
From: Anton Khirnov @ 2023-01-27 13:16 UTC (permalink / raw)
To: ffmpeg-devel
---
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".
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [FFmpeg-devel] [PATCH 1/4] lavfi/framesync: use a local variable to shorten code
2023-01-27 13:16 [FFmpeg-devel] [PATCH 1/4] lavfi/framesync: use a local variable to shorten code Anton Khirnov
` (2 preceding siblings ...)
2023-01-27 13:16 ` [FFmpeg-devel] [PATCH 4/4] lavfi/framesync: reindent after previous commit Anton Khirnov
@ 2023-01-27 14:39 ` Nicolas George
3 siblings, 0 replies; 10+ messages in thread
From: Nicolas George @ 2023-01-27 14:39 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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".
^ permalink raw reply [flat|nested] 10+ messages in thread