[FFmpeg-devel] [PATCH v0 09/14] avcodec: add private side data set to AVCodecInternal

[Anton Khirnov]

Quoting Jan Ekström (2023-03-27 08:37:07)
> On Sun, Mar 26, 2023 at 10:00 PM Anton Khirnov <anton at khirnov.net> wrote:
> > Quoting Jan Ekström (2023-03-24 18:34:28)
> > > The general idea was that if you want to make people utilize helpers
> > > and not touch entries willy-nilly,
> >
> > But do we? Why?
> >
> > In this case I see no advantage to having a public function over having
> > two public fields. The function is strictly worse because it cannot be
> > extended, and enlarges the symbol table.
> 
> The overarching idea has been that looking at the recent changes of
> adding private internal structs and deprecating previously public
> stuff from the public structs was to be careful.

The fields being removed from public structs are actually private,
that's the only reason they are being hidden.

The thing you are adding is not private, it is public API. Having a
setter for it rather than making it a field in a public struct is merely
a different way of making it public, which
* is inconsistent with other similar APIs
* has the disadvantages I mentioned in the previous email

> 1. Post a patch with things added to the relevant private struct.
> 2. Allow specific access with a helper
> 3. See if anyone requires the information to be part of the public
> struct. At which point it may be moved there.
> 
> Additionally, only having things available via helpers means that you
> know how exactly the fields were being poked (instead of "we don't
> know how API users utilize them, and thus we cannot do X" (see the
> avcodec context related helpers where we still have to support close &
> av_freep which popped up in another patch), and given that the
> argument is a struct, the function can technically be extended
> (although a struct being in the middle of a struct might stop from it
> being extended).

There is no constructor for the struct, so it cannot be extended without
a major bump.

-- 
Anton Khirnov