diff mbox series

[FFmpeg-devel,v3,02/25] avfilter/avfilter: Allow to free non-static pads generically

Message ID PR3PR03MB6665D7C98D139601BC6F0A058FC39@PR3PR03MB6665.eurprd03.prod.outlook.com
State Accepted
Commit f308f37441ec0d49166641843acd5302c1e26e6a
Headers show
Series [FFmpeg-devel,v3,01/25] avfilter/internal: Uninline ff_insert_(in|out)pad() | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Andreas Rheinhardt Aug. 22, 2021, 12:47 a.m. UTC 
This can be enabled/disabled on a per-pad basis by setting
the AVFILTERPAD_FLAG_FREE_NAME flag; variants of ff_append_(in|out)pads
that do this for you have been added and will be put to use in the
following commits.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavfilter/avfilter.c | 21 ++++++++++++++++++++-
 libavfilter/internal.h | 10 ++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

Comments

Nicolas George Aug. 22, 2021, 9:10 a.m. UTC | #1 
Andreas Rheinhardt (12021-08-22):
> This can be enabled/disabled on a per-pad basis by setting
> the AVFILTERPAD_FLAG_FREE_NAME flag; variants of ff_append_(in|out)pads
> that do this for you have been added and will be put to use in the
> following commits.

This is an even better solution, kudos.

> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavfilter/avfilter.c | 21 ++++++++++++++++++++-
>  libavfilter/internal.h | 10 ++++++++++
>  2 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index 2681d04fc0..4b6a3d1e8f 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -123,8 +123,11 @@ static int append_pad(unsigned *count, AVFilterPad **pads,
>          *pads  = newpads;
>      if (newlinks)
>          *links = newlinks;
> -    if (!newpads || !newlinks)
> +    if (!newpads || !newlinks) {
> +        if (newpad->flags & AVFILTERPAD_FLAG_FREE_NAME)
> +            av_freep(&newpad->name);
>          return AVERROR(ENOMEM);
> +    }
>  
>      memcpy(*pads + idx, newpad, sizeof(AVFilterPad));
>      (*links)[idx] = NULL;
> @@ -139,11 +142,23 @@ int ff_append_inpad(AVFilterContext *f, AVFilterPad *p)
>      return append_pad(&f->nb_inputs, &f->input_pads, &f->inputs, p);
>  }
>  

> +int ff_append_inpad_free_name(AVFilterContext *f, AVFilterPad *p)
> +{
> +    p->flags |= AVFILTERPAD_FLAG_FREE_NAME;
> +    return ff_append_inpad(f, p);
> +}

Suggestion for a later patch:

if (!p->name)
    return AVERROR(ENOMEM);

That way, we can remove the checks in the filters.

> +
>  int ff_append_outpad(AVFilterContext *f, AVFilterPad *p)
>  {
>      return append_pad(&f->nb_outputs, &f->output_pads, &f->outputs, p);
>  }
>  
> +int ff_append_outpad_free_name(AVFilterContext *f, AVFilterPad *p)
> +{
> +    p->flags |= AVFILTERPAD_FLAG_FREE_NAME;
> +    return ff_append_outpad(f, p);
> +}
> +
>  int avfilter_link(AVFilterContext *src, unsigned srcpad,
>                    AVFilterContext *dst, unsigned dstpad)
>  {
> @@ -754,9 +769,13 @@ void avfilter_free(AVFilterContext *filter)
>  
>      for (i = 0; i < filter->nb_inputs; i++) {
>          free_link(filter->inputs[i]);
> +        if (filter->input_pads[i].flags  & AVFILTERPAD_FLAG_FREE_NAME)
> +            av_freep(&filter->input_pads[i].name);
>      }
>      for (i = 0; i < filter->nb_outputs; i++) {
>          free_link(filter->outputs[i]);
> +        if (filter->output_pads[i].flags & AVFILTERPAD_FLAG_FREE_NAME)
> +            av_freep(&filter->output_pads[i].name);
>      }
>  
>      if (filter->filter->priv_class)
> diff --git a/libavfilter/internal.h b/libavfilter/internal.h
> index cc95f06c4c..6ddf024d93 100644
> --- a/libavfilter/internal.h
> +++ b/libavfilter/internal.h
> @@ -68,6 +68,11 @@ struct AVFilterPad {
>       */
>  #define AVFILTERPAD_FLAG_NEEDS_WRITABLE                  (1 << 0)
>  
> +    /**
> +     * The pad's name is allocated and should be freed generically.
> +     */
> +#define AVFILTERPAD_FLAG_FREE_NAME                       (1 << 1)
> +
>      /**
>       * A combination of AVFILTERPAD_FLAG_* flags.
>       */
> @@ -231,9 +236,14 @@ void ff_tlog_link(void *ctx, AVFilterLink *link, int end);
>  
>  /**
>   * Append a new input/output pad to the filter's list of such pads.
> + *
> + * The *_free_name versions will set the AVFILTERPAD_FLAG_FREE_NAME flag
> + * ensuring that the name will be freed generically (even on insertion error).
>   */
>  int ff_append_inpad (AVFilterContext *f, AVFilterPad *p);
>  int ff_append_outpad(AVFilterContext *f, AVFilterPad *p);
> +int ff_append_inpad_free_name (AVFilterContext *f, AVFilterPad *p);
> +int ff_append_outpad_free_name(AVFilterContext *f, AVFilterPad *p);
>  
>  /**
>   * Request an input frame from the filter at the other end of the link.

LGTM, and patches 1-24 too, I do not maintain most of them but they are
straightforward enough.

(Also, sorry for misspelling your name recently.)

Regards,
diff mbox series

Patch

diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
index 2681d04fc0..4b6a3d1e8f 100644
--- a/libavfilter/avfilter.c
+++ b/libavfilter/avfilter.c
@@ -123,8 +123,11 @@  static int append_pad(unsigned *count, AVFilterPad **pads,
         *pads  = newpads;
     if (newlinks)
         *links = newlinks;
-    if (!newpads || !newlinks)
+    if (!newpads || !newlinks) {
+        if (newpad->flags & AVFILTERPAD_FLAG_FREE_NAME)
+            av_freep(&newpad->name);
         return AVERROR(ENOMEM);
+    }
 
     memcpy(*pads + idx, newpad, sizeof(AVFilterPad));
     (*links)[idx] = NULL;
@@ -139,11 +142,23 @@  int ff_append_inpad(AVFilterContext *f, AVFilterPad *p)
     return append_pad(&f->nb_inputs, &f->input_pads, &f->inputs, p);
 }
 
+int ff_append_inpad_free_name(AVFilterContext *f, AVFilterPad *p)
+{
+    p->flags |= AVFILTERPAD_FLAG_FREE_NAME;
+    return ff_append_inpad(f, p);
+}
+
 int ff_append_outpad(AVFilterContext *f, AVFilterPad *p)
 {
     return append_pad(&f->nb_outputs, &f->output_pads, &f->outputs, p);
 }
 
+int ff_append_outpad_free_name(AVFilterContext *f, AVFilterPad *p)
+{
+    p->flags |= AVFILTERPAD_FLAG_FREE_NAME;
+    return ff_append_outpad(f, p);
+}
+
 int avfilter_link(AVFilterContext *src, unsigned srcpad,
                   AVFilterContext *dst, unsigned dstpad)
 {
@@ -754,9 +769,13 @@  void avfilter_free(AVFilterContext *filter)
 
     for (i = 0; i < filter->nb_inputs; i++) {
         free_link(filter->inputs[i]);
+        if (filter->input_pads[i].flags  & AVFILTERPAD_FLAG_FREE_NAME)
+            av_freep(&filter->input_pads[i].name);
     }
     for (i = 0; i < filter->nb_outputs; i++) {
         free_link(filter->outputs[i]);
+        if (filter->output_pads[i].flags & AVFILTERPAD_FLAG_FREE_NAME)
+            av_freep(&filter->output_pads[i].name);
     }
 
     if (filter->filter->priv_class)
diff --git a/libavfilter/internal.h b/libavfilter/internal.h
index cc95f06c4c..6ddf024d93 100644
--- a/libavfilter/internal.h
+++ b/libavfilter/internal.h
@@ -68,6 +68,11 @@  struct AVFilterPad {
      */
 #define AVFILTERPAD_FLAG_NEEDS_WRITABLE                  (1 << 0)
 
+    /**
+     * The pad's name is allocated and should be freed generically.
+     */
+#define AVFILTERPAD_FLAG_FREE_NAME                       (1 << 1)
+
     /**
      * A combination of AVFILTERPAD_FLAG_* flags.
      */
@@ -231,9 +236,14 @@  void ff_tlog_link(void *ctx, AVFilterLink *link, int end);
 
 /**
  * Append a new input/output pad to the filter's list of such pads.
+ *
+ * The *_free_name versions will set the AVFILTERPAD_FLAG_FREE_NAME flag
+ * ensuring that the name will be freed generically (even on insertion error).
  */
 int ff_append_inpad (AVFilterContext *f, AVFilterPad *p);
 int ff_append_outpad(AVFilterContext *f, AVFilterPad *p);
+int ff_append_inpad_free_name (AVFilterContext *f, AVFilterPad *p);
+int ff_append_outpad_free_name(AVFilterContext *f, AVFilterPad *p);
 
 /**
  * Request an input frame from the filter at the other end of the link.