diff mbox series

[FFmpeg-devel,05/17] lavu/opt: add a more general child class iteration API

Message ID 20200528201559.22618-5-anton@khirnov.net
State Accepted
Headers show
Series [FFmpeg-devel,01/17] Remove unnecessary use of avcodec_close(). | expand

Checks

Context Check Description
andriy/default pending
andriy/make_warn warning New warnings during build
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Anton Khirnov May 28, 2020, 8:15 p.m. UTC
Use opaque iteration state instead of the previous child class. This
mirrors similar changes done in lavf/lavc.

Deprecate the av_opt_child_class_next() API.
---
 libavutil/log.h     | 13 +++++++++++++
 libavutil/opt.c     | 24 ++++++++++++++++++++++--
 libavutil/opt.h     | 31 +++++++++++++++++++++++--------
 libavutil/version.h |  3 +++
 4 files changed, 61 insertions(+), 10 deletions(-)

Comments

Nicolas George May 29, 2020, 11:25 a.m. UTC | #1
Anton Khirnov (12020-05-28):
> Use opaque iteration state instead of the previous child class. This
> mirrors similar changes done in lavf/lavc.
> 
> Deprecate the av_opt_child_class_next() API.
> ---
>  libavutil/log.h     | 13 +++++++++++++
>  libavutil/opt.c     | 24 ++++++++++++++++++++++--
>  libavutil/opt.h     | 31 +++++++++++++++++++++++--------
>  libavutil/version.h |  3 +++
>  4 files changed, 61 insertions(+), 10 deletions(-)
> 
> diff --git a/libavutil/log.h b/libavutil/log.h
> index 9c14188a9c..4a7fdca8c2 100644
> --- a/libavutil/log.h
> +++ b/libavutil/log.h
> @@ -112,6 +112,7 @@ typedef struct AVClass {
>       */
>      void* (*child_next)(void *obj, void *prev);
>  
> +#if FF_API_CHILD_CLASS_NEXT
>      /**
>       * Return an AVClass corresponding to the next potential
>       * AVOptions-enabled child.
> @@ -120,7 +121,9 @@ typedef struct AVClass {
>       * child_next iterates over _already existing_ objects, while
>       * child_class_next iterates over _all possible_ children.
>       */
> +    attribute_deprecated
>      const struct AVClass* (*child_class_next)(const struct AVClass *prev);
> +#endif
>  
>      /**
>       * Category used for visualization (like color)
> @@ -140,6 +143,16 @@ typedef struct AVClass {
>       * available since version (52.12)
>       */
>      int (*query_ranges)(struct AVOptionRanges **, void *obj, const char *key, int flags);
> +

> +    /**
> +     * Return an AVClass corresponding to the next potential
> +     * AVOptions-enabled child.
> +     *
> +     * The difference between child_next and this is that
> +     * child_next iterates over _already existing_ objects, while
> +     * child_class_next iterates over _all possible_ children.
> +     */
> +    const struct AVClass* (*child_class_iterate)(void **iter);

Since "iter" is no longer named "prev", its semantic is no longer
obvious, and the documentation need to be a little more explicit.

	Iterate the AVClass corresponding to the potential
	AVOptions-enabled children.

	iter points to an opaque pointer that can be updated to keep
	track of the current child. On the first call, this pointer is
	NULL. 

>  } AVClass;
>  
>  /**
> diff --git a/libavutil/opt.c b/libavutil/opt.c
> index 423313bce2..2c3f998d97 100644
> --- a/libavutil/opt.c
> +++ b/libavutil/opt.c
> @@ -1679,8 +1679,9 @@ const AVOption *av_opt_find2(void *obj, const char *name, const char *unit,
>  
>      if (search_flags & AV_OPT_SEARCH_CHILDREN) {
>          if (search_flags & AV_OPT_SEARCH_FAKE_OBJ) {
> -            const AVClass *child = NULL;
> -            while (child = av_opt_child_class_next(c, child))
> +            void *iter = NULL;
> +            const AVClass *child;
> +            while (child = av_opt_child_class_iterate(c, &iter))
>                  if (o = av_opt_find2(&child, name, unit, opt_flags, search_flags, NULL))
>                      return o;
>          } else {
> @@ -1715,12 +1716,31 @@ void *av_opt_child_next(void *obj, void *prev)
>      return NULL;
>  }
>  
> +#if FF_API_CHILD_CLASS_NEXT
> +FF_DISABLE_DEPRECATION_WARNINGS
>  const AVClass *av_opt_child_class_next(const AVClass *parent, const AVClass *prev)
>  {
>      if (parent->child_class_next)
>          return parent->child_class_next(prev);
>      return NULL;
>  }
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
> +
> +const AVClass *av_opt_child_class_iterate(const AVClass *parent, void **iter)
> +{
> +    if (parent->child_class_iterate)
> +        return parent->child_class_iterate(iter);
> +#if FF_API_CHILD_CLASS_NEXT
> +FF_DISABLE_DEPRECATION_WARNINGS
> +    if (parent->child_class_next) {
> +        *iter = parent->child_class_next(*iter);
> +        return *iter;
> +    }
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
> +    return NULL;
> +}
>  
>  void *av_opt_ptr(const AVClass *class, void *obj, const char *name)
>  {
> diff --git a/libavutil/opt.h b/libavutil/opt.h
> index e46119572a..8dc020a820 100644
> --- a/libavutil/opt.h
> +++ b/libavutil/opt.h
> @@ -114,7 +114,7 @@
>   *      libavcodec exports generic options, while its priv_data field exports
>   *      codec-specific options). In such a case, it is possible to set up the
>   *      parent struct to export a child's options. To do that, simply
> - *      implement AVClass.child_next() and AVClass.child_class_next() in the
> + *      implement AVClass.child_next() and AVClass.child_class_iterate() in the
>   *      parent struct's AVClass.
>   *      Assuming that the test_struct from above now also contains a
>   *      child_struct field:
> @@ -143,23 +143,25 @@
>   *              return t->child_struct;
>   *          return NULL
>   *      }
> - *      const AVClass child_class_next(const AVClass *prev)
> + *      const AVClass child_class_iterate(void **iter)
>   *      {
> - *          return prev ? NULL : &child_class;
> + *          const AVClass *c = *iter ? NULL : &child_class;
> + *          *iter = (void*)(uintptr_t)c;
> + *          return c;
>   *      }
>   *      @endcode
> - *      Putting child_next() and child_class_next() as defined above into
> + *      Putting child_next() and child_class_iterate() as defined above into
>   *      test_class will now make child_struct's options accessible through
>   *      test_struct (again, proper setup as described above needs to be done on
>   *      child_struct right after it is created).
>   *
>   *      From the above example it might not be clear why both child_next()
> - *      and child_class_next() are needed. The distinction is that child_next()
> - *      iterates over actually existing objects, while child_class_next()
> + *      and child_class_iterate() are needed. The distinction is that child_next()
> + *      iterates over actually existing objects, while child_class_iterate()
>   *      iterates over all possible child classes. E.g. if an AVCodecContext
>   *      was initialized to use a codec which has private options, then its
>   *      child_next() will return AVCodecContext.priv_data and finish
> - *      iterating. OTOH child_class_next() on AVCodecContext.av_class will
> + *      iterating. OTOH child_class_iterate() on AVCodecContext.av_class will
>   *      iterate over all available codecs with private options.
>   *
>   * @subsection avoptions_implement_named_constants Named constants
> @@ -194,7 +196,7 @@
>   * For enumerating there are basically two cases. The first is when you want to
>   * get all options that may potentially exist on the struct and its children
>   * (e.g.  when constructing documentation). In that case you should call
> - * av_opt_child_class_next() recursively on the parent struct's AVClass.  The
> + * av_opt_child_class_iterate() recursively on the parent struct's AVClass.  The
>   * second case is when you have an already initialized struct with all its
>   * children and you want to get all options that can be actually written or read
>   * from it. In that case you should call av_opt_child_next() recursively (and
> @@ -646,13 +648,26 @@ const AVOption *av_opt_next(const void *obj, const AVOption *prev);
>   */
>  void *av_opt_child_next(void *obj, void *prev);
>  
> +#if FF_API_CHILD_CLASS_NEXT
>  /**
>   * Iterate over potential AVOptions-enabled children of parent.
>   *
>   * @param prev result of a previous call to this function or NULL
>   * @return AVClass corresponding to next potential child or NULL
> + *
> + * @deprecated use av_opt_child_class_iterate
>   */
> +attribute_deprecated
>  const AVClass *av_opt_child_class_next(const AVClass *parent, const AVClass *prev);
> +#endif
> +
> +/**
> + * Iterate over potential AVOptions-enabled children of parent.
> + *
> + * @param iter a pointer where iteration state is stored.
> + * @return AVClass corresponding to next potential child or NULL
> + */
> +const AVClass *av_opt_child_class_iterate(const AVClass *parent, void **iter);
>  
>  /**
>   * @defgroup opt_set_funcs Option setting functions
> diff --git a/libavutil/version.h b/libavutil/version.h
> index f8cd0b5f8a..b805455d06 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -132,6 +132,9 @@
>  #ifndef FF_API_DECLARE_ALIGNED
>  #define FF_API_DECLARE_ALIGNED          (LIBAVUTIL_VERSION_MAJOR < 57)
>  #endif
> +#ifndef FF_API_CHILD_CLASS_NEXT
> +#define FF_API_CHILD_CLASS_NEXT         (LIBAVUTIL_VERSION_MAJOR < 57)
> +#endif
>  
>  /**
>   * @}

Regards,
diff mbox series

Patch

diff --git a/libavutil/log.h b/libavutil/log.h
index 9c14188a9c..4a7fdca8c2 100644
--- a/libavutil/log.h
+++ b/libavutil/log.h
@@ -112,6 +112,7 @@  typedef struct AVClass {
      */
     void* (*child_next)(void *obj, void *prev);
 
+#if FF_API_CHILD_CLASS_NEXT
     /**
      * Return an AVClass corresponding to the next potential
      * AVOptions-enabled child.
@@ -120,7 +121,9 @@  typedef struct AVClass {
      * child_next iterates over _already existing_ objects, while
      * child_class_next iterates over _all possible_ children.
      */
+    attribute_deprecated
     const struct AVClass* (*child_class_next)(const struct AVClass *prev);
+#endif
 
     /**
      * Category used for visualization (like color)
@@ -140,6 +143,16 @@  typedef struct AVClass {
      * available since version (52.12)
      */
     int (*query_ranges)(struct AVOptionRanges **, void *obj, const char *key, int flags);
+
+    /**
+     * Return an AVClass corresponding to the next potential
+     * AVOptions-enabled child.
+     *
+     * The difference between child_next and this is that
+     * child_next iterates over _already existing_ objects, while
+     * child_class_next iterates over _all possible_ children.
+     */
+    const struct AVClass* (*child_class_iterate)(void **iter);
 } AVClass;
 
 /**
diff --git a/libavutil/opt.c b/libavutil/opt.c
index 423313bce2..2c3f998d97 100644
--- a/libavutil/opt.c
+++ b/libavutil/opt.c
@@ -1679,8 +1679,9 @@  const AVOption *av_opt_find2(void *obj, const char *name, const char *unit,
 
     if (search_flags & AV_OPT_SEARCH_CHILDREN) {
         if (search_flags & AV_OPT_SEARCH_FAKE_OBJ) {
-            const AVClass *child = NULL;
-            while (child = av_opt_child_class_next(c, child))
+            void *iter = NULL;
+            const AVClass *child;
+            while (child = av_opt_child_class_iterate(c, &iter))
                 if (o = av_opt_find2(&child, name, unit, opt_flags, search_flags, NULL))
                     return o;
         } else {
@@ -1715,12 +1716,31 @@  void *av_opt_child_next(void *obj, void *prev)
     return NULL;
 }
 
+#if FF_API_CHILD_CLASS_NEXT
+FF_DISABLE_DEPRECATION_WARNINGS
 const AVClass *av_opt_child_class_next(const AVClass *parent, const AVClass *prev)
 {
     if (parent->child_class_next)
         return parent->child_class_next(prev);
     return NULL;
 }
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+
+const AVClass *av_opt_child_class_iterate(const AVClass *parent, void **iter)
+{
+    if (parent->child_class_iterate)
+        return parent->child_class_iterate(iter);
+#if FF_API_CHILD_CLASS_NEXT
+FF_DISABLE_DEPRECATION_WARNINGS
+    if (parent->child_class_next) {
+        *iter = parent->child_class_next(*iter);
+        return *iter;
+    }
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+    return NULL;
+}
 
 void *av_opt_ptr(const AVClass *class, void *obj, const char *name)
 {
diff --git a/libavutil/opt.h b/libavutil/opt.h
index e46119572a..8dc020a820 100644
--- a/libavutil/opt.h
+++ b/libavutil/opt.h
@@ -114,7 +114,7 @@ 
  *      libavcodec exports generic options, while its priv_data field exports
  *      codec-specific options). In such a case, it is possible to set up the
  *      parent struct to export a child's options. To do that, simply
- *      implement AVClass.child_next() and AVClass.child_class_next() in the
+ *      implement AVClass.child_next() and AVClass.child_class_iterate() in the
  *      parent struct's AVClass.
  *      Assuming that the test_struct from above now also contains a
  *      child_struct field:
@@ -143,23 +143,25 @@ 
  *              return t->child_struct;
  *          return NULL
  *      }
- *      const AVClass child_class_next(const AVClass *prev)
+ *      const AVClass child_class_iterate(void **iter)
  *      {
- *          return prev ? NULL : &child_class;
+ *          const AVClass *c = *iter ? NULL : &child_class;
+ *          *iter = (void*)(uintptr_t)c;
+ *          return c;
  *      }
  *      @endcode
- *      Putting child_next() and child_class_next() as defined above into
+ *      Putting child_next() and child_class_iterate() as defined above into
  *      test_class will now make child_struct's options accessible through
  *      test_struct (again, proper setup as described above needs to be done on
  *      child_struct right after it is created).
  *
  *      From the above example it might not be clear why both child_next()
- *      and child_class_next() are needed. The distinction is that child_next()
- *      iterates over actually existing objects, while child_class_next()
+ *      and child_class_iterate() are needed. The distinction is that child_next()
+ *      iterates over actually existing objects, while child_class_iterate()
  *      iterates over all possible child classes. E.g. if an AVCodecContext
  *      was initialized to use a codec which has private options, then its
  *      child_next() will return AVCodecContext.priv_data and finish
- *      iterating. OTOH child_class_next() on AVCodecContext.av_class will
+ *      iterating. OTOH child_class_iterate() on AVCodecContext.av_class will
  *      iterate over all available codecs with private options.
  *
  * @subsection avoptions_implement_named_constants Named constants
@@ -194,7 +196,7 @@ 
  * For enumerating there are basically two cases. The first is when you want to
  * get all options that may potentially exist on the struct and its children
  * (e.g.  when constructing documentation). In that case you should call
- * av_opt_child_class_next() recursively on the parent struct's AVClass.  The
+ * av_opt_child_class_iterate() recursively on the parent struct's AVClass.  The
  * second case is when you have an already initialized struct with all its
  * children and you want to get all options that can be actually written or read
  * from it. In that case you should call av_opt_child_next() recursively (and
@@ -646,13 +648,26 @@  const AVOption *av_opt_next(const void *obj, const AVOption *prev);
  */
 void *av_opt_child_next(void *obj, void *prev);
 
+#if FF_API_CHILD_CLASS_NEXT
 /**
  * Iterate over potential AVOptions-enabled children of parent.
  *
  * @param prev result of a previous call to this function or NULL
  * @return AVClass corresponding to next potential child or NULL
+ *
+ * @deprecated use av_opt_child_class_iterate
  */
+attribute_deprecated
 const AVClass *av_opt_child_class_next(const AVClass *parent, const AVClass *prev);
+#endif
+
+/**
+ * Iterate over potential AVOptions-enabled children of parent.
+ *
+ * @param iter a pointer where iteration state is stored.
+ * @return AVClass corresponding to next potential child or NULL
+ */
+const AVClass *av_opt_child_class_iterate(const AVClass *parent, void **iter);
 
 /**
  * @defgroup opt_set_funcs Option setting functions
diff --git a/libavutil/version.h b/libavutil/version.h
index f8cd0b5f8a..b805455d06 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -132,6 +132,9 @@ 
 #ifndef FF_API_DECLARE_ALIGNED
 #define FF_API_DECLARE_ALIGNED          (LIBAVUTIL_VERSION_MAJOR < 57)
 #endif
+#ifndef FF_API_CHILD_CLASS_NEXT
+#define FF_API_CHILD_CLASS_NEXT         (LIBAVUTIL_VERSION_MAJOR < 57)
+#endif
 
 /**
  * @}