[FFmpeg-devel] [PATCH] avfilter/af_channelsplit: add channels option

[Alexander Strasser]

Hi Paul!

On 2018-03-20 19:11 +0100, Paul B Mahol wrote:
> So user can pick which channels to extract.
> 
> Signed-off-by: Paul B Mahol <onemda at gmail.com>
> ---
>  doc/filters.texi              | 15 ++++++++++++
>  libavfilter/af_channelsplit.c | 54 +++++++++++++++++++++++++++++++++++--------
>  2 files changed, 60 insertions(+), 9 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index bd43a7ac6e..81310e1cdf 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -2208,8 +2208,14 @@ It accepts the following parameters:
>  @table @option
>  @item channel_layout
>  The channel layout of the input stream. The default is "stereo".
> + at item channels
> +The channel layout of the channels for extraction. The default is "all".

  This sentence feels a bit too in-accurate to me. Maybe like this:

  A channel layout describing the channels to be extracted as separate output streams or "all" to extract each input channel as a separate stream. The default is "all".

  While writing this, I think maybe another way to implement the default
would be to set the channels to the channel_layout of the input stream.
So the documentation would read:

  A channel layout describing the channels to be extracted as separate output streams. Defaults to the channel layout of the input stream.

  Would also simplify the implementation. But I may sure be missing something...

>  @end table
>  
> + at subsection Examples
> +
> + at itemize
> + at item
>  For example, assuming a stereo input MP3 file,
>  @example
>  ffmpeg -i in.mp3 -filter_complex channelsplit out.mkv
> @@ -2217,6 +2223,7 @@ ffmpeg -i in.mp3 -filter_complex channelsplit out.mkv
>  will create an output Matroska file with two audio streams, one containing only
>  the left channel and the other the right channel.
>  
> + at item
>  Split a 5.1 WAV file into per-channel files:
>  @example
>  ffmpeg -i in.wav -filter_complex
> @@ -2226,6 +2233,14 @@ front_center.wav -map '[LFE]' lfe.wav -map '[SL]' side_left.wav -map '[SR]'
>  side_right.wav
>  @end example
>  
> + at item
> +Extract LFE from a 5.1 WAV file:
> + at example
> +ffmpeg -i in.wav -filter_complex 'channelsplit=channel_layout=5.1:LFE[LFE]'
> +-map '[LFE]' lfe.wav
> + at end example
> + at end itemize
> +
>  @section chorus
>  Add a chorus effect to the audio.
>  
> diff --git a/libavfilter/af_channelsplit.c b/libavfilter/af_channelsplit.c
> index 8c6b00fe4f..d9b9a60420 100644
> --- a/libavfilter/af_channelsplit.c
> +++ b/libavfilter/af_channelsplit.c
> @@ -38,6 +38,9 @@ typedef struct ChannelSplitContext {
>  
>      uint64_t channel_layout;
>      char    *channel_layout_str;
> +    char    *channels_str;
> +
> +    int      map[64];
>  } ChannelSplitContext;
>  
>  #define OFFSET(x) offsetof(ChannelSplitContext, x)
> @@ -45,6 +48,7 @@ typedef struct ChannelSplitContext {
>  #define F AV_OPT_FLAG_FILTERING_PARAM
>  static const AVOption channelsplit_options[] = {
>      { "channel_layout", "Input channel layout.", OFFSET(channel_layout_str), AV_OPT_TYPE_STRING, { .str = "stereo" }, .flags = A|F },
> +    { "channels",        "Channels to extract.", OFFSET(channels_str),       AV_OPT_TYPE_STRING, { .str = "all" },    .flags = A|F },
>      { NULL }
>  };
>  
> @@ -64,15 +68,46 @@ static av_cold int init(AVFilterContext *ctx)
>      }
>  
>      nb_channels = av_get_channel_layout_nb_channels(s->channel_layout);
> -    for (i = 0; i < nb_channels; i++) {
> -        uint64_t channel = av_channel_layout_extract_channel(s->channel_layout, i);
> -        AVFilterPad pad  = { 0 };
>  
> -        pad.type = AVMEDIA_TYPE_AUDIO;
> -        pad.name = av_get_channel_name(channel);
> +    if (!strcmp(s->channels_str, "all")) {
> +        for (i = 0; i < nb_channels; i++) {
> +            uint64_t channel = av_channel_layout_extract_channel(s->channel_layout, i);
> +            AVFilterPad pad  = { 0 };
> +
> +            pad.type = AVMEDIA_TYPE_AUDIO;
> +            pad.name = av_get_channel_name(channel);
> +
> +            s->map[i] = i;
>  
> -        if ((ret = ff_insert_outpad(ctx, i, &pad)) < 0) {
> +            if ((ret = ff_insert_outpad(ctx, i, &pad)) < 0) {
> +                return ret;
> +            }
> +        }
> +    } else {
> +        uint64_t channel_layout;
> +        int nb_extracted_channels;

Checking nb_channels above and nb_extracted_channels here against 
length of s->map would be more robust.

> +
> +        if ((ret = av_get_extended_channel_layout(s->channels_str, &channel_layout, &nb_extracted_channels)) < 0)
>              return ret;
> +
> +        for (i = 0; i < nb_extracted_channels; i++) {
> +            uint64_t channel = av_channel_layout_extract_channel(channel_layout, i);
> +            AVFilterPad pad  = { 0 };
> +
> +            if ((ret = av_get_channel_layout_channel_index(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);
> +                return ret;
> +            }
> +
> +            s->map[i] = ret;
> +
> +            pad.type = AVMEDIA_TYPE_AUDIO;
> +            pad.name = av_get_channel_name(channel);

Nit: If these assignments are moved above, you could use pad.name
in the av_log before the error return. I do not really care, but
in case you prefer it too...

> +
> +            if ((ret = ff_insert_outpad(ctx, i, &pad)) < 0) {
> +                return ret;
> +            }
>          }
>      }
>  
> @@ -96,7 +131,7 @@ static int query_formats(AVFilterContext *ctx)
>  
>      for (i = 0; i < ctx->nb_outputs; i++) {
>          AVFilterChannelLayouts *out_layouts = NULL;
> -        uint64_t channel = av_channel_layout_extract_channel(s->channel_layout, i);
> +        uint64_t channel = av_channel_layout_extract_channel(s->channel_layout, s->map[i]);
>  
>          if ((ret = ff_add_channel_layout(&out_layouts, channel)) < 0 ||
>              (ret = ff_channel_layouts_ref(out_layouts, &ctx->outputs[i]->in_channel_layouts)) < 0)
> @@ -109,6 +144,7 @@ static int query_formats(AVFilterContext *ctx)
>  static int filter_frame(AVFilterLink *inlink, AVFrame *buf)
>  {
>      AVFilterContext *ctx = inlink->dst;
> +    ChannelSplitContext *s = ctx->priv;
>      int i, ret = 0;
>  
>      for (i = 0; i < ctx->nb_outputs; i++) {
> @@ -119,9 +155,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *buf)
>              break;
>          }
>  
> -        buf_out->data[0] = buf_out->extended_data[0] = buf_out->extended_data[i];
> +        buf_out->data[0] = buf_out->extended_data[0] = buf_out->extended_data[s->map[i]];
>          buf_out->channel_layout =
> -            av_channel_layout_extract_channel(buf->channel_layout, i);
> +            av_channel_layout_extract_channel(buf->channel_layout, s->map[i]);
>          buf_out->channels = 1;
>  
>          ret = ff_filter_frame(ctx->outputs[i], buf_out);
> -- 


Looks like a good improvement for the channelsplit filter.

I only read the patch; I didn't test.


  Alexander