[FFmpeg-devel] [PATCH] lavu/opt: Mention that AVOptions is not reentrant

[Andrew Sayers]

On Thu, Jun 06, 2024 at 05:21:00PM +0200, Andreas Rheinhardt wrote:
> Andrew Sayers:
> > On Thu, Jun 06, 2024 at 04:24:11PM +0200, Michael Niedermayer wrote:
> >> On Thu, Jun 06, 2024 at 09:29:24AM +0100, Andrew Sayers wrote:
> >>> On Thu, Jun 06, 2024 at 01:17:48AM +0200, Michael Niedermayer wrote:
> >>> [...]
> >>>> AVOption simply provides light weight access to the struct fields.
> >>>> Calling AVOption non re-entrant in modifying a field you arent even allowed
> >>>> to modify from 2 threads is confusing
> >>>
> >>> I think you're saying there's already a rule about modifying AVOptions from
> >>> 2 threads.  Could you explain that in more detail?
> >>
> >> Well, one way this can be argued is this:
> >> Latest C draft: (but i doubt this is different) ISO/IEC 9899:2017   C17 ballot N2176
> >>
> >> "Two expression evaluations conflict if one of them modifies a memory location and the other one
> >>  reads or modifies the same memory location"
> >>
> >> so if you have 2 threads and one writes into a int and another reads it at the
> >> same time, the code is broken.
> >> The code doing said act through some API doesnt become less broken
> >>
> >> Calling AVOption non re-rentrant because of that is false thats as if one executed
> >> strtok_r(a,b,c) with the VERY same a,b,c from 2 threads and then said
> >> its not thread safe
> >>
> >> strtok_r() is thread safe and reentrant if its used correctly, so is AVOption
> > [...]
> > 
> > Ok, how about if the patch avoided the word "reentrant" and just said:
> > 
> > + * Note: AVOptions values should not be modified after a struct is initialized.
> 
> This is wrong either. As Paul has already pointed out to you, some
> options are allowed to be modified lateron.

Ah, I'd interpreted "runtime" to be the opposite of "compile-time", not
"initialization-time".  I'll propose a new patch that should be clearer.