diff mbox series

[FFmpeg-devel,v2,1/5] avutil/dict: Add av_dict_iterate

Message ID 20220924143659.74756-1-epirat07@gmail.com
State Accepted
Headers show
Series [FFmpeg-devel,v2,1/5] avutil/dict: Add av_dict_iterate | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Marvin Scholz Sept. 24, 2022, 2:36 p.m. UTC
This is a more explicit iteration API rather than using the "magic"
av_dict_get(d, "", t, AV_DICT_IGNORE_SUFFIX) which is not really
trivial to grasp what it does when casually reading through code.
---
 libavutil/dict.c    | 19 +++++++++++++++++++
 libavutil/dict.h    | 46 +++++++++++++++++++++++++++++++++++----------
 libavutil/version.h |  4 ++--
 3 files changed, 57 insertions(+), 12 deletions(-)

Comments

Marvin Scholz Oct. 11, 2022, 1:23 p.m. UTC | #1
On 24 Sep 2022, at 16:36, Marvin Scholz wrote:

> This is a more explicit iteration API rather than using the "magic"
> av_dict_get(d, "", t, AV_DICT_IGNORE_SUFFIX) which is not really
> trivial to grasp what it does when casually reading through code.
> ---
>  libavutil/dict.c    | 19 +++++++++++++++++++
>  libavutil/dict.h    | 46 +++++++++++++++++++++++++++++++++++----------
>  libavutil/version.h |  4 ++--
>  3 files changed, 57 insertions(+), 12 deletions(-)
>
> diff --git a/libavutil/dict.c b/libavutil/dict.c
> index 14ad780a79..ee059d712c 100644
> --- a/libavutil/dict.c
> +++ b/libavutil/dict.c
> @@ -20,6 +20,7 @@
>
>  #include <string.h>
>
> +#include "avassert.h"
>  #include "avstring.h"
>  #include "dict.h"
>  #include "dict_internal.h"
> @@ -38,6 +39,24 @@ int av_dict_count(const AVDictionary *m)
>      return m ? m->count : 0;
>  }
>
> +const AVDictionaryEntry *av_dict_iterate(const AVDictionary *m,
> +                                         const AVDictionaryEntry *prev)
> +{
> +    int i = 0;
> +
> +    if (!m)
> +        return NULL;
> +
> +    if (prev)
> +        i = prev - m->elems + 1;
> +
> +    av_assert2(i >= 0);
> +    if (i >= m->count)
> +        return NULL;
> +
> +    return &m->elems[i];
> +}
> +
>  AVDictionaryEntry *av_dict_get(const AVDictionary *m, const char *key,
>                                 const AVDictionaryEntry *prev, int flags)
>  {
> diff --git a/libavutil/dict.h b/libavutil/dict.h
> index 0d1afc6c64..344afb452b 100644
> --- a/libavutil/dict.h
> +++ b/libavutil/dict.h
> @@ -32,6 +32,8 @@
>
>  #include <stdint.h>
>
> +#include "attributes.h"
> +
>  /**
>   * @addtogroup lavu_dict AVDictionary
>   * @ingroup lavu_data
> @@ -43,9 +45,9 @@
>   * an AVDictionary, simply pass an address of a NULL pointer to
>   * av_dict_set(). NULL can be used as an empty dictionary wherever
>   * a pointer to an AVDictionary is required.
> - * Use av_dict_get() to retrieve an entry or iterate over all
> - * entries and finally av_dict_free() to free the dictionary
> - * and all its contents.
> + * Use av_dict_get() to retrieve an entry and av_dict_iterate() to
> + * iterate over all entries and finally av_dict_free() to free the
> + * dictionary and all its contents.
>   *
>   @code
>     AVDictionary *d = NULL;           // "create" an empty dictionary
> @@ -57,8 +59,8 @@
>     char *v = av_strdup("value");     // you can avoid copying them like this
>     av_dict_set(&d, k, v, AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL);
>
> -   while (t = av_dict_get(d, "", t, AV_DICT_IGNORE_SUFFIX)) {
> -       <....>                             // iterate over all entries in d
> +   while (t = av_dict_iterate(d, t)) {
> +       <....>                        // iterate over all entries in d
>     }
>     av_dict_free(&d);
>   @endcode
> @@ -89,9 +91,6 @@ typedef struct AVDictionary AVDictionary;
>   * The returned entry key or value must not be changed, or it will
>   * cause undefined behavior.
>   *
> - * To iterate through all the dictionary entries, you can set the matching key
> - * to the null string "" and set the AV_DICT_IGNORE_SUFFIX flag.
> - *
>   * @param prev Set to the previous matching element to find the next.
>   *             If set to NULL the first matching element is returned.
>   * @param key matching key
> @@ -101,6 +100,33 @@ typedef struct AVDictionary AVDictionary;
>  AVDictionaryEntry *av_dict_get(const AVDictionary *m, const char *key,
>                                 const AVDictionaryEntry *prev, int flags);
>
> +/**
> + * Iterate over a dictionary
> + *
> + * Iterates through all entries in the dictionary.
> + *
> + * @warning The returned AVDictionaryEntry key/value must not be changed.
> + *
> + * @warning As av_dict_set() invalidates all previous entries returned
> + * by this function, it must not be called while iterating over the dict.
> + *
> + * Typical usage:
> + * @code
> + * const AVDictionaryEntry *e = NULL;
> + * while ((e = av_dict_iterate(m, e))) {
> + *     // ...
> + * }
> + * @endcode
> + *
> + * @param m     The dictionary to iterate over
> + * @param prev  Pointer to the previous AVDictionaryEntry, NULL initially
> + *
> + * @retval AVDictionaryEntry* The next element in the dictionary
> + * @retval NULL               No more elements in the dictionary
> + */
> +const AVDictionaryEntry *av_dict_iterate(const AVDictionary *m,
> +                                         const AVDictionaryEntry *prev);
> +
>  /**
>   * Get number of entries in dictionary.
>   *
> @@ -115,8 +141,8 @@ int av_dict_count(const AVDictionary *m);
>   * Note: If AV_DICT_DONT_STRDUP_KEY or AV_DICT_DONT_STRDUP_VAL is set,
>   * these arguments will be freed on error.
>   *
> - * Warning: Adding a new entry to a dictionary invalidates all existing entries
> - * previously returned with av_dict_get.
> + * @warning Adding a new entry to a dictionary invalidates all existing entries
> + * previously returned with av_dict_get() or av_dict_iterate().
>   *
>   * @param pm pointer to a pointer to a dictionary struct. If *pm is NULL
>   * a dictionary struct is allocated and put in *pm.
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 0585fa7b80..9c44cef6aa 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,8 +79,8 @@
>   */
>
>  #define LIBAVUTIL_VERSION_MAJOR  57
> -#define LIBAVUTIL_VERSION_MINOR  36
> -#define LIBAVUTIL_VERSION_MICRO 102
> +#define LIBAVUTIL_VERSION_MINOR  37
> +#define LIBAVUTIL_VERSION_MICRO 100
>
>  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
>                                                 LIBAVUTIL_VERSION_MINOR, \
> -- 
> 2.37.0 (Apple Git-136)


Ping for review/merge, please.
Anton Khirnov Oct. 25, 2022, 9:41 a.m. UTC | #2
Quoting Marvin Scholz (2022-09-24 16:36:55)
> diff --git a/libavutil/dict.h b/libavutil/dict.h
> index 0d1afc6c64..344afb452b 100644
> --- a/libavutil/dict.h
> +++ b/libavutil/dict.h
> @@ -32,6 +32,8 @@
>  
>  #include <stdint.h>
>  
> +#include "attributes.h"

This seems unnecessary now.

Will push with this include removed, if noone objects.
diff mbox series

Patch

diff --git a/libavutil/dict.c b/libavutil/dict.c
index 14ad780a79..ee059d712c 100644
--- a/libavutil/dict.c
+++ b/libavutil/dict.c
@@ -20,6 +20,7 @@ 
 
 #include <string.h>
 
+#include "avassert.h"
 #include "avstring.h"
 #include "dict.h"
 #include "dict_internal.h"
@@ -38,6 +39,24 @@  int av_dict_count(const AVDictionary *m)
     return m ? m->count : 0;
 }
 
+const AVDictionaryEntry *av_dict_iterate(const AVDictionary *m,
+                                         const AVDictionaryEntry *prev)
+{
+    int i = 0;
+
+    if (!m)
+        return NULL;
+
+    if (prev)
+        i = prev - m->elems + 1;
+
+    av_assert2(i >= 0);
+    if (i >= m->count)
+        return NULL;
+
+    return &m->elems[i];
+}
+
 AVDictionaryEntry *av_dict_get(const AVDictionary *m, const char *key,
                                const AVDictionaryEntry *prev, int flags)
 {
diff --git a/libavutil/dict.h b/libavutil/dict.h
index 0d1afc6c64..344afb452b 100644
--- a/libavutil/dict.h
+++ b/libavutil/dict.h
@@ -32,6 +32,8 @@ 
 
 #include <stdint.h>
 
+#include "attributes.h"
+
 /**
  * @addtogroup lavu_dict AVDictionary
  * @ingroup lavu_data
@@ -43,9 +45,9 @@ 
  * an AVDictionary, simply pass an address of a NULL pointer to
  * av_dict_set(). NULL can be used as an empty dictionary wherever
  * a pointer to an AVDictionary is required.
- * Use av_dict_get() to retrieve an entry or iterate over all
- * entries and finally av_dict_free() to free the dictionary
- * and all its contents.
+ * Use av_dict_get() to retrieve an entry and av_dict_iterate() to
+ * iterate over all entries and finally av_dict_free() to free the
+ * dictionary and all its contents.
  *
  @code
    AVDictionary *d = NULL;           // "create" an empty dictionary
@@ -57,8 +59,8 @@ 
    char *v = av_strdup("value");     // you can avoid copying them like this
    av_dict_set(&d, k, v, AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL);
 
-   while (t = av_dict_get(d, "", t, AV_DICT_IGNORE_SUFFIX)) {
-       <....>                             // iterate over all entries in d
+   while (t = av_dict_iterate(d, t)) {
+       <....>                        // iterate over all entries in d
    }
    av_dict_free(&d);
  @endcode
@@ -89,9 +91,6 @@  typedef struct AVDictionary AVDictionary;
  * The returned entry key or value must not be changed, or it will
  * cause undefined behavior.
  *
- * To iterate through all the dictionary entries, you can set the matching key
- * to the null string "" and set the AV_DICT_IGNORE_SUFFIX flag.
- *
  * @param prev Set to the previous matching element to find the next.
  *             If set to NULL the first matching element is returned.
  * @param key matching key
@@ -101,6 +100,33 @@  typedef struct AVDictionary AVDictionary;
 AVDictionaryEntry *av_dict_get(const AVDictionary *m, const char *key,
                                const AVDictionaryEntry *prev, int flags);
 
+/**
+ * Iterate over a dictionary
+ *
+ * Iterates through all entries in the dictionary.
+ * 
+ * @warning The returned AVDictionaryEntry key/value must not be changed.
+ * 
+ * @warning As av_dict_set() invalidates all previous entries returned
+ * by this function, it must not be called while iterating over the dict.
+ *
+ * Typical usage:
+ * @code
+ * const AVDictionaryEntry *e = NULL;
+ * while ((e = av_dict_iterate(m, e))) {
+ *     // ...
+ * }
+ * @endcode
+ * 
+ * @param m     The dictionary to iterate over
+ * @param prev  Pointer to the previous AVDictionaryEntry, NULL initially
+ * 
+ * @retval AVDictionaryEntry* The next element in the dictionary
+ * @retval NULL               No more elements in the dictionary
+ */
+const AVDictionaryEntry *av_dict_iterate(const AVDictionary *m,
+                                         const AVDictionaryEntry *prev);
+
 /**
  * Get number of entries in dictionary.
  *
@@ -115,8 +141,8 @@  int av_dict_count(const AVDictionary *m);
  * Note: If AV_DICT_DONT_STRDUP_KEY or AV_DICT_DONT_STRDUP_VAL is set,
  * these arguments will be freed on error.
  *
- * Warning: Adding a new entry to a dictionary invalidates all existing entries
- * previously returned with av_dict_get.
+ * @warning Adding a new entry to a dictionary invalidates all existing entries
+ * previously returned with av_dict_get() or av_dict_iterate().
  *
  * @param pm pointer to a pointer to a dictionary struct. If *pm is NULL
  * a dictionary struct is allocated and put in *pm.
diff --git a/libavutil/version.h b/libavutil/version.h
index 0585fa7b80..9c44cef6aa 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,8 +79,8 @@ 
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  57
-#define LIBAVUTIL_VERSION_MINOR  36
-#define LIBAVUTIL_VERSION_MICRO 102
+#define LIBAVUTIL_VERSION_MINOR  37
+#define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
                                                LIBAVUTIL_VERSION_MINOR, \