diff mbox series

[FFmpeg-devel,02/31] fftools: use av_dict_get_string

Message ID 20221125013046.40904-3-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 
Instead of manually assembling the string, use av_dict_get_string
which handles things like proper escaping too (even though it is
not yet needed here).
---
 fftools/ffmpeg_filter.c | 31 +++++++++++++------------------
 1 file changed, 13 insertions(+), 18 deletions(-)

Comments

Andreas Rheinhardt Nov. 25, 2022, 12:48 p.m. UTC | #1 
Marvin Scholz:
> Instead of manually assembling the string, use av_dict_get_string
> which handles things like proper escaping too (even though it is
> not yet needed here).
> ---
>  fftools/ffmpeg_filter.c | 31 +++++++++++++------------------
>  1 file changed, 13 insertions(+), 18 deletions(-)
> 
> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> index b0c4c8ece3..29794fdc85 100644
> --- a/fftools/ffmpeg_filter.c
> +++ b/fftools/ffmpeg_filter.c
> @@ -972,7 +972,7 @@ int configure_filtergraph(FilterGraph *fg)
>  
>      if (simple) {
>          OutputStream *ost = fg->outputs[0]->ost;
> -        char args[512];
> +        char *args = NULL;
>          const AVDictionaryEntry *e = NULL;
>  
>          if (filter_nbthreads) {
> @@ -985,26 +985,21 @@ int configure_filtergraph(FilterGraph *fg)
>                  av_opt_set(fg->graph, "threads", e->value, 0);
>          }
>  
> -        args[0] = 0;
> -        e       = NULL;
> -        while ((e = av_dict_get(ost->sws_dict, "", e,
> -                                AV_DICT_IGNORE_SUFFIX))) {
> -            av_strlcatf(args, sizeof(args), "%s=%s:", e->key, e->value);
> -        }
> -        if (strlen(args)) {
> -            args[strlen(args)-1] = 0;
> -            fg->graph->scale_sws_opts = av_strdup(args);
> +        if (av_dict_count(ost->sws_dict)) {
> +            ret = av_dict_get_string(ost->sws_dict, &args, '=', ':');
> +            if (ret < 0)
> +                goto fail;
> +            fg->graph->scale_sws_opts = args;
> +            args = NULL;

It would be cleaner if you used a smaller scope for args (here and
below); or maybe just eliminate this variable here entirely by using
&fg->graph->scale_sws_opts as destination for av_dict_get_string()?

>          }
>  
> -        args[0] = 0;
> -        e       = NULL;
> -        while ((e = av_dict_get(ost->swr_opts, "", e,
> -                                AV_DICT_IGNORE_SUFFIX))) {
> -            av_strlcatf(args, sizeof(args), "%s=%s:", e->key, e->value);
> +        if (av_dict_count(ost->swr_opts)) {
> +            ret = av_dict_get_string(ost->swr_opts, &args, '=', ':');
> +            if (ret < 0)
> +                goto fail;
> +            av_opt_set(fg->graph, "aresample_swr_opts", args, 0);
> +            free(args);

Wrong deallocator.

>          }
> -        if (strlen(args))
> -            args[strlen(args)-1] = 0;
> -        av_opt_set(fg->graph, "aresample_swr_opts", args, 0);
>      } else {
>          fg->graph->nb_threads = filter_complex_nbthreads;
>      }
diff mbox series

Patch

diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index b0c4c8ece3..29794fdc85 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -972,7 +972,7 @@  int configure_filtergraph(FilterGraph *fg)
 
     if (simple) {
         OutputStream *ost = fg->outputs[0]->ost;
-        char args[512];
+        char *args = NULL;
         const AVDictionaryEntry *e = NULL;
 
         if (filter_nbthreads) {
@@ -985,26 +985,21 @@  int configure_filtergraph(FilterGraph *fg)
                 av_opt_set(fg->graph, "threads", e->value, 0);
         }
 
-        args[0] = 0;
-        e       = NULL;
-        while ((e = av_dict_get(ost->sws_dict, "", e,
-                                AV_DICT_IGNORE_SUFFIX))) {
-            av_strlcatf(args, sizeof(args), "%s=%s:", e->key, e->value);
-        }
-        if (strlen(args)) {
-            args[strlen(args)-1] = 0;
-            fg->graph->scale_sws_opts = av_strdup(args);
+        if (av_dict_count(ost->sws_dict)) {
+            ret = av_dict_get_string(ost->sws_dict, &args, '=', ':');
+            if (ret < 0)
+                goto fail;
+            fg->graph->scale_sws_opts = args;
+            args = NULL;
         }
 
-        args[0] = 0;
-        e       = NULL;
-        while ((e = av_dict_get(ost->swr_opts, "", e,
-                                AV_DICT_IGNORE_SUFFIX))) {
-            av_strlcatf(args, sizeof(args), "%s=%s:", e->key, e->value);
+        if (av_dict_count(ost->swr_opts)) {
+            ret = av_dict_get_string(ost->swr_opts, &args, '=', ':');
+            if (ret < 0)
+                goto fail;
+            av_opt_set(fg->graph, "aresample_swr_opts", args, 0);
+            free(args);
         }
-        if (strlen(args))
-            args[strlen(args)-1] = 0;
-        av_opt_set(fg->graph, "aresample_swr_opts", args, 0);
     } else {
         fg->graph->nb_threads = filter_complex_nbthreads;
     }