commit 500d6e7f142fddbcc2a6e04a76471eed15efa988
author Ivo Creusen <ivoc@webrtc.org> Wed Nov 24 12:32:07 2021 +0000
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Dec 02 11:48:17 2021 +0000
tree 5cba98980d38d01c986f3ef47760fb4398b89317
parent 258ed1a38ad9d4f0da798c40b6976eff2dce864f

Return kSampleUnderrun if the number of samples does not fit in an AudioFrame.

If the number of samples does not fit in an AudioFrame, we should return
kSampleUnderrun to avoid crashes further downstream.

Bug: chromium:1265806
Change-Id: Ie94e1de53810167fd9b52ade72b3cb669a2a4f06
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/238666
Reviewed-by: Jakob Ivarsson <jakobi@webrtc.org>
Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
Commit-Queue: Ivo Creusen <ivoc@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35459}

modules/audio_coding/neteq/neteq_impl.cc

diff --git a/modules/audio_coding/neteq/neteq_impl.cc b/modules/audio_coding/neteq/neteq_impl.cc
index 7e3c4ef..30886c3 100644
--- a/modules/audio_coding/neteq/neteq_impl.cc
+++ b/modules/audio_coding/neteq/neteq_impl.cc
@@ -798,6 +798,11 @@
     RTC_DCHECK(audio_frame->muted());  // Reset() should mute the frame.
     playout_timestamp_ += static_cast<uint32_t>(output_size_samples_);
     audio_frame->sample_rate_hz_ = fs_hz_;
+    // Make sure the total number of samples fits in the AudioFrame.
+    if (output_size_samples_ * sync_buffer_->Channels() >
+        AudioFrame::kMaxDataSizeSamples) {
+      return kSampleUnderrun;
+    }
     audio_frame->samples_per_channel_ = output_size_samples_;
     audio_frame->timestamp_ =
         first_packet_

modules/audio_coding/neteq/neteq_impl_unittest.cc

diff --git a/modules/audio_coding/neteq/neteq_impl_unittest.cc b/modules/audio_coding/neteq/neteq_impl_unittest.cc
index e6e3eb4..b39a880 100644
--- a/modules/audio_coding/neteq/neteq_impl_unittest.cc
+++ b/modules/audio_coding/neteq/neteq_impl_unittest.cc
@@ -18,6 +18,7 @@
 #include "api/neteq/default_neteq_controller_factory.h"
 #include "api/neteq/neteq.h"
 #include "api/neteq/neteq_controller.h"
+#include "modules/audio_coding/codecs/g711/audio_decoder_pcm.h"
 #include "modules/audio_coding/neteq/accelerate.h"
 #include "modules/audio_coding/neteq/decision_logic.h"
 #include "modules/audio_coding/neteq/default_neteq_factory.h"
@@ -75,6 +76,7 @@
   void CreateInstance(
       const rtc::scoped_refptr<AudioDecoderFactory>& decoder_factory) {
     ASSERT_TRUE(decoder_factory);
+    config_.enable_muted_state = enable_muted_state_;
     NetEqImpl::Dependencies deps(config_, &clock_, decoder_factory,
                                  DefaultNetEqControllerFactory());
 
@@ -245,6 +247,7 @@
   MockRedPayloadSplitter* mock_payload_splitter_ = nullptr;
   RedPayloadSplitter* red_payload_splitter_ = nullptr;
   bool use_mock_payload_splitter_ = true;
+  bool enable_muted_state_ = false;
 };
 
 // This tests the interface class NetEq.
@@ -1601,6 +1604,69 @@
   EXPECT_EQ(NetEq::kOK, neteq_->InsertPacket(rtp_header, payload));
 }
 
+// When using a codec with 1000 channels, there should be no crashes.
+TEST_F(NetEqImplTest, NoCrashWith1000Channels) {
+  using ::testing::AllOf;
+  using ::testing::Field;
+  UseNoMocks();
+  use_mock_decoder_database_ = true;
+  enable_muted_state_ = true;
+  CreateInstance();
+  const size_t kPayloadLength = 100;
+  const uint8_t kPayloadType = 0;
+  const uint16_t kFirstSequenceNumber = 0x1234;
+  const uint32_t kFirstTimestamp = 0x12345678;
+  const uint32_t kSsrc = 0x87654321;
+  uint8_t payload[kPayloadLength] = {0};
+  RTPHeader rtp_header;
+  rtp_header.payloadType = kPayloadType;
+  rtp_header.sequenceNumber = kFirstSequenceNumber;
+  rtp_header.timestamp = kFirstTimestamp;
+  rtp_header.ssrc = kSsrc;
+  Packet fake_packet;
+  fake_packet.payload_type = kPayloadType;
+  fake_packet.sequence_number = kFirstSequenceNumber;
+  fake_packet.timestamp = kFirstTimestamp;
+
+  AudioDecoder* decoder = nullptr;
+
+  auto mock_decoder_factory = rtc::make_ref_counted<MockAudioDecoderFactory>();
+  EXPECT_CALL(*mock_decoder_factory, MakeAudioDecoderMock(_, _, _))
+      .WillOnce(Invoke([&](const SdpAudioFormat& format,
+                           absl::optional<AudioCodecPairId> codec_pair_id,
+                           std::unique_ptr<AudioDecoder>* dec) {
+        EXPECT_EQ("pcmu", format.name);
+        *dec = std::make_unique<AudioDecoderPcmU>(1000);
+        decoder = dec->get();
+      }));
+  DecoderDatabase::DecoderInfo info(SdpAudioFormat("pcmu", 8000, 1),
+                                    absl::nullopt, mock_decoder_factory);
+  // Expectations for decoder database.
+  EXPECT_CALL(*mock_decoder_database_, GetDecoderInfo(kPayloadType))
+      .WillRepeatedly(Return(&info));
+  EXPECT_CALL(*mock_decoder_database_, GetActiveCngDecoder())
+      .WillRepeatedly(ReturnNull());
+  EXPECT_CALL(*mock_decoder_database_, GetActiveDecoder())
+      .WillRepeatedly(Return(decoder));
+  EXPECT_CALL(*mock_decoder_database_, SetActiveDecoder(_, _))
+      .WillOnce(Invoke([](uint8_t rtp_payload_type, bool* new_decoder) {
+        *new_decoder = true;
+        return 0;
+      }));
+
+  // Insert first packet.
+  neteq_->InsertPacket(rtp_header, payload);
+
+  AudioFrame audio_frame;
+  bool muted;
+
+  // Repeat 40 times to ensure we enter muted state.
+  for (int i = 0; i < 40; i++) {
+    // GetAudio should return an error, and not crash, even in muted state.
+    EXPECT_NE(0, neteq_->GetAudio(&audio_frame, &muted));
+  }
+}
+
 class Decoder120ms : public AudioDecoder {
  public:
   Decoder120ms(int sample_rate_hz, SpeechType speech_type)