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
.

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
  12 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-requ...@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-requ...@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
.

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-requ...@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  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, >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, , 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(>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 && 

[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, >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, , 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(>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 = >in[i];
+int64_t next_ts = av_rescale_q(fs->ts_map[fs->nb_events * 
fs->nb_in +