[FFmpeg-devel] [PATCH 275/281] avfilter: convert to new channel layout API

Anton Khirnov anton at khirnov.net
Tue Feb 15 13:50:04 EET 2022


Quoting James Almer (2022-01-13 03:09:07)
> diff --git a/libavfilter/af_aformat.c b/libavfilter/af_aformat.c
> index ed3c75311a..96704e041c 100644
> --- a/libavfilter/af_aformat.c
> +++ b/libavfilter/af_aformat.c
> @@ -104,9 +104,36 @@ static av_cold int init(AVFilterContext *ctx)
>                    ff_add_format, av_get_sample_fmt, AV_SAMPLE_FMT_NONE, "sample format");
>      PARSE_FORMATS(s->sample_rates_str, int, s->sample_rates, ff_add_format,
>                    get_sample_rate, 0, "sample rate");
> -    PARSE_FORMATS(s->channel_layouts_str, uint64_t, s->channel_layouts,
> -                  ff_add_channel_layout, av_get_channel_layout, 0,
> -                  "channel layout");
> +    {
> +        AVChannelLayout fmt = { 0 };
> +        const char *cur = s->channel_layouts_str;
> +        int ret;
> +
> +        if (s->channel_layouts_str && strchr(s->channel_layouts_str, ',')) {
> +            av_log(ctx, AV_LOG_WARNING, "This syntax is deprecated, use '|' to "
> +                   "separate channel layout.\n");

It might be unclear to the user what "this syntax" refers to, maybe make
it "Using ',' to separate channel layouts is deprecated"

> diff --git a/libavfilter/af_biquads.c b/libavfilter/af_biquads.c
> index ee42b2a034..2a0747a037 100644
> --- a/libavfilter/af_biquads.c
> +++ b/libavfilter/af_biquads.c
> @@ -125,7 +125,7 @@ typedef struct BiquadsContext {
>      double frequency;
>      double width;
>      double mix;
> -    uint64_t channels;
> +    AVChannelLayout ch_layout;
>      int normalize;
>      int order;
>  
> @@ -716,11 +716,11 @@ static int config_filter(AVFilterLink *outlink, int reset)
>          s->b2 *= factor;
>      }
>  
> -    s->cache = av_realloc_f(s->cache, sizeof(ChanCache), inlink->channels);
> +    s->cache = av_realloc_f(s->cache, sizeof(ChanCache), inlink->ch_layout.nb_channels);
>      if (!s->cache)
>          return AVERROR(ENOMEM);
>      if (reset)
> -        memset(s->cache, 0, sizeof(ChanCache) * inlink->channels);
> +        memset(s->cache, 0, sizeof(ChanCache) * inlink->ch_layout.nb_channels);
>  
>      switch (s->transform_type) {
>      case DI:
> @@ -838,12 +838,14 @@ static int filter_channel(AVFilterContext *ctx, void *arg, int jobnr, int nb_job
>      AVFrame *buf = td->in;
>      AVFrame *out_buf = td->out;
>      BiquadsContext *s = ctx->priv;
> -    const int start = (buf->channels * jobnr) / nb_jobs;
> -    const int end = (buf->channels * (jobnr+1)) / nb_jobs;
> +    const int start = (buf->ch_layout.nb_channels * jobnr) / nb_jobs;
> +    const int end = (buf->ch_layout.nb_channels * (jobnr+1)) / nb_jobs;
>      int ch;
>  
>      for (ch = start; ch < end; ch++) {
> -        if (!((av_channel_layout_extract_channel(inlink->channel_layout, ch) & s->channels))) {
> +        if (!(av_channel_layout_channel_from_index(&inlink->ch_layout, ch) >= 0 &&
> +              av_channel_layout_channel_from_index(&s->ch_layout, ch) >= 0)) {

This doesn't look right. The original code tests whether the channel
with index ch in inlink->channel_layout is present in s->channels.

The new code tests whether the channel with index ch exists in both
inlink->ch_layout and s->ch_layout (the test is also broken, because it
should compare against AV_CHAN_NONE).

Same applies to af_speechnorm

> diff --git a/libavfilter/af_headphone.c b/libavfilter/af_headphone.c
> index b2030dbbbb..2fe36368c1 100644
> --- a/libavfilter/af_headphone.c
> +++ b/libavfilter/af_headphone.c
> @@ -85,13 +85,13 @@ typedef struct HeadphoneContext {
>      uint64_t mapping[64];
>  } HeadphoneContext;
>  
> -static int parse_channel_name(const char *arg, uint64_t *rchannel)
> +static int parse_channel_name(const char *arg, int *rchannel)

enum AVChannel everywhere?

>  {
> -    uint64_t layout = av_get_channel_layout(arg);
> +    int channel = av_channel_from_string(arg);
>  
> -    if (av_get_channel_layout_nb_channels(layout) != 1)
> +    if (channel < 0)
>          return AVERROR(EINVAL);
> -    *rchannel = layout;
> +    *rchannel = channel;
>      return 0;
>  }
>  
> @@ -103,14 +103,14 @@ static void parse_map(AVFilterContext *ctx)
>  
>      p = s->map;
>      while ((arg = av_strtok(p, "|", &tokenizer))) {
> -        uint64_t out_channel;
> +        int out_channel;
>  
>          p = NULL;
>          if (parse_channel_name(arg, &out_channel)) {
>              av_log(ctx, AV_LOG_WARNING, "Failed to parse \'%s\' as channel name.\n", arg);
>              continue;
>          }
> -        if (used_channels & out_channel) {
> +        if (used_channels & (1ULL << out_channel)) {

should check that out_channel < 64

> diff --git a/libavfilter/af_surround.c b/libavfilter/af_surround.c
> index ccd85148e9..71d713e4ed 100644
> --- a/libavfilter/af_surround.c
> +++ b/libavfilter/af_surround.c
> @@ -92,8 +92,8 @@ typedef struct AudioSurroundContext {
>      float lowcut;
>      float highcut;
>  
> -    uint64_t out_channel_layout;
> -    uint64_t in_channel_layout;
> +    AVChannelLayout out_channel_layout;
> +    AVChannelLayout in_channel_layout;
>      int nb_in_channels;
>      int nb_out_channels;
>  
> @@ -171,7 +171,7 @@ static int query_formats(AVFilterContext *ctx)
>          return ret;
>  
>      layouts = NULL;
> -    ret = ff_add_channel_layout(&layouts, s->out_channel_layout);
> +    ret = ff_add_channel_layout(&layouts, &s->out_channel_layout);
>      if (ret)
>          return ret;
>  
> @@ -180,7 +180,7 @@ static int query_formats(AVFilterContext *ctx)
>          return ret;
>  
>      layouts = NULL;
> -    ret = ff_add_channel_layout(&layouts, s->in_channel_layout);
> +    ret = ff_add_channel_layout(&layouts, &s->in_channel_layout);
>      if (ret)
>          return ret;
>  
> @@ -197,12 +197,12 @@ static int config_input(AVFilterLink *inlink)
>      AudioSurroundContext *s = ctx->priv;
>      int ch;
>  
> -    s->rdft = av_calloc(inlink->channels, sizeof(*s->rdft));
> +    s->rdft = av_calloc(inlink->ch_layout.nb_channels, sizeof(*s->rdft));
>      if (!s->rdft)
>          return AVERROR(ENOMEM);
> -    s->nb_in_channels = inlink->channels;
> +    s->nb_in_channels = inlink->ch_layout.nb_channels;
>  
> -    for (ch = 0; ch < inlink->channels; ch++) {
> +    for (ch = 0; ch < inlink->ch_layout.nb_channels; ch++) {
>          s->rdft[ch]  = av_rdft_init(ff_log2(s->buf_size), DFT_R2C);
>          if (!s->rdft[ch])
>              return AVERROR(ENOMEM);
> @@ -212,31 +212,31 @@ static int config_input(AVFilterLink *inlink)
>          return AVERROR(ENOMEM);
>      for (ch = 0;  ch < s->nb_in_channels; ch++)
>          s->input_levels[ch] = s->level_in;
> -    ch = av_get_channel_layout_channel_index(inlink->channel_layout, AV_CH_FRONT_CENTER);
> +    ch = av_channel_layout_index_from_channel(&inlink->ch_layout, AV_CHAN_FRONT_CENTER);
>      if (ch >= 0)
>          s->input_levels[ch] *= s->fc_in;
> -    ch = av_get_channel_layout_channel_index(inlink->channel_layout, AV_CH_FRONT_LEFT);
> +    ch = av_channel_layout_index_from_channel(&inlink->ch_layout, AV_CHAN_FRONT_LEFT);
>      if (ch >= 0)
>          s->input_levels[ch] *= s->fl_in;
> -    ch = av_get_channel_layout_channel_index(inlink->channel_layout, AV_CH_FRONT_RIGHT);
> +    ch = av_channel_layout_index_from_channel(&inlink->ch_layout, AV_CHAN_FRONT_RIGHT);
>      if (ch >= 0)
>          s->input_levels[ch] *= s->fr_in;
> -    ch = av_get_channel_layout_channel_index(inlink->channel_layout, AV_CH_SIDE_LEFT);
> +    ch = av_channel_layout_index_from_channel(&inlink->ch_layout, AV_CHAN_SIDE_LEFT);
>      if (ch >= 0)
>          s->input_levels[ch] *= s->sl_in;
> -    ch = av_get_channel_layout_channel_index(inlink->channel_layout, AV_CH_SIDE_RIGHT);
> +    ch = av_channel_layout_index_from_channel(&inlink->ch_layout, AV_CHAN_SIDE_RIGHT);
>      if (ch >= 0)
>          s->input_levels[ch] *= s->sr_in;
> -    ch = av_get_channel_layout_channel_index(inlink->channel_layout, AV_CH_BACK_LEFT);
> +    ch = av_channel_layout_index_from_channel(&inlink->ch_layout, AV_CHAN_BACK_LEFT);
>      if (ch >= 0)
>          s->input_levels[ch] *= s->bl_in;
> -    ch = av_get_channel_layout_channel_index(inlink->channel_layout, AV_CH_BACK_RIGHT);
> +    ch = av_channel_layout_index_from_channel(&inlink->ch_layout, AV_CHAN_BACK_RIGHT);
>      if (ch >= 0)
>          s->input_levels[ch] *= s->br_in;
> -    ch = av_get_channel_layout_channel_index(inlink->channel_layout, AV_CH_BACK_CENTER);
> +    ch = av_channel_layout_index_from_channel(&inlink->ch_layout, AV_CHAN_BACK_CENTER);
>      if (ch >= 0)
>          s->input_levels[ch] *= s->bc_in;
> -    ch = av_get_channel_layout_channel_index(inlink->channel_layout, AV_CH_LOW_FREQUENCY);
> +    ch = av_channel_layout_index_from_channel(&inlink->ch_layout, AV_CHAN_LOW_FREQUENCY);
>      if (ch >= 0)

Make all those compare to AV_CHAN_NONE

-- 
Anton Khirnov


More information about the ffmpeg-devel mailing list