diff mbox series

[FFmpeg-devel,02/23] avfilter/internal: Uninline ff_insert_(in|out)pad()

Message ID PR3PR03MB66650D906AB24F699F5C3E728FF99@PR3PR03MB6665.eurprd03.prod.outlook.com
State Superseded
Headers show
Series [FFmpeg-devel,01/23] avfilter/vf_(guided|program_opencl): Add missing dynamic inputs flag | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt Aug. 12, 2021, 1:43 a.m. UTC
These functions are not hot at all and future commits will make them
bigger.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
I haven't found a caller that adds a pad somewhere else than the end
of the list, so the index parameter could be removed. Shall I do so
or is there a compelling reason to retain this functionality?

 libavfilter/avfilter.c | 32 +++++++++++++++++++++++++++++---
 libavfilter/internal.h | 32 ++------------------------------
 2 files changed, 31 insertions(+), 33 deletions(-)

Comments

Nicolas George Aug. 16, 2021, 12:40 p.m. UTC | #1
Andreas Rheinhardt (12021-08-12):
> These functions are not hot at all and future commits will make them
> bigger.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> I haven't found a caller that adds a pad somewhere else than the end
> of the list, so the index parameter could be removed. Shall I do so
> or is there a compelling reason to retain this functionality?

No objection. It would be easy to 

>  libavfilter/avfilter.c | 32 +++++++++++++++++++++++++++++---
>  libavfilter/internal.h | 32 ++------------------------------
>  2 files changed, 31 insertions(+), 33 deletions(-)
> 
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index f9d7226386..de7501c37b 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -101,9 +101,23 @@ void ff_command_queue_pop(AVFilterContext *filter)
>      av_free(c);
>  }
>  
> -int ff_insert_pad(unsigned idx, unsigned *count, size_t padidx_off,
> -                   AVFilterPad **pads, AVFilterLink ***links,
> -                   AVFilterPad *newpad)

> +/**
> + * Insert a new pad.
> + *
> + * @param idx Insertion point. Pad is inserted at the end if this point
> + *            is beyond the end of the list of pads.
> + * @param count Pointer to the number of pads in the list
> + * @param padidx_off Offset within an AVFilterLink structure to the element
> + *                   to increment when inserting a new pad causes link
> + *                   numbering to change
> + * @param pads Pointer to the pointer to the beginning of the list of pads
> + * @param links Pointer to the pointer to the beginning of the list of links
> + * @param newpad The new pad to add. A copy is made when adding.
> + * @return >= 0 in case of success, a negative AVERROR code on error
> + */

The documentation should be in the header where the function is
declared: this is where we would be looking for it.

> +static int insert_pad(unsigned idx, unsigned *count, size_t padidx_off,
> +                      AVFilterPad **pads, AVFilterLink ***links,
> +                      AVFilterPad *newpad)
>  {
>      AVFilterLink **newlinks;
>      AVFilterPad *newpads;
> @@ -133,6 +147,18 @@ int ff_insert_pad(unsigned idx, unsigned *count, size_t padidx_off,
>      return 0;
>  }
>  
> +int ff_insert_inpad(AVFilterContext *f, unsigned index, AVFilterPad *p)
> +{
> +    return insert_pad(index, &f->nb_inputs, offsetof(AVFilterLink, dstpad),
> +                  &f->input_pads, &f->inputs, p);
> +}
> +
> +int ff_insert_outpad(AVFilterContext *f, unsigned index, AVFilterPad *p)
> +{
> +    return insert_pad(index, &f->nb_outputs, offsetof(AVFilterLink, srcpad),
> +                  &f->output_pads, &f->outputs, p);
> +}
> +
>  int avfilter_link(AVFilterContext *src, unsigned srcpad,
>                    AVFilterContext *dst, unsigned dstpad)
>  {
> diff --git a/libavfilter/internal.h b/libavfilter/internal.h
> index 6c908690b4..615b725cab 100644
> --- a/libavfilter/internal.h
> +++ b/libavfilter/internal.h
> @@ -214,39 +214,11 @@ void ff_tlog_ref(void *ctx, AVFrame *ref, int end);
>  
>  void ff_tlog_link(void *ctx, AVFilterLink *link, int end);
>  
> -/**
> - * Insert a new pad.
> - *
> - * @param idx Insertion point. Pad is inserted at the end if this point
> - *            is beyond the end of the list of pads.
> - * @param count Pointer to the number of pads in the list
> - * @param padidx_off Offset within an AVFilterLink structure to the element
> - *                   to increment when inserting a new pad causes link
> - *                   numbering to change
> - * @param pads Pointer to the pointer to the beginning of the list of pads
> - * @param links Pointer to the pointer to the beginning of the list of links
> - * @param newpad The new pad to add. A copy is made when adding.
> - * @return >= 0 in case of success, a negative AVERROR code on error
> - */
> -int ff_insert_pad(unsigned idx, unsigned *count, size_t padidx_off,
> -                   AVFilterPad **pads, AVFilterLink ***links,
> -                   AVFilterPad *newpad);
> -
>  /** Insert a new input pad for the filter. */
> -static inline int ff_insert_inpad(AVFilterContext *f, unsigned index,
> -                                   AVFilterPad *p)
> -{
> -    return ff_insert_pad(index, &f->nb_inputs, offsetof(AVFilterLink, dstpad),
> -                  &f->input_pads, &f->inputs, p);
> -}
> +int ff_insert_inpad(AVFilterContext *f, unsigned index, AVFilterPad *p);
>  
>  /** Insert a new output pad for the filter. */
> -static inline int ff_insert_outpad(AVFilterContext *f, unsigned index,
> -                                    AVFilterPad *p)
> -{
> -    return ff_insert_pad(index, &f->nb_outputs, offsetof(AVFilterLink, srcpad),
> -                  &f->output_pads, &f->outputs, p);
> -}
> +int ff_insert_outpad(AVFilterContext *f, unsigned index, AVFilterPad *p);
>  
>  /**
>   * Request an input frame from the filter at the other end of the link.

Regards,
Andreas Rheinhardt Aug. 16, 2021, 1:29 p.m. UTC | #2
Nicolas George:
> Andreas Rheinhardt (12021-08-12):
>> These functions are not hot at all and future commits will make them
>> bigger.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>> I haven't found a caller that adds a pad somewhere else than the end
>> of the list, so the index parameter could be removed. Shall I do so
>> or is there a compelling reason to retain this functionality?
> 
> No objection. It would be easy to 
> 

Ok, I'll do it.

>>  libavfilter/avfilter.c | 32 +++++++++++++++++++++++++++++---
>>  libavfilter/internal.h | 32 ++------------------------------
>>  2 files changed, 31 insertions(+), 33 deletions(-)
>>
>> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
>> index f9d7226386..de7501c37b 100644
>> --- a/libavfilter/avfilter.c
>> +++ b/libavfilter/avfilter.c
>> @@ -101,9 +101,23 @@ void ff_command_queue_pop(AVFilterContext *filter)
>>      av_free(c);
>>  }
>>  
>> -int ff_insert_pad(unsigned idx, unsigned *count, size_t padidx_off,
>> -                   AVFilterPad **pads, AVFilterLink ***links,
>> -                   AVFilterPad *newpad)
> 
>> +/**
>> + * Insert a new pad.
>> + *
>> + * @param idx Insertion point. Pad is inserted at the end if this point
>> + *            is beyond the end of the list of pads.
>> + * @param count Pointer to the number of pads in the list
>> + * @param padidx_off Offset within an AVFilterLink structure to the element
>> + *                   to increment when inserting a new pad causes link
>> + *                   numbering to change
>> + * @param pads Pointer to the pointer to the beginning of the list of pads
>> + * @param links Pointer to the pointer to the beginning of the list of links
>> + * @param newpad The new pad to add. A copy is made when adding.
>> + * @return >= 0 in case of success, a negative AVERROR code on error
>> + */
> 
> The documentation should be in the header where the function is
> declared: this is where we would be looking for it.
> 
insert_pad() has been made static in this patch; it is not declared in
any header any more. Therefore I moved the documentation together with
the function.

>> +static int insert_pad(unsigned idx, unsigned *count, size_t padidx_off,
>> +                      AVFilterPad **pads, AVFilterLink ***links,
>> +                      AVFilterPad *newpad)

- Andreas
Nicolas George Aug. 16, 2021, 1:31 p.m. UTC | #3
Andreas Rheinhardt (12021-08-16):
> insert_pad() has been made static in this patch; it is not declared in
> any header any more. Therefore I moved the documentation together with
> the function.

Yes, but the documentation is not really for insert_pad(), it is just an
internal function for the two ff_ functions. They are declared in a
header, the documentation should be there.

Regards,
Andreas Rheinhardt Aug. 16, 2021, 1:59 p.m. UTC | #4
Nicolas George:
> Andreas Rheinhardt (12021-08-16):
>> insert_pad() has been made static in this patch; it is not declared in
>> any header any more. Therefore I moved the documentation together with
>> the function.
> 
> Yes, but the documentation is not really for insert_pad(), it is just an
> internal function for the two ff_ functions. They are declared in a
> header, the documentation should be there.
> 
Not really: The two ff_insert_(in|out)pad functions have a completely
different signature. Anyway I'll add documentation for them.

- Andreas
diff mbox series

Patch

diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
index f9d7226386..de7501c37b 100644
--- a/libavfilter/avfilter.c
+++ b/libavfilter/avfilter.c
@@ -101,9 +101,23 @@  void ff_command_queue_pop(AVFilterContext *filter)
     av_free(c);
 }
 
-int ff_insert_pad(unsigned idx, unsigned *count, size_t padidx_off,
-                   AVFilterPad **pads, AVFilterLink ***links,
-                   AVFilterPad *newpad)
+/**
+ * Insert a new pad.
+ *
+ * @param idx Insertion point. Pad is inserted at the end if this point
+ *            is beyond the end of the list of pads.
+ * @param count Pointer to the number of pads in the list
+ * @param padidx_off Offset within an AVFilterLink structure to the element
+ *                   to increment when inserting a new pad causes link
+ *                   numbering to change
+ * @param pads Pointer to the pointer to the beginning of the list of pads
+ * @param links Pointer to the pointer to the beginning of the list of links
+ * @param newpad The new pad to add. A copy is made when adding.
+ * @return >= 0 in case of success, a negative AVERROR code on error
+ */
+static int insert_pad(unsigned idx, unsigned *count, size_t padidx_off,
+                      AVFilterPad **pads, AVFilterLink ***links,
+                      AVFilterPad *newpad)
 {
     AVFilterLink **newlinks;
     AVFilterPad *newpads;
@@ -133,6 +147,18 @@  int ff_insert_pad(unsigned idx, unsigned *count, size_t padidx_off,
     return 0;
 }
 
+int ff_insert_inpad(AVFilterContext *f, unsigned index, AVFilterPad *p)
+{
+    return insert_pad(index, &f->nb_inputs, offsetof(AVFilterLink, dstpad),
+                  &f->input_pads, &f->inputs, p);
+}
+
+int ff_insert_outpad(AVFilterContext *f, unsigned index, AVFilterPad *p)
+{
+    return insert_pad(index, &f->nb_outputs, offsetof(AVFilterLink, srcpad),
+                  &f->output_pads, &f->outputs, p);
+}
+
 int avfilter_link(AVFilterContext *src, unsigned srcpad,
                   AVFilterContext *dst, unsigned dstpad)
 {
diff --git a/libavfilter/internal.h b/libavfilter/internal.h
index 6c908690b4..615b725cab 100644
--- a/libavfilter/internal.h
+++ b/libavfilter/internal.h
@@ -214,39 +214,11 @@  void ff_tlog_ref(void *ctx, AVFrame *ref, int end);
 
 void ff_tlog_link(void *ctx, AVFilterLink *link, int end);
 
-/**
- * Insert a new pad.
- *
- * @param idx Insertion point. Pad is inserted at the end if this point
- *            is beyond the end of the list of pads.
- * @param count Pointer to the number of pads in the list
- * @param padidx_off Offset within an AVFilterLink structure to the element
- *                   to increment when inserting a new pad causes link
- *                   numbering to change
- * @param pads Pointer to the pointer to the beginning of the list of pads
- * @param links Pointer to the pointer to the beginning of the list of links
- * @param newpad The new pad to add. A copy is made when adding.
- * @return >= 0 in case of success, a negative AVERROR code on error
- */
-int ff_insert_pad(unsigned idx, unsigned *count, size_t padidx_off,
-                   AVFilterPad **pads, AVFilterLink ***links,
-                   AVFilterPad *newpad);
-
 /** Insert a new input pad for the filter. */
-static inline int ff_insert_inpad(AVFilterContext *f, unsigned index,
-                                   AVFilterPad *p)
-{
-    return ff_insert_pad(index, &f->nb_inputs, offsetof(AVFilterLink, dstpad),
-                  &f->input_pads, &f->inputs, p);
-}
+int ff_insert_inpad(AVFilterContext *f, unsigned index, AVFilterPad *p);
 
 /** Insert a new output pad for the filter. */
-static inline int ff_insert_outpad(AVFilterContext *f, unsigned index,
-                                    AVFilterPad *p)
-{
-    return ff_insert_pad(index, &f->nb_outputs, offsetof(AVFilterLink, srcpad),
-                  &f->output_pads, &f->outputs, p);
-}
+int ff_insert_outpad(AVFilterContext *f, unsigned index, AVFilterPad *p);
 
 /**
  * Request an input frame from the filter at the other end of the link.