avfilter/af_headphone: Simplify parsing channel mapping string

When parsing the channel mapping string (a string containing '|'
delimited tokens each of which is supposed to contain a channel name
like "FR"), the old code would at each step read up to seven uppercase
characters from the input string and give this to
av_get_channel_layout() to parse. The returned layout is then checked
for being a layout with a single channel set by computing its logarithm.

Besides being overtly complicated this also has the drawback of relying
on the assumption that every channel name consists of at most seven
uppercase letters only; but said assumption is wrong: The abbreviation
of the second low frequency channel is LFE2. Furthermore it treats
garbage like "FRfoo" as valid channel.

This commit changes this by using av_get_channel_layout() directly;
furthermore, av_get_channel_layout_nb_channels() (which uses popcount)
is used to find out the number of channels instead of the custom code
to calculate the logarithm.

(As a consequence, certain other formats to specify the channel layouts
are now accepted (like the hex versions of av_get_channel_layout()); but
this is actually not bad at all.)

Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>

diff --git a/libavfilter/af_headphone.c b/libavfilter/af_headphone.c
index 8db712e9a04f24e7e4aa343d71de10e61ecb3ed6..32939af8546e3e5ed5fc292a7013fe05dee67ac3 100644 (file)
--- a/libavfilter/af_headphone.c
+++ b/libavfilter/af_headphone.c
@@ -87,26 +87,14 @@ typedef struct HeadphoneContext {
     uint64_t mapping[64];
 } HeadphoneContext;
 
-static int parse_channel_name(char **arg, uint64_t *rchannel, char *buf)
+static int parse_channel_name(const char *arg, uint64_t *rchannel)
 {
-    int len, i, channel_id = 0;
-    uint64_t layout, layout0;
-
-    if (sscanf(*arg, "%7[A-Z]%n", buf, &len)) {
-        layout0 = layout = av_get_channel_layout(buf);
-        for (i = 32; i > 0; i >>= 1) {
-            if (layout >= 1LL << i) {
-                channel_id += i;
-                layout >>= i;
-            }
-        }
-        if (channel_id >= 64 || layout0 != 1ULL << channel_id)
-            return AVERROR(EINVAL);
-        *rchannel = layout0;
-        *arg += len;
-        return 0;
-    }
-    return AVERROR(EINVAL);
+    uint64_t layout = av_get_channel_layout(arg);
+
+    if (av_get_channel_layout_nb_channels(layout) != 1)
+        return AVERROR(EINVAL);
+    *rchannel = layout;
+    return 0;
 }
 
 static void parse_map(AVFilterContext *ctx)
@@ -124,15 +112,14 @@ static void parse_map(AVFilterContext *ctx)
 
     while ((arg = av_strtok(p, "|", &tokenizer))) {
         uint64_t out_channel;
-        char buf[8];
 
         p = NULL;
-        if (parse_channel_name(&arg, &out_channel, buf)) {
+        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) {
-            av_log(ctx, AV_LOG_WARNING, "Ignoring duplicate channel '%s'.\n", buf);
+            av_log(ctx, AV_LOG_WARNING, "Ignoring duplicate channel '%s'.\n", arg);
             continue;
         }
         used_channels        |= out_channel;