Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH 1/2] avutil/dict: Add av_dict_iterate
@ 2022-09-22  2:02 Marvin Scholz
  2022-09-22  2:02 ` [FFmpeg-devel] [PATCH 2/2] avutil/dict: Use av_dict_iterate in av_dict_get Marvin Scholz
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Marvin Scholz @ 2022-09-22  2:02 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Marvin Scholz

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    | 27 +++++++++++++++++++++++++++
 libavutil/version.h |  2 +-
 3 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/libavutil/dict.c b/libavutil/dict.c
index 14ad780a79..2f690a5b8e 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;
 }
 
+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..b42b3f07fd 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
@@ -101,6 +103,31 @@ 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.
+ *
+ * @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
+ *
+ * Typical usage:
+ * @code
+ * AVDictionaryEntry *e = NULL;
+ * while (e = av_dict_iterate(m, e)) {
+ *     // ...
+ * }
+ * @endcode
+ */
+av_warn_unused_result
+AVDictionaryEntry *av_dict_iterate(const AVDictionary *m,
+                                   const AVDictionaryEntry *prev);
+
 /**
  * Get number of entries in dictionary.
  *
diff --git a/libavutil/version.h b/libavutil/version.h
index 0585fa7b80..6fd07ed2a4 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -80,7 +80,7 @@
 
 #define LIBAVUTIL_VERSION_MAJOR  57
 #define LIBAVUTIL_VERSION_MINOR  36
-#define LIBAVUTIL_VERSION_MICRO 102
+#define LIBAVUTIL_VERSION_MICRO 103
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
                                                LIBAVUTIL_VERSION_MINOR, \
-- 
2.37.0 (Apple Git-136)

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [FFmpeg-devel] [PATCH 2/2] avutil/dict: Use av_dict_iterate in av_dict_get
  2022-09-22  2:02 [FFmpeg-devel] [PATCH 1/2] avutil/dict: Add av_dict_iterate Marvin Scholz
@ 2022-09-22  2:02 ` Marvin Scholz
  2022-09-22 11:33 ` [FFmpeg-devel] [PATCH 1/2] avutil/dict: Add av_dict_iterate Andreas Rheinhardt
  2022-09-24 14:36 ` [FFmpeg-devel] [PATCH v2 1/5] " Marvin Scholz
  2 siblings, 0 replies; 13+ messages in thread
From: Marvin Scholz @ 2022-09-22  2:02 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Marvin Scholz

---
 libavutil/dict.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/libavutil/dict.c b/libavutil/dict.c
index 2f690a5b8e..1a348d6710 100644
--- a/libavutil/dict.c
+++ b/libavutil/dict.c
@@ -60,18 +60,14 @@ AVDictionaryEntry *av_dict_iterate(const AVDictionary *m,
 AVDictionaryEntry *av_dict_get(const AVDictionary *m, const char *key,
                                const AVDictionaryEntry *prev, int flags)
 {
-    unsigned int i, j;
+    AVDictionaryEntry *entry = (AVDictionaryEntry *)prev;
+    unsigned int j;
 
-    if (!m || !key)
+    if (!key)
         return NULL;
 
-    if (prev)
-        i = prev - m->elems + 1;
-    else
-        i = 0;
-
-    for (; i < m->count; i++) {
-        const char *s = m->elems[i].key;
+    while (entry = av_dict_iterate(m, entry)) {
+        const char *s = entry->key;
         if (flags & AV_DICT_MATCH_CASE)
             for (j = 0; s[j] == key[j] && key[j]; j++)
                 ;
@@ -82,7 +78,7 @@ AVDictionaryEntry *av_dict_get(const AVDictionary *m, const char *key,
             continue;
         if (s[j] && !(flags & AV_DICT_IGNORE_SUFFIX))
             continue;
-        return &m->elems[i];
+        return entry;
     }
     return NULL;
 }
-- 
2.37.0 (Apple Git-136)

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avutil/dict: Add av_dict_iterate
  2022-09-22  2:02 [FFmpeg-devel] [PATCH 1/2] avutil/dict: Add av_dict_iterate Marvin Scholz
  2022-09-22  2:02 ` [FFmpeg-devel] [PATCH 2/2] avutil/dict: Use av_dict_iterate in av_dict_get Marvin Scholz
@ 2022-09-22 11:33 ` Andreas Rheinhardt
  2022-09-22 11:47   ` Marvin Scholz
  2022-09-24 14:36 ` [FFmpeg-devel] [PATCH v2 1/5] " Marvin Scholz
  2 siblings, 1 reply; 13+ messages in thread
From: Andreas Rheinhardt @ 2022-09-22 11:33 UTC (permalink / raw)
  To: ffmpeg-devel

Marvin Scholz:
> 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    | 27 +++++++++++++++++++++++++++
>  libavutil/version.h |  2 +-
>  3 files changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/libavutil/dict.c b/libavutil/dict.c
> index 14ad780a79..2f690a5b8e 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;
>  }
>  
> +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..b42b3f07fd 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
> @@ -101,6 +103,31 @@ 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.
> + *
> + * @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
> + *
> + * Typical usage:
> + * @code
> + * AVDictionaryEntry *e = NULL;
> + * while (e = av_dict_iterate(m, e)) {
> + *     // ...
> + * }
> + * @endcode
> + */
> +av_warn_unused_result
> +AVDictionaryEntry *av_dict_iterate(const AVDictionary *m,
> +                                   const AVDictionaryEntry *prev);

The user is not allowed to modify the returned AVDictionaryEntries, so
you should return a const AVDictionaryEntry here. And there is no reason
to use av_warn_unused_result at all; nothing bad happens if you ignore
the result (except that you called av_dict_iterate unnecessarily).

Furthermore, av_dict_set's documentation contains "Adding a new entry to
a dictionary invalidates all existing entries previously returned with
av_dict_get." This needs to be updated, too.

> +
>  /**
>   * Get number of entries in dictionary.
>   *
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 0585fa7b80..6fd07ed2a4 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -80,7 +80,7 @@
>  
>  #define LIBAVUTIL_VERSION_MAJOR  57
>  #define LIBAVUTIL_VERSION_MINOR  36
> -#define LIBAVUTIL_VERSION_MICRO 102
> +#define LIBAVUTIL_VERSION_MICRO 103

New API additions need a minor bump (and need to reset micro).

>  
>  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
>                                                 LIBAVUTIL_VERSION_MINOR, \

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avutil/dict: Add av_dict_iterate
  2022-09-22 11:33 ` [FFmpeg-devel] [PATCH 1/2] avutil/dict: Add av_dict_iterate Andreas Rheinhardt
@ 2022-09-22 11:47   ` Marvin Scholz
  2022-09-22 11:56     ` Andreas Rheinhardt
  0 siblings, 1 reply; 13+ messages in thread
From: Marvin Scholz @ 2022-09-22 11:47 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



On 22 Sep 2022, at 13:33, Andreas Rheinhardt wrote:

> Marvin Scholz:
>> 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    | 27 +++++++++++++++++++++++++++
>>  libavutil/version.h |  2 +-
>>  3 files changed, 47 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavutil/dict.c b/libavutil/dict.c
>> index 14ad780a79..2f690a5b8e 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;
>>  }
>>
>> +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..b42b3f07fd 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
>> @@ -101,6 +103,31 @@ 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.
>> + *
>> + * @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
>> + *
>> + * Typical usage:
>> + * @code
>> + * AVDictionaryEntry *e = NULL;
>> + * while (e = av_dict_iterate(m, e)) {
>> + *     // ...
>> + * }
>> + * @endcode
>> + */
>> +av_warn_unused_result
>> +AVDictionaryEntry *av_dict_iterate(const AVDictionary *m,
>> +                                   const AVDictionaryEntry *prev);
>
> The user is not allowed to modify the returned AVDictionaryEntries, so
> you should return a const AVDictionaryEntry here.

Ok.

Shouldn't _get return const as well then? If so, can that be changed or would
it break ABI?

> And there is no reason
> to use av_warn_unused_result at all; nothing bad happens if you ignore
> the result (except that you called av_dict_iterate unnecessarily).

Yeah nothing bad happens but it is probably still wrong use of the API
that seemed beneficial to highlight or is there any valid use-case
to call it without using the return? I could not think of any…

>
> Furthermore, av_dict_set's documentation contains "Adding a new entry to
> a dictionary invalidates all existing entries previously returned with
> av_dict_get." This needs to be updated, too.

Sure, I will update the docs more, I just wanted to have some general
feedback if this is acceptable addition before I invest more time to
polish it.

>
>> +
>>  /**
>>   * Get number of entries in dictionary.
>>   *
>> diff --git a/libavutil/version.h b/libavutil/version.h
>> index 0585fa7b80..6fd07ed2a4 100644
>> --- a/libavutil/version.h
>> +++ b/libavutil/version.h
>> @@ -80,7 +80,7 @@
>>
>>  #define LIBAVUTIL_VERSION_MAJOR  57
>>  #define LIBAVUTIL_VERSION_MINOR  36
>> -#define LIBAVUTIL_VERSION_MICRO 102
>> +#define LIBAVUTIL_VERSION_MICRO 103
>
> New API additions need a minor bump (and need to reset micro).

Oh indeed, sorry, will fix that in the next version.

>
>>
>>  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
>>                                                 LIBAVUTIL_VERSION_MINOR, \
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avutil/dict: Add av_dict_iterate
  2022-09-22 11:47   ` Marvin Scholz
@ 2022-09-22 11:56     ` Andreas Rheinhardt
  0 siblings, 0 replies; 13+ messages in thread
From: Andreas Rheinhardt @ 2022-09-22 11:56 UTC (permalink / raw)
  To: ffmpeg-devel

Marvin Scholz:
> 
> 
> On 22 Sep 2022, at 13:33, Andreas Rheinhardt wrote:
> 
>> Marvin Scholz:
>>> 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    | 27 +++++++++++++++++++++++++++
>>>  libavutil/version.h |  2 +-
>>>  3 files changed, 47 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavutil/dict.c b/libavutil/dict.c
>>> index 14ad780a79..2f690a5b8e 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;
>>>  }
>>>
>>> +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..b42b3f07fd 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
>>> @@ -101,6 +103,31 @@ 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.
>>> + *
>>> + * @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
>>> + *
>>> + * Typical usage:
>>> + * @code
>>> + * AVDictionaryEntry *e = NULL;
>>> + * while (e = av_dict_iterate(m, e)) {
>>> + *     // ...
>>> + * }
>>> + * @endcode
>>> + */
>>> +av_warn_unused_result
>>> +AVDictionaryEntry *av_dict_iterate(const AVDictionary *m,
>>> +                                   const AVDictionaryEntry *prev);
>>
>> The user is not allowed to modify the returned AVDictionaryEntries, so
>> you should return a const AVDictionaryEntry here.
> 
> Ok.
> 
> Shouldn't _get return const as well then? If so, can that be changed or would
> it break ABI?
> 

It is an API break and therefore has to wait until a major bump.

>> And there is no reason
>> to use av_warn_unused_result at all; nothing bad happens if you ignore
>> the result (except that you called av_dict_iterate unnecessarily).
> 
> Yeah nothing bad happens but it is probably still wrong use of the API
> that seemed beneficial to highlight or is there any valid use-case
> to call it without using the return? I could not think of any…
> 

By that philosophy, basically all our API functions would need the
av_warn_unused_result attribute. I think this attribute should stand out
to alert the user that he is doing something with grave consequences if
he ignores the result.

>>
>> Furthermore, av_dict_set's documentation contains "Adding a new entry to
>> a dictionary invalidates all existing entries previously returned with
>> av_dict_get." This needs to be updated, too.
> 
> Sure, I will update the docs more, I just wanted to have some general
> feedback if this is acceptable addition before I invest more time to
> polish it.
> 
>>
>>> +
>>>  /**
>>>   * Get number of entries in dictionary.
>>>   *
>>> diff --git a/libavutil/version.h b/libavutil/version.h
>>> index 0585fa7b80..6fd07ed2a4 100644
>>> --- a/libavutil/version.h
>>> +++ b/libavutil/version.h
>>> @@ -80,7 +80,7 @@
>>>
>>>  #define LIBAVUTIL_VERSION_MAJOR  57
>>>  #define LIBAVUTIL_VERSION_MINOR  36
>>> -#define LIBAVUTIL_VERSION_MICRO 102
>>> +#define LIBAVUTIL_VERSION_MICRO 103
>>
>> New API additions need a minor bump (and need to reset micro).
> 
> Oh indeed, sorry, will fix that in the next version.
> 
>>
>>>
>>>  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
>>>                                                 LIBAVUTIL_VERSION_MINOR, \
>>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [FFmpeg-devel] [PATCH v2 1/5] avutil/dict: Add av_dict_iterate
  2022-09-22  2:02 [FFmpeg-devel] [PATCH 1/2] avutil/dict: Add av_dict_iterate Marvin Scholz
  2022-09-22  2:02 ` [FFmpeg-devel] [PATCH 2/2] avutil/dict: Use av_dict_iterate in av_dict_get Marvin Scholz
  2022-09-22 11:33 ` [FFmpeg-devel] [PATCH 1/2] avutil/dict: Add av_dict_iterate Andreas Rheinhardt
@ 2022-09-24 14:36 ` Marvin Scholz
  2022-09-24 14:36   ` [FFmpeg-devel] [PATCH v2 2/5] avutil/dict: Use av_dict_iterate in av_dict_get Marvin Scholz
                     ` (5 more replies)
  2 siblings, 6 replies; 13+ messages in thread
From: Marvin Scholz @ 2022-09-24 14:36 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Marvin Scholz

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)

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [FFmpeg-devel] [PATCH v2 2/5] avutil/dict: Use av_dict_iterate in av_dict_get
  2022-09-24 14:36 ` [FFmpeg-devel] [PATCH v2 1/5] " Marvin Scholz
@ 2022-09-24 14:36   ` Marvin Scholz
  2022-10-25  9:46     ` Anton Khirnov
  2022-09-24 14:36   ` [FFmpeg-devel] [PATCH v2 3/5] avutil/dict: Use av_dict_iterate in av_dict_copy Marvin Scholz
                     ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Marvin Scholz @ 2022-09-24 14:36 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Marvin Scholz

---
 libavutil/dict.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/libavutil/dict.c b/libavutil/dict.c
index ee059d712c..ea03590d0f 100644
--- a/libavutil/dict.c
+++ b/libavutil/dict.c
@@ -60,18 +60,14 @@ const AVDictionaryEntry *av_dict_iterate(const AVDictionary *m,
 AVDictionaryEntry *av_dict_get(const AVDictionary *m, const char *key,
                                const AVDictionaryEntry *prev, int flags)
 {
-    unsigned int i, j;
+   const AVDictionaryEntry *entry = prev;
+    unsigned int j;
 
-    if (!m || !key)
+    if (!key)
         return NULL;
 
-    if (prev)
-        i = prev - m->elems + 1;
-    else
-        i = 0;
-
-    for (; i < m->count; i++) {
-        const char *s = m->elems[i].key;
+    while ((entry = av_dict_iterate(m, entry))) {
+        const char *s = entry->key;
         if (flags & AV_DICT_MATCH_CASE)
             for (j = 0; s[j] == key[j] && key[j]; j++)
                 ;
@@ -82,7 +78,7 @@ AVDictionaryEntry *av_dict_get(const AVDictionary *m, const char *key,
             continue;
         if (s[j] && !(flags & AV_DICT_IGNORE_SUFFIX))
             continue;
-        return &m->elems[i];
+        return (AVDictionaryEntry *)entry;
     }
     return NULL;
 }
-- 
2.37.0 (Apple Git-136)

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [FFmpeg-devel] [PATCH v2 3/5] avutil/dict: Use av_dict_iterate in av_dict_copy
  2022-09-24 14:36 ` [FFmpeg-devel] [PATCH v2 1/5] " Marvin Scholz
  2022-09-24 14:36   ` [FFmpeg-devel] [PATCH v2 2/5] avutil/dict: Use av_dict_iterate in av_dict_get Marvin Scholz
@ 2022-09-24 14:36   ` Marvin Scholz
  2022-09-24 14:36   ` [FFmpeg-devel] [PATCH v2 4/5] avutil/dict: Use av_dict_iterate in av_dict_get_string Marvin Scholz
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Marvin Scholz @ 2022-09-24 14:36 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Marvin Scholz

---
 libavutil/dict.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavutil/dict.c b/libavutil/dict.c
index ea03590d0f..3c05ed4c84 100644
--- a/libavutil/dict.c
+++ b/libavutil/dict.c
@@ -236,9 +236,9 @@ void av_dict_free(AVDictionary **pm)
 
 int av_dict_copy(AVDictionary **dst, const AVDictionary *src, int flags)
 {
-    AVDictionaryEntry *t = NULL;
+    const AVDictionaryEntry *t = NULL;
 
-    while ((t = av_dict_get(src, "", t, AV_DICT_IGNORE_SUFFIX))) {
+    while ((t = av_dict_iterate(src, t))) {
         int ret = av_dict_set(dst, t->key, t->value, flags);
         if (ret < 0)
             return ret;
-- 
2.37.0 (Apple Git-136)

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [FFmpeg-devel] [PATCH v2 4/5] avutil/dict: Use av_dict_iterate in av_dict_get_string
  2022-09-24 14:36 ` [FFmpeg-devel] [PATCH v2 1/5] " Marvin Scholz
  2022-09-24 14:36   ` [FFmpeg-devel] [PATCH v2 2/5] avutil/dict: Use av_dict_iterate in av_dict_get Marvin Scholz
  2022-09-24 14:36   ` [FFmpeg-devel] [PATCH v2 3/5] avutil/dict: Use av_dict_iterate in av_dict_copy Marvin Scholz
@ 2022-09-24 14:36   ` Marvin Scholz
  2022-09-24 14:36   ` [FFmpeg-devel] [PATCH v2 5/5] avutil/dict: Improve documentation Marvin Scholz
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Marvin Scholz @ 2022-09-24 14:36 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Marvin Scholz

---
 libavutil/dict.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavutil/dict.c b/libavutil/dict.c
index 3c05ed4c84..122606e3aa 100644
--- a/libavutil/dict.c
+++ b/libavutil/dict.c
@@ -250,7 +250,7 @@ int av_dict_copy(AVDictionary **dst, const AVDictionary *src, int flags)
 int av_dict_get_string(const AVDictionary *m, char **buffer,
                        const char key_val_sep, const char pairs_sep)
 {
-    AVDictionaryEntry *t = NULL;
+    const AVDictionaryEntry *t = NULL;
     AVBPrint bprint;
     int cnt = 0;
     char special_chars[] = {pairs_sep, key_val_sep, '\0'};
@@ -265,7 +265,7 @@ int av_dict_get_string(const AVDictionary *m, char **buffer,
     }
 
     av_bprint_init(&bprint, 64, AV_BPRINT_SIZE_UNLIMITED);
-    while ((t = av_dict_get(m, "", t, AV_DICT_IGNORE_SUFFIX))) {
+    while ((t = av_dict_iterate(m, t))) {
         if (cnt++)
             av_bprint_append_data(&bprint, &pairs_sep, 1);
         av_bprint_escape(&bprint, t->key, special_chars, AV_ESCAPE_MODE_BACKSLASH, 0);
-- 
2.37.0 (Apple Git-136)

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [FFmpeg-devel] [PATCH v2 5/5] avutil/dict: Improve documentation
  2022-09-24 14:36 ` [FFmpeg-devel] [PATCH v2 1/5] " Marvin Scholz
                     ` (2 preceding siblings ...)
  2022-09-24 14:36   ` [FFmpeg-devel] [PATCH v2 4/5] avutil/dict: Use av_dict_iterate in av_dict_get_string Marvin Scholz
@ 2022-09-24 14:36   ` Marvin Scholz
  2022-10-11 13:23   ` [FFmpeg-devel] [PATCH v2 1/5] avutil/dict: Add av_dict_iterate Marvin Scholz
  2022-10-25  9:41   ` Anton Khirnov
  5 siblings, 0 replies; 13+ messages in thread
From: Marvin Scholz @ 2022-09-24 14:36 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Marvin Scholz

Mostly consistent formatting and consistently ordering of
warnings/notes to be next to the description.

Additionally group the AV_DICT_* macros.
---
 libavutil/dict.h | 91 +++++++++++++++++++++++++++++-------------------
 1 file changed, 55 insertions(+), 36 deletions(-)

diff --git a/libavutil/dict.h b/libavutil/dict.h
index 344afb452b..e4bc330a44 100644
--- a/libavutil/dict.h
+++ b/libavutil/dict.h
@@ -41,13 +41,15 @@
  * @brief Simple key:value store
  *
  * @{
- * Dictionaries are used for storing key:value pairs. To create
- * 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 and av_dict_iterate() to
- * iterate over all entries and finally av_dict_free() to free the
- * dictionary and all its contents.
+ * Dictionaries are used for storing key-value pairs.
+ * 
+ * - To **create 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.
+ * - To **insert an entry**, use av_dict_set().
+ * - Use av_dict_get() to **retrieve an entry**.
+ * - To **iterate over all entries**, use av_dict_iterate().
+ * - In order to **free the dictionary and all its contents**, use av_dict_free().
  *
  @code
    AVDictionary *d = NULL;           // "create" an empty dictionary
@@ -59,13 +61,18 @@
    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_iterate(d, t)) {
+   while ((t = av_dict_iterate(d, t))) {
        <....>                        // iterate over all entries in d
    }
    av_dict_free(&d);
  @endcode
  */
 
+/**
+ * @name AVDictionary Flags
+ * Flags that influence behavior of the matching of keys or insertion to the dictionary.
+ * @{
+ */
 #define AV_DICT_MATCH_CASE      1   /**< Only get an entry with exact-case key match. Only relevant in av_dict_get(). */
 #define AV_DICT_IGNORE_SUFFIX   2   /**< Return first entry in a dictionary whose first part corresponds to the search key,
                                          ignoring the suffix of the found key string. Only relevant in av_dict_get(). */
@@ -73,10 +80,13 @@
                                          allocated with av_malloc() or another memory allocation function. */
 #define AV_DICT_DONT_STRDUP_VAL 8   /**< Take ownership of a value that's been
                                          allocated with av_malloc() or another memory allocation function. */
-#define AV_DICT_DONT_OVERWRITE 16   ///< Don't overwrite existing entries.
+#define AV_DICT_DONT_OVERWRITE 16   /**< Don't overwrite existing entries. */
 #define AV_DICT_APPEND         32   /**< If the entry already exists, append to it.  Note that no
-                                      delimiter is added, the strings are simply concatenated. */
+                                         delimiter is added, the strings are simply concatenated. */
 #define AV_DICT_MULTIKEY       64   /**< Allow to store several equal keys in the dictionary */
+/**
+ * @}
+ */
 
 typedef struct AVDictionaryEntry {
     char *key;
@@ -91,11 +101,13 @@ typedef struct AVDictionary AVDictionary;
  * The returned entry key or value must not be changed, or it will
  * cause undefined behavior.
  *
- * @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
- * @param flags a collection of AV_DICT_* flags controlling how the entry is retrieved
- * @return found entry or NULL in case no matching entry was found in the dictionary
+ * @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
+ * @param flags A collection of AV_DICT_* flags controlling how the
+ *              entry is retrieved
+ *
+ * @return      Found entry or NULL in case no matching entry was found in the dictionary
  */
 AVDictionaryEntry *av_dict_get(const AVDictionary *m, const char *key,
                                const AVDictionaryEntry *prev, int flags);
@@ -144,20 +156,21 @@ int av_dict_count(const AVDictionary *m);
  * @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.
- * @param key entry key to add to *pm (will either be av_strduped or added as a new key depending on flags)
- * @param value entry value to add to *pm (will be av_strduped or added as a new key depending on flags).
- *        Passing a NULL value will cause an existing entry to be deleted.
- * @return >= 0 on success otherwise an error code <0
+ * @param pm        Pointer to a pointer to a dictionary struct. If *pm is NULL
+ *                  a dictionary struct is allocated and put in *pm.
+ * @param key       Entry key to add to *pm (will either be av_strduped or added as a new key depending on flags)
+ * @param value     Entry value to add to *pm (will be av_strduped or added as a new key depending on flags).
+ *                  Passing a NULL value will cause an existing entry to be deleted.
+ *
+ * @return          >= 0 on success otherwise an error code <0
  */
 int av_dict_set(AVDictionary **pm, const char *key, const char *value, int flags);
 
 /**
- * Convenience wrapper for av_dict_set that converts the value to a string
+ * Convenience wrapper for av_dict_set() that converts the value to a string
  * and stores it.
  *
- * Note: If AV_DICT_DONT_STRDUP_KEY is set, key will be freed on error.
+ * Note: If ::AV_DICT_DONT_STRDUP_KEY is set, key will be freed on error.
  */
 int av_dict_set_int(AVDictionary **pm, const char *key, int64_t value, int flags);
 
@@ -167,14 +180,15 @@ int av_dict_set_int(AVDictionary **pm, const char *key, int64_t value, int flags
  * In case of failure, all the successfully set entries are stored in
  * *pm. You may need to manually free the created dictionary.
  *
- * @param key_val_sep  a 0-terminated list of characters used to separate
+ * @param key_val_sep  A 0-terminated list of characters used to separate
  *                     key from value
- * @param pairs_sep    a 0-terminated list of characters used to separate
+ * @param pairs_sep    A 0-terminated list of characters used to separate
  *                     two pairs from each other
- * @param flags        flags to use when adding to dictionary.
- *                     AV_DICT_DONT_STRDUP_KEY and AV_DICT_DONT_STRDUP_VAL
+ * @param flags        Flags to use when adding to the dictionary.
+ *                     ::AV_DICT_DONT_STRDUP_KEY and ::AV_DICT_DONT_STRDUP_VAL
  *                     are ignored since the key/value tokens will always
  *                     be duplicated.
+ *
  * @return             0 on success, negative AVERROR code on failure
  */
 int av_dict_parse_string(AVDictionary **pm, const char *str,
@@ -183,11 +197,14 @@ int av_dict_parse_string(AVDictionary **pm, const char *str,
 
 /**
  * Copy entries from one AVDictionary struct into another.
- * @param dst pointer to a pointer to a AVDictionary struct. If *dst is NULL,
- *            this function will allocate a struct for you and put it in *dst
- * @param src pointer to source AVDictionary struct
- * @param flags flags to use when setting entries in *dst
- * @note metadata is read using the AV_DICT_IGNORE_SUFFIX flag
+ *
+ * @note Metadata is read using the ::AV_DICT_IGNORE_SUFFIX flag
+ * 
+ * @param dst   Pointer to a pointer to a AVDictionary struct to copy into. If *dst is NULL,
+ *              this function will allocate a struct for you and put it in *dst
+ * @param src   Pointer to the source AVDictionary struct to copy items from.
+ * @param flags Flags to use when setting entries in *dst
+ *
  * @return 0 on success, negative AVERROR code on failure. If dst was allocated
  *           by this function, callers should free the associated memory.
  */
@@ -206,13 +223,15 @@ void av_dict_free(AVDictionary **m);
  * Such string may be passed back to av_dict_parse_string().
  * @note String is escaped with backslashes ('\').
  *
- * @param[in]  m             dictionary
+ * @warning Separators cannot be neither '\\' nor '\0'. They also cannot be the same.
+ *
+ * @param[in]  m             The dictionary
  * @param[out] buffer        Pointer to buffer that will be allocated with string containg entries.
  *                           Buffer must be freed by the caller when is no longer needed.
- * @param[in]  key_val_sep   character used to separate key from value
- * @param[in]  pairs_sep     character used to separate two pairs from each other
+ * @param[in]  key_val_sep   Character used to separate key from value
+ * @param[in]  pairs_sep     Character used to separate two pairs from each other
+ *
  * @return                   >= 0 on success, negative on error
- * @warning Separators cannot be neither '\\' nor '\0'. They also cannot be the same.
  */
 int av_dict_get_string(const AVDictionary *m, char **buffer,
                        const char key_val_sep, const char pairs_sep);
-- 
2.37.0 (Apple Git-136)

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2 1/5] avutil/dict: Add av_dict_iterate
  2022-09-24 14:36 ` [FFmpeg-devel] [PATCH v2 1/5] " Marvin Scholz
                     ` (3 preceding siblings ...)
  2022-09-24 14:36   ` [FFmpeg-devel] [PATCH v2 5/5] avutil/dict: Improve documentation Marvin Scholz
@ 2022-10-11 13:23   ` Marvin Scholz
  2022-10-25  9:41   ` Anton Khirnov
  5 siblings, 0 replies; 13+ messages in thread
From: Marvin Scholz @ 2022-10-11 13:23 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Marvin Scholz

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.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2 1/5] avutil/dict: Add av_dict_iterate
  2022-09-24 14:36 ` [FFmpeg-devel] [PATCH v2 1/5] " Marvin Scholz
                     ` (4 preceding siblings ...)
  2022-10-11 13:23   ` [FFmpeg-devel] [PATCH v2 1/5] avutil/dict: Add av_dict_iterate Marvin Scholz
@ 2022-10-25  9:41   ` Anton Khirnov
  5 siblings, 0 replies; 13+ messages in thread
From: Anton Khirnov @ 2022-10-25  9:41 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Marvin Scholz

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.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [FFmpeg-devel] [PATCH v2 2/5] avutil/dict: Use av_dict_iterate in av_dict_get
  2022-09-24 14:36   ` [FFmpeg-devel] [PATCH v2 2/5] avutil/dict: Use av_dict_iterate in av_dict_get Marvin Scholz
@ 2022-10-25  9:46     ` Anton Khirnov
  0 siblings, 0 replies; 13+ messages in thread
From: Anton Khirnov @ 2022-10-25  9:46 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Marvin Scholz

Quoting Marvin Scholz (2022-09-24 16:36:56)
> ---
>  libavutil/dict.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/libavutil/dict.c b/libavutil/dict.c
> index ee059d712c..ea03590d0f 100644
> --- a/libavutil/dict.c
> +++ b/libavutil/dict.c
> @@ -60,18 +60,14 @@ const AVDictionaryEntry *av_dict_iterate(const AVDictionary *m,
>  AVDictionaryEntry *av_dict_get(const AVDictionary *m, const char *key,
>                                 const AVDictionaryEntry *prev, int flags)
>  {
> -    unsigned int i, j;
> +   const AVDictionaryEntry *entry = prev;

Broken indentation, otherwise looks good.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-10-25  9:46 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-22  2:02 [FFmpeg-devel] [PATCH 1/2] avutil/dict: Add av_dict_iterate Marvin Scholz
2022-09-22  2:02 ` [FFmpeg-devel] [PATCH 2/2] avutil/dict: Use av_dict_iterate in av_dict_get Marvin Scholz
2022-09-22 11:33 ` [FFmpeg-devel] [PATCH 1/2] avutil/dict: Add av_dict_iterate Andreas Rheinhardt
2022-09-22 11:47   ` Marvin Scholz
2022-09-22 11:56     ` Andreas Rheinhardt
2022-09-24 14:36 ` [FFmpeg-devel] [PATCH v2 1/5] " Marvin Scholz
2022-09-24 14:36   ` [FFmpeg-devel] [PATCH v2 2/5] avutil/dict: Use av_dict_iterate in av_dict_get Marvin Scholz
2022-10-25  9:46     ` Anton Khirnov
2022-09-24 14:36   ` [FFmpeg-devel] [PATCH v2 3/5] avutil/dict: Use av_dict_iterate in av_dict_copy Marvin Scholz
2022-09-24 14:36   ` [FFmpeg-devel] [PATCH v2 4/5] avutil/dict: Use av_dict_iterate in av_dict_get_string Marvin Scholz
2022-09-24 14:36   ` [FFmpeg-devel] [PATCH v2 5/5] avutil/dict: Improve documentation Marvin Scholz
2022-10-11 13:23   ` [FFmpeg-devel] [PATCH v2 1/5] avutil/dict: Add av_dict_iterate Marvin Scholz
2022-10-25  9:41   ` Anton Khirnov

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git