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

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(-)

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".
>

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.

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 (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.