From 500d6e7f142fddbcc2a6e04a76471eed15efa988 Mon Sep 17 00:00:00 2001 From: Ivo Creusen Date: Wed, 24 Nov 2021 12:32:07 +0000 Subject: [PATCH] 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 Reviewed-by: Ivo Creusen Commit-Queue: Ivo Creusen Cr-Commit-Position: refs/heads/main@{#35459} --- modules/audio_coding/neteq/neteq_impl.cc | 5 ++ .../audio_coding/neteq/neteq_impl_unittest.cc | 66 +++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/modules/audio_coding/neteq/neteq_impl.cc b/modules/audio_coding/neteq/neteq_impl.cc index 7e3c4efab9..30886c3ace 100644 --- a/modules/audio_coding/neteq/neteq_impl.cc +++ b/modules/audio_coding/neteq/neteq_impl.cc @@ -798,6 +798,11 @@ int NetEqImpl::GetAudioInternal(AudioFrame* audio_frame, RTC_DCHECK(audio_frame->muted()); // Reset() should mute the frame. playout_timestamp_ += static_cast(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_ diff --git a/modules/audio_coding/neteq/neteq_impl_unittest.cc b/modules/audio_coding/neteq/neteq_impl_unittest.cc index e6e3eb493b..b39a880292 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 @@ class NetEqImplTest : public ::testing::Test { void CreateInstance( const rtc::scoped_refptr& 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 @@ class NetEqImplTest : public ::testing::Test { 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 @@ TEST_F(NetEqImplTest, NotifyControllerOfReorderedPacket) { 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(); + EXPECT_CALL(*mock_decoder_factory, MakeAudioDecoderMock(_, _, _)) + .WillOnce(Invoke([&](const SdpAudioFormat& format, + absl::optional codec_pair_id, + std::unique_ptr* dec) { + EXPECT_EQ("pcmu", format.name); + *dec = std::make_unique(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)