From 257d15cb760fbb2e48949af8cd79a4c85ef808ee Mon Sep 17 00:00:00 2001
From: Syed AbuTalib <lowkey@google.com>
Date: Mon, 27 Apr 2026 14:17:10 -0700
Subject: [PATCH] Enable Channel Mixing for DISCRETE with differing counts

It is still worth using ChannelMixStrategy for `DISCRETE` cases as it is
not a guarantee that the input/output params have the same channel
count.

Also, the unittest file was optimized for testing out a max of stereo
(2) layout, updated the unittests to handle higher channel counts and
mixing between the higher channel counts.

Bug: 501667839
Change-Id: I7e3cacddebb85f7cbe346087f8a58048947b4293
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7759235
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Syed AbuTalib <lowkey@google.com>
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1621314}
---

diff --git a/services/audio/snooper_node.cc b/services/audio/snooper_node.cc
index 06709f97..f4af944 100644
--- a/services/audio/snooper_node.cc
+++ b/services/audio/snooper_node.cc
@@ -73,7 +73,8 @@
           base::BindRepeating(&SnooperNode::ReadFromDelayBuffer,
                               base::Unretained(this))),
       channel_mix_strategy_(
-          (input_params_.channel_layout() == output_params_.channel_layout())
+          (input_params_.channel_layout_config() ==
+           output_params_.channel_layout_config())
               ? ChannelMixStrategy::kNone
               : ((output_params_.channels() < input_params_.channels())
                      ? ChannelMixStrategy::kBefore
diff --git a/services/audio/snooper_node_unittest.cc b/services/audio/snooper_node_unittest.cc
index 9ad44924..945fe20 100644
--- a/services/audio/snooper_node_unittest.cc
+++ b/services/audio/snooper_node_unittest.cc
@@ -32,9 +32,10 @@
 // something finite.
 constexpr float kInvalidAudioSample = std::numeric_limits<float>::infinity();
 
-// The tones the source should generate into the left and right channels.
-constexpr double kLeftChannelFrequency = 500.0;
-constexpr double kRightChannelFrequency = 1200.0;
+// The tones the source should generate into the channels, and the alternate
+// frequency to use when testing swapped/discontinuous input.
+constexpr double kDefaultFrequency = 500.0;
+constexpr double kSwappedFrequency = 1200.0;
 constexpr double kSourceVolume = 0.5;
 
 // The duration of the audio that flows through the SnooperNode for each test.
@@ -50,29 +51,59 @@
 constexpr std::string_view kDumpAsWavSwitch = "dump-as-wav";
 
 // Test parameters.
-struct InputAndOutputParams {
-  media::AudioParameters input;
-  media::AudioParameters output;
-};
+class AudioParametersBuilder {
+ public:
+  AudioParametersBuilder() = default;
 
-// Helper so that gtest can produce useful logging of the test parameters.
-std::ostream& operator<<(std::ostream& out,
-                         const InputAndOutputParams& test_params) {
-  return out << "{input=" << test_params.input.AsHumanReadableString()
-             << ", output=" << test_params.output.AsHumanReadableString()
-             << "}";
-}
+  AudioParametersBuilder& WithLayout(media::ChannelLayout layout,
+                                     int channels) {
+    layout_ = layout;
+    channels_ = channels;
+    return *this;
+  }
+  AudioParametersBuilder& WithRate(int rate) {
+    rate_ = rate;
+    return *this;
+  }
+  AudioParametersBuilder& WithFrames(int frames) {
+    frames_ = frames;
+    return *this;
+  }
+
+  media::AudioParameters Build() const {
+    return media::AudioParameters(
+        media::AudioParameters::AUDIO_PCM_LOW_LATENCY,
+        media::ChannelLayoutConfig(layout_, channels_), rate_, frames_);
+  }
+
+ private:
+  media::ChannelLayout layout_ = media::CHANNEL_LAYOUT_STEREO;
+  int channels_ = 2;
+  int rate_ = 48000;
+  int frames_ = 480;
+};
+struct InputAndOutputParams {
+  AudioParametersBuilder input;
+  AudioParametersBuilder output;
+  std::string name;
+};
 
 class SnooperNodeTest : public testing::TestWithParam<InputAndOutputParams> {
  public:
   SnooperNodeTest() = default;
   ~SnooperNodeTest() override = default;
 
-  const media::AudioParameters& input_params() const {
-    return GetParam().input;
+  media::AudioParameters input_params() {
+    if (!input_params_.IsValid()) {
+      input_params_ = GetParam().input.Build();
+    }
+    return input_params_;
   }
-  const media::AudioParameters& output_params() const {
-    return GetParam().output;
+  media::AudioParameters output_params() {
+    if (!output_params_.IsValid()) {
+      output_params_ = GetParam().output.Build();
+    }
+    return output_params_;
   }
   base::TimeDelta output_delay() const { return output_delay_; }
   double max_relative_error() const { return max_relative_error_; }
@@ -80,6 +111,40 @@
   base::TestMockTimeTaskRunner* task_runner() const {
     return task_runner_.get();
   }
+  // Builds a gain matrix so tests can compute the expected output without
+  // hardcoding the values.
+  void PrecomputeExpectedMixMatrix() {
+    const int in_channels = input_params().channels();
+    const int out_channels = output_params().channels();
+    expected_mix_matrix_.assign(in_channels, std::vector<double>(out_channels));
+
+    auto input_bus = media::AudioBus::Create(in_channels, /*frames=*/1);
+    auto output_bus = media::AudioBus::Create(out_channels, /*frames=*/1);
+    media::ChannelMixer mixer(input_params(), output_params());
+
+    for (int in_ch = 0; in_ch < in_channels; ++in_ch) {
+      input_bus->Zero();
+      input_bus->channel(in_ch)[0] = 1.0f;
+      mixer.Transform(input_bus.get(), output_bus.get());
+      for (int out_ch = 0; out_ch < out_channels; ++out_ch) {
+        expected_mix_matrix_[in_ch][out_ch] =
+            static_cast<double>(output_bus->channel(out_ch)[0]);
+      }
+    }
+  }
+
+  double GetExpectedGain(int input_ch, int output_ch) const {
+    return expected_mix_matrix_[input_ch][output_ch];
+  }
+
+  bool IsOutputChannelSilent(int out_ch) {
+    for (int in_ch = 0; in_ch < input_params().channels(); ++in_ch) {
+      if (GetExpectedGain(in_ch, out_ch) > 0.0) {
+        return false;
+      }
+    }
+    return true;
+  }
   FakeLoopbackGroupMember* group_member() { return &*group_member_; }
   SnooperNode* node() { return &*node_; }
   FakeConsumer* consumer() { return &*consumer_; }
@@ -131,6 +196,8 @@
     // "huge" to ensure time calculations are being tested for overflow cases.
     task_runner_ = base::MakeRefCounted<base::TestMockTimeTaskRunner>(
         base::Time(), base::TimeTicks() + base::Microseconds(INT64_C(1) << 62));
+
+    PrecomputeExpectedMixMatrix();
   }
 
   void TearDown() override {
@@ -148,54 +215,10 @@
     }
   }
 
-  // Selects which frequency to return for each channel based on the input and
-  // output channel layout.
-  enum WhichFlow : int8_t {
-    FOR_INPUT,
-    FOR_SWAPPED_INPUT,
-    FOR_OUTPUT,
-    FOR_SWAPPED_OUTPUT,
-  };
-
-  double GetLeftChannelFrequency(WhichFlow which) const {
-    switch (which) {
-      case FOR_INPUT:
-      case FOR_OUTPUT:
-        return kLeftChannelFrequency;
-      case FOR_SWAPPED_INPUT:
-      case FOR_SWAPPED_OUTPUT:
-        return kRightChannelFrequency;

diff --git a/services/audio/snooper_node_unittest.cc b/services/audio/snooper_node_unittest.cc
index 9ad44924..945fe20 100644
--- a/services/audio/snooper_node_unittest.cc
+++ b/services/audio/snooper_node_unittest.cc
@@ -32,9 +32,10 @@
 // something finite.
 constexpr float kInvalidAudioSample = std::numeric_limits<float>::infinity();
 
-// The tones the source should generate into the left and right channels.
-constexpr double kLeftChannelFrequency = 500.0;
-constexpr double kRightChannelFrequency = 1200.0;
+// The tones the source should generate into the channels, and the alternate
+// frequency to use when testing swapped/discontinuous input.
+constexpr double kDefaultFrequency = 500.0;
+constexpr double kSwappedFrequency = 1200.0;
 constexpr double kSourceVolume = 0.5;
 
 // The duration of the audio that flows through the SnooperNode for each test.
@@ -50,29 +51,59 @@
 constexpr std::string_view kDumpAsWavSwitch = "dump-as-wav";
 
 // Test parameters.
-struct InputAndOutputParams {
-  media::AudioParameters input;
-  media::AudioParameters output;
-};
+class AudioParametersBuilder {
+ public:
+  AudioParametersBuilder() = default;
 
-// Helper so that gtest can produce useful logging of the test parameters.
-std::ostream& operator<<(std::ostream& out,
-                         const InputAndOutputParams& test_params) {
-  return out << "{input=" << test_params.input.AsHumanReadableString()
-             << ", output=" << test_params.output.AsHumanReadableString()
-             << "}";
-}
+  AudioParametersBuilder& WithLayout(media::ChannelLayout layout,
+                                     int channels) {
+    layout_ = layout;
+    channels_ = channels;
+    return *this;
+  }
+  AudioParametersBuilder& WithRate(int rate) {
+    rate_ = rate;
+    return *this;
+  }
+  AudioParametersBuilder& WithFrames(int frames) {
+    frames_ = frames;
+    return *this;
+  }
+
+  media::AudioParameters Build() const {
+    return media::AudioParameters(
+        media::AudioParameters::AUDIO_PCM_LOW_LATENCY,
+        media::ChannelLayoutConfig(layout_, channels_), rate_, frames_);
+  }
+
+ private:
+  media::ChannelLayout layout_ = media::CHANNEL_LAYOUT_STEREO;
+  int channels_ = 2;
+  int rate_ = 48000;
+  int frames_ = 480;
+};
+struct InputAndOutputParams {
+  AudioParametersBuilder input;
+  AudioParametersBuilder output;
+  std::string name;
+};
 
 class SnooperNodeTest : public testing::TestWithParam<InputAndOutputParams> {
  public:
   SnooperNodeTest() = default;
   ~SnooperNodeTest() override = default;
 
-  const media::AudioParameters& input_params() const {
-    return GetParam().input;
+  media::AudioParameters input_params() {
+    if (!input_params_.IsValid()) {
+      input_params_ = GetParam().input.Build();
+    }
+    return input_params_;
   }
-  const media::AudioParameters& output_params() const {
-    return GetParam().output;
+  media::AudioParameters output_params() {
+    if (!output_params_.IsValid()) {
+      output_params_ = GetParam().output.Build();
+    }
+    return output_params_;
   }
   base::TimeDelta output_delay() const { return output_delay_; }
   double max_relative_error() const { return max_relative_error_; }
@@ -80,6 +111,40 @@
   base::TestMockTimeTaskRunner* task_runner() const {
     return task_runner_.get();
   }
+  // Builds a gain matrix so tests can compute the expected output without
+  // hardcoding the values.
+  void PrecomputeExpectedMixMatrix() {
+    const int in_channels = input_params().channels();
+    const int out_channels = output_params().channels();
+    expected_mix_matrix_.assign(in_channels, std::vector<double>(out_channels));
+
+    auto input_bus = media::AudioBus::Create(in_channels, /*frames=*/1);
+    auto output_bus = media::AudioBus::Create(out_channels, /*frames=*/1);
+    media::ChannelMixer mixer(input_params(), output_params());
+
+    for (int in_ch = 0; in_ch < in_channels; ++in_ch) {
+      input_bus->Zero();
+      input_bus->channel(in_ch)[0] = 1.0f;
+      mixer.Transform(input_bus.get(), output_bus.get());
+      for (int out_ch = 0; out_ch < out_channels; ++out_ch) {
+        expected_mix_matrix_[in_ch][out_ch] =
+            static_cast<double>(output_bus->channel(out_ch)[0]);
+      }
+    }
+  }
+
+  double GetExpectedGain(int input_ch, int output_ch) const {
+    return expected_mix_matrix_[input_ch][output_ch];
+  }
+
+  bool IsOutputChannelSilent(int out_ch) {
+    for (int in_ch = 0; in_ch < input_params().channels(); ++in_ch) {
+      if (GetExpectedGain(in_ch, out_ch) > 0.0) {
+        return false;
+      }
+    }
+    return true;
+  }
   FakeLoopbackGroupMember* group_member() { return &*group_member_; }
   SnooperNode* node() { return &*node_; }
   FakeConsumer* consumer() { return &*consumer_; }
@@ -131,6 +196,8 @@
     // "huge" to ensure time calculations are being tested for overflow cases.
     task_runner_ = base::MakeRefCounted<base::TestMockTimeTaskRunner>(
         base::Time(), base::TimeTicks() + base::Microseconds(INT64_C(1) << 62));
+
+    PrecomputeExpectedMixMatrix();
   }
 
   void TearDown() override {
@@ -148,54 +215,10 @@
     }
   }
 
-  // Selects which frequency to return for each channel based on the input and
-  // output channel layout.
-  enum WhichFlow : int8_t {
-    FOR_INPUT,
-    FOR_SWAPPED_INPUT,
-    FOR_OUTPUT,
-    FOR_SWAPPED_OUTPUT,
-  };
-
-  double GetLeftChannelFrequency(WhichFlow which) const {
-    switch (which) {
-      case FOR_INPUT:
-      case FOR_OUTPUT:
-        return kLeftChannelFrequency;
-      case FOR_SWAPPED_INPUT:
-      case FOR_SWAPPED_OUTPUT:
-        return kRightChannelFrequency;
-    }
-  }
-
-  double GetRightChannelFrequency(WhichFlow which) const {
-    switch (which) {
-      // If the test parameters call for stereo→mono channel down-mixing, use
-      // the left channel frequency again for the right channel input. Down-
-      // mixing is tested elsewhere.
-      case FOR_INPUT:
-        return (output_params().channels() == 1 ? kLeftChannelFrequency
-                                                : kRightChannelFrequency);
-      case FOR_SWAPPED_INPUT:
-        return (output_params().channels() == 1 ? kRightChannelFrequency
-                                                : kLeftChannelFrequency);
-
-      // If the input was monaural, the output's right channel should contain
-      // the input's "left" channel frequency.
-      case FOR_OUTPUT:
-        return (input_params().channels() == 1 ? kLeftChannelFrequency
-                                               : kRightChannelFrequency);
-      case FOR_SWAPPED_OUTPUT:
-        return (input_params().channels() == 1 ? kRightChannelFrequency
-                                               : kLeftChannelFrequency);
-    }
-  }
-
   void CreateNewPipeline() {
     group_member_.emplace(input_params());
-    group_member_->SetChannelTone(0, GetLeftChannelFrequency(FOR_INPUT));
-    if (input_params().channels() > 1) {
-      group_member_->SetChannelTone(1, GetRightChannelFrequency(FOR_INPUT));
+    for (int ch = 0; ch < input_params().channels(); ++ch) {
+      group_member_->SetChannelTone(ch, kDefaultFrequency);
     }
     group_member_->SetVolume(kSourceVolume);
 
@@ -239,6 +262,32 @@
     consumer_->Consume(*bus);
   }
 
+  // Used to check each output channel is producing the correct tones at
+  // `position`.
+  void VerifyTonesAt(int position, double freq) {
+    for (int out_ch = 0; out_ch < output_params().channels(); ++out_ch) {
+      double expected_vol = 0.0;
+      for (int in_ch = 0; in_ch < input_params().channels(); ++in_ch) {
+        expected_vol += GetExpectedGain(in_ch, out_ch) * kSourceVolume;
+      }
+      if (expected_vol == 0.0) {
+        continue;
+      }
+      EXPECT_NEAR(expected_vol,
+                  consumer()->ComputeAmplitudeAt(out_ch, freq, position),
+                  expected_vol * max_relative_error())
+          << "Failure for output ch " << out_ch;
+    }
+  }
+
+  void VerifySilenceInRange(int start_frame, int end_frame) {
+    for (int ch = 0; ch < output_params().channels(); ++ch) {
+      EXPECT_TRUE(consumer()->IsSilentInRange(ch, start_frame, end_frame))
+          << "Channel " << ch << " expected to be silent between "
+          << start_frame << " and " << end_frame;
+    }
+  }
+
   // Post delayed tasks to schedule normal, uninterrupted input with the default
   // kInputAdvanceTime delay.
   void ScheduleDefaultInputTasks(double skew = 1.0) {
@@ -286,6 +335,11 @@
  private:
   scoped_refptr<base::TestMockTimeTaskRunner> task_runner_;
 
+  media::AudioParameters input_params_;
+  media::AudioParameters output_params_;
+
+  std::vector<std::vector<double>> expected_mix_matrix_;
+
   // A suitable output delay to use for rendering audio from the pipeline. See
   // comments in SetUp() for further details.
   base::TimeDelta output_delay_;
@@ -333,17 +387,7 @@
           ((input_skew * kInputAdvanceTime.InSecondsF()) +
            (output_skew * output_delay().InSecondsF())) *
           output_params().sample_rate();
-      const double frames_in_one_millisecond =
-          output_params().sample_rate() /
-          double{base::Time::kMillisecondsPerSecond};
-      EXPECT_NEAR(expected_end_of_silence_position,
-                  consumer()->FindEndOfSilence(0, 0),
-                  frames_in_one_millisecond);
-      if (output_params().channels() > 1) {
-        EXPECT_NEAR(expected_end_of_silence_position,
-                    consumer()->FindEndOfSilence(1, 0),
-                    frames_in_one_millisecond);
-      }
+      VerifySilenceInRange(0, expected_end_of_silence_position);
 
       // Analyze the recording in several places for the expected tones.
       constexpr int kNumToneChecks = 16;
@@ -351,16 +395,7 @@
         const int end_frame =
             consumer()->GetRecordedFrameCount() * i / kNumToneChecks;
         SCOPED_TRACE(testing::Message() << "end_frame=" << end_frame);
-        EXPECT_NEAR(kSourceVolume,
-                    consumer()->ComputeAmplitudeAt(
-                        0, GetLeftChannelFrequency(FOR_OUTPUT), end_frame),
-                    kSourceVolume * max_relative_error());
-        if (output_params().channels() > 1) {
-          EXPECT_NEAR(kSourceVolume,
-                      consumer()->ComputeAmplitudeAt(
-                          1, GetRightChannelFrequency(FOR_OUTPUT), end_frame),
-                      kSourceVolume * max_relative_error());
-        }
+        VerifyTonesAt(end_frame, kDefaultFrequency);
       }
 
       if (HasFailure()) {
@@ -425,18 +460,7 @@
     // Just before the drop, there should be a tone.
     const int position_a_little_before_silence_begins =
         output_silence_position - output_frames_in_20_milliseconds;
-    EXPECT_NEAR(
-        kSourceVolume,
-        consumer()->ComputeAmplitudeAt(0, GetLeftChannelFrequency(FOR_OUTPUT),
-                                       position_a_little_before_silence_begins),
-        kSourceVolume * max_relative_error());
-    if (output_params().channels() > 1) {
-      EXPECT_NEAR(kSourceVolume,
-                  consumer()->ComputeAmplitudeAt(
-                      1, GetRightChannelFrequency(FOR_OUTPUT),
-                      position_a_little_before_silence_begins),
-                  kSourceVolume * max_relative_error());
-    }
+    VerifyTonesAt(position_a_little_before_silence_begins, kDefaultFrequency);
 
... (truncated)