[FFmpeg-devel] [PATCH 1/6] avfilter/af_channelmap: fix error message if FL source channel was missing

[FFmpeg-devel] [PATCH 1/6] avfilter/af_channelmap: fix error message if FL source channel was missing

[Marton Balint]

FL channel ID is 0, so for an unset value we must check for ID < 0.

Regression since 1f96db959c1235bb7079d354e09914a0a2608f62.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavfilter/af_channelmap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavfilter/af_channelmap.c b/libavfilter/af_channelmap.c
index 5e1cf57680..d13dcc317d 100644
--- a/libavfilter/af_channelmap.c
+++ b/libavfilter/af_channelmap.c
@@ -165,7 +165,7 @@ static av_cold int channelmap_init(AVFilterContext *ctx)
 
     for (i = 0; i < map_entries; i++) {
         int in_ch_idx = -1, out_ch_idx = -1;
-        int in_ch = 0, out_ch = 0;
+        int in_ch = -1, out_ch = -1;
         static const char err[] = "Failed to parse channel map\n";
         switch (mode) {
         case MAP_ONE_INT:
@@ -349,7 +349,7 @@ static int channelmap_config_input(AVFilterLink *inlink)
 
         if (m->in_channel_idx < 0 || m->in_channel_idx >= nb_channels) {
             av_channel_layout_describe(&inlink->ch_layout, layout_name, sizeof(layout_name));
-            if (m->in_channel) {
+            if (m->in_channel >= 0) {
                 av_channel_name(channel_name, sizeof(channel_name), m->in_channel);
                 av_log(ctx, AV_LOG_ERROR,
                        "input channel '%s' not available from input layout '%s'\n",
-- 
2.35.3

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/6] avfilter/af_channelmap: fix mapping if in_channel was a string but out_channel was not specified

[Marton Balint]

In this case in_channel_idx was never set and the default 0 was used.
Suprisingly no one noticed that the respective fate test output was wrong.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavfilter/af_channelmap.c | 2 +-
 tests/fate/filter-audio.mak | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavfilter/af_channelmap.c b/libavfilter/af_channelmap.c
index d13dcc317d..35dc4c4618 100644
--- a/libavfilter/af_channelmap.c
+++ b/libavfilter/af_channelmap.c
@@ -342,7 +342,7 @@ static int channelmap_config_input(AVFilterLink *inlink)
     for (i = 0; i < s->nch; i++) {
         struct ChannelMap *m = &s->map[i];
 
-        if (s->mode == MAP_PAIR_STR_INT || s->mode == MAP_PAIR_STR_STR) {
+        if (s->mode == MAP_PAIR_STR_INT || s->mode == MAP_PAIR_STR_STR || s->mode == MAP_ONE_STR) {
             m->in_channel_idx = av_channel_layout_index_from_channel(
                 &inlink->ch_layout, m->in_channel);
         }
diff --git a/tests/fate/filter-audio.mak b/tests/fate/filter-audio.mak
index 308969c4ac..88299a932f 100644
--- a/tests/fate/filter-audio.mak
+++ b/tests/fate/filter-audio.mak
@@ -302,7 +302,7 @@ fate-filter-channelmap-one-str: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.w
 fate-filter-channelmap-one-str: tests/data/asynth-44100-2.wav
 fate-filter-channelmap-one-str: CMD = md5 -auto_conversion_filters -i $(SRC) -/filter_complex $(TARGET_PATH)/tests/data/filtergraphs/channelmap_one_str -f wav -fflags +bitexact
 fate-filter-channelmap-one-str: CMP = oneline
-fate-filter-channelmap-one-str: REF = 0ea3052e482c95d5d3bd9da6dac1b5fa
+fate-filter-channelmap-one-str: REF = e18791f65ce5861e130b2c3e472ab90a
 
 FATE_AFILTER-$(call FILTERDEMDECENCMUX, CHANNELMAP ARESAMPLE, WAV, PCM_S16LE, PCM_S16LE, WAV) += $(FATE_FILTER_CHANNELMAP)
 
-- 
2.35.3

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/6] avfilter/af_channelmap: disallow channel index 64

[Marton Balint]

MAX_CH is 64, therefore the maximum index is 63.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavfilter/af_channelmap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavfilter/af_channelmap.c b/libavfilter/af_channelmap.c
index 35dc4c4618..1ecbddd462 100644
--- a/libavfilter/af_channelmap.c
+++ b/libavfilter/af_channelmap.c
@@ -84,7 +84,7 @@ static char* split(char *message, char delim) {
     return next;
 }
 
-static int get_channel_idx(char **map, int *ch, char delim, int max_ch)
+static int get_channel_idx(char **map, int *ch, char delim, int max_nb_channels)
 {
     char *next;
     int len;
@@ -98,7 +98,7 @@ static int get_channel_idx(char **map, int *ch, char delim, int max_ch)
     sscanf(*map, "%d%n", ch, &n);
     if (n != len)
         return AVERROR(EINVAL);
-    if (*ch < 0 || *ch > max_ch)
+    if (*ch < 0 || *ch >= max_nb_channels)
         return AVERROR(EINVAL);
     *map = next;
     return 0;
-- 
2.35.3

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 4/6] avfilter/af_channelmap: factorize checking indexes against a channel layout

[Marton Balint]

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavfilter/af_channelmap.c | 41 ++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/libavfilter/af_channelmap.c b/libavfilter/af_channelmap.c
index 1ecbddd462..62e11103ad 100644
--- a/libavfilter/af_channelmap.c
+++ b/libavfilter/af_channelmap.c
@@ -116,6 +116,30 @@ static int get_channel(char **map, int *ch, char delim)
     return 0;
 }
 
+static int check_idx_and_id(AVFilterContext *ctx, int channel_idx, int channel, AVChannelLayout *ch_layout, const char *io)
+{
+    char channel_name[64];
+    char layout_name[256];
+    int nb_channels = ch_layout->nb_channels;
+
+    if (channel_idx < 0 || channel_idx >= nb_channels) {
+        av_channel_layout_describe(ch_layout, layout_name, sizeof(layout_name));
+        if (channel >= 0) {
+            av_channel_name(channel_name, sizeof(channel_name), channel);
+            av_log(ctx, AV_LOG_ERROR,
+                   "%sput channel '%s' not available from %sput layout '%s'\n",
+                   io, channel_name, io, layout_name);
+        } else {
+            av_log(ctx, AV_LOG_ERROR,
+                   "%sput channel #%d not available from %sput layout '%s'\n",
+                   io, channel_idx, io, layout_name);
+        }
+        return AVERROR(EINVAL);
+    }
+
+    return 0;
+}
+
 static av_cold int channelmap_init(AVFilterContext *ctx)
 {
     ChannelMapContext *s = ctx->priv;
@@ -334,10 +358,7 @@ static int channelmap_config_input(AVFilterLink *inlink)
 {
     AVFilterContext *ctx = inlink->dst;
     ChannelMapContext *s = ctx->priv;
-    int nb_channels = inlink->ch_layout.nb_channels;
     int i, err = 0;
-    char channel_name[64];
-    char layout_name[256];
 
     for (i = 0; i < s->nch; i++) {
         struct ChannelMap *m = &s->map[i];
@@ -347,20 +368,8 @@ static int channelmap_config_input(AVFilterLink *inlink)
                 &inlink->ch_layout, m->in_channel);
         }
 
-        if (m->in_channel_idx < 0 || m->in_channel_idx >= nb_channels) {
-            av_channel_layout_describe(&inlink->ch_layout, layout_name, sizeof(layout_name));
-            if (m->in_channel >= 0) {
-                av_channel_name(channel_name, sizeof(channel_name), m->in_channel);
-                av_log(ctx, AV_LOG_ERROR,
-                       "input channel '%s' not available from input layout '%s'\n",
-                       channel_name, layout_name);
-            } else {
-                av_log(ctx, AV_LOG_ERROR,
-                       "input channel #%d not available from input layout '%s'\n",
-                       m->in_channel_idx, layout_name);
-            }
+        if (check_idx_and_id(ctx, m->in_channel_idx, m->in_channel, &inlink->ch_layout, "in") < 0)
             err = AVERROR(EINVAL);
-        }
     }
 
     return err;
-- 
2.35.3

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 5/6] avfilter/af_channelmap: add some additional checks for the mappings

[Marton Balint]

- Properly initialize all the mappings to -1 by default.
- Make sure every output channel is assigned exactly once
- Autodetect a native layout when only native channels are present
- Always honor the user specified layout, but make sure the mapping is
  compatible with it

The last item is a regression from 4af412be7153405e43930ac73fc7d91d7ded19eb.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavfilter/af_channelmap.c | 53 +++++++++++++++++++++++++++++--------
 1 file changed, 42 insertions(+), 11 deletions(-)

diff --git a/libavfilter/af_channelmap.c b/libavfilter/af_channelmap.c
index 62e11103ad..64efacb576 100644
--- a/libavfilter/af_channelmap.c
+++ b/libavfilter/af_channelmap.c
@@ -146,7 +146,8 @@ static av_cold int channelmap_init(AVFilterContext *ctx)
     char *mapping, separator = '|';
     int map_entries = 0;
     enum MappingMode mode;
-    uint64_t out_ch_mask = 0;
+    int64_t out_ch_mask = 0;
+    uint64_t presence_mask;
     int i;
 
     mapping = s->mapping_str;
@@ -187,6 +188,13 @@ static av_cold int channelmap_init(AVFilterContext *ctx)
         return AVERROR(EINVAL);
     }
 
+    for (i = 0; i < MAX_CH; i++) {
+        s->map[i].in_channel_idx  = -1;
+        s->map[i].out_channel_idx = -1;
+        s->map[i].in_channel      = -1;
+        s->map[i].out_channel     = -1;
+    }
+
     for (i = 0; i < map_entries; i++) {
         int in_ch_idx = -1, out_ch_idx = -1;
         int in_ch = -1, out_ch = -1;
@@ -219,14 +227,16 @@ static av_cold int channelmap_init(AVFilterContext *ctx)
             break;
         case MAP_PAIR_INT_STR:
             if (get_channel_idx(&mapping, &in_ch_idx, '-', MAX_CH) < 0 ||
-                get_channel(&mapping, &out_ch, separator) < 0 ||
-                (1ULL << out_ch) & out_ch_mask) {
+                get_channel(&mapping, &out_ch, separator) < 0) {
                 av_log(ctx, AV_LOG_ERROR, err);
                 return AVERROR(EINVAL);
             }
             s->map[i].in_channel_idx  = in_ch_idx;
             s->map[i].out_channel     = out_ch;
-            out_ch_mask |= 1ULL << out_ch;
+            if (out_ch < 63)
+                out_ch_mask |= 1ULL << out_ch;
+            else
+                out_ch_mask = -1;
             break;
         case MAP_PAIR_STR_INT:
             if (get_channel(&mapping, &in_ch, '-') < 0 ||
@@ -239,23 +249,27 @@ static av_cold int channelmap_init(AVFilterContext *ctx)
             break;
         case MAP_PAIR_STR_STR:
             if (get_channel(&mapping, &in_ch, '-') < 0 ||
-                get_channel(&mapping, &out_ch, separator) < 0 ||
-                (1ULL << out_ch) & out_ch_mask) {
+                get_channel(&mapping, &out_ch, separator) < 0) {
                 av_log(ctx, AV_LOG_ERROR, err);
                 return AVERROR(EINVAL);
             }
             s->map[i].in_channel = in_ch;
             s->map[i].out_channel = out_ch;
-            out_ch_mask |= 1ULL << out_ch;
+            if (out_ch < 63)
+                out_ch_mask |= 1ULL << out_ch;
+            else
+                out_ch_mask = -1;
             break;
         }
     }
     s->mode          = mode;
     s->nch           = map_entries;
-    if (out_ch_mask)
-        av_channel_layout_from_mask(&s->output_layout, out_ch_mask);
-    else if (map_entries && s->output_layout.nb_channels == 0)
-        av_channel_layout_default(&s->output_layout, map_entries);
+    if (s->output_layout.nb_channels == 0) {
+        if (out_ch_mask > 0)
+            av_channel_layout_from_mask(&s->output_layout, out_ch_mask);
+        else if (map_entries)
+            av_channel_layout_default(&s->output_layout, map_entries);
+    }
 
     if (mode == MAP_NONE) {
         int i;
@@ -286,6 +300,23 @@ static av_cold int channelmap_init(AVFilterContext *ctx)
         }
     }
 
+    presence_mask = 0;
+    for (i = 0; i < s->nch; i++) {
+        uint64_t idx_mask;
+        int ret = check_idx_and_id(ctx, s->map[i].out_channel_idx, s->map[i].out_channel, &s->output_layout, "out");
+        if (ret < 0)
+            return ret;
+        idx_mask = (1ULL << s->map[i].out_channel_idx);
+        if (presence_mask & idx_mask) {
+            char layout_name[256];
+            av_channel_layout_describe(&s->output_layout, layout_name, sizeof(layout_name));
+            av_log(ctx, AV_LOG_ERROR, "Mapping %d assigns channel #%d twice in output layout '%s'.\n",
+                   i + 1, s->map[i].out_channel_idx, layout_name);
+            return AVERROR(EINVAL);
+        }
+        presence_mask |= idx_mask;
+    }
+
     return 0;
 }
 
-- 
2.35.3

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 6/6] doc/filters: extend af_channelmap documentation

[Marton Balint]

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 doc/filters.texi | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index 913365671d..18f0d1c5a7 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -4291,14 +4291,18 @@ It accepts the following parameters:
 @item map
 Map channels from input to output. The argument is a '|'-separated list of
 mappings, each in the @code{@var{in_channel}-@var{out_channel}} or
-@var{in_channel} form. @var{in_channel} can be either the name of the input
-channel (e.g. FL for front left) or its index in the input channel layout.
-@var{out_channel} is the name of the output channel or its index in the output
-channel layout. If @var{out_channel} is not given then it is implicitly an
-index, starting with zero and increasing by one for each mapping.
+@code{@var{in_channel}} form. @var{in_channel} can be either the name of the
+input channel (e.g. FL for front left) or its index in the input channel
+layout. @var{out_channel} is the name of the output channel or its index in the
+output channel layout. If @var{out_channel} is not given then it is implicitly
+an index, starting with zero and increasing by one for each mapping. Mixing
+different types of mappings is not allowed and will result in a parse error.
 
 @item channel_layout
-The channel layout of the output stream.
+The channel layout of the output stream. If not specified, then filter will
+guess it based on the @var{out_channel} names or the number of mappings.
+Guessed layouts will not necessarily contain channels in the order of the
+mappings.
 @end table
 
 If no mapping is present, the filter will implicitly map input channels to
-- 
2.35.3

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/6] avfilter/af_channelmap: fix error message if FL source channel was missing

[Marton Balint]

On Tue, 19 Mar 2024, Marton Balint wrote:

> FL channel ID is 0, so for an unset value we must check for ID < 0.
>
> Regression since 1f96db959c1235bb7079d354e09914a0a2608f62.

Will apply the series soon.

Regards,
Marton

>
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
> libavfilter/af_channelmap.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavfilter/af_channelmap.c b/libavfilter/af_channelmap.c
> index 5e1cf57680..d13dcc317d 100644
> --- a/libavfilter/af_channelmap.c
> +++ b/libavfilter/af_channelmap.c
> @@ -165,7 +165,7 @@ static av_cold int channelmap_init(AVFilterContext *ctx)
>
>     for (i = 0; i < map_entries; i++) {
>         int in_ch_idx = -1, out_ch_idx = -1;
> -        int in_ch = 0, out_ch = 0;
> +        int in_ch = -1, out_ch = -1;
>         static const char err[] = "Failed to parse channel map\n";
>         switch (mode) {
>         case MAP_ONE_INT:
> @@ -349,7 +349,7 @@ static int channelmap_config_input(AVFilterLink *inlink)
>
>         if (m->in_channel_idx < 0 || m->in_channel_idx >= nb_channels) {
>             av_channel_layout_describe(&inlink->ch_layout, layout_name, sizeof(layout_name));
> -            if (m->in_channel) {
> +            if (m->in_channel >= 0) {
>                 av_channel_name(channel_name, sizeof(channel_name), m->in_channel);
>                 av_log(ctx, AV_LOG_ERROR,
>                        "input channel '%s' not available from input layout '%s'\n",
> -- 
> 2.35.3
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/6] avfilter/af_channelmap: fix mapping if in_channel was a string but out_channel was not specified

[Paul B Mahol]

Please add coverage for regression cases in FATE.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".