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

2020-11-20 Thread Michael Niedermayer
On Wed, Nov 18, 2020 at 06:49:10PM -0300, James Almer wrote:
> On 11/18/2020 6:41 PM, Michael Niedermayer wrote:
> > On Mon, Nov 16, 2020 at 09:46:01PM -0300, James Almer wrote:
> > > On 11/16/2020 12:25 PM, Hendrik Leppkes wrote:
> > > > On Mon, Nov 16, 2020 at 4:20 PM James Almer  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);
> > 
> > If the (only) goal is to export the whole list then a better option might be
> > to use an opaque state and some sort of iterator.
> 
> The index entries are in a flat array. A function like what i suggested
> above would simply do st->internal->index_entries[idx].

Yes, if we never want to change the internal data structure, this makes sense.
But then if you never want to change the internal data structure then
allowing direct access could improve performance for some apps potentially

on this topic, a flat array has issues in some circumstances
repeatedly inserting of new elements is one, it is not fast unless the
elements are only added at the end


> 
> > Because the suggestion requires N index based lookups and this is only fast
> > with some conceivable implementations.
> > for example consider an index stored in some sort of tree structure
> > a index based lookup might be O(log n) while one step iterating over all
> > elements could be O(1)
> > 
> > AVIndexEntry *av_index_iterate(AVStream *st, AVIndexIterator **i);
> > 
> > and
> > 
> > AVIndexIterator *i = NULL;
> > AVIndexEntry *entry;
> > for (entry = av_index_iterate(st, ); i; entry = av_index_iterate(st, ))
> >  ...
> 
> But do we want to return a pointer to an entry in the index array, or just
> some of the fields for a given entry?
> As i said above, if the former was intended, then it wouldn't have been an
> internal field all this time to being with.

I dont know but a struct seems simpler

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2020-11-18 Thread James Almer

On 11/18/2020 6:41 PM, Michael Niedermayer wrote:

On Mon, Nov 16, 2020 at 09:46:01PM -0300, James Almer wrote:

On 11/16/2020 12:25 PM, Hendrik Leppkes wrote:

On Mon, Nov 16, 2020 at 4:20 PM James Almer  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);


If the (only) goal is to export the whole list then a better option might be
to use an opaque state and some sort of iterator.


The index entries are in a flat array. A function like what i suggested 
above would simply do st->internal->index_entries[idx].



Because the suggestion requires N index based lookups and this is only fast
with some conceivable implementations.
for example consider an index stored in some sort of tree structure
a index based lookup might be O(log n) while one step iterating over all
elements could be O(1)

AVIndexEntry *av_index_iterate(AVStream *st, AVIndexIterator **i);

and

AVIndexIterator *i = NULL;
AVIndexEntry *entry;
for (entry = av_index_iterate(st, ); i; entry = av_index_iterate(st, ))
 ...


But do we want to return a pointer to an entry in the index array, or 
just some of the fields for a given entry?
As i said above, if the former was intended, then it wouldn't have been 
an internal field all this time to being with.


 


[...]


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".



___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2020-11-18 Thread Michael Niedermayer
On Mon, Nov 16, 2020 at 09:46:01PM -0300, James Almer wrote:
> On 11/16/2020 12:25 PM, Hendrik Leppkes wrote:
> > On Mon, Nov 16, 2020 at 4:20 PM James Almer  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);

If the (only) goal is to export the whole list then a better option might be
to use an opaque state and some sort of iterator.
Because the suggestion requires N index based lookups and this is only fast
with some conceivable implementations.
for example consider an index stored in some sort of tree structure
a index based lookup might be O(log n) while one step iterating over all
elements could be O(1)

AVIndexEntry *av_index_iterate(AVStream *st, AVIndexIterator **i);

and

AVIndexIterator *i = NULL;
AVIndexEntry *entry;
for (entry = av_index_iterate(st, ); i; entry = av_index_iterate(st, ))
...


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2020-11-17 Thread 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  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@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".



___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2020-11-17 Thread Hendrik Leppkes
On Tue, Nov 17, 2020 at 1:46 AM James Almer  wrote:
>
> On 11/16/2020 12:25 PM, Hendrik Leppkes wrote:
> > On Mon, Nov 16, 2020 at 4:20 PM James Almer  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.
>

I agree that just giving out the field is not a good public API, but I
just needed a quick fix. :)
A proper get_count + accessor that takes the index is much more
sensible, as you suggested.

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2020-11-16 Thread 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  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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2020-11-16 Thread Xiang, Haihao
On Mon, 2020-11-16 at 21:46 -0300, James Almer wrote:
> On 11/16/2020 12:25 PM, Hendrik Leppkes wrote:
> > On Mon, Nov 16, 2020 at 4:20 PM James Almer  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.

It sounds good to me if FFmpeg can provide these functions.

Thanks
Haihao


> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2020-11-16 Thread James Almer

On 11/16/2020 12:25 PM, Hendrik Leppkes wrote:

On Mon, Nov 16, 2020 at 4:20 PM James Almer  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.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2020-11-16 Thread Hendrik Leppkes
On Mon, Nov 16, 2020 at 4:20 PM James Almer  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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2020-11-16 Thread James Almer

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

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2020-11-15 Thread Anton Khirnov
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.

-- 
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2020-11-15 Thread Xiang, Haihao

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.

Thanks
Haihao


> Those are private fields, no reason to have them exposed in a public
> header. Since there are some (semi-)public fields located after these,
> even though this section is supposed to be private, keep some dummy
> padding there until the next major bump to preserve ABI compatibility.
> ---
>  libavformat/ape.c |   2 +-
>  libavformat/asfdec_f.c|   4 +-
>  libavformat/asfdec_o.c|   6 +-
>  libavformat/avformat.h|  10 +-
>  libavformat/avidec.c  | 100 ++--
>  libavformat/bink.c|  10 +-
>  libavformat/cafdec.c  |  22 ++---
>  libavformat/cinedec.c |   2 +-
>  libavformat/dhav.c|   4 +-
>  libavformat/flacdec.c |   8 +-
>  libavformat/flic.c|   6 +-
>  libavformat/flvdec.c  |  10 +-
>  libavformat/gxf.c |   6 +-
>  libavformat/ifv.c |   8 +-
>  libavformat/img2dec.c |   2 +-
>  libavformat/internal.h|   5 +
>  libavformat/jvdec.c   |  28 +++---
>  libavformat/matroskadec.c |  40 
>  libavformat/mlvdec.c  |  20 ++--
>  libavformat/mov.c | 190 +++---
>  libavformat/mp3dec.c  |   6 +-
>  libavformat/mpc.c |   4 +-
>  libavformat/mpc8.c|   4 +-
>  libavformat/mux.c |   2 +-
>  libavformat/mvdec.c   |   4 +-
>  libavformat/nsvdec.c  |   4 +-
>  libavformat/nutdec.c  |   6 +-
>  libavformat/nutenc.c  |  12 +--
>  libavformat/rl2.c |   8 +-
>  libavformat/rpl.c |   4 +-
>  libavformat/segafilm.c|   2 +-
>  libavformat/tta.c |   8 +-
>  libavformat/utils.c   |  46 -
>  libavformat/vocdec.c  |   8 +-
>  34 files changed, 304 insertions(+), 297 deletions(-)
> 
> diff --git a/libavformat/ape.c b/libavformat/ape.c
> index d92cb2867d..33b7237fb0 100644
> --- a/libavformat/ape.c
> +++ b/libavformat/ape.c
> @@ -469,7 +469,7 @@ static int ape_read_seek(AVFormatContext *s, int
> stream_index, int64_t timestamp
>  if (index < 0)
>  return -1;
>  
> -if ((ret = avio_seek(s->pb, st->index_entries[index].pos, SEEK_SET)) < 0)
> +if ((ret = avio_seek(s->pb, st->internal->index_entries[index].pos,
> SEEK_SET)) < 0)
>  return ret;
>  ape->currentframe = index;
>  return 0;
> diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
> index b92434db9e..deb7c266ed 100644
> --- a/libavformat/asfdec_f.c
> +++ b/libavformat/asfdec_f.c
> @@ -1678,11 +1678,11 @@ static int asf_read_seek(AVFormatContext *s, int
> stream_index,
>  asf->index_read = -1;
>  }
>  
> -if (asf->index_read > 0 && st->index_entries) {
> +if (asf->index_read > 0 && st->internal->index_entries) {
>  int index = av_index_search_timestamp(st, pts, flags);
>  if (index >= 0) {
>  /* find the position */
> -uint64_t pos = st->index_entries[index].pos;
> +uint64_t pos = st->internal->index_entries[index].pos;
>  
>  /* do the seek */
>  av_log(s, AV_LOG_DEBUG, "SEEKTO: %"PRId64"\n", pos);
> diff --git a/libavformat/asfdec_o.c b/libavformat/asfdec_o.c
> index 1b10e47907..b142f83541 100644
> --- a/libavformat/asfdec_o.c
> +++ b/libavformat/asfdec_o.c
> @@ -1640,11 +1640,11 @@ static int asf_read_seek(AVFormatContext *s, int
> stream_index,
>  ASFContext *asf = s->priv_data;
>  int idx, ret;
>  
> -if (s->streams[stream_index]->nb_index_entries && asf->is_simple_index) {
> +if (s->streams[stream_index]->internal->nb_index_entries && asf-
> >is_simple_index) {
>  idx = av_index_search_timestamp(s->streams[stream_index], timestamp,
> flags);
> -if (idx < 0 || idx >= s->streams[stream_index]->nb_index_entries)
> +if (idx < 0 || idx >= s->streams[stream_index]->internal-
> >nb_index_entries)
>  return AVERROR_INVALIDDATA;
> -avio_seek(s->pb, s->streams[stream_index]->index_entries[idx].pos,
> SEEK_SET);
> +avio_seek(s->pb, s->streams[stream_index]->internal-
> >index_entries[idx].pos, SEEK_SET);
>  } else {
>  if ((ret = ff_seek_frame_binary(s, stream_index, timestamp, flags)) <
> 0)
>  return ret;
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 1253021452..dbef1b21dd 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -1065,10 +1065,12 @@ typedef struct AVStream {
>  #define MAX_REORDER_DELAY 16
>  int64_t pts_buffer[MAX_REORDER_DELAY+1];
>  
> -AVIndexEntry *index_entries; /**< Only used if the format does not
> -support seeking natively. */
> -int nb_index_entries;
> -unsigned int index_entries_allocated_size;
> +#if LIBAVFORMAT_VERSION_MAJOR < 59
> +// kept for ABI compatibility only, do not access in any way
> +

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

2020-10-09 Thread Anton Khirnov
Those are private fields, no reason to have them exposed in a public
header. Since there are some (semi-)public fields located after these,
even though this section is supposed to be private, keep some dummy
padding there until the next major bump to preserve ABI compatibility.
---
 libavformat/ape.c |   2 +-
 libavformat/asfdec_f.c|   4 +-
 libavformat/asfdec_o.c|   6 +-
 libavformat/avformat.h|  10 +-
 libavformat/avidec.c  | 100 ++--
 libavformat/bink.c|  10 +-
 libavformat/cafdec.c  |  22 ++---
 libavformat/cinedec.c |   2 +-
 libavformat/dhav.c|   4 +-
 libavformat/flacdec.c |   8 +-
 libavformat/flic.c|   6 +-
 libavformat/flvdec.c  |  10 +-
 libavformat/gxf.c |   6 +-
 libavformat/ifv.c |   8 +-
 libavformat/img2dec.c |   2 +-
 libavformat/internal.h|   5 +
 libavformat/jvdec.c   |  28 +++---
 libavformat/matroskadec.c |  40 
 libavformat/mlvdec.c  |  20 ++--
 libavformat/mov.c | 190 +++---
 libavformat/mp3dec.c  |   6 +-
 libavformat/mpc.c |   4 +-
 libavformat/mpc8.c|   4 +-
 libavformat/mux.c |   2 +-
 libavformat/mvdec.c   |   4 +-
 libavformat/nsvdec.c  |   4 +-
 libavformat/nutdec.c  |   6 +-
 libavformat/nutenc.c  |  12 +--
 libavformat/rl2.c |   8 +-
 libavformat/rpl.c |   4 +-
 libavformat/segafilm.c|   2 +-
 libavformat/tta.c |   8 +-
 libavformat/utils.c   |  46 -
 libavformat/vocdec.c  |   8 +-
 34 files changed, 304 insertions(+), 297 deletions(-)

diff --git a/libavformat/ape.c b/libavformat/ape.c
index d92cb2867d..33b7237fb0 100644
--- a/libavformat/ape.c
+++ b/libavformat/ape.c
@@ -469,7 +469,7 @@ static int ape_read_seek(AVFormatContext *s, int 
stream_index, int64_t timestamp
 if (index < 0)
 return -1;
 
-if ((ret = avio_seek(s->pb, st->index_entries[index].pos, SEEK_SET)) < 0)
+if ((ret = avio_seek(s->pb, st->internal->index_entries[index].pos, 
SEEK_SET)) < 0)
 return ret;
 ape->currentframe = index;
 return 0;
diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index b92434db9e..deb7c266ed 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -1678,11 +1678,11 @@ static int asf_read_seek(AVFormatContext *s, int 
stream_index,
 asf->index_read = -1;
 }
 
-if (asf->index_read > 0 && st->index_entries) {
+if (asf->index_read > 0 && st->internal->index_entries) {
 int index = av_index_search_timestamp(st, pts, flags);
 if (index >= 0) {
 /* find the position */
-uint64_t pos = st->index_entries[index].pos;
+uint64_t pos = st->internal->index_entries[index].pos;
 
 /* do the seek */
 av_log(s, AV_LOG_DEBUG, "SEEKTO: %"PRId64"\n", pos);
diff --git a/libavformat/asfdec_o.c b/libavformat/asfdec_o.c
index 1b10e47907..b142f83541 100644
--- a/libavformat/asfdec_o.c
+++ b/libavformat/asfdec_o.c
@@ -1640,11 +1640,11 @@ static int asf_read_seek(AVFormatContext *s, int 
stream_index,
 ASFContext *asf = s->priv_data;
 int idx, ret;
 
-if (s->streams[stream_index]->nb_index_entries && asf->is_simple_index) {
+if (s->streams[stream_index]->internal->nb_index_entries && 
asf->is_simple_index) {
 idx = av_index_search_timestamp(s->streams[stream_index], timestamp, 
flags);
-if (idx < 0 || idx >= s->streams[stream_index]->nb_index_entries)
+if (idx < 0 || idx >= 
s->streams[stream_index]->internal->nb_index_entries)
 return AVERROR_INVALIDDATA;
-avio_seek(s->pb, s->streams[stream_index]->index_entries[idx].pos, 
SEEK_SET);
+avio_seek(s->pb, 
s->streams[stream_index]->internal->index_entries[idx].pos, SEEK_SET);
 } else {
 if ((ret = ff_seek_frame_binary(s, stream_index, timestamp, flags)) < 
0)
 return ret;
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 1253021452..dbef1b21dd 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -1065,10 +1065,12 @@ typedef struct AVStream {
 #define MAX_REORDER_DELAY 16
 int64_t pts_buffer[MAX_REORDER_DELAY+1];
 
-AVIndexEntry *index_entries; /**< Only used if the format does not
-support seeking natively. */
-int nb_index_entries;
-unsigned int index_entries_allocated_size;
+#if LIBAVFORMAT_VERSION_MAJOR < 59
+// kept for ABI compatibility only, do not access in any way
+void *unused2;
+int  unused3;
+unsigned int unused4;
+#endif
 
 /**
  * Stream Identifier
diff --git a/libavformat/avidec.c b/libavformat/avidec.c
index 89a9443821..80e5563bc6 100644
--- a/libavformat/avidec.c
+++ b/libavformat/avidec.c
@@ -272,7 +272,7 @@ static void clean_index(AVFormatContext *s)
 for (i = 0; i < s->nb_streams; i++) {
 AVStream *st   = s->streams[i];