[FFmpeg-devel] [PATCH 05/29] lavu/opt: distinguish between native and foreign access for AVOption fields

[Diederick C. Niehorster]

On Mon, Mar 4, 2024 at 11:39 PM Marton Balint <cus at passwd.hu> wrote:

> On Mon, 4 Mar 2024, Anton Khirnov wrote:
>
> > Native access is from the code that declared the options, foreign access
> > is from code that is using the options. Forbid foreign access to
> > AVOption.offset/default_val, for which there is no good reason, and
> > which should allow us more freedom in extending their semantics in a
> > compatible way.
> > ---
> > libavutil/opt.h | 14 ++++++++++++++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/libavutil/opt.h b/libavutil/opt.h
> > index e34b8506f8..e402f6a0a0 100644
> > --- a/libavutil/opt.h
> > +++ b/libavutil/opt.h
> > @@ -43,6 +43,16 @@
> >  * ("objects"). An option can have a help text, a type and a range of
> possible
> >  * values. Options may then be enumerated, read and written to.
> >  *
> > + * There are two modes of access to members of AVOption and its child
> structs.
> > + * One is called 'native access', and refers to access from the code
> that
> > + * declares the AVOption in question.  The other is 'foreign access',
> and refers
> > + * to access from other code.
> > + *
> > + * Certain struct members in this header are documented as 'native
> access only'
> > + * or similar - it means that only the code that declared the AVOption
> in
> > + * question is allowed to access the field. This allows us to extend the
> > + * semantics of those fields without breaking API compatibility.
> > + *
>
> Changing private/public status of existing fields retrospecitvely can be
> considered an API break.
>
> >      */
> > @@ -308,6 +320,8 @@ typedef struct AVOption {
> >     enum AVOptionType type;
> >
> >     /**
> > +     * Native access only.
> > +     *
> >      * the default value for scalar options
> >      */
>
> One could argue that it will be more difficult to get the default value of
> an option (you'd have to create an object, call av_opt_set_defaults() and
> finally do av_opt_get), but what I find more problematic is the
> inconsistency. You are not allowed to access default_val, unless it is an
> array type, in which case you might access it to get array settings, but -
> oh well - not the default value.
>

There is no helper function for getting the default value of an option. If
you disallow reading this field directly (as in one of your other patches),
please add such a helper function(s), since library users need it. It
should also work without instantiating the object, but directly on the
class definition