[FFmpeg-devel] [PATCH v3 1/3] doc: Explain what "context" means

[Stefano Sabatini]

On date Wednesday 2024-05-22 13:47:36 +0100, Andrew Sayers wrote:
> On Wed, May 22, 2024 at 12:37:37PM +0200, Stefano Sabatini wrote:
> > On date Sunday 2024-05-05 22:04:36 +0100, Andrew Sayers wrote:
> > > I'm still travelling, so the following thoughts might be a bit
> > > half-formed.  But I wanted to get some feedback before sitting down
> > > for a proper think.
> > [...]
> > > > > I've also gone through the code looking for edge cases we haven't covered.
> > > > > Here are some questions trying to prompt an "oh yeah I forgot to mention
> > > > > that"-type answer.  Anything where the answer is more like "that should
> > > > > probably be rewritten to be clearer", let me know and I'll avoid confusing
> > > > > newbies with it.
> > > > > 
> > > > 
> > > > > av_ambient_viewing_environment_create_side_data() takes an AVFrame as its
> > > > > first argument, and returns a new AVAmbientViewingEnvironment.  What is the
> > > > > context object for that function - AVFrame or AVAmbientViewingEnvironment?
> > > > 
> > > > But this should be clear from the doxy:
> > > > 
> > > > /**
> > > >  * Allocate and add an AVAmbientViewingEnvironment structure to an existing
> > > >  * AVFrame as side data.
> > > >  *
> > > >  * @return the newly allocated struct, or NULL on failure
> > > >  */
> > > > AVAmbientViewingEnvironment *av_ambient_viewing_environment_create_side_data(AVFrame *frame);
> > > 
> > > I'm afraid it's not clear, at least to me.  I think you're saying the
> > > AVFrame is the context because a "create" function can't have a
> > > context any more than a C++ "new" can be called as a member.  But the
> > > function's prefix points to the other conclusion, and neither signal
> > > is clear enough on its own.
> > 
> > No, what I'm saying is that in some cases you don't need to think in
> > terms of contexts, in this case there is no context at all, the
> > function takes a frame and modify it, and returns the ambient
> > environment to be used by the following functions. This should be very
> > clear by reading the doxy. There is no rule dictating the first param
> > of each FFmpeg function should be a "context".
> 
> I'm still writing up a reply to your longer feedback, but on this topic...
> 

> This function is in the "av_ambient_viewing_environment" namespace, and returns
> an object that is clearly used as a context for other functions.  So saying
> "stop thinking about contexts" just leaves a negative space and a bad thing
> to fill it with (confusion in my case).

>av_ambient_viewing_environment_create_side_data() takes an AVFrame as its
>first argument, and returns a new AVAmbientViewingEnvironment.  What is the
>context object for that function - AVFrame or AVAmbientViewingEnvironment?

av_ambient_viewing_environment_ defines the namespace. In this case we
are not using the av_frame_ API since we want to have all the ambient
API defined in a single place.

Alternatively we might have extended the av_frame API (e.g. 
av_frame_create_ambient_viewing_environment_side_data()), but we
wanted to avoid to reference the ambient API in the minimal frame API,
so this approach makes perfect sense to me and I cannot see major
inconsistencies.

You can think about it in terms of a static or class constructor
function, where AVFrame is simply an argument of the contructor.

What is the approach that you would have expected in this case?
 
> I've found it useful to think about "receiving" vs. "producing" a context:
> 
> * avcodec_alloc_context3() produces a context, but does not receive one
> * sws_init_context() receives a context, but does not produce one
> * av_ambient_viewing_environment_create_side_data() receives one context,
>   and produces another
> 
> How about if the document mostly talks about functions as having contexts,
> then follows it up with something like:
> 
>     There are some edge cases where this doesn't work.  <examples>.
>     If you find contexts a useful metaphor in these cases, you might
>     prefer to think about them as "receiving" and "producing" contexts.
> 
> ... or something similar that acknowledges contexts are unnecessary here,
> but provides a solution for people that want to use them anyway.

I see, but I still see an excessive use of the context concept in
place of the simpler and more natural use of parameters (in this case
AVFrame is simply the input element), and this scheme is pretty common
in C APIs (note that ambient is a simple structure, no need to use
AVClass or options for this simple struct), so I see no need to tag
this as an edge case.