[FFmpeg-devel] [PATCH 1/2] avformat/avformat: Introduced `AVInputFormat.read_timestamp2` to fix keyframe seeking for formats that rely on `read_timestamp` for seeking
Michael Niedermayer
michael at niedermayer.cc
Mon Sep 2 17:33:56 EEST 2019
On Sat, Aug 31, 2019 at 03:10:38AM -0700, fumoboy007 at me.com wrote:
> From: fumoboy007 <fumoboy007 at me.com>
>
> If the user omits `AVSEEK_FLAG_ANY`, then the result of the seek should be a keyframe. `ff_gen_search` did not respect that contract because it had no good way to determine whether a timestamp returned by `read_timestamp` was for a keyframe packet.
>
> Therefore, we introduce `read_timestamp2`, which adds a new parameter named `prefer_keyframe`. This new parameter tells the input format whether to skip non-keyframe packets. The parameter is named `prefer_keyframe` instead of something like `keyframe_only` because some formats do not distinguish between keyframe and non-keyframe packets.
>
> This commit adds the new function and deprecates the old function. Subsequent commits will migrate input formats to the new function.
>
> Signed-off-by: fumoboy007 <fumoboy007 at me.com>
> ---
> libavformat/avformat.h | 9 +++++++
> libavformat/internal.h | 6 +++--
> libavformat/nutdec.c | 6 ++---
> libavformat/utils.c | 56 +++++++++++++++++++++++++++++++++---------
> 4 files changed, 60 insertions(+), 17 deletions(-)
>
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 6eb329f13f..1db548663c 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -741,6 +741,7 @@ typedef struct AVInputFormat {
> * Get the next timestamp in stream[stream_index].time_base units.
> * @return the timestamp or AV_NOPTS_VALUE if an error occurred
> */
> + attribute_deprecated
> int64_t (*read_timestamp)(struct AVFormatContext *s, int stream_index,
> int64_t *pos, int64_t pos_limit);
>
> @@ -781,6 +782,14 @@ typedef struct AVInputFormat {
> * @see avdevice_capabilities_free() for more details.
> */
> int (*free_device_capabilities)(struct AVFormatContext *s, struct AVDeviceCapabilitiesQuery *caps);
> +
> + /**
> + * Get the next timestamp in stream[stream_index].time_base units.
> + * @param prefer_keyframe Whether to skip over non-keyframe packets (if possible).
> + * @return the timestamp or AV_NOPTS_VALUE if an error occurred
> + */
> + int64_t (*read_timestamp2)(struct AVFormatContext *s, int stream_index,
> + int64_t *pos, int64_t pos_limit, int prefer_keyframe);
> } AVInputFormat;
> /**
> * @}
> diff --git a/libavformat/internal.h b/libavformat/internal.h
> index d6a039c497..a45c538b21 100644
> --- a/libavformat/internal.h
> +++ b/libavformat/internal.h
> @@ -443,7 +443,8 @@ int ff_seek_frame_binary(AVFormatContext *s, int stream_index,
> void ff_update_cur_dts(AVFormatContext *s, AVStream *ref_st, int64_t timestamp);
>
> int ff_find_last_ts(AVFormatContext *s, int stream_index, int64_t *ts, int64_t *pos,
> - int64_t (*read_timestamp)(struct AVFormatContext *, int , int64_t *, int64_t ));
> + int64_t (*read_timestamp)(struct AVFormatContext *, int, int64_t *, int64_t),
> + int64_t (*read_timestamp2)(struct AVFormatContext *, int, int64_t *, int64_t, int));
>
> /**
> * Perform a binary search using read_timestamp().
> @@ -456,7 +457,8 @@ int64_t ff_gen_search(AVFormatContext *s, int stream_index,
> int64_t pos_max, int64_t pos_limit,
> int64_t ts_min, int64_t ts_max,
> int flags, int64_t *ts_ret,
> - int64_t (*read_timestamp)(struct AVFormatContext *, int , int64_t *, int64_t ));
> + int64_t (*read_timestamp)(struct AVFormatContext *, int, int64_t *, int64_t),
> + int64_t (*read_timestamp2)(struct AVFormatContext *, int, int64_t *, int64_t, int));
>
> /**
> * Set the time base and wrapping info for a given stream. This will be used
> diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
> index 979cb9a031..f28fb2297e 100644
> --- a/libavformat/nutdec.c
> +++ b/libavformat/nutdec.c
> @@ -653,7 +653,7 @@ static int64_t find_duration(NUTContext *nut, int64_t filesize)
> AVFormatContext *s = nut->avf;
> int64_t duration = 0;
>
> - ff_find_last_ts(s, -1, &duration, NULL, nut_read_timestamp);
> + ff_find_last_ts(s, -1, &duration, NULL, nut_read_timestamp, NULL);
>
> if(duration > 0)
> s->duration_estimation_method = AVFMT_DURATION_FROM_PTS;
> @@ -1251,7 +1251,7 @@ static int read_seek(AVFormatContext *s, int stream_index,
> pos = ff_gen_search(s, -1, dummy.ts, next_node[0]->pos,
> next_node[1]->pos, next_node[1]->pos,
> next_node[0]->ts, next_node[1]->ts,
> - AVSEEK_FLAG_BACKWARD, &ts, nut_read_timestamp);
> + AVSEEK_FLAG_BACKWARD, &ts, nut_read_timestamp, NULL);
> if (pos < 0)
> return pos;
>
> @@ -1263,7 +1263,7 @@ static int read_seek(AVFormatContext *s, int stream_index,
> pos2 = ff_gen_search(s, -2, dummy.pos, next_node[0]->pos,
> next_node[1]->pos, next_node[1]->pos,
> next_node[0]->back_ptr, next_node[1]->back_ptr,
> - flags, &ts, nut_read_timestamp);
> + flags, &ts, nut_read_timestamp, NULL);
> if (pos2 >= 0)
> pos = pos2;
> // FIXME dir but I think it does not matter
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index b57e680089..3cb2599f42 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -2166,6 +2166,15 @@ static int64_t ff_read_timestamp(AVFormatContext *s, int stream_index, int64_t *
> return ts;
> }
>
> +static int64_t ff_read_timestamp2(AVFormatContext *s, int stream_index, int64_t *ppos, int64_t pos_limit, int prefer_keyframe,
> + int64_t (*read_timestamp2)(struct AVFormatContext *, int, int64_t *, int64_t, int))
> +{
> + int64_t ts = read_timestamp2(s, stream_index, ppos, pos_limit, prefer_keyframe);
> + if (stream_index >= 0)
> + ts = wrap_timestamp(s->streams[stream_index], ts);
> + return ts;
> +}
> +
> int ff_seek_frame_binary(AVFormatContext *s, int stream_index,
> int64_t target_ts, int flags)
> {
> @@ -2220,7 +2229,7 @@ int ff_seek_frame_binary(AVFormatContext *s, int stream_index,
> }
>
> pos = ff_gen_search(s, stream_index, target_ts, pos_min, pos_max, pos_limit,
> - ts_min, ts_max, flags, &ts, avif->read_timestamp);
> + ts_min, ts_max, flags, &ts, avif->read_timestamp, avif->read_timestamp2);
> if (pos < 0)
> return -1;
>
> @@ -2235,7 +2244,8 @@ int ff_seek_frame_binary(AVFormatContext *s, int stream_index,
> }
>
> int ff_find_last_ts(AVFormatContext *s, int stream_index, int64_t *ts, int64_t *pos,
> - int64_t (*read_timestamp)(struct AVFormatContext *, int , int64_t *, int64_t ))
> + int64_t (*read_timestamp)(struct AVFormatContext *, int, int64_t *, int64_t),
> + int64_t (*read_timestamp2)(struct AVFormatContext *, int, int64_t *, int64_t, int))
> {
> int64_t step = 1024;
> int64_t limit, ts_max;
> @@ -2244,8 +2254,13 @@ int ff_find_last_ts(AVFormatContext *s, int stream_index, int64_t *ts, int64_t *
> do {
> limit = pos_max;
> pos_max = FFMAX(0, (pos_max) - step);
> - ts_max = ff_read_timestamp(s, stream_index,
> - &pos_max, limit, read_timestamp);
> + if (read_timestamp2) {
> + ts_max = ff_read_timestamp2(s, stream_index,
> + &pos_max, limit, 0, read_timestamp2);
> + } else {
> + ts_max = ff_read_timestamp(s, stream_index,
> + &pos_max, limit, read_timestamp);
> + }
> step += step;
> } while (ts_max == AV_NOPTS_VALUE && 2*limit > step);
> if (ts_max == AV_NOPTS_VALUE)
> @@ -2253,8 +2268,14 @@ int ff_find_last_ts(AVFormatContext *s, int stream_index, int64_t *ts, int64_t *
>
> for (;;) {
> int64_t tmp_pos = pos_max + 1;
> - int64_t tmp_ts = ff_read_timestamp(s, stream_index,
> - &tmp_pos, INT64_MAX, read_timestamp);
> + int64_t tmp_ts;
> + if (read_timestamp2) {
> + tmp_ts = ff_read_timestamp2(s, stream_index,
> + &tmp_pos, INT64_MAX, 0, read_timestamp2);
> + } else {
> + tmp_ts = ff_read_timestamp(s, stream_index,
> + &tmp_pos, INT64_MAX, read_timestamp);
> + }
> if (tmp_ts == AV_NOPTS_VALUE)
> break;
> av_assert0(tmp_pos > pos_max);
> @@ -2277,7 +2298,9 @@ int64_t ff_gen_search(AVFormatContext *s, int stream_index, int64_t target_ts,
> int64_t ts_min, int64_t ts_max,
> int flags, int64_t *ts_ret,
> int64_t (*read_timestamp)(struct AVFormatContext *, int,
> - int64_t *, int64_t))
> + int64_t *, int64_t),
> + int64_t (*read_timestamp2)(struct AVFormatContext *, int,
> + int64_t *, int64_t, int))
> {
> int64_t pos, ts;
> int64_t start_pos;
> @@ -2288,7 +2311,11 @@ int64_t ff_gen_search(AVFormatContext *s, int stream_index, int64_t target_ts,
>
> if (ts_min == AV_NOPTS_VALUE) {
> pos_min = s->internal->data_offset;
> - ts_min = ff_read_timestamp(s, stream_index, &pos_min, INT64_MAX, read_timestamp);
> + if (read_timestamp2) {
> + ts_min = ff_read_timestamp2(s, stream_index, &pos_min, INT64_MAX, 0, read_timestamp2);
> + } else {
> + ts_min = ff_read_timestamp(s, stream_index, &pos_min, INT64_MAX, read_timestamp);
> + }
> if (ts_min == AV_NOPTS_VALUE)
> return -1;
> }
> @@ -2299,7 +2326,7 @@ int64_t ff_gen_search(AVFormatContext *s, int stream_index, int64_t target_ts,
> }
>
> if (ts_max == AV_NOPTS_VALUE) {
> - if ((ret = ff_find_last_ts(s, stream_index, &ts_max, &pos_max, read_timestamp)) < 0)
> + if ((ret = ff_find_last_ts(s, stream_index, &ts_max, &pos_max, read_timestamp, read_timestamp2)) < 0)
> return ret;
> pos_limit = pos_max;
> }
> @@ -2339,7 +2366,12 @@ int64_t ff_gen_search(AVFormatContext *s, int stream_index, int64_t target_ts,
> start_pos = pos;
>
> // May pass pos_limit instead of -1.
> - ts = ff_read_timestamp(s, stream_index, &pos, INT64_MAX, read_timestamp);
> + if (read_timestamp2) {
> + int prefer_keyframe = (flags & AVSEEK_FLAG_ANY) == 0;
> + ts = ff_read_timestamp2(s, stream_index, &pos, INT64_MAX, prefer_keyframe, read_timestamp2);
> + } else {
> + ts = ff_read_timestamp(s, stream_index, &pos, INT64_MAX, read_timestamp);
> + }
i think this is the wrong way to fix the issue unless iam missing something
Bisecting on only keyframe packets needs to read
framesize * goplength * log(all frames)
Bisecting on any frame and only at the end searching for the next keyframe needs
framesize * (log(all frames) + goplength)
that is more than an order of magnitude less data to read even with
small gop sizes
Thanks
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190902/7593a322/attachment.sig>
More information about the ffmpeg-devel
mailing list