diff mbox series

[FFmpeg-devel,v6,2/4] lavu: Clarify relationship between AVClass, AVOption and context

Message ID 20240604144919.213799-3-ffmpeg-devel@pileofstuff.org
State New
Headers show
Series doc: Explain what "context" means | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Andrew Sayers June 4, 2024, 2:47 p.m. UTC
---
 libavutil/log.h | 16 +++++++++++++---
 libavutil/opt.h | 26 +++++++++++++++++++++-----
 2 files changed, 34 insertions(+), 8 deletions(-)

Comments

Stefano Sabatini June 5, 2024, 10:34 a.m. UTC | #1
On date Tuesday 2024-06-04 15:47:22 +0100, Andrew Sayers wrote:
> ---
>  libavutil/log.h | 16 +++++++++++++---
>  libavutil/opt.h | 26 +++++++++++++++++++++-----
>  2 files changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/libavutil/log.h b/libavutil/log.h
> index ab7ceabe22..88b35897c6 100644
> --- a/libavutil/log.h
> +++ b/libavutil/log.h
> @@ -59,9 +59,19 @@ typedef enum {
>  struct AVOptionRanges;
>  
>  /**
> - * Describe the class of an AVClass context structure. That is an
> - * arbitrary struct of which the first field is a pointer to an
> - * AVClass struct (e.g. AVCodecContext, AVFormatContext etc.).

> + * Generic Logging facilities and configuration options

Generic logging and options handling facilities.

> + *
> + * Logging and AVOptions functions expect to be passed structs

Logging and options handling functions ...

> + * whose first member is a pointer-to-@ref AVClass.
> + *

> + * Structs that only use logging facilities are often referred to as
> + * "AVClass context structures", while those that provide configuration
> + * options are called "AVOptions-enabled structs".

A struct with an AVClass as its first member can be used for both
logging and options management, there is no difference between the
two. My take:
|Structs that use AVClass might be referred to as AVClass
|contexts/structures, those that in addition define options might be
|called AVOptions contexts/structures.

> + *
> + * @see
> + * * @ref lavu_log
> + * * @ref avoptions
> + * * @ref Context
>   */
>  typedef struct AVClass {
>      /**
> diff --git a/libavutil/opt.h b/libavutil/opt.h
> index 07e27a9208..cdee8f7d28 100644
> --- a/libavutil/opt.h
> +++ b/libavutil/opt.h
> @@ -39,9 +39,16 @@
>   * @defgroup avoptions AVOptions
>   * @ingroup lavu_data
>   * @{
> - * AVOptions provide a generic system to declare options on arbitrary structs
> - * ("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.
> + *
> + * Inspection and configuration for AVClass context structures
> + *
> + * Provides a generic API to declare and manage options on any struct
> + * whose first member is a pointer-to-@ref AVClass.  Structs with private
> + * contexts can use that AVClass to return further @ref AVClass "AVClass"es
> + * that allow access to options in the private structs.
> + *
> + * Each option can have a help text, a type and a range of possible values.
> + * Options may 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
> @@ -53,11 +60,20 @@
>   * question is allowed to access the field. This allows us to extend the
>   * semantics of those fields without breaking API compatibility.
>   *

> + * Note that AVOptions is not reentrant, and that many FFmpeg functions access

... AVOptions access is not reeentrant ...

> + * options from separate threads.  Unless otherwise indicated, it is best to
> + * avoid modifying options once a struct has been initialized.

But this note is not relevant to the change, and should probably be
discussed separately

[...]
Andrew Sayers June 5, 2024, 12:46 p.m. UTC | #2
On Wed, Jun 05, 2024 at 12:34:48PM +0200, Stefano Sabatini wrote:
> On date Tuesday 2024-06-04 15:47:22 +0100, Andrew Sayers wrote:
<snip - language choices we can talk about in future>
> > + * Structs that only use logging facilities are often referred to as
> > + * "AVClass context structures", while those that provide configuration
> > + * options are called "AVOptions-enabled structs".
> 
> A struct with an AVClass as its first member can be used for both
> logging and options management, there is no difference between the
> two. My take:
> |Structs that use AVClass might be referred to as AVClass
> |contexts/structures, those that in addition define options might be
> |called AVOptions contexts/structures.

I think you were away when this came up, and anyway this thread is getting
quite unwieldy.  See [1] and [2] for the full version, but in short, defining
AVClass by layout leads to conclusions that are at best unintuitive.

<snip - AVOptions text>
> > + * Note that AVOptions is not reentrant, and that many FFmpeg functions access
> 
> ... AVOptions access is not reeentrant ...
> 
> > + * options from separate threads.  Unless otherwise indicated, it is best to
> > + * avoid modifying options once a struct has been initialized.
> 
> But this note is not relevant to the change, and should probably be
> discussed separately

Short version: I'll make a separate patch now, let's come back to this after

Long version...

If you assume options can be set at any time, they broadly resemble a reflection
mechanism.  If you assume they can only be set during the configuration stage,
they broadly resemble an OOP constructor.  The document needs to address the one
they resemble (even if just to say "this is why they're different"), and needs
to steer clear of any possible comparison with the one they don't resemble.
So it would be too risky to bump this to the upcoming omnibus patchset,
but it's fine to apply this *before* the context document.

[1] https://ffmpeg.org/pipermail/ffmpeg-devel/2024-May/328058.html
[2] https://ffmpeg.org/pipermail/ffmpeg-devel/2024-May/328087.html
diff mbox series

Patch

diff --git a/libavutil/log.h b/libavutil/log.h
index ab7ceabe22..88b35897c6 100644
--- a/libavutil/log.h
+++ b/libavutil/log.h
@@ -59,9 +59,19 @@  typedef enum {
 struct AVOptionRanges;
 
 /**
- * Describe the class of an AVClass context structure. That is an
- * arbitrary struct of which the first field is a pointer to an
- * AVClass struct (e.g. AVCodecContext, AVFormatContext etc.).
+ * Generic Logging facilities and configuration options
+ *
+ * Logging and AVOptions functions expect to be passed structs
+ * whose first member is a pointer-to-@ref AVClass.
+ *
+ * Structs that only use logging facilities are often referred to as
+ * "AVClass context structures", while those that provide configuration
+ * options are called "AVOptions-enabled structs".
+ *
+ * @see
+ * * @ref lavu_log
+ * * @ref avoptions
+ * * @ref Context
  */
 typedef struct AVClass {
     /**
diff --git a/libavutil/opt.h b/libavutil/opt.h
index 07e27a9208..cdee8f7d28 100644
--- a/libavutil/opt.h
+++ b/libavutil/opt.h
@@ -39,9 +39,16 @@ 
  * @defgroup avoptions AVOptions
  * @ingroup lavu_data
  * @{
- * AVOptions provide a generic system to declare options on arbitrary structs
- * ("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.
+ *
+ * Inspection and configuration for AVClass context structures
+ *
+ * Provides a generic API to declare and manage options on any struct
+ * whose first member is a pointer-to-@ref AVClass.  Structs with private
+ * contexts can use that AVClass to return further @ref AVClass "AVClass"es
+ * that allow access to options in the private structs.
+ *
+ * Each option can have a help text, a type and a range of possible values.
+ * Options may 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
@@ -53,11 +60,20 @@ 
  * question is allowed to access the field. This allows us to extend the
  * semantics of those fields without breaking API compatibility.
  *
+ * Note that AVOptions is not reentrant, and that many FFmpeg functions access
+ * options from separate threads.  Unless otherwise indicated, it is best to
+ * avoid modifying options once a struct has been initialized.
+ *
+ * @see
+ * * @ref lavu_log
+ * * @ref Context
+ *
  * @section avoptions_scope Scope of AVOptions
  *
  * AVOptions is designed to support any set of multimedia configuration options
- * that can be defined at compile-time.  Although it is mainly used to expose
- * FFmpeg options, you are welcome to adapt it to your own use case.
+ * that can be defined at compile-time and set at object creation time.  Although
+ * it is mainly used to expose FFmpeg options, you are welcome to adapt it
+ * to your own use case.
  *
  * No single approach can ever fully solve the problem of configuration,
  * but please submit a patch if you believe you have found a problem