[FFmpeg-devel,v3,2/3] avformat/avio{, buf}: deprecate AVIOContext::written

Originally added as a private entry in commit
3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
the comment noting its private state was missed during merging of
the field from Libav (most likely due to an already existing field
in between).
---
 doc/APIchanges        | 6 ++++++
 libavformat/avio.h    | 6 ++++++
 libavformat/aviobuf.c | 9 +++++++++
 libavformat/version.h | 5 ++++-
 4 files changed, 25 insertions(+), 1 deletion(-)

On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström <jeebjp@gmail.com> wrote:
>
> Originally added as a private entry in commit
> 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
> the comment noting its private state was missed during merging of
> the field from Libav (most likely due to an already existing field
> in between).
> ---

As noted in the previous two iterations' cover letter etc, I would
like to have some opinions on whether to remove or deprecate this
struct member.

Personally I see the struct member being one of those private ones
that Andreas removed earlier, and thus deprecating it feels weird
while all of the other private variables got removed from AVIOContext
already.

I can also do this deprecation dance, as this patch already shows that
change being done :) .

Jan

On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström wrote:
> On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström <jeebjp@gmail.com> wrote:
> >
> > Originally added as a private entry in commit
> > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
> > the comment noting its private state was missed during merging of
> > the field from Libav (most likely due to an already existing field
> > in between).
> > ---

Its a public struct in a public header so deprecate

thx

[...]

On Thu, Oct 21, 2021 at 8:26 PM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström wrote:
> > On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström <jeebjp@gmail.com> wrote:
> > >
> > > Originally added as a private entry in commit
> > > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
> > > the comment noting its private state was missed during merging of
> > > the field from Libav (most likely due to an already existing field
> > > in between).
> > > ---
>
> Its a public struct in a public header so deprecate
>
> thx
>

OK, just confirming but so you don't consider this to be of the same
type as these http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=a7b56ab6674f0a25be3b480df314e2d1292f397b;hp=0b35409787c925d681a121aa8d8715c3921fddf2;hb=45bfe8b838275235412777dd430206d9a24eb3ee;hpb=530ac6aa305aeda631c77f8a17e96c14c7ab1a1c
?

The field was brought in with
http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=7bf7985c5e04a551e3a2841decc6b32ded7c0799;hp=49721aa1315fc44bf46d73d343ace4e7c3dc785e;hb=3f75e5116b900f1428aa13041fc7d6301bf1988a;hpb=fc85646ad495f3418042468da415af73a7a07334
(you can see the comment noting its private state on the top of the context)

which then was merged as
http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=525eb7129eb5b5a5a2a8dc015275526b9b25a0cb;hp=6f4ed8440d684e03e70fd296849b76d216d6fbbb;hb=34d7f337c1608c72be8c36355018dc894f0560ce;hpb=c5fd47fa8a300fc51489a47da94041609545803c
(it went just under another private struct member from the FFmpeg side)

Jan

On Thu, Oct 21, 2021 at 08:48:35PM +0300, Jan Ekström wrote:
> On Thu, Oct 21, 2021 at 8:26 PM Michael Niedermayer
> <michael@niedermayer.cc> wrote:
> >
> > On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström wrote:
> > > On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström <jeebjp@gmail.com> wrote:
> > > >
> > > > Originally added as a private entry in commit
> > > > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
> > > > the comment noting its private state was missed during merging of
> > > > the field from Libav (most likely due to an already existing field
> > > > in between).
> > > > ---
> >
> > Its a public struct in a public header so deprecate
> >
> > thx
> >
> 
> OK, just confirming but so you don't consider this to be of the same
> type as these http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=a7b56ab6674f0a25be3b480df314e2d1292f397b;hp=0b35409787c925d681a121aa8d8715c3921fddf2;hb=45bfe8b838275235412777dd430206d9a24eb3ee;hpb=530ac6aa305aeda631c77f8a17e96c14c7ab1a1c
> ?

i assume this was during ABI bumping, 
when the major version numbers change then ABI/API breaking changes can be
done.

Try to write an application which links to a shared lib and then change the
public structs of that lib by removing a field. 
the ABI/API major numbers have to be bumped when incompatible changes are
done. That way they can co exist on a machiene, application build against
the old continue working and so on ...

[...]

On Thu, Oct 21, 2021 at 11:49 PM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> On Thu, Oct 21, 2021 at 08:48:35PM +0300, Jan Ekström wrote:
> > On Thu, Oct 21, 2021 at 8:26 PM Michael Niedermayer
> > <michael@niedermayer.cc> wrote:
> > >
> > > On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström wrote:
> > > > On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström <jeebjp@gmail.com> wrote:
> > > > >
> > > > > Originally added as a private entry in commit
> > > > > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
> > > > > the comment noting its private state was missed during merging of
> > > > > the field from Libav (most likely due to an already existing field
> > > > > in between).
> > > > > ---
> > >
> > > Its a public struct in a public header so deprecate
> > >
> > > thx
> > >
> >
> > OK, just confirming but so you don't consider this to be of the same
> > type as these http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=a7b56ab6674f0a25be3b480df314e2d1292f397b;hp=0b35409787c925d681a121aa8d8715c3921fddf2;hb=45bfe8b838275235412777dd430206d9a24eb3ee;hpb=530ac6aa305aeda631c77f8a17e96c14c7ab1a1c
> > ?
>
> i assume this was during ABI bumping,
> when the major version numbers change then ABI/API breaking changes can be
> done.
>

IIRC ABI breakage after bumping was supposed to be a ~month but then
IIRC people posted such things much later. Was there a message or note
when this period ended?

> Try to write an application which links to a shared lib and then change the
> public structs of that lib by removing a field.
> the ABI/API major numbers have to be bumped when incompatible changes are
> done. That way they can co exist on a machiene, application build against
> the old continue working and so on ...
>

For the record, I know what ABI bumping is. I was just unclear whether
we were already past ABI instability period or not due to
4.5/5.0/whatever not still being made and due to there (as far as I
know) there being no clear message on when the ABI instability period
would end.

Jan

On Fri, Oct 22, 2021 at 12:21 AM Jan Ekström <jeebjp@gmail.com> wrote:
>
> On Thu, Oct 21, 2021 at 11:49 PM Michael Niedermayer
> <michael@niedermayer.cc> wrote:
> >
> > On Thu, Oct 21, 2021 at 08:48:35PM +0300, Jan Ekström wrote:
> > > On Thu, Oct 21, 2021 at 8:26 PM Michael Niedermayer
> > > <michael@niedermayer.cc> wrote:
> > > >
> > > > On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström wrote:
> > > > > On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström <jeebjp@gmail.com> wrote:
> > > > > >
> > > > > > Originally added as a private entry in commit
> > > > > > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
> > > > > > the comment noting its private state was missed during merging of
> > > > > > the field from Libav (most likely due to an already existing field
> > > > > > in between).
> > > > > > ---
> > > >
> > > > Its a public struct in a public header so deprecate
> > > >
> > > > thx
> > > >
> > >
> > > OK, just confirming but so you don't consider this to be of the same
> > > type as these http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=a7b56ab6674f0a25be3b480df314e2d1292f397b;hp=0b35409787c925d681a121aa8d8715c3921fddf2;hb=45bfe8b838275235412777dd430206d9a24eb3ee;hpb=530ac6aa305aeda631c77f8a17e96c14c7ab1a1c
> > > ?
> >
> > i assume this was during ABI bumping,
> > when the major version numbers change then ABI/API breaking changes can be
> > done.
> >
>
> IIRC ABI breakage after bumping was supposed to be a ~month but then
> IIRC people posted such things much later. Was there a message or note
> when this period ended?
>

This commit (http://git.videolan.org/?p=ffmpeg.git;a=commit;h=45bfe8b838275235412777dd430206d9a24eb3ee)
was applied in late August (as the entries were private - kind of like
this one remaining entry), and I can find Anton noting in May that the
project is in ABI instability period. So do excuse me for not knowing
which mode we are in right now if it went on for at least four months
:D

Jan

On Fri, Oct 22, 2021 at 12:32:18AM +0300, Jan Ekström wrote:
> On Fri, Oct 22, 2021 at 12:21 AM Jan Ekström <jeebjp@gmail.com> wrote:
> >
> > On Thu, Oct 21, 2021 at 11:49 PM Michael Niedermayer
> > <michael@niedermayer.cc> wrote:
> > >
> > > On Thu, Oct 21, 2021 at 08:48:35PM +0300, Jan Ekström wrote:
> > > > On Thu, Oct 21, 2021 at 8:26 PM Michael Niedermayer
> > > > <michael@niedermayer.cc> wrote:
> > > > >
> > > > > On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström wrote:
> > > > > > On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström <jeebjp@gmail.com> wrote:
> > > > > > >
> > > > > > > Originally added as a private entry in commit
> > > > > > > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
> > > > > > > the comment noting its private state was missed during merging of
> > > > > > > the field from Libav (most likely due to an already existing field
> > > > > > > in between).
> > > > > > > ---
> > > > >
> > > > > Its a public struct in a public header so deprecate
> > > > >
> > > > > thx
> > > > >
> > > >
> > > > OK, just confirming but so you don't consider this to be of the same
> > > > type as these http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=a7b56ab6674f0a25be3b480df314e2d1292f397b;hp=0b35409787c925d681a121aa8d8715c3921fddf2;hb=45bfe8b838275235412777dd430206d9a24eb3ee;hpb=530ac6aa305aeda631c77f8a17e96c14c7ab1a1c
> > > > ?
> > >
> > > i assume this was during ABI bumping,
> > > when the major version numbers change then ABI/API breaking changes can be
> > > done.
> > >
> >
> > IIRC ABI breakage after bumping was supposed to be a ~month but then
> > IIRC people posted such things much later. Was there a message or note
> > when this period ended?

Maybe someone assumed that everyone agreed what "a ~month" is ?


> >
> 
> This commit (http://git.videolan.org/?p=ffmpeg.git;a=commit;h=45bfe8b838275235412777dd430206d9a24eb3ee)
> was applied in late August (as the entries were private - kind of like
> this one remaining entry), and I can find Anton noting in May that the
> project is in ABI instability period. So do excuse me for not knowing
> which mode we are in right now if it went on for at least four months
> :D

I think you should really implement a full statistics API instead of
this as you clearly have alot of time.
you can leave the field, you can deprecate it you can also suggest to
bump major. What you cannot do is break ABI/API. If Iam missing something
and this is not breaking ABI/API then you can also explain how that would
not break it

thx


[...]

On Fri, Oct 22, 2021 at 1:13 AM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> On Fri, Oct 22, 2021 at 12:32:18AM +0300, Jan Ekström wrote:
> > On Fri, Oct 22, 2021 at 12:21 AM Jan Ekström <jeebjp@gmail.com> wrote:
> > >
> > > On Thu, Oct 21, 2021 at 11:49 PM Michael Niedermayer
> > > <michael@niedermayer.cc> wrote:
> > > >
> > > > On Thu, Oct 21, 2021 at 08:48:35PM +0300, Jan Ekström wrote:
> > > > > On Thu, Oct 21, 2021 at 8:26 PM Michael Niedermayer
> > > > > <michael@niedermayer.cc> wrote:
> > > > > >
> > > > > > On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström wrote:
> > > > > > > On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström <jeebjp@gmail.com> wrote:
> > > > > > > >
> > > > > > > > Originally added as a private entry in commit
> > > > > > > > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
> > > > > > > > the comment noting its private state was missed during merging of
> > > > > > > > the field from Libav (most likely due to an already existing field
> > > > > > > > in between).
> > > > > > > > ---
> > > > > >
> > > > > > Its a public struct in a public header so deprecate
> > > > > >
> > > > > > thx
> > > > > >
> > > > >
> > > > > OK, just confirming but so you don't consider this to be of the same
> > > > > type as these http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=a7b56ab6674f0a25be3b480df314e2d1292f397b;hp=0b35409787c925d681a121aa8d8715c3921fddf2;hb=45bfe8b838275235412777dd430206d9a24eb3ee;hpb=530ac6aa305aeda631c77f8a17e96c14c7ab1a1c
> > > > > ?
> > > >
> > > > i assume this was during ABI bumping,
> > > > when the major version numbers change then ABI/API breaking changes can be
> > > > done.
> > > >
> > >
> > > IIRC ABI breakage after bumping was supposed to be a ~month but then
> > > IIRC people posted such things much later. Was there a message or note
> > > when this period ended?
>
> Maybe someone assumed that everyone agreed what "a ~month" is ?
>
>
> > >
> >
> > This commit (http://git.videolan.org/?p=ffmpeg.git;a=commit;h=45bfe8b838275235412777dd430206d9a24eb3ee)
> > was applied in late August (as the entries were private - kind of like
> > this one remaining entry), and I can find Anton noting in May that the
> > project is in ABI instability period. So do excuse me for not knowing
> > which mode we are in right now if it went on for at least four months
> > :D
>
> I think you should really implement a full statistics API instead of
> this as you clearly have alot of time.

I do not have a lot of time, I just saw this as a finalization of the
clean-up of private fields that Andreas started and I wanted to ask
about which people would prefer, since we have not yet had a release
branching with the API major version that this clean-up is included
in.

If you disagree with Andreas's actions from late August, then I would
have preferred you to note that. I would have been OK with something a
la "It was not clearly marked in our (FFmpeg's) merge so we consider
it public and not private.", instead I got what (at least to me) felt
like re-education on what API/ABI breaks are, plus what seemed to be
not reading all those references I provided to why I thought this was
a private field and why this in my opinion was just a continuation of
the previous commit done by Andreas. I am sorry for the knee-jerk
reflex-like response, it is not productive. I think I just felt like
getting ignored and getting a standard "he doesn't understand the
basics of this basic thing" response, even though I attempted to
provide all the information regarding *why* I was asking.

Jan

On Fri, Oct 22, 2021 at 12:20:19PM +0300, Jan Ekström wrote:
> On Fri, Oct 22, 2021 at 1:13 AM Michael Niedermayer
> <michael@niedermayer.cc> wrote:
> >
> > On Fri, Oct 22, 2021 at 12:32:18AM +0300, Jan Ekström wrote:
> > > On Fri, Oct 22, 2021 at 12:21 AM Jan Ekström <jeebjp@gmail.com> wrote:
> > > >
> > > > On Thu, Oct 21, 2021 at 11:49 PM Michael Niedermayer
> > > > <michael@niedermayer.cc> wrote:
> > > > >
> > > > > On Thu, Oct 21, 2021 at 08:48:35PM +0300, Jan Ekström wrote:
> > > > > > On Thu, Oct 21, 2021 at 8:26 PM Michael Niedermayer
> > > > > > <michael@niedermayer.cc> wrote:
> > > > > > >
> > > > > > > On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström wrote:
> > > > > > > > On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström <jeebjp@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > Originally added as a private entry in commit
> > > > > > > > > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
> > > > > > > > > the comment noting its private state was missed during merging of
> > > > > > > > > the field from Libav (most likely due to an already existing field
> > > > > > > > > in between).
> > > > > > > > > ---
> > > > > > >
> > > > > > > Its a public struct in a public header so deprecate
> > > > > > >
> > > > > > > thx
> > > > > > >
> > > > > >
> > > > > > OK, just confirming but so you don't consider this to be of the same
> > > > > > type as these http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=a7b56ab6674f0a25be3b480df314e2d1292f397b;hp=0b35409787c925d681a121aa8d8715c3921fddf2;hb=45bfe8b838275235412777dd430206d9a24eb3ee;hpb=530ac6aa305aeda631c77f8a17e96c14c7ab1a1c
> > > > > > ?
> > > > >
> > > > > i assume this was during ABI bumping,
> > > > > when the major version numbers change then ABI/API breaking changes can be
> > > > > done.
> > > > >
> > > >
> > > > IIRC ABI breakage after bumping was supposed to be a ~month but then
> > > > IIRC people posted such things much later. Was there a message or note
> > > > when this period ended?
> >
> > Maybe someone assumed that everyone agreed what "a ~month" is ?
> >
> >
> > > >
> > >
> > > This commit (http://git.videolan.org/?p=ffmpeg.git;a=commit;h=45bfe8b838275235412777dd430206d9a24eb3ee)
> > > was applied in late August (as the entries were private - kind of like
> > > this one remaining entry), and I can find Anton noting in May that the
> > > project is in ABI instability period. So do excuse me for not knowing
> > > which mode we are in right now if it went on for at least four months
> > > :D
> >
> > I think you should really implement a full statistics API instead of
> > this as you clearly have alot of time.
> 
> I do not have a lot of time, I just saw this as a finalization of the
> clean-up of private fields that Andreas started and I wanted to ask
> about which people would prefer, since we have not yet had a release
> branching with the API major version that this clean-up is included
> in.
> 
> If you disagree with Andreas's actions from late August, then I would
> have preferred you to note that. I would have been OK with something a
> la "It was not clearly marked in our (FFmpeg's) merge so we consider
> it public and not private.", instead I got what (at least to me) felt
> like re-education on what API/ABI breaks are, plus what seemed to be
> not reading all those references I provided to why I thought this was
> a private field and why this in my opinion was just a continuation of
> the previous commit done by Andreas. I am sorry for the knee-jerk
> reflex-like response, it is not productive. I think I just felt like
> getting ignored and getting a standard "he doesn't understand the
> basics of this basic thing" response, even though I attempted to
> provide all the information regarding *why* I was asking.

Well, the idea of ABI compatibility is that fields which are not
clearly marked as private cannot be removed or move around in the binary
layout of a struct. So a user can depend on code continuing to work
A removial would violate that. Now the arguments i see here are
1. There was no release
That is true but our download page first and biggest points to master
not a release. So IMHO if we point to master then master should be
working in "all circumstances" and that includes its shared libs and ABI

2. probably noone used that field
I agree but you asked for my oppinion. 
I have no strong oppinion but when you ask me what is the "proper" thing
to do then its, all removed public fields need deprecation. 
And its also less work than proofing that nothing
used a field and that its removial which may reshuffle other things also
wont be an issue. More so as its hard to proof this given we cannot know
all code that uses our libs. 

Theres alot that can be improved IMHO
* master during any period of instability should not allow a simple
--enable-shared and then make install without something like a extra --allow-unstable-abi

* periods of ABI/API instability should be short and better coordinated
people should arrange their planned changes before and not keep extending
these periods

* maybe we should use a special ABI version for these unstable period
that way half of the problems disappear, its then clear when that period
ends. And its not possible to install that under a stable ABI/API soname

* maybe our download page should not just point to a master snapshot when
  our ABI/API is in a transition period 

so, if you want to just remove the field, sure do it it probably will be
fine but the proper way from ABI/API compatibility POV is to deprecate it

thx
  
[...]

On Fri, Oct 22, 2021 at 6:16 PM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> On Fri, Oct 22, 2021 at 12:20:19PM +0300, Jan Ekström wrote:
> > On Fri, Oct 22, 2021 at 1:13 AM Michael Niedermayer
> > <michael@niedermayer.cc> wrote:
> > >
> > > On Fri, Oct 22, 2021 at 12:32:18AM +0300, Jan Ekström wrote:
> > > > On Fri, Oct 22, 2021 at 12:21 AM Jan Ekström <jeebjp@gmail.com> wrote:
> > > > >
> > > > > On Thu, Oct 21, 2021 at 11:49 PM Michael Niedermayer
> > > > > <michael@niedermayer.cc> wrote:
> > > > > >
> > > > > > On Thu, Oct 21, 2021 at 08:48:35PM +0300, Jan Ekström wrote:
> > > > > > > On Thu, Oct 21, 2021 at 8:26 PM Michael Niedermayer
> > > > > > > <michael@niedermayer.cc> wrote:
> > > > > > > >
> > > > > > > > On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström wrote:
> > > > > > > > > On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström <jeebjp@gmail.com> wrote:
> > > > > > > > > >
> > > > > > > > > > Originally added as a private entry in commit
> > > > > > > > > > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
> > > > > > > > > > the comment noting its private state was missed during merging of
> > > > > > > > > > the field from Libav (most likely due to an already existing field
> > > > > > > > > > in between).
> > > > > > > > > > ---
> > > > > > > >
> > > > > > > > Its a public struct in a public header so deprecate
> > > > > > > >
> > > > > > > > thx
> > > > > > > >
> > > > > > >
> > > > > > > OK, just confirming but so you don't consider this to be of the same
> > > > > > > type as these http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=a7b56ab6674f0a25be3b480df314e2d1292f397b;hp=0b35409787c925d681a121aa8d8715c3921fddf2;hb=45bfe8b838275235412777dd430206d9a24eb3ee;hpb=530ac6aa305aeda631c77f8a17e96c14c7ab1a1c
> > > > > > > ?
> > > > > >
> > > > > > i assume this was during ABI bumping,
> > > > > > when the major version numbers change then ABI/API breaking changes can be
> > > > > > done.
> > > > > >
> > > > >
> > > > > IIRC ABI breakage after bumping was supposed to be a ~month but then
> > > > > IIRC people posted such things much later. Was there a message or note
> > > > > when this period ended?
> > >
> > > Maybe someone assumed that everyone agreed what "a ~month" is ?
> > >
> > >
> > > > >
> > > >
> > > > This commit (http://git.videolan.org/?p=ffmpeg.git;a=commit;h=45bfe8b838275235412777dd430206d9a24eb3ee)
> > > > was applied in late August (as the entries were private - kind of like
> > > > this one remaining entry), and I can find Anton noting in May that the
> > > > project is in ABI instability period. So do excuse me for not knowing
> > > > which mode we are in right now if it went on for at least four months
> > > > :D
> > >
> > > I think you should really implement a full statistics API instead of
> > > this as you clearly have alot of time.
> >
> > I do not have a lot of time, I just saw this as a finalization of the
> > clean-up of private fields that Andreas started and I wanted to ask
> > about which people would prefer, since we have not yet had a release
> > branching with the API major version that this clean-up is included
> > in.
> >
> > If you disagree with Andreas's actions from late August, then I would
> > have preferred you to note that. I would have been OK with something a
> > la "It was not clearly marked in our (FFmpeg's) merge so we consider
> > it public and not private.", instead I got what (at least to me) felt
> > like re-education on what API/ABI breaks are, plus what seemed to be
> > not reading all those references I provided to why I thought this was
> > a private field and why this in my opinion was just a continuation of
> > the previous commit done by Andreas. I am sorry for the knee-jerk
> > reflex-like response, it is not productive. I think I just felt like
> > getting ignored and getting a standard "he doesn't understand the
> > basics of this basic thing" response, even though I attempted to
> > provide all the information regarding *why* I was asking.
>
> Well, the idea of ABI compatibility is that fields which are not
> clearly marked as private cannot be removed or move around in the binary
> layout of a struct. So a user can depend on code continuing to work
> A removial would violate that.

Agreed. I was trying to lay before people the things I had noticed
during my research into this field's history, which clearly showed
that it was intended to be 100% private when added in Libav, and that
it ended up being right next to private ones in FFmpeg, albeit without
a clear comment on top of it due to merge conflicts. If people saw it
still technically being public due to how the comment was no longer
right next to the struct member, then so be it :) .

Just to recap:
The field was brought in with
http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=7bf7985c5e04a551e3a2841decc6b32ded7c0799;hp=49721aa1315fc44bf46d73d343ace4e7c3dc785e;hb=3f75e5116b900f1428aa13041fc7d6301bf1988a;hpb=fc85646ad495f3418042468da415af73a7a07334
(you can see the comment noting its private state on the top of the context)

which then was merged as
http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=525eb7129eb5b5a5a2a8dc015275526b9b25a0cb;hp=6f4ed8440d684e03e70fd296849b76d216d6fbbb;hb=34d7f337c1608c72be8c36355018dc894f0560ce;hpb=c5fd47fa8a300fc51489a47da94041609545803c
(it went just under another private struct member from the FFmpeg side)

> Now the arguments i see here are
> 1. There was no release
> That is true but our download page first and biggest points to master
> not a release. So IMHO if we point to master then master should be
> working in "all circumstances" and that includes its shared libs and ABI
>

True. But then instead of having major bumps and periods afterwards,
we would have to properly bump major with each such change. Or work in
a separate branch and then pull it into master as the Big Bump.

First one is mostly something people dislike due to cosmetics (or
having to move deprecations onto a higher number dynamically if a
major version gets bumped multiple times in close proximity). Second
one means that people are much less likely to test that branch, as
well as incompatibilities when a person would be going backwards in
time in the branch. That said, the latter is what we already have in
master during ABI instability :) .

> 2. probably noone used that field

I have so far tried really hard to not utilize that argument, since at
least for me it seems like to attempt to get consensus on whether a
field is private or public is simpler than trying to prove that the
field wasn't utilized by anyone.

- If private, it can be removed.
- If public, it needs deprecation.

> I agree but you asked for my oppinion.
> I have no strong oppinion but when you ask me what is the "proper" thing
> to do then its, all removed public fields need deprecation.
> And its also less work than proofing that nothing
> used a field and that its removial which may reshuffle other things also
> wont be an issue. More so as its hard to proof this given we cannot know
> all code that uses our libs.
>
> Theres alot that can be improved IMHO

Massive yes from here on this :) I think we've noticed various things
that are sub-optimal with these things.

> * master during any period of instability should not allow a simple
> --enable-shared and then make install without something like a extra --allow-unstable-abi
>

Interesting proposition, and could possibly be matched with your later
proposition with regards to a specific major version/SONAME.

> * periods of ABI/API instability should be short and better coordinated
> people should arrange their planned changes before and not keep extending
> these periods

Definitely. The question is how we could make queuing these things
easier for people. On a merge request style platform I'd probably see
this handled by tags/flags that mark that those open change sets
should be handled at the next bump (tagging lets one easily either
filter the view of change sets for just those, or excluding those).
Then if a bump is announced (let's say a month or two before it
happens, so that people can get their change sets ready for rebase),
reviews are started so that when a bump happens, it's swift and done
with.

>
> * maybe we should use a special ABI version for these unstable period
> that way half of the problems disappear, its then clear when that period
> ends. And its not possible to install that under a stable ABI/API soname
>

That's an interesting proposition. Just not sure what's the best way
for this? Negative or very large SONAME?

> * maybe our download page should not just point to a master snapshot when
>   our ABI/API is in a transition period
>
> so, if you want to just remove the field, sure do it it probably will be
> fine but the proper way from ABI/API compatibility POV is to deprecate it
>

So just to bring this set to the finish line, do I understand that for
you the field was not well enough flagged as private, and should thus
be treated as a public struct member?

I would prefer removal as I've looked into the history of the field
and I saw Andreas working hard on removing the private entries from
this struct, but technically I can see that due to the Libav->FFmpeg
merge breaking the connection it could be argued that the field was
not flagged as private when added to FFmpeg. Which is why Andreas most
likely missed it from the struct in his clean-up commit.

If you think it is public, then I will just go with this version of
the patch set, including deprecation. For me the main thing was to
highlight and make people think whether this struct member was meant
as public or private, and then decide how it should be interpreted in
current FFmpeg master. Poking James on IRC, he seems to be more
towards removal when looking at it in the context of how and where it
was added, but also first noted that deprecation would be the cleanest
way to remove it.

Jan

On Fri, Oct 22, 2021 at 08:42:14PM +0300, Jan Ekström wrote:
> On Fri, Oct 22, 2021 at 6:16 PM Michael Niedermayer
> <michael@niedermayer.cc> wrote:
> >
> > On Fri, Oct 22, 2021 at 12:20:19PM +0300, Jan Ekström wrote:
> > > On Fri, Oct 22, 2021 at 1:13 AM Michael Niedermayer
> > > <michael@niedermayer.cc> wrote:
> > > >
> > > > On Fri, Oct 22, 2021 at 12:32:18AM +0300, Jan Ekström wrote:
> > > > > On Fri, Oct 22, 2021 at 12:21 AM Jan Ekström <jeebjp@gmail.com> wrote:
> > > > > >
> > > > > > On Thu, Oct 21, 2021 at 11:49 PM Michael Niedermayer
> > > > > > <michael@niedermayer.cc> wrote:
> > > > > > >
> > > > > > > On Thu, Oct 21, 2021 at 08:48:35PM +0300, Jan Ekström wrote:
> > > > > > > > On Thu, Oct 21, 2021 at 8:26 PM Michael Niedermayer
> > > > > > > > <michael@niedermayer.cc> wrote:
> > > > > > > > >
> > > > > > > > > On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström wrote:
> > > > > > > > > > On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström <jeebjp@gmail.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > Originally added as a private entry in commit
> > > > > > > > > > > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
> > > > > > > > > > > the comment noting its private state was missed during merging of
> > > > > > > > > > > the field from Libav (most likely due to an already existing field
> > > > > > > > > > > in between).
> > > > > > > > > > > ---
> > > > > > > > >
> > > > > > > > > Its a public struct in a public header so deprecate
> > > > > > > > >
> > > > > > > > > thx
> > > > > > > > >
> > > > > > > >
> > > > > > > > OK, just confirming but so you don't consider this to be of the same
> > > > > > > > type as these http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=a7b56ab6674f0a25be3b480df314e2d1292f397b;hp=0b35409787c925d681a121aa8d8715c3921fddf2;hb=45bfe8b838275235412777dd430206d9a24eb3ee;hpb=530ac6aa305aeda631c77f8a17e96c14c7ab1a1c
> > > > > > > > ?
> > > > > > >
> > > > > > > i assume this was during ABI bumping,
> > > > > > > when the major version numbers change then ABI/API breaking changes can be
> > > > > > > done.
> > > > > > >
> > > > > >
> > > > > > IIRC ABI breakage after bumping was supposed to be a ~month but then
> > > > > > IIRC people posted such things much later. Was there a message or note
> > > > > > when this period ended?
> > > >
> > > > Maybe someone assumed that everyone agreed what "a ~month" is ?
> > > >
> > > >
> > > > > >
> > > > >
> > > > > This commit (http://git.videolan.org/?p=ffmpeg.git;a=commit;h=45bfe8b838275235412777dd430206d9a24eb3ee)
> > > > > was applied in late August (as the entries were private - kind of like
> > > > > this one remaining entry), and I can find Anton noting in May that the
> > > > > project is in ABI instability period. So do excuse me for not knowing
> > > > > which mode we are in right now if it went on for at least four months
> > > > > :D
> > > >
> > > > I think you should really implement a full statistics API instead of
> > > > this as you clearly have alot of time.
> > >
> > > I do not have a lot of time, I just saw this as a finalization of the
> > > clean-up of private fields that Andreas started and I wanted to ask
> > > about which people would prefer, since we have not yet had a release
> > > branching with the API major version that this clean-up is included
> > > in.
> > >
> > > If you disagree with Andreas's actions from late August, then I would
> > > have preferred you to note that. I would have been OK with something a
> > > la "It was not clearly marked in our (FFmpeg's) merge so we consider
> > > it public and not private.", instead I got what (at least to me) felt
> > > like re-education on what API/ABI breaks are, plus what seemed to be
> > > not reading all those references I provided to why I thought this was
> > > a private field and why this in my opinion was just a continuation of
> > > the previous commit done by Andreas. I am sorry for the knee-jerk
> > > reflex-like response, it is not productive. I think I just felt like
> > > getting ignored and getting a standard "he doesn't understand the
> > > basics of this basic thing" response, even though I attempted to
> > > provide all the information regarding *why* I was asking.
> >
> > Well, the idea of ABI compatibility is that fields which are not
> > clearly marked as private cannot be removed or move around in the binary
> > layout of a struct. So a user can depend on code continuing to work
> > A removial would violate that.
> 
> Agreed. I was trying to lay before people the things I had noticed
> during my research into this field's history, which clearly showed
> that it was intended to be 100% private when added in Libav, and that
> it ended up being right next to private ones in FFmpeg, albeit without
> a clear comment on top of it due to merge conflicts. If people saw it
> still technically being public due to how the comment was no longer
> right next to the struct member, then so be it :) .
> 
> Just to recap:
> The field was brought in with
> http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=7bf7985c5e04a551e3a2841decc6b32ded7c0799;hp=49721aa1315fc44bf46d73d343ace4e7c3dc785e;hb=3f75e5116b900f1428aa13041fc7d6301bf1988a;hpb=fc85646ad495f3418042468da415af73a7a07334
> (you can see the comment noting its private state on the top of the context)
> 
> which then was merged as
> http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=525eb7129eb5b5a5a2a8dc015275526b9b25a0cb;hp=6f4ed8440d684e03e70fd296849b76d216d6fbbb;hb=34d7f337c1608c72be8c36355018dc894f0560ce;hpb=c5fd47fa8a300fc51489a47da94041609545803c
> (it went just under another private struct member from the FFmpeg side)
> 
> > Now the arguments i see here are
> > 1. There was no release
> > That is true but our download page first and biggest points to master
> > not a release. So IMHO if we point to master then master should be
> > working in "all circumstances" and that includes its shared libs and ABI
> >
> 
> True. But then instead of having major bumps and periods afterwards,
> we would have to properly bump major with each such change. Or work in
> a separate branch and then pull it into master as the Big Bump.
> 
> First one is mostly something people dislike due to cosmetics (or
> having to move deprecations onto a higher number dynamically if a
> major version gets bumped multiple times in close proximity). Second
> one means that people are much less likely to test that branch, as
> well as incompatibilities when a person would be going backwards in
> time in the branch. That said, the latter is what we already have in
> master during ABI instability :) .
> 
> > 2. probably noone used that field
> 
> I have so far tried really hard to not utilize that argument, since at
> least for me it seems like to attempt to get consensus on whether a
> field is private or public is simpler than trying to prove that the
> field wasn't utilized by anyone.
> 
> - If private, it can be removed.
> - If public, it needs deprecation.
> 
> > I agree but you asked for my oppinion.
> > I have no strong oppinion but when you ask me what is the "proper" thing
> > to do then its, all removed public fields need deprecation.
> > And its also less work than proofing that nothing
> > used a field and that its removial which may reshuffle other things also
> > wont be an issue. More so as its hard to proof this given we cannot know
> > all code that uses our libs.
> >
> > Theres alot that can be improved IMHO
> 
> Massive yes from here on this :) I think we've noticed various things
> that are sub-optimal with these things.
> 
> > * master during any period of instability should not allow a simple
> > --enable-shared and then make install without something like a extra --allow-unstable-abi
> >
> 
> Interesting proposition, and could possibly be matched with your later
> proposition with regards to a specific major version/SONAME.
> 
> > * periods of ABI/API instability should be short and better coordinated
> > people should arrange their planned changes before and not keep extending
> > these periods
> 
> Definitely. The question is how we could make queuing these things
> easier for people. On a merge request style platform I'd probably see
> this handled by tags/flags that mark that those open change sets
> should be handled at the next bump (tagging lets one easily either
> filter the view of change sets for just those, or excluding those).
> Then if a bump is announced (let's say a month or two before it
> happens, so that people can get their change sets ready for rebase),
> reviews are started so that when a bump happens, it's swift and done
> with.
> 
> >
> > * maybe we should use a special ABI version for these unstable period
> > that way half of the problems disappear, its then clear when that period
> > ends. And its not possible to install that under a stable ABI/API soname
> >
> 
> That's an interesting proposition. Just not sure what's the best way
> for this? Negative or very large SONAME?
> 
> > * maybe our download page should not just point to a master snapshot when
> >   our ABI/API is in a transition period
> >
> > so, if you want to just remove the field, sure do it it probably will be
> > fine but the proper way from ABI/API compatibility POV is to deprecate it
> >
> 
> So just to bring this set to the finish line, do I understand that for
> you the field was not well enough flagged as private, and should thus
> be treated as a public struct member?
> 
> I would prefer removal as I've looked into the history of the field
> and I saw Andreas working hard on removing the private entries from
> this struct, but technically I can see that due to the Libav->FFmpeg
> merge breaking the connection it could be argued that the field was
> not flagged as private when added to FFmpeg. Which is why Andreas most
> likely missed it from the struct in his clean-up commit.
> 
> If you think it is public, then I will just go with this version of
> the patch set, including deprecation. For me the main thing was to
> highlight and make people think whether this struct member was meant
> as public or private, and then decide how it should be interpreted in
> current FFmpeg master. Poking James on IRC, he seems to be more
> towards removal when looking at it in the context of how and where it
> was added, but also first noted that deprecation would be the cleanest
> way to remove it.

why would one think its private ?

The struct documentation says this:
/**
 * Bytestream IO Context.
 * New public fields can be added with minor version bumps.
 * Removal, reordering and changes to existing public fields require
 * a major version bump.
 * sizeof(AVIOContext) must not be used outside libav*.
 *
 * @note None of the function pointers in AVIOContext should be called
 *       directly, they should only be set by the client application
 *       when implementing custom I/O. Normally these are set to the
 *       function pointers specified in avio_alloc_context()
 */

 
This speaks only of public fields
it also excludes the function pointers, but nothing else is excluded

and then theres

int64_t written;

why would this not be a public field ?
what is the reasoning behind that?

You can also poll maybe 3 developers unrelated to FFmpeg and not knowing
your preferrance or the codebase. "Is this a public field an application
can use if it needs to ?"

If someone says "No" then iam really currious why someone came to
this conclusion. I dont think anyone would investigate the git log
of when a field was added or other related cleanups.

thx

[...]

On Fri, Oct 22, 2021 at 11:26 PM Michael Niedermayer
<michael@niedermayer.cc> wrote:
>
> On Fri, Oct 22, 2021 at 08:42:14PM +0300, Jan Ekström wrote:
> > On Fri, Oct 22, 2021 at 6:16 PM Michael Niedermayer
> > <michael@niedermayer.cc> wrote:
> > >
> > > On Fri, Oct 22, 2021 at 12:20:19PM +0300, Jan Ekström wrote:
> > > > On Fri, Oct 22, 2021 at 1:13 AM Michael Niedermayer
> > > > <michael@niedermayer.cc> wrote:
> > > > >
> > > > > On Fri, Oct 22, 2021 at 12:32:18AM +0300, Jan Ekström wrote:
> > > > > > On Fri, Oct 22, 2021 at 12:21 AM Jan Ekström <jeebjp@gmail.com> wrote:
> > > > > > >
> > > > > > > On Thu, Oct 21, 2021 at 11:49 PM Michael Niedermayer
> > > > > > > <michael@niedermayer.cc> wrote:
> > > > > > > >
> > > > > > > > On Thu, Oct 21, 2021 at 08:48:35PM +0300, Jan Ekström wrote:
> > > > > > > > > On Thu, Oct 21, 2021 at 8:26 PM Michael Niedermayer
> > > > > > > > > <michael@niedermayer.cc> wrote:
> > > > > > > > > >
> > > > > > > > > > On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström wrote:
> > > > > > > > > > > On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström <jeebjp@gmail.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Originally added as a private entry in commit
> > > > > > > > > > > > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
> > > > > > > > > > > > the comment noting its private state was missed during merging of
> > > > > > > > > > > > the field from Libav (most likely due to an already existing field
> > > > > > > > > > > > in between).
> > > > > > > > > > > > ---
> > > > > > > > > >
> > > > > > > > > > Its a public struct in a public header so deprecate
> > > > > > > > > >
> > > > > > > > > > thx
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > OK, just confirming but so you don't consider this to be of the same
> > > > > > > > > type as these http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=a7b56ab6674f0a25be3b480df314e2d1292f397b;hp=0b35409787c925d681a121aa8d8715c3921fddf2;hb=45bfe8b838275235412777dd430206d9a24eb3ee;hpb=530ac6aa305aeda631c77f8a17e96c14c7ab1a1c
> > > > > > > > > ?
> > > > > > > >
> > > > > > > > i assume this was during ABI bumping,
> > > > > > > > when the major version numbers change then ABI/API breaking changes can be
> > > > > > > > done.
> > > > > > > >
> > > > > > >
> > > > > > > IIRC ABI breakage after bumping was supposed to be a ~month but then
> > > > > > > IIRC people posted such things much later. Was there a message or note
> > > > > > > when this period ended?
> > > > >
> > > > > Maybe someone assumed that everyone agreed what "a ~month" is ?
> > > > >
> > > > >
> > > > > > >
> > > > > >
> > > > > > This commit (http://git.videolan.org/?p=ffmpeg.git;a=commit;h=45bfe8b838275235412777dd430206d9a24eb3ee)
> > > > > > was applied in late August (as the entries were private - kind of like
> > > > > > this one remaining entry), and I can find Anton noting in May that the
> > > > > > project is in ABI instability period. So do excuse me for not knowing
> > > > > > which mode we are in right now if it went on for at least four months
> > > > > > :D
> > > > >
> > > > > I think you should really implement a full statistics API instead of
> > > > > this as you clearly have alot of time.
> > > >
> > > > I do not have a lot of time, I just saw this as a finalization of the
> > > > clean-up of private fields that Andreas started and I wanted to ask
> > > > about which people would prefer, since we have not yet had a release
> > > > branching with the API major version that this clean-up is included
> > > > in.
> > > >
> > > > If you disagree with Andreas's actions from late August, then I would
> > > > have preferred you to note that. I would have been OK with something a
> > > > la "It was not clearly marked in our (FFmpeg's) merge so we consider
> > > > it public and not private.", instead I got what (at least to me) felt
> > > > like re-education on what API/ABI breaks are, plus what seemed to be
> > > > not reading all those references I provided to why I thought this was
> > > > a private field and why this in my opinion was just a continuation of
> > > > the previous commit done by Andreas. I am sorry for the knee-jerk
> > > > reflex-like response, it is not productive. I think I just felt like
> > > > getting ignored and getting a standard "he doesn't understand the
> > > > basics of this basic thing" response, even though I attempted to
> > > > provide all the information regarding *why* I was asking.
> > >
> > > Well, the idea of ABI compatibility is that fields which are not
> > > clearly marked as private cannot be removed or move around in the binary
> > > layout of a struct. So a user can depend on code continuing to work
> > > A removial would violate that.
> >
> > Agreed. I was trying to lay before people the things I had noticed
> > during my research into this field's history, which clearly showed
> > that it was intended to be 100% private when added in Libav, and that
> > it ended up being right next to private ones in FFmpeg, albeit without
> > a clear comment on top of it due to merge conflicts. If people saw it
> > still technically being public due to how the comment was no longer
> > right next to the struct member, then so be it :) .
> >
> > Just to recap:
> > The field was brought in with
> > http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=7bf7985c5e04a551e3a2841decc6b32ded7c0799;hp=49721aa1315fc44bf46d73d343ace4e7c3dc785e;hb=3f75e5116b900f1428aa13041fc7d6301bf1988a;hpb=fc85646ad495f3418042468da415af73a7a07334
> > (you can see the comment noting its private state on the top of the context)
> >
> > which then was merged as
> > http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=525eb7129eb5b5a5a2a8dc015275526b9b25a0cb;hp=6f4ed8440d684e03e70fd296849b76d216d6fbbb;hb=34d7f337c1608c72be8c36355018dc894f0560ce;hpb=c5fd47fa8a300fc51489a47da94041609545803c
> > (it went just under another private struct member from the FFmpeg side)
> >
> > > Now the arguments i see here are
> > > 1. There was no release
> > > That is true but our download page first and biggest points to master
> > > not a release. So IMHO if we point to master then master should be
> > > working in "all circumstances" and that includes its shared libs and ABI
> > >
> >
> > True. But then instead of having major bumps and periods afterwards,
> > we would have to properly bump major with each such change. Or work in
> > a separate branch and then pull it into master as the Big Bump.
> >
> > First one is mostly something people dislike due to cosmetics (or
> > having to move deprecations onto a higher number dynamically if a
> > major version gets bumped multiple times in close proximity). Second
> > one means that people are much less likely to test that branch, as
> > well as incompatibilities when a person would be going backwards in
> > time in the branch. That said, the latter is what we already have in
> > master during ABI instability :) .
> >
> > > 2. probably noone used that field
> >
> > I have so far tried really hard to not utilize that argument, since at
> > least for me it seems like to attempt to get consensus on whether a
> > field is private or public is simpler than trying to prove that the
> > field wasn't utilized by anyone.
> >
> > - If private, it can be removed.
> > - If public, it needs deprecation.
> >
> > > I agree but you asked for my oppinion.
> > > I have no strong oppinion but when you ask me what is the "proper" thing
> > > to do then its, all removed public fields need deprecation.
> > > And its also less work than proofing that nothing
> > > used a field and that its removial which may reshuffle other things also
> > > wont be an issue. More so as its hard to proof this given we cannot know
> > > all code that uses our libs.
> > >
> > > Theres alot that can be improved IMHO
> >
> > Massive yes from here on this :) I think we've noticed various things
> > that are sub-optimal with these things.
> >
> > > * master during any period of instability should not allow a simple
> > > --enable-shared and then make install without something like a extra --allow-unstable-abi
> > >
> >
> > Interesting proposition, and could possibly be matched with your later
> > proposition with regards to a specific major version/SONAME.
> >
> > > * periods of ABI/API instability should be short and better coordinated
> > > people should arrange their planned changes before and not keep extending
> > > these periods
> >
> > Definitely. The question is how we could make queuing these things
> > easier for people. On a merge request style platform I'd probably see
> > this handled by tags/flags that mark that those open change sets
> > should be handled at the next bump (tagging lets one easily either
> > filter the view of change sets for just those, or excluding those).
> > Then if a bump is announced (let's say a month or two before it
> > happens, so that people can get their change sets ready for rebase),
> > reviews are started so that when a bump happens, it's swift and done
> > with.
> >
> > >
> > > * maybe we should use a special ABI version for these unstable period
> > > that way half of the problems disappear, its then clear when that period
> > > ends. And its not possible to install that under a stable ABI/API soname
> > >
> >
> > That's an interesting proposition. Just not sure what's the best way
> > for this? Negative or very large SONAME?
> >
> > > * maybe our download page should not just point to a master snapshot when
> > >   our ABI/API is in a transition period
> > >
> > > so, if you want to just remove the field, sure do it it probably will be
> > > fine but the proper way from ABI/API compatibility POV is to deprecate it
> > >
> >
> > So just to bring this set to the finish line, do I understand that for
> > you the field was not well enough flagged as private, and should thus
> > be treated as a public struct member?
> >
> > I would prefer removal as I've looked into the history of the field
> > and I saw Andreas working hard on removing the private entries from
> > this struct, but technically I can see that due to the Libav->FFmpeg
> > merge breaking the connection it could be argued that the field was
> > not flagged as private when added to FFmpeg. Which is why Andreas most
> > likely missed it from the struct in his clean-up commit.
> >
> > If you think it is public, then I will just go with this version of
> > the patch set, including deprecation. For me the main thing was to
> > highlight and make people think whether this struct member was meant
> > as public or private, and then decide how it should be interpreted in
> > current FFmpeg master. Poking James on IRC, he seems to be more
> > towards removal when looking at it in the context of how and where it
> > was added, but also first noted that deprecation would be the cleanest
> > way to remove it.
>
> why would one think its private ?
>
> The struct documentation says this:
> /**
>  * Bytestream IO Context.
>  * New public fields can be added with minor version bumps.
>  * Removal, reordering and changes to existing public fields require
>  * a major version bump.
>  * sizeof(AVIOContext) must not be used outside libav*.
>  *
>  * @note None of the function pointers in AVIOContext should be called
>  *       directly, they should only be set by the client application
>  *       when implementing custom I/O. Normally these are set to the
>  *       function pointers specified in avio_alloc_context()
>  */
>
>
> This speaks only of public fields
> it also excludes the function pointers, but nothing else is excluded
>
> and then theres
>
> int64_t written;
>
> why would this not be a public field ?
> what is the reasoning behind that?
>

Because I thought the how it was meant to be and how close in context
it was to groups of private members would add some value when making
the decision of whether to put it into one group of struct members or
into another. Thus at the very least I thought it was worth bringing
it up and hearing opinions.

Until very recently the region of the struct would be more like

    /**
     * Internal, not meant to be used from outside of AVIOContext.
     */
    enum AVIODataMarkerType current_type;
    int64_t last_time;

    /**
     * A callback that is used instead of short_seek_threshold.
     * This is current internal only, do not use from outside.
     */
    int (*short_seek_get)(void *opaque);

    int64_t written;

    /**
     * Maximum reached position before a backward seek in the write buffer,
     * used keeping track of already written data for a later flush.
     */
    unsigned char *buf_ptr_max;

    /**
     * Try to buffer at least this amount of data before flushing it
     */
    int min_packet_size;

If you strictly read only into what's in this context, it is an
undocumented member near some internal ones.

But sure, it seems like I am not going to get a straight answer here,
for whatever reason. I will just take this as the implied "it is
private in my opinion". Which is fine. It was one of the alternatives
I noted, and a valid reading based on the lack of comments stuck right
next to the field.

I have no idea why you respond in this way, but I hope you have a good evening.

Jan

On Sat, Oct 23, 2021 at 12:11 AM Jan Ekström <jeebjp@gmail.com> wrote:
>
> On Fri, Oct 22, 2021 at 11:26 PM Michael Niedermayer
> <michael@niedermayer.cc> wrote:
> >
> > On Fri, Oct 22, 2021 at 08:42:14PM +0300, Jan Ekström wrote:
> > > On Fri, Oct 22, 2021 at 6:16 PM Michael Niedermayer
> > > <michael@niedermayer.cc> wrote:
> > > >
> > > > On Fri, Oct 22, 2021 at 12:20:19PM +0300, Jan Ekström wrote:
> > > > > On Fri, Oct 22, 2021 at 1:13 AM Michael Niedermayer
> > > > > <michael@niedermayer.cc> wrote:
> > > > > >
> > > > > > On Fri, Oct 22, 2021 at 12:32:18AM +0300, Jan Ekström wrote:
> > > > > > > On Fri, Oct 22, 2021 at 12:21 AM Jan Ekström <jeebjp@gmail.com> wrote:
> > > > > > > >
> > > > > > > > On Thu, Oct 21, 2021 at 11:49 PM Michael Niedermayer
> > > > > > > > <michael@niedermayer.cc> wrote:
> > > > > > > > >
> > > > > > > > > On Thu, Oct 21, 2021 at 08:48:35PM +0300, Jan Ekström wrote:
> > > > > > > > > > On Thu, Oct 21, 2021 at 8:26 PM Michael Niedermayer
> > > > > > > > > > <michael@niedermayer.cc> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström wrote:
> > > > > > > > > > > > On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström <jeebjp@gmail.com> wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > Originally added as a private entry in commit
> > > > > > > > > > > > > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
> > > > > > > > > > > > > the comment noting its private state was missed during merging of
> > > > > > > > > > > > > the field from Libav (most likely due to an already existing field
> > > > > > > > > > > > > in between).
> > > > > > > > > > > > > ---
> > > > > > > > > > >
> > > > > > > > > > > Its a public struct in a public header so deprecate
> > > > > > > > > > >
> > > > > > > > > > > thx
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > OK, just confirming but so you don't consider this to be of the same
> > > > > > > > > > type as these http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=a7b56ab6674f0a25be3b480df314e2d1292f397b;hp=0b35409787c925d681a121aa8d8715c3921fddf2;hb=45bfe8b838275235412777dd430206d9a24eb3ee;hpb=530ac6aa305aeda631c77f8a17e96c14c7ab1a1c
> > > > > > > > > > ?
> > > > > > > > >
> > > > > > > > > i assume this was during ABI bumping,
> > > > > > > > > when the major version numbers change then ABI/API breaking changes can be
> > > > > > > > > done.
> > > > > > > > >
> > > > > > > >
> > > > > > > > IIRC ABI breakage after bumping was supposed to be a ~month but then
> > > > > > > > IIRC people posted such things much later. Was there a message or note
> > > > > > > > when this period ended?
> > > > > >
> > > > > > Maybe someone assumed that everyone agreed what "a ~month" is ?
> > > > > >
> > > > > >
> > > > > > > >
> > > > > > >
> > > > > > > This commit (http://git.videolan.org/?p=ffmpeg.git;a=commit;h=45bfe8b838275235412777dd430206d9a24eb3ee)
> > > > > > > was applied in late August (as the entries were private - kind of like
> > > > > > > this one remaining entry), and I can find Anton noting in May that the
> > > > > > > project is in ABI instability period. So do excuse me for not knowing
> > > > > > > which mode we are in right now if it went on for at least four months
> > > > > > > :D
> > > > > >
> > > > > > I think you should really implement a full statistics API instead of
> > > > > > this as you clearly have alot of time.
> > > > >
> > > > > I do not have a lot of time, I just saw this as a finalization of the
> > > > > clean-up of private fields that Andreas started and I wanted to ask
> > > > > about which people would prefer, since we have not yet had a release
> > > > > branching with the API major version that this clean-up is included
> > > > > in.
> > > > >
> > > > > If you disagree with Andreas's actions from late August, then I would
> > > > > have preferred you to note that. I would have been OK with something a
> > > > > la "It was not clearly marked in our (FFmpeg's) merge so we consider
> > > > > it public and not private.", instead I got what (at least to me) felt
> > > > > like re-education on what API/ABI breaks are, plus what seemed to be
> > > > > not reading all those references I provided to why I thought this was
> > > > > a private field and why this in my opinion was just a continuation of
> > > > > the previous commit done by Andreas. I am sorry for the knee-jerk
> > > > > reflex-like response, it is not productive. I think I just felt like
> > > > > getting ignored and getting a standard "he doesn't understand the
> > > > > basics of this basic thing" response, even though I attempted to
> > > > > provide all the information regarding *why* I was asking.
> > > >
> > > > Well, the idea of ABI compatibility is that fields which are not
> > > > clearly marked as private cannot be removed or move around in the binary
> > > > layout of a struct. So a user can depend on code continuing to work
> > > > A removial would violate that.
> > >
> > > Agreed. I was trying to lay before people the things I had noticed
> > > during my research into this field's history, which clearly showed
> > > that it was intended to be 100% private when added in Libav, and that
> > > it ended up being right next to private ones in FFmpeg, albeit without
> > > a clear comment on top of it due to merge conflicts. If people saw it
> > > still technically being public due to how the comment was no longer
> > > right next to the struct member, then so be it :) .
> > >
> > > Just to recap:
> > > The field was brought in with
> > > http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=7bf7985c5e04a551e3a2841decc6b32ded7c0799;hp=49721aa1315fc44bf46d73d343ace4e7c3dc785e;hb=3f75e5116b900f1428aa13041fc7d6301bf1988a;hpb=fc85646ad495f3418042468da415af73a7a07334
> > > (you can see the comment noting its private state on the top of the context)
> > >
> > > which then was merged as
> > > http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=525eb7129eb5b5a5a2a8dc015275526b9b25a0cb;hp=6f4ed8440d684e03e70fd296849b76d216d6fbbb;hb=34d7f337c1608c72be8c36355018dc894f0560ce;hpb=c5fd47fa8a300fc51489a47da94041609545803c
> > > (it went just under another private struct member from the FFmpeg side)
> > >
> > > > Now the arguments i see here are
> > > > 1. There was no release
> > > > That is true but our download page first and biggest points to master
> > > > not a release. So IMHO if we point to master then master should be
> > > > working in "all circumstances" and that includes its shared libs and ABI
> > > >
> > >
> > > True. But then instead of having major bumps and periods afterwards,
> > > we would have to properly bump major with each such change. Or work in
> > > a separate branch and then pull it into master as the Big Bump.
> > >
> > > First one is mostly something people dislike due to cosmetics (or
> > > having to move deprecations onto a higher number dynamically if a
> > > major version gets bumped multiple times in close proximity). Second
> > > one means that people are much less likely to test that branch, as
> > > well as incompatibilities when a person would be going backwards in
> > > time in the branch. That said, the latter is what we already have in
> > > master during ABI instability :) .
> > >
> > > > 2. probably noone used that field
> > >
> > > I have so far tried really hard to not utilize that argument, since at
> > > least for me it seems like to attempt to get consensus on whether a
> > > field is private or public is simpler than trying to prove that the
> > > field wasn't utilized by anyone.
> > >
> > > - If private, it can be removed.
> > > - If public, it needs deprecation.
> > >
> > > > I agree but you asked for my oppinion.
> > > > I have no strong oppinion but when you ask me what is the "proper" thing
> > > > to do then its, all removed public fields need deprecation.
> > > > And its also less work than proofing that nothing
> > > > used a field and that its removial which may reshuffle other things also
> > > > wont be an issue. More so as its hard to proof this given we cannot know
> > > > all code that uses our libs.
> > > >
> > > > Theres alot that can be improved IMHO
> > >
> > > Massive yes from here on this :) I think we've noticed various things
> > > that are sub-optimal with these things.
> > >
> > > > * master during any period of instability should not allow a simple
> > > > --enable-shared and then make install without something like a extra --allow-unstable-abi
> > > >
> > >
> > > Interesting proposition, and could possibly be matched with your later
> > > proposition with regards to a specific major version/SONAME.
> > >
> > > > * periods of ABI/API instability should be short and better coordinated
> > > > people should arrange their planned changes before and not keep extending
> > > > these periods
> > >
> > > Definitely. The question is how we could make queuing these things
> > > easier for people. On a merge request style platform I'd probably see
> > > this handled by tags/flags that mark that those open change sets
> > > should be handled at the next bump (tagging lets one easily either
> > > filter the view of change sets for just those, or excluding those).
> > > Then if a bump is announced (let's say a month or two before it
> > > happens, so that people can get their change sets ready for rebase),
> > > reviews are started so that when a bump happens, it's swift and done
> > > with.
> > >
> > > >
> > > > * maybe we should use a special ABI version for these unstable period
> > > > that way half of the problems disappear, its then clear when that period
> > > > ends. And its not possible to install that under a stable ABI/API soname
> > > >
> > >
> > > That's an interesting proposition. Just not sure what's the best way
> > > for this? Negative or very large SONAME?
> > >
> > > > * maybe our download page should not just point to a master snapshot when
> > > >   our ABI/API is in a transition period
> > > >
> > > > so, if you want to just remove the field, sure do it it probably will be
> > > > fine but the proper way from ABI/API compatibility POV is to deprecate it
> > > >
> > >
> > > So just to bring this set to the finish line, do I understand that for
> > > you the field was not well enough flagged as private, and should thus
> > > be treated as a public struct member?
> > >
> > > I would prefer removal as I've looked into the history of the field
> > > and I saw Andreas working hard on removing the private entries from
> > > this struct, but technically I can see that due to the Libav->FFmpeg
> > > merge breaking the connection it could be argued that the field was
> > > not flagged as private when added to FFmpeg. Which is why Andreas most
> > > likely missed it from the struct in his clean-up commit.
> > >
> > > If you think it is public, then I will just go with this version of
> > > the patch set, including deprecation. For me the main thing was to
> > > highlight and make people think whether this struct member was meant
> > > as public or private, and then decide how it should be interpreted in
> > > current FFmpeg master. Poking James on IRC, he seems to be more
> > > towards removal when looking at it in the context of how and where it
> > > was added, but also first noted that deprecation would be the cleanest
> > > way to remove it.
> >
> > why would one think its private ?
> >
> > The struct documentation says this:
> > /**
> >  * Bytestream IO Context.
> >  * New public fields can be added with minor version bumps.
> >  * Removal, reordering and changes to existing public fields require
> >  * a major version bump.
> >  * sizeof(AVIOContext) must not be used outside libav*.
> >  *
> >  * @note None of the function pointers in AVIOContext should be called
> >  *       directly, they should only be set by the client application
> >  *       when implementing custom I/O. Normally these are set to the
> >  *       function pointers specified in avio_alloc_context()
> >  */
> >
> >
> > This speaks only of public fields
> > it also excludes the function pointers, but nothing else is excluded
> >
> > and then theres
> >
> > int64_t written;
> >
> > why would this not be a public field ?
> > what is the reasoning behind that?
> >
>
> Because I thought the how it was meant to be and how close in context
> it was to groups of private members would add some value when making
> the decision of whether to put it into one group of struct members or
> into another. Thus at the very least I thought it was worth bringing
> it up and hearing opinions.
>
> Until very recently the region of the struct would be more like
>
>     /**
>      * Internal, not meant to be used from outside of AVIOContext.
>      */
>     enum AVIODataMarkerType current_type;
>     int64_t last_time;
>
>     /**
>      * A callback that is used instead of short_seek_threshold.
>      * This is current internal only, do not use from outside.
>      */
>     int (*short_seek_get)(void *opaque);
>
>     int64_t written;
>
>     /**
>      * Maximum reached position before a backward seek in the write buffer,
>      * used keeping track of already written data for a later flush.
>      */
>     unsigned char *buf_ptr_max;
>
>     /**
>      * Try to buffer at least this amount of data before flushing it
>      */
>     int min_packet_size;
>
> If you strictly read only into what's in this context, it is an
> undocumented member near some internal ones.
>
> But sure, it seems like I am not going to get a straight answer here,
> for whatever reason. I will just take this as the implied "it is
> private in my opinion". Which is fine. It was one of the alternatives
> I noted, and a valid reading based on the lack of comments stuck right
> next to the field.

I meant, "it is public in my opinion" here, of course. Sorry. Kind of
missed I had typed the wrong words while going through thoughts.

Jan

On Sat, Oct 23, 2021 at 12:13 AM Jan Ekström <jeebjp@gmail.com> wrote:
>
> On Sat, Oct 23, 2021 at 12:11 AM Jan Ekström <jeebjp@gmail.com> wrote:
> >
> > On Fri, Oct 22, 2021 at 11:26 PM Michael Niedermayer
> > <michael@niedermayer.cc> wrote:
> > >
> > > On Fri, Oct 22, 2021 at 08:42:14PM +0300, Jan Ekström wrote:
> > > > On Fri, Oct 22, 2021 at 6:16 PM Michael Niedermayer
> > > > <michael@niedermayer.cc> wrote:
> > > > >
> > > > > On Fri, Oct 22, 2021 at 12:20:19PM +0300, Jan Ekström wrote:
> > > > > > On Fri, Oct 22, 2021 at 1:13 AM Michael Niedermayer
> > > > > > <michael@niedermayer.cc> wrote:
> > > > > > >
> > > > > > > On Fri, Oct 22, 2021 at 12:32:18AM +0300, Jan Ekström wrote:
> > > > > > > > On Fri, Oct 22, 2021 at 12:21 AM Jan Ekström <jeebjp@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > On Thu, Oct 21, 2021 at 11:49 PM Michael Niedermayer
> > > > > > > > > <michael@niedermayer.cc> wrote:
> > > > > > > > > >
> > > > > > > > > > On Thu, Oct 21, 2021 at 08:48:35PM +0300, Jan Ekström wrote:
> > > > > > > > > > > On Thu, Oct 21, 2021 at 8:26 PM Michael Niedermayer
> > > > > > > > > > > <michael@niedermayer.cc> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström wrote:
> > > > > > > > > > > > > On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström <jeebjp@gmail.com> wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Originally added as a private entry in commit
> > > > > > > > > > > > > > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
> > > > > > > > > > > > > > the comment noting its private state was missed during merging of
> > > > > > > > > > > > > > the field from Libav (most likely due to an already existing field
> > > > > > > > > > > > > > in between).
> > > > > > > > > > > > > > ---
> > > > > > > > > > > >
> > > > > > > > > > > > Its a public struct in a public header so deprecate
> > > > > > > > > > > >
> > > > > > > > > > > > thx
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > OK, just confirming but so you don't consider this to be of the same
> > > > > > > > > > > type as these http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=a7b56ab6674f0a25be3b480df314e2d1292f397b;hp=0b35409787c925d681a121aa8d8715c3921fddf2;hb=45bfe8b838275235412777dd430206d9a24eb3ee;hpb=530ac6aa305aeda631c77f8a17e96c14c7ab1a1c
> > > > > > > > > > > ?
> > > > > > > > > >
> > > > > > > > > > i assume this was during ABI bumping,
> > > > > > > > > > when the major version numbers change then ABI/API breaking changes can be
> > > > > > > > > > done.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > IIRC ABI breakage after bumping was supposed to be a ~month but then
> > > > > > > > > IIRC people posted such things much later. Was there a message or note
> > > > > > > > > when this period ended?
> > > > > > >
> > > > > > > Maybe someone assumed that everyone agreed what "a ~month" is ?
> > > > > > >
> > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > > > This commit (http://git.videolan.org/?p=ffmpeg.git;a=commit;h=45bfe8b838275235412777dd430206d9a24eb3ee)
> > > > > > > > was applied in late August (as the entries were private - kind of like
> > > > > > > > this one remaining entry), and I can find Anton noting in May that the
> > > > > > > > project is in ABI instability period. So do excuse me for not knowing
> > > > > > > > which mode we are in right now if it went on for at least four months
> > > > > > > > :D
> > > > > > >
> > > > > > > I think you should really implement a full statistics API instead of
> > > > > > > this as you clearly have alot of time.
> > > > > >
> > > > > > I do not have a lot of time, I just saw this as a finalization of the
> > > > > > clean-up of private fields that Andreas started and I wanted to ask
> > > > > > about which people would prefer, since we have not yet had a release
> > > > > > branching with the API major version that this clean-up is included
> > > > > > in.
> > > > > >
> > > > > > If you disagree with Andreas's actions from late August, then I would
> > > > > > have preferred you to note that. I would have been OK with something a
> > > > > > la "It was not clearly marked in our (FFmpeg's) merge so we consider
> > > > > > it public and not private.", instead I got what (at least to me) felt
> > > > > > like re-education on what API/ABI breaks are, plus what seemed to be
> > > > > > not reading all those references I provided to why I thought this was
> > > > > > a private field and why this in my opinion was just a continuation of
> > > > > > the previous commit done by Andreas. I am sorry for the knee-jerk
> > > > > > reflex-like response, it is not productive. I think I just felt like
> > > > > > getting ignored and getting a standard "he doesn't understand the
> > > > > > basics of this basic thing" response, even though I attempted to
> > > > > > provide all the information regarding *why* I was asking.
> > > > >
> > > > > Well, the idea of ABI compatibility is that fields which are not
> > > > > clearly marked as private cannot be removed or move around in the binary
> > > > > layout of a struct. So a user can depend on code continuing to work
> > > > > A removial would violate that.
> > > >
> > > > Agreed. I was trying to lay before people the things I had noticed
> > > > during my research into this field's history, which clearly showed
> > > > that it was intended to be 100% private when added in Libav, and that
> > > > it ended up being right next to private ones in FFmpeg, albeit without
> > > > a clear comment on top of it due to merge conflicts. If people saw it
> > > > still technically being public due to how the comment was no longer
> > > > right next to the struct member, then so be it :) .
> > > >
> > > > Just to recap:
> > > > The field was brought in with
> > > > http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=7bf7985c5e04a551e3a2841decc6b32ded7c0799;hp=49721aa1315fc44bf46d73d343ace4e7c3dc785e;hb=3f75e5116b900f1428aa13041fc7d6301bf1988a;hpb=fc85646ad495f3418042468da415af73a7a07334
> > > > (you can see the comment noting its private state on the top of the context)
> > > >
> > > > which then was merged as
> > > > http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=525eb7129eb5b5a5a2a8dc015275526b9b25a0cb;hp=6f4ed8440d684e03e70fd296849b76d216d6fbbb;hb=34d7f337c1608c72be8c36355018dc894f0560ce;hpb=c5fd47fa8a300fc51489a47da94041609545803c
> > > > (it went just under another private struct member from the FFmpeg side)
> > > >
> > > > > Now the arguments i see here are
> > > > > 1. There was no release
> > > > > That is true but our download page first and biggest points to master
> > > > > not a release. So IMHO if we point to master then master should be
> > > > > working in "all circumstances" and that includes its shared libs and ABI
> > > > >
> > > >
> > > > True. But then instead of having major bumps and periods afterwards,
> > > > we would have to properly bump major with each such change. Or work in
> > > > a separate branch and then pull it into master as the Big Bump.
> > > >
> > > > First one is mostly something people dislike due to cosmetics (or
> > > > having to move deprecations onto a higher number dynamically if a
> > > > major version gets bumped multiple times in close proximity). Second
> > > > one means that people are much less likely to test that branch, as
> > > > well as incompatibilities when a person would be going backwards in
> > > > time in the branch. That said, the latter is what we already have in
> > > > master during ABI instability :) .
> > > >
> > > > > 2. probably noone used that field
> > > >
> > > > I have so far tried really hard to not utilize that argument, since at
> > > > least for me it seems like to attempt to get consensus on whether a
> > > > field is private or public is simpler than trying to prove that the
> > > > field wasn't utilized by anyone.
> > > >
> > > > - If private, it can be removed.
> > > > - If public, it needs deprecation.
> > > >
> > > > > I agree but you asked for my oppinion.
> > > > > I have no strong oppinion but when you ask me what is the "proper" thing
> > > > > to do then its, all removed public fields need deprecation.
> > > > > And its also less work than proofing that nothing
> > > > > used a field and that its removial which may reshuffle other things also
> > > > > wont be an issue. More so as its hard to proof this given we cannot know
> > > > > all code that uses our libs.
> > > > >
> > > > > Theres alot that can be improved IMHO
> > > >
> > > > Massive yes from here on this :) I think we've noticed various things
> > > > that are sub-optimal with these things.
> > > >
> > > > > * master during any period of instability should not allow a simple
> > > > > --enable-shared and then make install without something like a extra --allow-unstable-abi
> > > > >
> > > >
> > > > Interesting proposition, and could possibly be matched with your later
> > > > proposition with regards to a specific major version/SONAME.
> > > >
> > > > > * periods of ABI/API instability should be short and better coordinated
> > > > > people should arrange their planned changes before and not keep extending
> > > > > these periods
> > > >
> > > > Definitely. The question is how we could make queuing these things
> > > > easier for people. On a merge request style platform I'd probably see
> > > > this handled by tags/flags that mark that those open change sets
> > > > should be handled at the next bump (tagging lets one easily either
> > > > filter the view of change sets for just those, or excluding those).
> > > > Then if a bump is announced (let's say a month or two before it
> > > > happens, so that people can get their change sets ready for rebase),
> > > > reviews are started so that when a bump happens, it's swift and done
> > > > with.
> > > >
> > > > >
> > > > > * maybe we should use a special ABI version for these unstable period
> > > > > that way half of the problems disappear, its then clear when that period
> > > > > ends. And its not possible to install that under a stable ABI/API soname
> > > > >
> > > >
> > > > That's an interesting proposition. Just not sure what's the best way
> > > > for this? Negative or very large SONAME?
> > > >
> > > > > * maybe our download page should not just point to a master snapshot when
> > > > >   our ABI/API is in a transition period
> > > > >
> > > > > so, if you want to just remove the field, sure do it it probably will be
> > > > > fine but the proper way from ABI/API compatibility POV is to deprecate it
> > > > >
> > > >
> > > > So just to bring this set to the finish line, do I understand that for
> > > > you the field was not well enough flagged as private, and should thus
> > > > be treated as a public struct member?
> > > >
> > > > I would prefer removal as I've looked into the history of the field
> > > > and I saw Andreas working hard on removing the private entries from
> > > > this struct, but technically I can see that due to the Libav->FFmpeg
> > > > merge breaking the connection it could be argued that the field was
> > > > not flagged as private when added to FFmpeg. Which is why Andreas most
> > > > likely missed it from the struct in his clean-up commit.
> > > >
> > > > If you think it is public, then I will just go with this version of
> > > > the patch set, including deprecation. For me the main thing was to
> > > > highlight and make people think whether this struct member was meant
> > > > as public or private, and then decide how it should be interpreted in
> > > > current FFmpeg master. Poking James on IRC, he seems to be more
> > > > towards removal when looking at it in the context of how and where it
> > > > was added, but also first noted that deprecation would be the cleanest
> > > > way to remove it.
> > >
> > > why would one think its private ?
> > >
> > > The struct documentation says this:
> > > /**
> > >  * Bytestream IO Context.
> > >  * New public fields can be added with minor version bumps.
> > >  * Removal, reordering and changes to existing public fields require
> > >  * a major version bump.
> > >  * sizeof(AVIOContext) must not be used outside libav*.
> > >  *
> > >  * @note None of the function pointers in AVIOContext should be called
> > >  *       directly, they should only be set by the client application
> > >  *       when implementing custom I/O. Normally these are set to the
> > >  *       function pointers specified in avio_alloc_context()
> > >  */
> > >
> > >
> > > This speaks only of public fields
> > > it also excludes the function pointers, but nothing else is excluded
> > >
> > > and then theres
> > >
> > > int64_t written;
> > >
> > > why would this not be a public field ?
> > > what is the reasoning behind that?
> > >
> >
> > Because I thought the how it was meant to be and how close in context
> > it was to groups of private members would add some value when making
> > the decision of whether to put it into one group of struct members or
> > into another. Thus at the very least I thought it was worth bringing
> > it up and hearing opinions.
> >
> > Until very recently the region of the struct would be more like
> >
> >     /**
> >      * Internal, not meant to be used from outside of AVIOContext.
> >      */
> >     enum AVIODataMarkerType current_type;
> >     int64_t last_time;
> >
> >     /**
> >      * A callback that is used instead of short_seek_threshold.
> >      * This is current internal only, do not use from outside.
> >      */
> >     int (*short_seek_get)(void *opaque);
> >
> >     int64_t written;
> >
> >     /**
> >      * Maximum reached position before a backward seek in the write buffer,
> >      * used keeping track of already written data for a later flush.
> >      */
> >     unsigned char *buf_ptr_max;
> >
> >     /**
> >      * Try to buffer at least this amount of data before flushing it
> >      */
> >     int min_packet_size;
> >
> > If you strictly read only into what's in this context, it is an
> > undocumented member near some internal ones.
> >
> > But sure, it seems like I am not going to get a straight answer here,
> > for whatever reason. I will just take this as the implied "it is
> > private in my opinion". Which is fine. It was one of the alternatives
> > I noted, and a valid reading based on the lack of comments stuck right
> > next to the field.
>
> I meant, "it is public in my opinion" here, of course. Sorry. Kind of
> missed I had typed the wrong words while going through thoughts.

And here in shame I notice that this indeed was your initial response.
OK, sorry about that. Somehow I had gotten swept into this so I had
forgotten about it until I scrolled through the patchwork comment
thread again.

This is what I guess being tired does to one, trying to request an
answer to finalize a thread while that answer was already there in the
beginning.

Jan