CRAS: apm_list - Construct channel layout that works with APM

For input device we could have two channels PCM with the first
channel unconnected. And that introduces a bug in APM use case
because APM processing only uses data in the first channel.
This change fixes the issue by always reconstruct a channel
layout that works with APM.

BUG=b:181818480
TEST=Test online voice recording on 'barla' device.
Unittest to verify all other possible capture channel maps
works.

Change-Id: I1e6ba79d2752a7a541bfbea91dd96bf0f9d38f4d
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/2755524
Reviewed-by: Cheng-Yi Chiang <cychiang@chromium.org>
Commit-Queue: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>

2 files changed, 67 insertions, 7 deletions

diff --git a/cras/src/server/cras_apm_list.c b/cras/src/server/cras_apm_list.c
index ac57d86a..264bc85e 100644
--- a/cras/src/server/cras_apm_list.c
+++ b/cras/src/server/cras_apm_list.c
@@ -239,13 +239,12 @@ static void get_best_channels(struct cras_audio_format *apm_fmt)
 	int ch;
 	int8_t layout[CRAS_CH_MAX];
 
-	/* Assume device format has correct channel layout populated. */
-	if (apm_fmt->num_channels <= 2)
-		return;
-
-	/* If the device provides recording from more channels than we care
-	 * about, construct a new channel layout containing subset of original
-	 * channels that matches either FL, FR, or FC.
+	/* Using the format from dev_fmt is dangerous because input device
+	 * could have wild configurations like unuse the 1st channel and
+	 * connects 2nd channel to the only mic. Data in the first channel
+	 * is what APM cares about so always construct a new channel layout
+	 * containing subset of original channels that matches either FL, FR,
+	 * or FC.
 	 * TODO(hychao): extend the logic when we have a stream that wants
 	 * to record channels like RR(rear right).
 	 */
diff --git a/cras/src/tests/apm_list_unittest.cc b/cras/src/tests/apm_list_unittest.cc
index 09c7b866..65e712fb 100644
--- a/cras/src/tests/apm_list_unittest.cc
+++ b/cras/src/tests/apm_list_unittest.cc
@@ -79,6 +79,64 @@ static void delete_tempdir(char* dir) {
   rmdir(dir);
 }
 
+static void init_channel_layout(struct cras_audio_format* fmt) {
+  int i;
+  for (i = 0; i < CRAS_CH_MAX; i++)
+    fmt->channel_layout[i] = -1;
+}
+
+TEST(ApmList, AddApmInputDevUnuseFirstChannel) {
+  struct cras_audio_format fmt;
+  struct cras_audio_format* val;
+  struct cras_apm* apm;
+  int ch;
+  const int num_test_casts = 9;
+  int test_layouts[num_test_casts][CRAS_CH_MAX] = {
+      {0, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1},
+      {0, 0, -1, -1, -1, -1, -1, -1, -1, -1, -1},
+      {0, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},
+      {1, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},
+      {1, 0, -1, -1, -1, -1, -1, -1, -1, -1, -1},
+      {2, 2, -1, -1, -1, -1, -1, -1, -1, -1, -1},
+      {2, 3, -1, -1, -1, -1, -1, -1, -1, -1, -1},
+      {3, 3, -1, -1, -1, -1, -1, -1, -1, -1, -1},
+      {3, 2, -1, -1, -1, -1, -1, -1, -1, -1, -1}};
+  int test_num_channels[num_test_casts] = {1, 2, 2, 2, 2, 3, 4, 4, 4};
+
+  fmt.frame_rate = 48000;
+  fmt.format = SND_PCM_FORMAT_S16_LE;
+
+  cras_apm_list_init("");
+  list = cras_apm_list_create(stream_ptr, APM_ECHO_CANCELLATION);
+  EXPECT_NE((void*)NULL, list);
+
+  for (int i = 0; i < num_test_casts; i++) {
+    fmt.num_channels = test_num_channels[i];
+    init_channel_layout(&fmt);
+    for (ch = 0; ch < CRAS_CH_MAX; ch++)
+      fmt.channel_layout[ch] = test_layouts[i][ch];
+
+    /* Input dev is of aec use case. */
+    apm = cras_apm_list_add_apm(list, dev_ptr, &fmt, 1);
+    EXPECT_NE((void*)NULL, apm);
+
+    /* Assert that the post-processing format never has an unset
+     * first channel in the layout. */
+    bool first_channel_found_in_layout = 0;
+    val = cras_apm_list_get_format(apm);
+    for (ch = 0; ch < CRAS_CH_MAX; ch++)
+      if (0 == val->channel_layout[ch])
+        first_channel_found_in_layout = 1;
+
+    EXPECT_EQ(1, first_channel_found_in_layout);
+
+    cras_apm_list_remove_apm(list, dev_ptr);
+  }
+
+  cras_apm_list_destroy(list);
+  cras_apm_list_deinit();
+}
+
 TEST(ApmList, AddRemoveApm) {
   struct cras_audio_format fmt;
   char* dir;
@@ -170,6 +228,9 @@ TEST(ApmList, ApmProcessForwardBuffer) {
   fmt.num_channels = 2;
   fmt.frame_rate = 48000;
   fmt.format = SND_PCM_FORMAT_S16_LE;
+  init_channel_layout(&fmt);
+  fmt.channel_layout[CRAS_CH_FL] = 0;
+  fmt.channel_layout[CRAS_CH_FR] = 1;
 
   cras_apm_list_init("");