Make AudioFrame::channel_layout_ private and check for valid values

Bug: chromium:335805780
Change-Id: Ida671d317c07983cc51faa1a498642747dbb810c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/349322
Reviewed-by: Per Åhgren <peah@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42199}

api/audio/audio_frame.cc

@@ -131,6 +131,21 @@ bool AudioFrame::muted() const {
   return muted_;
 }
 
+void AudioFrame::SetLayoutAndNumChannels(ChannelLayout layout,
+                                         size_t num_channels) {
+  channel_layout_ = layout;
+  num_channels_ = num_channels;
+#if RTC_DCHECK_IS_ON
+  // Do a sanity check that the layout and num_channels match.
+  // If this lookup yield 0u, then the layout is likely CHANNEL_LAYOUT_DISCRETE.
+  auto expected_num_channels = ChannelLayoutToChannelCount(layout);
+  if (expected_num_channels) {  // If expected_num_channels is 0
+    RTC_DCHECK_EQ(expected_num_channels, num_channels_);
+  }
+#endif
+  RTC_CHECK_LE(samples_per_channel_ * num_channels_, kMaxDataSizeSamples);
+}
+
 // static
 const int16_t* AudioFrame::empty_data() {
   static int16_t* null_data = new int16_t[kMaxDataSizeSamples]();

api/audio/audio_frame.h

@@ -103,7 +103,11 @@ class AudioFrame {
   size_t max_16bit_samples() const { return kMaxDataSizeSamples; }
   size_t samples_per_channel() const { return samples_per_channel_; }
   size_t num_channels() const { return num_channels_; }
+
   ChannelLayout channel_layout() const { return channel_layout_; }
+  // Sets the `channel_layout` property as well as `num_channels`.
+  void SetLayoutAndNumChannels(ChannelLayout layout, size_t num_channels);
+
   int sample_rate_hz() const { return sample_rate_hz_; }
 
   void set_absolute_capture_timestamp_ms(
@@ -126,7 +130,6 @@ class AudioFrame {
   size_t samples_per_channel_ = 0;
   int sample_rate_hz_ = 0;
   size_t num_channels_ = 0;
-  ChannelLayout channel_layout_ = CHANNEL_LAYOUT_NONE;
   SpeechType speech_type_ = kUndefined;
   VADActivity vad_activity_ = kVadUnknown;
   // Monotonically increasing timestamp intended for profiling of audio frames.
@@ -160,6 +163,7 @@ class AudioFrame {
 
   int16_t data_[kMaxDataSizeSamples];
   bool muted_ = true;
+  ChannelLayout channel_layout_ = CHANNEL_LAYOUT_NONE;
 
   // Absolute capture timestamp when this audio frame was originally captured.
   // This is only valid for audio frames captured on this machine. The absolute

audio/utility/channel_mixer.cc

@@ -18,17 +18,26 @@
 namespace webrtc {
 
 ChannelMixer::ChannelMixer(ChannelLayout input_layout,
-                           ChannelLayout output_layout)
+                           size_t input_channels,
+                           ChannelLayout output_layout,
+                           size_t output_channels)
     : input_layout_(input_layout),
       output_layout_(output_layout),
-      input_channels_(ChannelLayoutToChannelCount(input_layout)),
+      input_channels_(input_channels),
-      output_channels_(ChannelLayoutToChannelCount(output_layout)) {
+      output_channels_(output_channels) {
   // Create the transformation matrix.
   ChannelMixingMatrix matrix_builder(input_layout_, input_channels_,
                                      output_layout_, output_channels_);
   remapping_ = matrix_builder.CreateTransformationMatrix(&matrix_);
 }
 
+ChannelMixer::ChannelMixer(ChannelLayout input_layout,
+                           ChannelLayout output_layout)
+    : ChannelMixer(input_layout,
+                   ChannelLayoutToChannelCount(input_layout),
+                   output_layout,
+                   ChannelLayoutToChannelCount(output_layout)) {}
+
 ChannelMixer::~ChannelMixer() = default;
 
 void ChannelMixer::Transform(AudioFrame* frame) {
@@ -49,8 +58,7 @@ void ChannelMixer::Transform(AudioFrame* frame) {
 
   // Only change the number of output channels if the audio frame is muted.
   if (frame->muted()) {
-    frame->num_channels_ = output_channels_;
+    frame->SetLayoutAndNumChannels(output_layout_, output_channels_);
-    frame->channel_layout_ = output_layout_;
     return;
   }
 
@@ -87,8 +95,7 @@ void ChannelMixer::Transform(AudioFrame* frame) {
   }
 
   // Update channel information.
-  frame->num_channels_ = output_channels_;
+  frame->SetLayoutAndNumChannels(output_layout_, output_channels_);
-  frame->channel_layout_ = output_layout_;
 
   // Copy the output result to the audio frame in `frame`.
   memcpy(

audio/utility/channel_mixer.h

@@ -35,6 +35,10 @@ class ChannelMixer {
   // (1 / sqrt(2)) gain to each.
   static constexpr float kHalfPower = 0.707106781186547524401f;
 
+  ChannelMixer(ChannelLayout input_layout,
+               size_t input_channels,
+               ChannelLayout output_layout,
+               size_t output_channels);
   ChannelMixer(ChannelLayout input_layout, ChannelLayout output_layout);
   ~ChannelMixer();
 

audio/utility/channel_mixing_matrix.cc

@@ -29,6 +29,21 @@ bool UseChannelMappingAdjustmentsByDefault() {
       "WebRTC-VoIPChannelRemixingAdjustmentKillSwitch");
 }
 
+ChannelLayout CheckInputLayout(ChannelLayout input_layout,
+                               ChannelLayout output_layout) {
+  // Special case for 5.0, 5.1 with back channels when upmixed to 7.0, 7.1,
+  // which should map the back LR to side LR.
+  if (input_layout == CHANNEL_LAYOUT_5_0_BACK &&
+      output_layout == CHANNEL_LAYOUT_7_0) {
+    return CHANNEL_LAYOUT_5_0;
+  } else if (input_layout == CHANNEL_LAYOUT_5_1_BACK &&
+             output_layout == CHANNEL_LAYOUT_7_1) {
+    return CHANNEL_LAYOUT_5_1;
+  }
+
+  return input_layout;
+}
+
 }  // namespace
 
 static void ValidateLayout(ChannelLayout layout) {
@@ -68,7 +83,7 @@ ChannelMixingMatrix::ChannelMixingMatrix(ChannelLayout input_layout,
                                          int output_channels)
     : use_voip_channel_mapping_adjustments_(
           UseChannelMappingAdjustmentsByDefault()),
-      input_layout_(input_layout),
+      input_layout_(CheckInputLayout(input_layout, output_layout)),
       input_channels_(input_channels),
       output_layout_(output_layout),
       output_channels_(output_channels) {
@@ -80,16 +95,6 @@ ChannelMixingMatrix::ChannelMixingMatrix(ChannelLayout input_layout,
     ValidateLayout(input_layout);
   if (output_layout != CHANNEL_LAYOUT_DISCRETE)
     ValidateLayout(output_layout);
-
-  // Special case for 5.0, 5.1 with back channels when upmixed to 7.0, 7.1,
-  // which should map the back LR to side LR.
-  if (input_layout_ == CHANNEL_LAYOUT_5_0_BACK &&
-      output_layout_ == CHANNEL_LAYOUT_7_0) {
-    input_layout_ = CHANNEL_LAYOUT_5_0;
-  } else if (input_layout_ == CHANNEL_LAYOUT_5_1_BACK &&
-             output_layout_ == CHANNEL_LAYOUT_7_1) {
-    input_layout_ = CHANNEL_LAYOUT_5_1;
-  }
 }
 
 ChannelMixingMatrix::~ChannelMixingMatrix() = default;

audio/utility/channel_mixing_matrix.h

@@ -42,10 +42,10 @@ class ChannelMixingMatrix {
   std::vector<std::vector<float>>* matrix_;
 
   // Input and output channel layout provided during construction.
-  ChannelLayout input_layout_;
+  const ChannelLayout input_layout_;
-  int input_channels_;
+  const int input_channels_;
-  ChannelLayout output_layout_;
+  const ChannelLayout output_layout_;
-  int output_channels_;
+  const int output_channels_;
 
   // Helper variable for tracking which inputs are currently unaccounted,
   // should be empty after construction completes.

modules/audio_mixer/audio_frame_manipulator.cc

@@ -79,8 +79,9 @@ void RemixFrame(size_t target_number_of_channels, AudioFrame* frame) {
     // instead of guessing based on number of channels.
     const ChannelLayout output_layout(
         GuessChannelLayout(target_number_of_channels));
-    ChannelMixer mixer(GuessChannelLayout(frame->num_channels()),
+    const ChannelLayout input_layout(GuessChannelLayout(frame->num_channels()));
-                       output_layout);
+    ChannelMixer mixer(input_layout, frame->num_channels(), output_layout,
+                       target_number_of_channels);
     mixer.Transform(frame);
     RTC_DCHECK_EQ(frame->channel_layout(), output_layout);
   }