[1/2] avconv: make sure the filtergraph is freed on init failure

Message ID 1475349749-4255-1-git-send-email-anton@khirnov.net
State Committed
Commit f6772e9bf8251d3943f52f6f34d97d2ce6c4b8af
Headers show

Commit Message

Anton Khirnov Oct. 1, 2016, 7:22 p.m.
The filtergraph's existence is used in several places to mean that the
filtergraph is fully configured. This causes problems if it's allocated,
but the initialization fails (e.g. if a non-existent filter is
specified).
---
 avconv_filter.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

Comments

Vittorio Giovara Oct. 1, 2016, 9:09 p.m. | #1
On Sat, Oct 1, 2016 at 3:22 PM, Anton Khirnov <anton@khirnov.net> wrote:
> The filtergraph's existence is used in several places to mean that the
> filtergraph is fully configured. This causes problems if it's allocated,
> but the initialization fails (e.g. if a non-existent filter is
> specified).
> ---
>  avconv_filter.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/avconv_filter.c b/avconv_filter.c
> index 4330802..e53dcd2 100644
> --- a/avconv_filter.c
> +++ b/avconv_filter.c
> @@ -709,7 +709,7 @@ int configure_filtergraph(FilterGraph *fg)
>      }
>
>      if ((ret = avfilter_graph_parse2(fg->graph, graph_desc, &inputs, &outputs)) < 0)
> -        return ret;
> +        goto fail;
>
>      if (hw_device_ctx) {
>          for (i = 0; i < fg->graph->nb_filters; i++) {
> @@ -720,12 +720,13 @@ int configure_filtergraph(FilterGraph *fg)
>      if (simple && (!inputs || inputs->next || !outputs || outputs->next)) {
>          av_log(NULL, AV_LOG_ERROR, "Simple filtergraph '%s' does not have "
>                 "exactly one input and output.\n", graph_desc);
> -        return AVERROR(EINVAL);
> +        ret = AVERROR(EINVAL);
> +        goto fail;
>      }
>
>      for (cur = inputs, i = 0; cur; cur = cur->next, i++)
>          if ((ret = configure_input_filter(fg, fg->inputs[i], cur)) < 0)
> -            return ret;
> +            goto fail;
>      avfilter_inout_free(&inputs);
>
>      for (cur = outputs, i = 0; cur; cur = cur->next, i++) {
> @@ -737,7 +738,7 @@ int configure_filtergraph(FilterGraph *fg)
>      avfilter_inout_free(&outputs);
>
>      if ((ret = avfilter_graph_config(fg->graph, NULL)) < 0)
> -        return ret;
> +        goto fail;
>
>      /* limit the lists of allowed formats to the ones selected, to
>       * make sure they stay the same if the filtergraph is reconfigured later */
> @@ -761,7 +762,7 @@ int configure_filtergraph(FilterGraph *fg)
>              ret = av_buffersrc_add_frame(fg->inputs[i]->filter, tmp);
>              av_frame_free(&tmp);
>              if (ret < 0)
> -                return ret;
> +                goto fail;
>          }
>      }
>
> @@ -770,11 +771,14 @@ int configure_filtergraph(FilterGraph *fg)
>          if (fg->inputs[i]->eof) {
>              ret = av_buffersrc_add_frame(fg->inputs[i]->filter, NULL);
>              if (ret < 0)
> -                return ret;
> +                goto fail;
>          }
>      }
>
>      return 0;
> +fail:
> +    avfilter_graph_free(&fg->graph);
> +    return ret;
>  }
>
>  int ifilter_parameters_from_frame(InputFilter *ifilter, const AVFrame *frame)
> --

ok I think

Patch

diff --git a/avconv_filter.c b/avconv_filter.c
index 4330802..e53dcd2 100644
--- a/avconv_filter.c
+++ b/avconv_filter.c
@@ -709,7 +709,7 @@  int configure_filtergraph(FilterGraph *fg)
     }
 
     if ((ret = avfilter_graph_parse2(fg->graph, graph_desc, &inputs, &outputs)) < 0)
-        return ret;
+        goto fail;
 
     if (hw_device_ctx) {
         for (i = 0; i < fg->graph->nb_filters; i++) {
@@ -720,12 +720,13 @@  int configure_filtergraph(FilterGraph *fg)
     if (simple && (!inputs || inputs->next || !outputs || outputs->next)) {
         av_log(NULL, AV_LOG_ERROR, "Simple filtergraph '%s' does not have "
                "exactly one input and output.\n", graph_desc);
-        return AVERROR(EINVAL);
+        ret = AVERROR(EINVAL);
+        goto fail;
     }
 
     for (cur = inputs, i = 0; cur; cur = cur->next, i++)
         if ((ret = configure_input_filter(fg, fg->inputs[i], cur)) < 0)
-            return ret;
+            goto fail;
     avfilter_inout_free(&inputs);
 
     for (cur = outputs, i = 0; cur; cur = cur->next, i++) {
@@ -737,7 +738,7 @@  int configure_filtergraph(FilterGraph *fg)
     avfilter_inout_free(&outputs);
 
     if ((ret = avfilter_graph_config(fg->graph, NULL)) < 0)
-        return ret;
+        goto fail;
 
     /* limit the lists of allowed formats to the ones selected, to
      * make sure they stay the same if the filtergraph is reconfigured later */
@@ -761,7 +762,7 @@  int configure_filtergraph(FilterGraph *fg)
             ret = av_buffersrc_add_frame(fg->inputs[i]->filter, tmp);
             av_frame_free(&tmp);
             if (ret < 0)
-                return ret;
+                goto fail;
         }
     }
 
@@ -770,11 +771,14 @@  int configure_filtergraph(FilterGraph *fg)
         if (fg->inputs[i]->eof) {
             ret = av_buffersrc_add_frame(fg->inputs[i]->filter, NULL);
             if (ret < 0)
-                return ret;
+                goto fail;
         }
     }
 
     return 0;
+fail:
+    avfilter_graph_free(&fg->graph);
+    return ret;
 }
 
 int ifilter_parameters_from_frame(InputFilter *ifilter, const AVFrame *frame)