diff mbox series

[FFmpeg-devel,v2,07/31] avutil: use av_dict_iterate

Message ID 20221126144648.73162-8-epirat07@gmail.com
State Accepted
Commit 81747b5259faa343aab1834e8949154d30d7994c
Headers show
Series Use av_dict_iterate where approproate | 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

Marvin Scholz Nov. 26, 2022, 2:46 p.m. UTC 
---
 libavutil/opt.c        | 12 ++++++------
 libavutil/tests/dict.c |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

Comments

James Almer Nov. 27, 2022, 3:06 p.m. UTC | #1 
On 11/26/2022 11:46 AM, Marvin Scholz wrote:
> ---
>   libavutil/opt.c        | 12 ++++++------
>   libavutil/tests/dict.c |  2 +-
>   2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/libavutil/opt.c b/libavutil/opt.c
> index a3940f47fb..0a909a8b22 100644
> --- a/libavutil/opt.c
> +++ b/libavutil/opt.c
> @@ -1742,14 +1742,14 @@ void av_opt_free(void *obj)
>   
>   int av_opt_set_dict2(void *obj, AVDictionary **options, int search_flags)
>   {
> -    AVDictionaryEntry *t = NULL;
> +    const AVDictionaryEntry *t = NULL;
>       AVDictionary    *tmp = NULL;
>       int ret;
>   
>       if (!options)
>           return 0;
>   
> -    while ((t = av_dict_get(*options, "", t, AV_DICT_IGNORE_SUFFIX))) {
> +    while ((t = av_dict_iterate(*options, t))) {
>           ret = av_opt_set(obj, t->key, t->value, search_flags);
>           if (ret == AVERROR_OPTION_NOT_FOUND)
>               ret = av_dict_set(&tmp, t->key, t->value, 0);
> @@ -2137,16 +2137,16 @@ FF_ENABLE_DEPRECATION_WARNINGS
>       case AV_OPT_TYPE_DICT: {
>           AVDictionary *dict1 = NULL;
>           AVDictionary *dict2 = *(AVDictionary **)dst;
> -        AVDictionaryEntry *en1 = NULL;
> -        AVDictionaryEntry *en2 = NULL;
> +        const AVDictionaryEntry *en1 = NULL;
> +        const AVDictionaryEntry *en2 = NULL;
>           ret = av_dict_parse_string(&dict1, o->default_val.str, "=", ":", 0);
>           if (ret < 0) {
>               av_dict_free(&dict1);
>               return ret;
>           }
>           do {
> -            en1 = av_dict_get(dict1, "", en1, AV_DICT_IGNORE_SUFFIX);
> -            en2 = av_dict_get(dict2, "", en2, AV_DICT_IGNORE_SUFFIX);
> +            en1 = av_dict_iterate(dict1, en1);
> +            en2 = av_dict_iterate(dict2, en2);
>           } while (en1 && en2 && !strcmp(en1->key, en2->key) && !strcmp(en1->value, en2->value));
>           av_dict_free(&dict1);
>           return (!en1 && !en2);
> diff --git a/libavutil/tests/dict.c b/libavutil/tests/dict.c
> index d053545f4d..91567289c2 100644
> --- a/libavutil/tests/dict.c
> +++ b/libavutil/tests/dict.c
> @@ -52,7 +52,7 @@ static void test_separators(const AVDictionary *m, const char pair, const char v
>   int main(void)
>   {
>       AVDictionary *dict = NULL;
> -    AVDictionaryEntry *e;
> +    const AVDictionaryEntry *e;

Spurious change? You're not touching the test otherwise.

In any case, feel free to add the following to this patch:

> diff --git a/libavutil/tests/dict.c b/libavutil/tests/dict.c
> index d053545f4d..468391a9f3 100644
> --- a/libavutil/tests/dict.c
> +++ b/libavutil/tests/dict.c
> @@ -52,7 +52,7 @@ static void test_separators(const AVDictionary *m, const char pair, const char v
>  int main(void)
>  {
>      AVDictionary *dict = NULL;
> -    AVDictionaryEntry *e;
> +    const AVDictionaryEntry *e;
>      char *buffer = NULL;
> 
>      printf("Testing av_dict_get_string() and av_dict_parse_string()\n");
> @@ -94,8 +94,13 @@ int main(void)
>      if (av_dict_get(dict, NULL, NULL, 0))
>          printf("av_dict_get() does not correctly handle NULL key.\n");
>      e = NULL;
> +    printf("Iterating with av_dict_get()\n");
>      while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
>          printf("%s %s\n", e->key, e->value);
> +    e = NULL;
> +    printf("Iterating with av_dict_iterate()\n");
> +    while ((e = av_dict_iterate(dict, e)))
> +        printf("%s %s\n", e->key, e->value);
>      av_dict_free(&dict);
> 
>      if (av_dict_set(&dict, NULL, "a", 0) >= 0 ||
> diff --git a/tests/ref/fate/dict b/tests/ref/fate/dict
> index 7205e4c845..a519b49163 100644
> --- a/tests/ref/fate/dict
> +++ b/tests/ref/fate/dict
> @@ -30,6 +30,14 @@ ret 0
>  aaa aaa   bbb bbb   ccc ccc   \,='" \,='"
> 
>  Testing av_dict_set()
> +Iterating with av_dict_get()
> +a a
> +b b
> +c c
> +d d
> +e e
> +ff ff
> +Iterating with av_dict_iterate()
>  a a
>  b b
>  c c

That should be enough to compare the output of both functions.
Marvin Scholz Nov. 30, 2022, 10:06 p.m. UTC | #2 
On 27 Nov 2022, at 16:06, James Almer wrote:

> On 11/26/2022 11:46 AM, Marvin Scholz wrote:
>> ---
>>   libavutil/opt.c        | 12 ++++++------
>>   libavutil/tests/dict.c |  2 +-
>>   2 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavutil/opt.c b/libavutil/opt.c
>> index a3940f47fb..0a909a8b22 100644
>> --- a/libavutil/opt.c
>> +++ b/libavutil/opt.c
>> @@ -1742,14 +1742,14 @@ void av_opt_free(void *obj)
>>    int av_opt_set_dict2(void *obj, AVDictionary **options, int search_flags)
>>   {
>> -    AVDictionaryEntry *t = NULL;
>> +    const AVDictionaryEntry *t = NULL;
>>       AVDictionary    *tmp = NULL;
>>       int ret;
>>        if (!options)
>>           return 0;
>>  -    while ((t = av_dict_get(*options, "", t, AV_DICT_IGNORE_SUFFIX))) {
>> +    while ((t = av_dict_iterate(*options, t))) {
>>           ret = av_opt_set(obj, t->key, t->value, search_flags);
>>           if (ret == AVERROR_OPTION_NOT_FOUND)
>>               ret = av_dict_set(&tmp, t->key, t->value, 0);
>> @@ -2137,16 +2137,16 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>       case AV_OPT_TYPE_DICT: {
>>           AVDictionary *dict1 = NULL;
>>           AVDictionary *dict2 = *(AVDictionary **)dst;
>> -        AVDictionaryEntry *en1 = NULL;
>> -        AVDictionaryEntry *en2 = NULL;
>> +        const AVDictionaryEntry *en1 = NULL;
>> +        const AVDictionaryEntry *en2 = NULL;
>>           ret = av_dict_parse_string(&dict1, o->default_val.str, "=", ":", 0);
>>           if (ret < 0) {
>>               av_dict_free(&dict1);
>>               return ret;
>>           }
>>           do {
>> -            en1 = av_dict_get(dict1, "", en1, AV_DICT_IGNORE_SUFFIX);
>> -            en2 = av_dict_get(dict2, "", en2, AV_DICT_IGNORE_SUFFIX);
>> +            en1 = av_dict_iterate(dict1, en1);
>> +            en2 = av_dict_iterate(dict2, en2);
>>           } while (en1 && en2 && !strcmp(en1->key, en2->key) && !strcmp(en1->value, en2->value));
>>           av_dict_free(&dict1);
>>           return (!en1 && !en2);
>> diff --git a/libavutil/tests/dict.c b/libavutil/tests/dict.c
>> index d053545f4d..91567289c2 100644
>> --- a/libavutil/tests/dict.c
>> +++ b/libavutil/tests/dict.c
>> @@ -52,7 +52,7 @@ static void test_separators(const AVDictionary *m, const char pair, const char v
>>   int main(void)
>>   {
>>       AVDictionary *dict = NULL;
>> -    AVDictionaryEntry *e;
>> +    const AVDictionaryEntry *e;
>
> Spurious change? You're not touching the test otherwise.

Oh indeed!.

>
> In any case, feel free to add the following to this patch:
>
>> diff --git a/libavutil/tests/dict.c b/libavutil/tests/dict.c
>> index d053545f4d..468391a9f3 100644
>> --- a/libavutil/tests/dict.c
>> +++ b/libavutil/tests/dict.c
>> @@ -52,7 +52,7 @@ static void test_separators(const AVDictionary *m, const char pair, const char v
>>  int main(void)
>>  {
>>      AVDictionary *dict = NULL;
>> -    AVDictionaryEntry *e;
>> +    const AVDictionaryEntry *e;
>>      char *buffer = NULL;
>>
>>      printf("Testing av_dict_get_string() and av_dict_parse_string()\n");
>> @@ -94,8 +94,13 @@ int main(void)
>>      if (av_dict_get(dict, NULL, NULL, 0))
>>          printf("av_dict_get() does not correctly handle NULL key.\n");
>>      e = NULL;
>> +    printf("Iterating with av_dict_get()\n");
>>      while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
>>          printf("%s %s\n", e->key, e->value);
>> +    e = NULL;
>> +    printf("Iterating with av_dict_iterate()\n");
>> +    while ((e = av_dict_iterate(dict, e)))
>> +        printf("%s %s\n", e->key, e->value);
>>      av_dict_free(&dict);
>>
>>      if (av_dict_set(&dict, NULL, "a", 0) >= 0 ||
>> diff --git a/tests/ref/fate/dict b/tests/ref/fate/dict
>> index 7205e4c845..a519b49163 100644
>> --- a/tests/ref/fate/dict
>> +++ b/tests/ref/fate/dict
>> @@ -30,6 +30,14 @@ ret 0
>>  aaa aaa   bbb bbb   ccc ccc   \,='" \,='"
>>
>>  Testing av_dict_set()
>> +Iterating with av_dict_get()
>> +a a
>> +b b
>> +c c
>> +d d
>> +e e
>> +ff ff
>> +Iterating with av_dict_iterate()
>>  a a
>>  b b
>>  c c
>
> That should be enough to compare the output of both functions.

I see, thanks. Do you want to apply this change to the patch
when merging saving an unnecessary round trip to the ML?

> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/libavutil/opt.c b/libavutil/opt.c
index a3940f47fb..0a909a8b22 100644
--- a/libavutil/opt.c
+++ b/libavutil/opt.c
@@ -1742,14 +1742,14 @@  void av_opt_free(void *obj)
 
 int av_opt_set_dict2(void *obj, AVDictionary **options, int search_flags)
 {
-    AVDictionaryEntry *t = NULL;
+    const AVDictionaryEntry *t = NULL;
     AVDictionary    *tmp = NULL;
     int ret;
 
     if (!options)
         return 0;
 
-    while ((t = av_dict_get(*options, "", t, AV_DICT_IGNORE_SUFFIX))) {
+    while ((t = av_dict_iterate(*options, t))) {
         ret = av_opt_set(obj, t->key, t->value, search_flags);
         if (ret == AVERROR_OPTION_NOT_FOUND)
             ret = av_dict_set(&tmp, t->key, t->value, 0);
@@ -2137,16 +2137,16 @@  FF_ENABLE_DEPRECATION_WARNINGS
     case AV_OPT_TYPE_DICT: {
         AVDictionary *dict1 = NULL;
         AVDictionary *dict2 = *(AVDictionary **)dst;
-        AVDictionaryEntry *en1 = NULL;
-        AVDictionaryEntry *en2 = NULL;
+        const AVDictionaryEntry *en1 = NULL;
+        const AVDictionaryEntry *en2 = NULL;
         ret = av_dict_parse_string(&dict1, o->default_val.str, "=", ":", 0);
         if (ret < 0) {
             av_dict_free(&dict1);
             return ret;
         }
         do {
-            en1 = av_dict_get(dict1, "", en1, AV_DICT_IGNORE_SUFFIX);
-            en2 = av_dict_get(dict2, "", en2, AV_DICT_IGNORE_SUFFIX);
+            en1 = av_dict_iterate(dict1, en1);
+            en2 = av_dict_iterate(dict2, en2);
         } while (en1 && en2 && !strcmp(en1->key, en2->key) && !strcmp(en1->value, en2->value));
         av_dict_free(&dict1);
         return (!en1 && !en2);
diff --git a/libavutil/tests/dict.c b/libavutil/tests/dict.c
index d053545f4d..91567289c2 100644
--- a/libavutil/tests/dict.c
+++ b/libavutil/tests/dict.c
@@ -52,7 +52,7 @@  static void test_separators(const AVDictionary *m, const char pair, const char v
 int main(void)
 {
     AVDictionary *dict = NULL;
-    AVDictionaryEntry *e;
+    const AVDictionaryEntry *e;
     char *buffer = NULL;
 
     printf("Testing av_dict_get_string() and av_dict_parse_string()\n");