[FFmpeg-devel] [PATCH 4/7] avfilter/formats: Avoid code duplication when merging samplerates

[Nicolas George]

Andreas Rheinhardt (12020-08-15):
> by adapting the MERGE_FORMATS() so that only one instance of the
> MERGE_REF() macro needs to exist in ff_merge_samplerates().

Nit: the first line of the commit message should be a short summary.

> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
>  libavfilter/formats.c | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/libavfilter/formats.c b/libavfilter/formats.c
> index 4efbcbebfe..e8a43a434d 100644
> --- a/libavfilter/formats.c
> +++ b/libavfilter/formats.c
> @@ -56,12 +56,21 @@ do {                                                                       \
>  
>  /**
>   * Add all formats common to a and b to a, add b's refs to a and destroy b.
> + * If empty_allowed is set and one of a,b->nb is zero, the lists are
> + * merged; otherwise, it is treated as error.
>   */
> -#define MERGE_FORMATS(a, b, fmts, nb, type, fail_statement)                \
> +#define MERGE_FORMATS(a, b, fmts, nb, type, fail_statement, empty_allowed) \
>  do {                                                                            \
>      int i, j, k = 0;                                                       \
>      void *tmp;                                                             \
>                                                                                  \

> +    if (empty_allowed) {                                                   \
> +        if (!a->nb || !b->nb) {                                            \
> +            if (!a->nb)                                                    \
> +                FFSWAP(type *, a, b);                                      \
> +            goto merge_ref;                                                \
> +        }                                                                  \
> +    }                                                                      \

I think a big if () / else would be better than a goto.

>          for (i = 0; i < a->nb; i++)                                             \
>              for (j = 0; j < b->nb; j++)                                         \
>                  if (a->fmts[i] == b->fmts[j]) {                                 \
> @@ -77,6 +86,7 @@ do {
>      if (tmp)                                                               \
>          a->fmts = tmp;                                                     \
>                                                                                  \
> +merge_ref:                                                                 \
>      MERGE_REF(a, b, fmts, type, fail_statement);                           \
>  } while (0)
>  
> @@ -114,7 +124,7 @@ AVFilterFormats *ff_merge_formats(AVFilterFormats *a, AVFilterFormats *b,
>      if (alpha2 > alpha1 || chroma2 > chroma1)
>          return NULL;
>  
> -    MERGE_FORMATS(a, b, formats, nb_formats, AVFilterFormats, return NULL;);
> +    MERGE_FORMATS(a, b, formats, nb_formats, AVFilterFormats, return NULL;, 0);
>  
>      return a;
>  }
> @@ -124,16 +134,8 @@ AVFilterFormats *ff_merge_samplerates(AVFilterFormats *a,
>  {
>      if (a == b) return a;
>  
> -    if (a->nb_formats && b->nb_formats) {
> -        MERGE_FORMATS(a, b, formats, nb_formats, AVFilterFormats, return NULL;);
> -        return a;
> -    } else if (a->nb_formats) {
> -        MERGE_REF(a, b, formats, AVFilterFormats, return NULL;);
> -        return a;
> -    } else {
> -        MERGE_REF(b, a, formats, AVFilterFormats, return NULL;);
> -        return b;
> -    }
> +    MERGE_FORMATS(a, b, formats, nb_formats, AVFilterFormats, return NULL;, 1);
> +    return a;
>  }
>  
>  AVFilterChannelLayouts *ff_merge_channel_layouts(AVFilterChannelLayouts *a,

Overall, it does not look very beneficial, but I suppose it is ok.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200819/f0719242/attachment.sig>