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

Anton Khirnov anton at khirnov.net
Wed Feb 16 20:15:37 EET 2022


Quoting James Almer (2022-01-13 03:09:07)
> diff --git a/libavfilter/af_channelsplit.c b/libavfilter/af_channelsplit.c
> index 1a2519dd32..31453823c5 100644
> --- a/libavfilter/af_channelsplit.c
> +++ b/libavfilter/af_channelsplit.c
> @@ -36,7 +36,7 @@
>  typedef struct ChannelSplitContext {
>      const AVClass *class;
>  
> -    uint64_t channel_layout;
> +    AVChannelLayout channel_layout;
>      char    *channel_layout_str;
>      char    *channels_str;
>  
> @@ -57,11 +57,11 @@ AVFILTER_DEFINE_CLASS(channelsplit);
>  static av_cold int init(AVFilterContext *ctx)
>  {
>      ChannelSplitContext *s = ctx->priv;
> -    uint64_t channel_layout;
> +    AVChannelLayout channel_layout = { 0 };
>      int nb_channels;
>      int all = 0, ret = 0, i;
>  
> -    if (!(s->channel_layout = av_get_channel_layout(s->channel_layout_str))) {
> +    if ((ret = av_channel_layout_from_string(&s->channel_layout, s->channel_layout_str)) < 0) {
>          av_log(ctx, AV_LOG_ERROR, "Error parsing channel layout '%s'.\n",
>                 s->channel_layout_str);
>          ret = AVERROR(EINVAL);
> @@ -70,27 +70,32 @@ static av_cold int init(AVFilterContext *ctx)
>  
>  
>      if (!strcmp(s->channels_str, "all")) {
> -        nb_channels = av_get_channel_layout_nb_channels(s->channel_layout);
> +        nb_channels = s->channel_layout.nb_channels;
>          channel_layout = s->channel_layout;
>          all = 1;
>      } else {
> -        if ((ret = av_get_extended_channel_layout(s->channels_str, &channel_layout, &nb_channels)) < 0)
> +        if ((ret = av_channel_layout_from_string(&channel_layout, s->channels_str)) < 0)
>              return ret;
>      }
>  
>      for (i = 0; i < nb_channels; i++) {
> -        uint64_t channel = av_channel_layout_extract_channel(channel_layout, i);
> -        AVFilterPad pad  = { 0 };
> +        int channel = av_channel_layout_channel_from_index(&channel_layout, i);
> +        char buf[64];
> +        AVFilterPad pad = { .flags = AVFILTERPAD_FLAG_FREE_NAME };
>  
> +        av_channel_name(buf, sizeof(buf), channel);
>          pad.type = AVMEDIA_TYPE_AUDIO;
> -        pad.name = av_get_channel_name(channel);
> +        pad.name = av_strdup(buf);
> +        if (!pad.name)
> +            return AVERROR(ENOMEM);
>  
>          if (all) {
>              s->map[i] = i;

Should either make map dynamically allocated or check that there are
fewer than 64 channels in the layout.

>          } else {
> -            if ((ret = av_get_channel_layout_channel_index(s->channel_layout, channel)) < 0) {
> +            if ((ret = av_channel_layout_index_from_channel(&s->channel_layout, channel)) < 0) {
>                  av_log(ctx, AV_LOG_ERROR, "Channel name '%s' not present in channel layout '%s'.\n",
> -                       av_get_channel_name(channel), s->channel_layout_str);
> +                       pad.name, s->channel_layout_str);
> +                av_freep(&pad.name);
>                  return ret;
>              }
>  
> @@ -115,15 +120,18 @@ static int query_formats(AVFilterContext *ctx)
>          (ret = ff_set_common_all_samplerates(ctx)) < 0)
>          return ret;
>  
> -    if ((ret = ff_add_channel_layout(&in_layouts, s->channel_layout)) < 0 ||
> +    if ((ret = ff_add_channel_layout(&in_layouts, &s->channel_layout)) < 0 ||
>          (ret = ff_channel_layouts_ref(in_layouts, &ctx->inputs[0]->outcfg.channel_layouts)) < 0)
>          return ret;
>  
>      for (i = 0; i < ctx->nb_outputs; i++) {
> +        AVChannelLayout channel_layout = { 0 };
>          AVFilterChannelLayouts *out_layouts = NULL;
> -        uint64_t channel = av_channel_layout_extract_channel(s->channel_layout, s->map[i]);
> +        int channel = av_channel_layout_channel_from_index(&s->channel_layout, s->map[i]);
>  
> -        if ((ret = ff_add_channel_layout(&out_layouts, channel)) < 0 ||
> +        if ((channel < 0) ||
> +            (ret = av_channel_layout_from_mask(&channel_layout, 1ULL << channel)) < 0 ||
> +            (ret = ff_add_channel_layout(&out_layouts, &channel_layout)) < 0 ||
>              (ret = ff_channel_layouts_ref(out_layouts, &ctx->outputs[i]->incfg.channel_layouts)) < 0)
>              return ret;
>      }
> @@ -139,6 +147,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *buf)
>  
>      for (i = 0; i < ctx->nb_outputs; i++) {
>          AVFrame *buf_out = av_frame_clone(buf);
> +        int channel = av_channel_layout_channel_from_index(&buf->ch_layout, s->map[i]);
>  
>          if (!buf_out) {
>              ret = AVERROR(ENOMEM);
> @@ -146,9 +155,16 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *buf)
>          }
>  
>          buf_out->data[0] = buf_out->extended_data[0] = buf_out->extended_data[s->map[i]];
> +        ret = av_channel_layout_from_mask(&buf_out->ch_layout, 1ULL << channel);

potential invalid shift?

> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index 7362bcdab5..acb2d7db51 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -204,6 +204,7 @@ void avfilter_link_free(AVFilterLink **link)
>  
>      ff_framequeue_free(&(*link)->fifo);
>      ff_frame_pool_uninit((FFFramePool**)&(*link)->frame_pool);
> +    av_channel_layout_uninit(&(*link)->ch_layout);
>  
>      av_freep(link);
>  }
> @@ -405,7 +406,7 @@ void ff_tlog_link(void *ctx, AVFilterLink *link, int end)
>                  end ? "\n" : "");
>      } else {
>          char buf[128];
> -        av_get_channel_layout_string(buf, sizeof(buf), -1, link->channel_layout);
> +        av_channel_layout_describe(&link->ch_layout, buf, sizeof(buf));
>  
>          ff_tlog(ctx,
>                  "link[%p r:%d cl:%s fmt:%s %s->%s]%s",
> @@ -1036,11 +1037,21 @@ int ff_filter_frame(AVFilterLink *link, AVFrame *frame)
>              av_log(link->dst, AV_LOG_ERROR, "Format change is not supported\n");
>              goto error;
>          }
> -        if (frame->channels != link->channels) {
> +#if FF_API_OLD_CHANNEL_LAYOUT
> +FF_DISABLE_DEPRECATION_WARNINGS
> +        if (frame->channels != link->ch_layout.nb_channels) {
>              av_log(link->dst, AV_LOG_ERROR, "Channel count change is not supported\n");
>              goto error;
>          }
> -        if (frame->channel_layout != link->channel_layout) {
> +        if (frame->channel_layout && frame->channel_layout != link->channel_layout) {
> +            av_log(link->dst, AV_LOG_ERROR, "Channel layout change is not supported\n");
> +            goto error;
> +        }
> +FF_ENABLE_DEPRECATION_WARNINGS
> +        if (av_channel_layout_check(&frame->ch_layout) && av_channel_layout_compare(&frame->ch_layout, &link->ch_layout)) {
> +#else
> +        if (av_channel_layout_compare(&frame->ch_layout, &link->ch_layout)) {
> +#endif

Is all this still needed? Presumably after this patch all sources make
sure the old and new fields are in sync, so we could just assert here,
or not check at all.

> diff --git a/libavfilter/src_movie.c b/libavfilter/src_movie.c
> index b89a680883..3ebd279df7 100644
> --- a/libavfilter/src_movie.c
> +++ b/libavfilter/src_movie.c
> @@ -189,23 +189,24 @@ static int guess_channel_layout(MovieStream *st, int st_index, void *log_ctx)
>  {
>      AVCodecParameters *dec_par = st->st->codecpar;
>      char buf[256];
> -    int64_t chl = av_get_default_channel_layout(dec_par->channels);
> +    AVChannelLayout chl = { 0 };
>  
> -    if (!chl) {
> +    av_channel_layout_default(&chl, dec_par->ch_layout.nb_channels);
> +
> +    if (!KNOWN(&chl)) {
>          av_log(log_ctx, AV_LOG_ERROR,
>                 "Channel layout is not set in stream %d, and could not "
>                 "be guessed from the number of channels (%d)\n",
> -               st_index, dec_par->channels);
> +               st_index, dec_par->ch_layout.nb_channels);
>          return AVERROR(EINVAL);

Should this still be an error? Unspec layouts should now be properly
supported by (almost?) everything.

-- 
Anton Khirnov


More information about the ffmpeg-devel mailing list