[FFmpeg-devel] [PATCH v4 4/4] lavf: Add documentation for private "Context" classes

[Andrew Sayers]

On Wed, May 22, 2024 at 12:08:29PM +0200, Stefano Sabatini wrote:
> On date Wednesday 2024-05-15 16:54:22 +0100, Andrew Sayers wrote:
> > Doxygen thinks any text like "Context for foo" is a link to a struct called "Context".
> > Add a description and a better link, to avoid confusing readers.
> > ---
> >  libavformat/async.c | 3 +++
> >  libavformat/cache.c | 3 +++
> >  2 files changed, 6 insertions(+)
> > 
> > diff --git a/libavformat/async.c b/libavformat/async.c
> > index e096b0bc6f..3c28d418ae 100644
> > --- a/libavformat/async.c
> > +++ b/libavformat/async.c
> > @@ -53,6 +53,9 @@ typedef struct RingBuffer
> >      int           read_pos;
> >  } RingBuffer;
> >  
> > +/**
> > + * @brief @ref md_doc_2context "Context" for testing async
> > + */
> >  typedef struct Context {
> >      AVClass        *class;
> >      URLContext     *inner;
> > diff --git a/libavformat/cache.c b/libavformat/cache.c
> > index 5f78adba9d..3cc0edec82 100644
> > --- a/libavformat/cache.c
> > +++ b/libavformat/cache.c
> > @@ -52,6 +52,9 @@ typedef struct CacheEntry {
> >      int size;
> >  } CacheEntry;
> >  
> > +/**
> > + * @brief @ref md_doc_2context "Context" for a cache
> > + */
> >  typedef struct Context {
> >      AVClass *class;
> >      int fd;
> 
> Not sure, these are private structs and we enforce no use of internal
> markup for those, and we can assume internals developers are already
> acquainted with contexts and such so this is probably adding no value.

Ah, yeah the use case isn't obvious if you haven't tripped over it...

Imagine you're a new user trying to learn FFmpeg, and you find yourself at
https://ffmpeg.org/doxygen/trunk/structAVAudioFifo.html - the first word
appears to be a link for this "context" thing you've been hearing about[1],
so you drop what you're doing to investigate.  It links you to a struct that
looks promisingly general, but eventually turns out to be some random internal
struct with a misleading name.  Now you've wasted a bunch of time and forgotten
what you were doing.

The other patch fixes the examples I've found in the code, but doxygen links
all instances of the word "Context" to this struct, confusing newbies.  This
patch ensures that when future developers say "Context" in a comment, the page
doxygen links them to will point them in the right direction.

I had previously assumed it was a deliberate decision to include private files
in the documentation, so I didn't look that carefully into workarounds that
would break links to these private structures.  But you've implied elsewhere
that's not the case, so is it worth looking into solutions that made "Context"
link to the context document, at the cost of making it impossible to link to
these private structs at all?

[1] to be clear, this is the current behaviour of the page on the live site