[FFmpeg-devel] [PATCH 10/18] lavf: move AVStream.*index_entries* to AVStreamInternal

[Andreas Rheinhardt]

James Almer:
> On 11/16/2020 12:25 PM, Hendrik Leppkes wrote:
>> On Mon, Nov 16, 2020 at 4:20 PM James Almer <jamrial at gmail.com> wrote:
>>>
>>> On 11/16/2020 3:01 AM, Anton Khirnov wrote:
>>>> Quoting Xiang, Haihao (2020-11-16 06:16:55)
>>>>>
>>>>> This change breaks the compiling of gst-libav (
>>>>> https://gitlab.freedesktop.org/gstreamer/gst-libav), I filed
>>>>> https://trac.ffmpeg.org/ticket/8988 to track this regression.
>>>>
>>>> This is not a regression, it's a bug in gst-libav. These fields are
>>>> private and have always been private, applications accessing them are
>>>> broken.
>>>
>>> Looking at that ticket, it seems gst-libav wants the timestamp for a
>>> given index entry. Unless this can already be achieved in some other
>>> way, it could maybe be implemented cleanly with a new public function
>>> (We have av_index_search_timestamp() to fetch an index from a timestamp
>>> after all).
>>
>> I also like being able to access the index, it can be used by players
>> to efficiently offer seeking straight to keyframes (eg. index
>> entries), which basically requires iterating over the full index and
>> getting the full information (timestamp, flags at least)
>>
>> - Hendrik
> 
> What do you suggest, implement something like what you added to you
> LAVFilters fork? I don't know if returning a pointer to a given
> AVIndexEntry is ideal. If that were the case, then it wouldn't be an
> internal field.
> 
> Perhaps something like
> 
> int av_index_get_entries_count(AVStream *st);
> int av_index_get_timestamp(AVStream *st, int64_t *timestamp, int *flags,
> int idx);
> 
> Where timestamp and flags are output parameters in the latter,
> containing the relevant AVIndexEntry values.

You missed pos and size which should also be exported; I don't know
whether the same applies to min_distance as I don't really see what this
is good for (seems to be only used by mov). And this already leads me to
the next point: We might want to change what we export in the future and
using pointer arguments for the each field is not good for this. Using a
struct with version information (or a flags parameter for
av_index_get_timestamp()) would solve this problem; too bad AVIndexEntry
is not available as name for this struct (btw: the current AVIndexEntry
struct should be made private after the appropriate deprecation period).

- Andreas