[FFmpeg-devel] [PATCH] lavfi: merge AVFilterChannelLayouts into AVFilterFormats.

[Andreas Rheinhardt]

Nicolas George:
> Make AVFilterFormats.formats uint64_t.
> They are short-lived lists and usually quite short too.
> 
> Signed-off-by: Nicolas George <george at nsup.org>
> ---
> 
> 
> I think the simplification in formats.c is well worth the cost in
> memory: most of the lines removed are hard-to-maintain macro code.

I do not really agree that such simple macros are hard to maintain. But
I am also not concerned about the increased memory usage (or the fact
that 32bit machines probably won't like it). And I like that it allows
to remove several duplicate functions.
If you think that it brings you closer to your other goals, then go
ahead (but Michael or Anton or Paul might want to voice their opinions,
too).

> 
> Furthermore, it will make easier to attach a score to the formats to
> replace the logic in swap_*() and pick_formats() without adding yet more
> macro code.
> 
> This patch has the risk of causing many conflicts, I would like to avoir
> keeping it in the air too long. Please comment without taking too much
> time.
> 
> 
>  libavfilter/aeval.c                |   6 +-
>  libavfilter/af_afir.c              |  10 +-
>  libavfilter/af_aformat.c           |   4 +-
>  libavfilter/af_agate.c             |  10 +-
>  libavfilter/af_amerge.c            |  12 +--
>  libavfilter/af_anequalizer.c       |   6 +-
>  libavfilter/af_apulsator.c         |   2 +-
>  libavfilter/af_aresample.c         |   6 +-
>  libavfilter/af_asr.c               |   2 +-
>  libavfilter/af_bs2b.c              |   2 +-
>  libavfilter/af_channelmap.c        |  10 +-
>  libavfilter/af_channelsplit.c      |   8 +-
>  libavfilter/af_crossfeed.c         |   2 +-
>  libavfilter/af_earwax.c            |   2 +-
>  libavfilter/af_extrastereo.c       |   2 +-
>  libavfilter/af_haas.c              |   2 +-
>  libavfilter/af_hdcd.c              |   2 +-
>  libavfilter/af_headphone.c         |  14 +--
>  libavfilter/af_join.c              |   6 +-
>  libavfilter/af_ladspa.c            |   8 +-
>  libavfilter/af_lv2.c               |   8 +-
>  libavfilter/af_pan.c               |   6 +-
>  libavfilter/af_replaygain.c        |   2 +-
>  libavfilter/af_sidechaincompress.c |  10 +-
>  libavfilter/af_silencedetect.c     |   2 +-
>  libavfilter/af_sofalizer.c         |   6 +-
>  libavfilter/af_stereotools.c       |   2 +-
>  libavfilter/af_stereowiden.c       |   2 +-
>  libavfilter/af_surround.c          |   6 +-
>  libavfilter/asrc_flite.c           |   2 +-
>  libavfilter/avf_abitscope.c        |   4 +-
>  libavfilter/avf_ahistogram.c       |   4 +-
>  libavfilter/avf_aphasemeter.c      |   6 +-
>  libavfilter/avf_avectorscope.c     |   4 +-
>  libavfilter/avf_concat.c           |   6 +-
>  libavfilter/avf_showcqt.c          |   4 +-
>  libavfilter/avf_showfreqs.c        |   4 +-
>  libavfilter/avf_showspatial.c      |   4 +-
>  libavfilter/avf_showspectrum.c     |   4 +-
>  libavfilter/avf_showvolume.c       |   4 +-
>  libavfilter/avf_showwaves.c        |   4 +-
>  libavfilter/avfilter.c             |   8 +-
>  libavfilter/avfilter.h             |   4 +-
>  libavfilter/avfiltergraph.c        |  44 ++++-----
>  libavfilter/buffersink.c           |   2 +-
>  libavfilter/buffersrc.c            |   2 +-
>  libavfilter/f_ebur128.c            |   6 +-
>  libavfilter/formats.c              | 152 ++++++++++++-----------------
>  libavfilter/formats.h              |  56 +++--------
>  libavfilter/src_movie.c            |   2 +-
>  libavfilter/tests/filtfmts.c       |   6 +-
>  libavfilter/vaf_spectrumsynth.c    |   4 +-
>  52 files changed, 221 insertions(+), 275 deletions(-)
> 
> diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
> index 5a225ffc44..19d26d6561 100644
> --- a/libavfilter/avfilter.h
> +++ b/libavfilter/avfilter.h
> @@ -68,7 +68,7 @@ typedef struct AVFilterContext AVFilterContext;
>  typedef struct AVFilterLink    AVFilterLink;
>  typedef struct AVFilterPad     AVFilterPad;
>  typedef struct AVFilterFormats AVFilterFormats;
> -typedef struct AVFilterChannelLayouts AVFilterChannelLayouts;
> +typedef struct AVFilterFormats AVFilterChannelLayouts;

That line should just be removed without replacement.

>  
>  /**
>   * Get the number of elements in a NULL-terminated array of AVFilterPads (e.g.
> @@ -437,7 +437,7 @@ typedef struct AVFilterFormatsConfig {
>      /**
>       * Lists of supported channel layouts, only for audio.
>       */
> -    AVFilterChannelLayouts  *channel_layouts;
> +    AVFilterFormats *channel_layouts;
>  
>  } AVFilterFormatsConfig;
>  
> diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
> index 8c6d43a0c7..2b8bac2a4c 100644
> --- a/libavfilter/avfiltergraph.c
> +++ b/libavfilter/avfiltergraph.c
> @@ -969,8 +969,8 @@ static void swap_channel_layouts_on_filter(AVFilterContext *filter)
>              }
>          }
>          av_assert0(best_idx >= 0);
> -        FFSWAP(uint64_t, outlink->incfg.channel_layouts->channel_layouts[0],
> -               outlink->incfg.channel_layouts->channel_layouts[best_idx]);
> +        FFSWAP(uint64_t, outlink->incfg.channel_layouts->formats[0],
> +               outlink->incfg.channel_layouts->formats[best_idx]);

Have you checked whether there are any macros of this type for the
current formats, where the type is switched from int to uint64_t with
this patch? They should be ok given that the list in AVFilterFormats
uses a length field and not a sentinel, so that all entries are >= 0,
but it would not be nice to use a wrong type here.

>      }
>  
>  }
> diff --git a/libavfilter/formats.h b/libavfilter/formats.h
> index 7c8258ed08..dafc40e915 100644
> --- a/libavfilter/formats.h
> +++ b/libavfilter/formats.h
> @@ -178,22 +166,10 @@ av_warn_unused_result
>  int ff_set_common_formats_from_list(AVFilterContext *ctx, const int *fmts);
>  
>  av_warn_unused_result
> -int ff_add_channel_layout(AVFilterChannelLayouts **l, uint64_t channel_layout);
> -
> -/**
> - * Add *ref as a new reference to f.
> - */
> -av_warn_unused_result
> -int ff_channel_layouts_ref(AVFilterChannelLayouts *f,
> -                           AVFilterChannelLayouts **ref);
> -
> -/**
> - * Remove a reference to a channel layouts list.
> - */
> -void ff_channel_layouts_unref(AVFilterChannelLayouts **ref);
> +int ff_add_channel_layout(AVFilterFormats **l, uint64_t channel_layout);
>  
> -void ff_channel_layouts_changeref(AVFilterChannelLayouts **oldref,
> -                                  AVFilterChannelLayouts **newref);
> +void ff_channel_layouts_changeref(AVFilterFormats **oldref,
> +                                  AVFilterFormats **newref);

This function does no longer exist; the declaration should be removed, too.

>  
>  av_warn_unused_result
>  int ff_default_query_formats(AVFilterContext *ctx);