diff mbox

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

Message ID 20180321230220.13585-1-onemda@gmail.com
State Accepted
Commit 4e1307c0f76d015941a1217bcd44a20f25e04fad
Headers show

Commit Message

Paul B Mahol March 21, 2018, 11:02 p.m. UTC
So user can pick which channels to extract.

Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 doc/filters.texi              | 18 ++++++++++++++++++
 libavfilter/af_channelsplit.c | 39 +++++++++++++++++++++++++++++++++------
 2 files changed, 51 insertions(+), 6 deletions(-)

Comments

Nicolas George March 22, 2018, 3:04 p.m. UTC | #1
Paul B Mahol (2018-03-22):
> So user can pick which channels to extract.
> 
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  doc/filters.texi              | 18 ++++++++++++++++++
>  libavfilter/af_channelsplit.c | 39 +++++++++++++++++++++++++++++++++------
>  2 files changed, 51 insertions(+), 6 deletions(-)

Thanks for simplifying the code. I have no more remarks. But Alexander
commented too, so please let him time to follow-up.

Regards,
Alexander Strasser March 22, 2018, 11:56 p.m. UTC | #2
On 2018-03-22 16:04 +0100, Nicolas George wrote:
> Paul B Mahol (2018-03-22):
> > So user can pick which channels to extract.
> > 
> > Signed-off-by: Paul B Mahol <onemda@gmail.com>
> > ---
> >  doc/filters.texi              | 18 ++++++++++++++++++
> >  libavfilter/af_channelsplit.c | 39 +++++++++++++++++++++++++++++++++------
> >  2 files changed, 51 insertions(+), 6 deletions(-)
> 
> Thanks for simplifying the code. I have no more remarks. But Alexander
> commented too, so please let him time to follow-up.

I still think checking the number of channels to be less then or
equal to the size of map is more robust.

There are two things that could change in future

1. the size of map (for whatever reason; maybe someone thinks 16
channels are max or similar...)
2. the number of channels we support could be increased

As each channel is represented by a bit in a 64 bit integer, it seems
kind of unlikely we get more channels soon. So I won't insist on the
check to be added.

Patch LGTM.


Thank you,
  Alexander
diff mbox

Patch

diff --git a/doc/filters.texi b/doc/filters.texi
index aa7d4fc2d9..14e3b74a83 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -2208,8 +2208,17 @@  It accepts the following parameters:
 @table @option
 @item channel_layout
 The channel layout of the input stream. The default is "stereo".
+@item channels
+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".
+
+Choosing channels not present in channel layout in the input will result in an error.
 @end table
 
+@subsection Examples
+
+@itemize
+@item
 For example, assuming a stereo input MP3 file,
 @example
 ffmpeg -i in.mp3 -filter_complex channelsplit out.mkv
@@ -2217,6 +2226,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.
 
+@item
 Split a 5.1 WAV file into per-channel files:
 @example
 ffmpeg -i in.wav -filter_complex
@@ -2226,6 +2236,14 @@  front_center.wav -map '[LFE]' lfe.wav -map '[SL]' side_left.wav -map '[SR]'
 side_right.wav
 @end example
 
+@item
+Extract only LFE from a 5.1 WAV file:
+@example
+ffmpeg -i in.wav -filter_complex 'channelsplit=channel_layout=5.1:channels=LFE[LFE]'
+-map '[LFE]' lfe.wav
+@end example
+@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..821bb73801 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 }
 };
 
@@ -53,8 +57,9 @@  AVFILTER_DEFINE_CLASS(channelsplit);
 static av_cold int init(AVFilterContext *ctx)
 {
     ChannelSplitContext *s = ctx->priv;
+    uint64_t channel_layout;
     int nb_channels;
-    int ret = 0, i;
+    int all = 0, ret = 0, i;
 
     if (!(s->channel_layout = av_get_channel_layout(s->channel_layout_str))) {
         av_log(ctx, AV_LOG_ERROR, "Error parsing channel layout '%s'.\n",
@@ -63,14 +68,35 @@  static av_cold int init(AVFilterContext *ctx)
         goto fail;
     }
 
-    nb_channels = av_get_channel_layout_nb_channels(s->channel_layout);
+
+    if (!strcmp(s->channels_str, "all")) {
+        nb_channels = av_get_channel_layout_nb_channels(s->channel_layout);
+        channel_layout = s->channel_layout;
+        all = 1;
+    } else {
+        if ((ret = av_get_extended_channel_layout(s->channels_str, &channel_layout, &nb_channels)) < 0)
+            return ret;
+    }
+
     for (i = 0; i < nb_channels; i++) {
-        uint64_t channel = av_channel_layout_extract_channel(s->channel_layout, i);
+        uint64_t channel = av_channel_layout_extract_channel(channel_layout, i);
         AVFilterPad pad  = { 0 };
 
         pad.type = AVMEDIA_TYPE_AUDIO;
         pad.name = av_get_channel_name(channel);
 
+        if (all) {
+            s->map[i] = i;
+        } else {
+            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;
+        }
+
         if ((ret = ff_insert_outpad(ctx, i, &pad)) < 0) {
             return ret;
         }
@@ -96,7 +122,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 +135,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 +146,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);