[FFmpeg-devel] [PATCH] avformat/utils: add helper functions to retrieve index entries from an AVStream

[Nicolas George]

James Almer (12021-03-23):
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
> Following "lavf: move AVStream.*index_entries* to AVStreamInternal", it was
> revealed that some library users apparently found these fields useful and were
> accessing them despite not being public.
> This patch introduces a few functions to recover this functionality in a proper
> and supported way.
> 

> We can't simply return a const pointer to the corresponding AVIndexEntry in the
> array because it may change at any time by for example calling
> av_add_index_entry(), so it's either a copy of the AVIndexEntry, or adding half
> a dozen output parameters to the function signature for each field in
> AVIndexEntry.

We can return a const pointer to the AVIndexEntry if we document it:

	Warning: the returned pointer is only valid until the next
	operation that may modify the AVStream or the AVFormatContext it
	belongs to. Accessing it after av_read_frame() or other
	functions results in undefined behavior.

Users can still copy it immediately if they need it longer.

> 
>  libavformat/avformat.h | 39 +++++++++++++++++++++++++++++++++++++++
>  libavformat/utils.c    | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 71 insertions(+)
> 
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 765bc3b6f5..ac8b57149e 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -2754,6 +2754,45 @@ int av_find_default_stream_index(AVFormatContext *s);
>   */
>  int av_index_search_timestamp(AVStream *st, int64_t timestamp, int flags);
>  
> +/**
> + * Get the index entry count for the given AVStream.
> + *
> + * @param st stream
> + * @return the number of index entries in the stream
> + */
> +int av_index_get_entries_count(AVStream *st);
> +
> +/**
> + * Get the AVIndexEntry corresponding to the given index.
> + *
> + * @param st          stream containing the requested AVIndexEntry
> + * @param index_entry pointer to an AVIndexEntry where to store the entry. On failure,
> + *                    or if the index entry could not be found, this struct will remain
> + *                    untouched
> + * @param idx         the desired index
> + * @return >= 0 on success. AVERROR(ENOENT) if no entry for the requested index could
> +           be found. other negative AVERROR codes on failure.
> + */
> +int av_index_get_entry(AVStream *st, AVIndexEntry *index_entry, int idx);
> +
> +/**
> + * Get the AVIndexEntry corresponding to the given timestamp.
> + *
> + * @param st          stream containing the requested AVIndexEntry
> + * @param index_entry pointer to an AVIndexEntry where to store the entry. On failure,
> + *                    or if the index entry could not be found, this struct will remain
> + *                    untouched
> + * @param timestamp   timestamp to retrieve the index entry for
> + * @param flags       if AVSEEK_FLAG_BACKWARD then the returned entry will correspond
> + *                    to the timestamp which is <= the requested one, if backward
> + *                    is 0, then it will be >=
> + *                    if AVSEEK_FLAG_ANY seek to any frame, only keyframes otherwise
> + * @return >= 0 on success. AVERROR(ENOENT) if no entry for the requested timestamp could
> + *          be found. other negative AVERROR codes on failure.
> + */
> +int av_index_get_entry_from_timestamp(AVStream *st, AVIndexEntry *index_entry,
> +                                      int64_t wanted_timestamp, int flags);
> +
>  /**
>   * Add an index entry into a sorted list. Update the entry if the list
>   * already contains it.
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index f31826f2ea..4e7a8bf23e 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -2129,6 +2129,38 @@ int av_index_search_timestamp(AVStream *st, int64_t wanted_timestamp, int flags)
>                                       wanted_timestamp, flags);
>  }
>  
> +int av_index_get_entries_count(AVStream *st)
> +{
> +    return st->internal->nb_index_entries;
> +}
> +
> +int av_index_get_entry(AVStream *st, AVIndexEntry *index_entry, int idx)
> +{

> +    if (idx < 0)
> +        return AVERROR(EINVAL);

Make idx unsigned.

> +    if (idx >= st->internal->nb_index_entries)
> +        return AVERROR(ENOENT);
> +

> +    memcpy(index_entry, &st->internal->index_entries[idx], sizeof(*index_entry));

Why not using an assignment?

But this makes sizeof(AVIndexEntry) part of the ABI. Probably not what
we want.

> +
> +    return 0;
> +}
> +
> +int av_index_get_entry_from_timestamp(AVStream *st, AVIndexEntry *index_entry,
> +                                      int64_t wanted_timestamp, int flags)
> +{
> +    int idx = ff_index_search_timestamp(st->internal->index_entries,
> +                                        st->internal->nb_index_entries,
> +                                        wanted_timestamp, flags);
> +
> +    if (idx < 0)
> +        return AVERROR(ENOENT);
> +

> +    memcpy(index_entry, &st->internal->index_entries[idx], sizeof(*index_entry));

Same, of course.

> +
> +    return 0;
> +}
> +
>  static int64_t ff_read_timestamp(AVFormatContext *s, int stream_index, int64_t *ppos, int64_t pos_limit,
>                                   int64_t (*read_timestamp)(struct AVFormatContext *, int , int64_t *, int64_t ))
>  {

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210323/fc2b7eec/attachment.sig>