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

Fri Oct 22 18:15:49 EEST 2021

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 at 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 at gmail.com> wrote:
> > > >
> > > > On Thu, Oct 21, 2021 at 11:49 PM Michael Niedermayer
> > > > <michael at 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 at 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 at 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

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

Never trust a computer, one day, it may think you are the virus. -- Compn
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20211022/4e01bc1a/attachment.sig>