diff mbox series

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

Message ID 20221125013046.40904-8-epirat07@gmail.com
State Accepted
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. 25, 2022, 1:30 a.m. UTC 
---
 libavutil/opt.c        | 12 ++++++------
 libavutil/tests/dict.c | 10 +++++-----
 2 files changed, 11 insertions(+), 11 deletions(-)

Comments

Andreas Rheinhardt Nov. 25, 2022, 12:50 p.m. UTC | #1 
Marvin Scholz:
> ---
>  libavutil/opt.c        | 12 ++++++------
>  libavutil/tests/dict.c | 10 +++++-----
>  2 files changed, 11 insertions(+), 11 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..8c05752ea7 100644
> --- a/libavutil/tests/dict.c
> +++ b/libavutil/tests/dict.c
> @@ -22,8 +22,8 @@
>  
>  static void print_dict(const AVDictionary *m)
>  {
> -    AVDictionaryEntry *t = NULL;
> -    while ((t = av_dict_get(m, "", t, AV_DICT_IGNORE_SUFFIX)))
> +    const AVDictionaryEntry *t = NULL;
> +    while ((t = av_dict_iterate(m, t)))
>          printf("%s %s   ", t->key, t->value);
>      printf("\n");
>  }
> @@ -94,7 +94,7 @@ int main(void)
>      if (av_dict_get(dict, NULL, NULL, 0))
>          printf("av_dict_get() does not correctly handle NULL key.\n");
>      e = NULL;
> -    while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
> +    while ((e = av_dict_iterate(dict, e)))
>          printf("%s %s\n", e->key, e->value);
>      av_dict_free(&dict);
>  
> @@ -106,7 +106,7 @@ int main(void)
>          printf("av_dict_set does not correctly handle NULL key\n");
>  
>      e = NULL;
> -    while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
> +    while ((e = av_dict_iterate(dict, e)))
>          printf("'%s' '%s'\n", e->key, e->value);
>      av_dict_free(&dict);
>  
> @@ -122,7 +122,7 @@ int main(void)
>      av_dict_set_int(&dict, "12", 1, 0);
>      av_dict_set_int(&dict, "12", 2, AV_DICT_APPEND);
>      e = NULL;
> -    while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
> +    while ((e = av_dict_iterate(dict, e)))
>          printf("%s %s\n", e->key, e->value);
>      av_dict_free(&dict);
>  

This stops testing the old iterating pattern; instead it should
explicitly test that both patterns coincide.

- Andreas
Marvin Scholz Nov. 26, 2022, 2:42 p.m. UTC | #2 
On 25 Nov 2022, at 13:50, Andreas Rheinhardt wrote:

> Marvin Scholz:
>> ---
>>  libavutil/opt.c        | 12 ++++++------
>>  libavutil/tests/dict.c | 10 +++++-----
>>  2 files changed, 11 insertions(+), 11 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..8c05752ea7 100644
>> --- a/libavutil/tests/dict.c
>> +++ b/libavutil/tests/dict.c
>> @@ -22,8 +22,8 @@
>>
>>  static void print_dict(const AVDictionary *m)
>>  {
>> -    AVDictionaryEntry *t = NULL;
>> -    while ((t = av_dict_get(m, "", t, AV_DICT_IGNORE_SUFFIX)))
>> +    const AVDictionaryEntry *t = NULL;
>> +    while ((t = av_dict_iterate(m, t)))
>>          printf("%s %s   ", t->key, t->value);
>>      printf("\n");
>>  }
>> @@ -94,7 +94,7 @@ int main(void)
>>      if (av_dict_get(dict, NULL, NULL, 0))
>>          printf("av_dict_get() does not correctly handle NULL key.\n");
>>      e = NULL;
>> -    while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
>> +    while ((e = av_dict_iterate(dict, e)))
>>          printf("%s %s\n", e->key, e->value);
>>      av_dict_free(&dict);
>>
>> @@ -106,7 +106,7 @@ int main(void)
>>          printf("av_dict_set does not correctly handle NULL key\n");
>>
>>      e = NULL;
>> -    while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
>> +    while ((e = av_dict_iterate(dict, e)))
>>          printf("'%s' '%s'\n", e->key, e->value);
>>      av_dict_free(&dict);
>>
>> @@ -122,7 +122,7 @@ int main(void)
>>      av_dict_set_int(&dict, "12", 1, 0);
>>      av_dict_set_int(&dict, "12", 2, AV_DICT_APPEND);
>>      e = NULL;
>> -    while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
>> +    while ((e = av_dict_iterate(dict, e)))
>>          printf("%s %s\n", e->key, e->value);
>>      av_dict_free(&dict);
>>
>
> This stops testing the old iterating pattern; instead it should
> explicitly test that both patterns coincide.
>

Ok, I've removed this change for now from the pachset
as adding tests is not really what this patchset is about.

Additionally I do not fully understand how the testing works yet
so I don't know how I could write such a test.

> - Andreas
>
> _______________________________________________
> 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".
James Almer Nov. 27, 2022, 1:59 p.m. UTC | #3 
On 11/26/2022 11:42 AM, Marvin Scholz wrote:
> 
> 
> On 25 Nov 2022, at 13:50, Andreas Rheinhardt wrote:
> 
>> Marvin Scholz:
>>> ---
>>>   libavutil/opt.c        | 12 ++++++------
>>>   libavutil/tests/dict.c | 10 +++++-----
>>>   2 files changed, 11 insertions(+), 11 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..8c05752ea7 100644
>>> --- a/libavutil/tests/dict.c
>>> +++ b/libavutil/tests/dict.c
>>> @@ -22,8 +22,8 @@
>>>
>>>   static void print_dict(const AVDictionary *m)
>>>   {
>>> -    AVDictionaryEntry *t = NULL;
>>> -    while ((t = av_dict_get(m, "", t, AV_DICT_IGNORE_SUFFIX)))
>>> +    const AVDictionaryEntry *t = NULL;
>>> +    while ((t = av_dict_iterate(m, t)))
>>>           printf("%s %s   ", t->key, t->value);
>>>       printf("\n");
>>>   }
>>> @@ -94,7 +94,7 @@ int main(void)
>>>       if (av_dict_get(dict, NULL, NULL, 0))
>>>           printf("av_dict_get() does not correctly handle NULL key.\n");
>>>       e = NULL;
>>> -    while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
>>> +    while ((e = av_dict_iterate(dict, e)))
>>>           printf("%s %s\n", e->key, e->value);
>>>       av_dict_free(&dict);
>>>
>>> @@ -106,7 +106,7 @@ int main(void)
>>>           printf("av_dict_set does not correctly handle NULL key\n");
>>>
>>>       e = NULL;
>>> -    while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
>>> +    while ((e = av_dict_iterate(dict, e)))
>>>           printf("'%s' '%s'\n", e->key, e->value);
>>>       av_dict_free(&dict);
>>>
>>> @@ -122,7 +122,7 @@ int main(void)
>>>       av_dict_set_int(&dict, "12", 1, 0);
>>>       av_dict_set_int(&dict, "12", 2, AV_DICT_APPEND);
>>>       e = NULL;
>>> -    while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
>>> +    while ((e = av_dict_iterate(dict, e)))
>>>           printf("%s %s\n", e->key, e->value);
>>>       av_dict_free(&dict);
>>>
>>
>> This stops testing the old iterating pattern; instead it should
>> explicitly test that both patterns coincide.
>>
> 
> Ok, I've removed this change for now from the pachset
> as adding tests is not really what this patchset is about.
> 
> Additionally I do not fully understand how the testing works yet
> so I don't know how I could write such a test.

This specific test just prints some hardcoded lines plus the output of 
av_dict_get() to stderr, and compares it with the reference in 
tests/ref/fate/dict.

What could be done is to either print both the output of the 
av_dict_get() and av_dict_iterate() iterations, or to compare the 
returned AVDictionaryEntry on each iteration and if they differ, print a 
line about it.
I can send a patch for that later.

> 
>> - Andreas
>>
>> _______________________________________________
>> 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".
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..8c05752ea7 100644
--- a/libavutil/tests/dict.c
+++ b/libavutil/tests/dict.c
@@ -22,8 +22,8 @@ 
 
 static void print_dict(const AVDictionary *m)
 {
-    AVDictionaryEntry *t = NULL;
-    while ((t = av_dict_get(m, "", t, AV_DICT_IGNORE_SUFFIX)))
+    const AVDictionaryEntry *t = NULL;
+    while ((t = av_dict_iterate(m, t)))
         printf("%s %s   ", t->key, t->value);
     printf("\n");
 }
@@ -94,7 +94,7 @@  int main(void)
     if (av_dict_get(dict, NULL, NULL, 0))
         printf("av_dict_get() does not correctly handle NULL key.\n");
     e = NULL;
-    while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
+    while ((e = av_dict_iterate(dict, e)))
         printf("%s %s\n", e->key, e->value);
     av_dict_free(&dict);
 
@@ -106,7 +106,7 @@  int main(void)
         printf("av_dict_set does not correctly handle NULL key\n");
 
     e = NULL;
-    while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
+    while ((e = av_dict_iterate(dict, e)))
         printf("'%s' '%s'\n", e->key, e->value);
     av_dict_free(&dict);
 
@@ -122,7 +122,7 @@  int main(void)
     av_dict_set_int(&dict, "12", 1, 0);
     av_dict_set_int(&dict, "12", 2, AV_DICT_APPEND);
     e = NULL;
-    while ((e = av_dict_get(dict, "", e, AV_DICT_IGNORE_SUFFIX)))
+    while ((e = av_dict_iterate(dict, e)))
         printf("%s %s\n", e->key, e->value);
     av_dict_free(&dict);