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

[James Almer]

On 11/17/2020 3:43 AM, Andreas Rheinhardt wrote:
> 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;

pos and size sound effectively like internal values only lavf should 
care about, for seeking purposes. Why would the library user care about 
the offset of seek points in a stream described within an AVStream? They 
are using lavf API to handle demuxing, after all.

Timestamps for seek points and the fact they are keyframes or not are 
useful for the user to make certain choices, but i have doubts about the 
rest.

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

If we need a public struct then AVIndexEntry can remain as such, and any 
field in it we don't want to export can be deprecated and then removed.
The current AVIndexEntry can then be copied/renamed to FFIndexEntry or 
similar, and moved to internal.h

> 
> - Andreas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>