From ec38238af7c1ca1c5c2957777af189b2ac0e4a01 Mon Sep 17 00:00:00 2001 From: Lionel Koenig Date: Thu, 12 Sep 2024 15:50:46 +0200 Subject: [PATCH] Ensure the AudioCodingModule is reset when sending is stopped. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:42226041 Change-Id: Ife3548bda3042a7447b7c50f48f023a2bc0bc443 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/362103 Commit-Queue: Lionel Koenig Reviewed-by: Jesus de Vicente Pena Reviewed-by: Jakob Ivarsson‎ Cr-Commit-Position: refs/heads/main@{#43017} --- audio/channel_send.cc | 2 +- audio/channel_send_unittest.cc | 1 + .../audio_coding/acm2/audio_coding_module.cc | 12 ++++- .../acm2/audio_coding_module_unittest.cc | 45 +++++++++++++++++++ .../include/audio_coding_module.h | 4 ++ 5 files changed, 62 insertions(+), 2 deletions(-) diff --git a/audio/channel_send.cc b/audio/channel_send.cc index d1683b74d7..fa8a42146c 100644 --- a/audio/channel_send.cc +++ b/audio/channel_send.cc @@ -562,7 +562,7 @@ void ChannelSend::StopSend() { rtc::Event flush; encoder_queue_->PostTask([this, &flush]() { RTC_DCHECK_RUN_ON(&encoder_queue_checker_); - CallEncoder([](AudioEncoder* encoder) { encoder->Reset(); }); + audio_coding_->Reset(); flush.Set(); }); flush.Wait(rtc::Event::kForever); diff --git a/audio/channel_send_unittest.cc b/audio/channel_send_unittest.cc index 2260c810c4..31cdaa0c0d 100644 --- a/audio/channel_send_unittest.cc +++ b/audio/channel_send_unittest.cc @@ -140,6 +140,7 @@ TEST_F(ChannelSendTest, StopSendShouldResetEncoder) { ProcessNextFrame(); // StopSend should clear the previous audio frame stored in the encoder. channel_->StopSend(); + channel_->StartSend(); // The following frame should not trigger a new packet since the encoder // needs 20 ms audio. diff --git a/modules/audio_coding/acm2/audio_coding_module.cc b/modules/audio_coding/acm2/audio_coding_module.cc index e1a775d704..030033547a 100644 --- a/modules/audio_coding/acm2/audio_coding_module.cc +++ b/modules/audio_coding/acm2/audio_coding_module.cc @@ -43,6 +43,8 @@ class AudioCodingModuleImpl final : public AudioCodingModule { explicit AudioCodingModuleImpl(); ~AudioCodingModuleImpl() override; + void Reset() override; + ///////////////////////////////////////// // Sender // @@ -280,7 +282,7 @@ int32_t AudioCodingModuleImpl::Encode( absolute_capture_timestamp_ms_.value_or(-1)); } } - absolute_capture_timestamp_ms_ = std::nullopt; + absolute_capture_timestamp_ms_.reset(); previous_pltype_ = encoded_info.payload_type; return static_cast(encode_buffer_.size()); } @@ -289,6 +291,14 @@ int32_t AudioCodingModuleImpl::Encode( // Sender // +void AudioCodingModuleImpl::Reset() { + MutexLock lock(&acm_mutex_); + absolute_capture_timestamp_ms_.reset(); + if (HaveValidEncoder("Reset")) { + encoder_stack_->Reset(); + } +} + void AudioCodingModuleImpl::ModifyEncoder( rtc::FunctionView*)> modifier) { MutexLock lock(&acm_mutex_); diff --git a/modules/audio_coding/acm2/audio_coding_module_unittest.cc b/modules/audio_coding/acm2/audio_coding_module_unittest.cc index c8ef9dfbf1..c7e9dfc1c1 100644 --- a/modules/audio_coding/acm2/audio_coding_module_unittest.cc +++ b/modules/audio_coding/acm2/audio_coding_module_unittest.cc @@ -522,6 +522,17 @@ class AudioPacketizationCallbackMock : public AudioPacketizationCallback { (override)); }; +TEST(AudioCodingModule, DoesResetEncoder) { + std::unique_ptr acm = AudioCodingModule::Create(); + auto encoder = std::make_unique(); + MockAudioEncoder* encoder_mock = encoder.get(); + + acm->SetEncoder(std::move(encoder)); + + EXPECT_CALL(*encoder_mock, Reset()).Times(1); + acm->Reset(); +} + class AcmAbsoluteCaptureTimestamp : public ::testing::Test { public: AcmAbsoluteCaptureTimestamp() : audio_frame_(kSampleRateHz, kNumChannels) {} @@ -587,6 +598,40 @@ TEST_F(AcmAbsoluteCaptureTimestamp, HaveBeginningOfFrameCaptureTime) { } } +TEST_F(AcmAbsoluteCaptureTimestamp, DoesResetWhenAudioCodingModuleDo) { + constexpr int64_t first_absolute_capture_timestamp_ms = 123456789; + + int64_t absolute_capture_timestamp_ms = first_absolute_capture_timestamp_ms; + EXPECT_CALL(transport_, + SendData(_, _, _, _, _, first_absolute_capture_timestamp_ms)) + .Times(1); + EXPECT_CALL( + transport_, + SendData(_, _, _, _, _, first_absolute_capture_timestamp_ms + kPTimeMs)) + .Times(1); + for (int k = 0; k < 5; ++k) { + acm_->Add10MsData( + GetAudioWithAbsoluteCaptureTimestamp(absolute_capture_timestamp_ms)); + absolute_capture_timestamp_ms += 10; + } + + acm_->Reset(); + constexpr int64_t after_reset_absolute_capture_timestamp_ms = 523456789; + EXPECT_CALL(transport_, SendData(_, _, _, _, _, + after_reset_absolute_capture_timestamp_ms)) + .Times(1); + EXPECT_CALL(transport_, + SendData(_, _, _, _, _, + after_reset_absolute_capture_timestamp_ms + kPTimeMs)) + .Times(1); + absolute_capture_timestamp_ms = after_reset_absolute_capture_timestamp_ms; + for (int k = 0; k < 5; ++k) { + acm_->Add10MsData( + GetAudioWithAbsoluteCaptureTimestamp(absolute_capture_timestamp_ms)); + absolute_capture_timestamp_ms += 10; + } +} + // Disabling all of these tests on iOS until file support has been added. // See https://code.google.com/p/webrtc/issues/detail?id=4752 for details. #if !defined(WEBRTC_IOS) diff --git a/modules/audio_coding/include/audio_coding_module.h b/modules/audio_coding/include/audio_coding_module.h index ec8255e576..094ef0eb2b 100644 --- a/modules/audio_coding/include/audio_coding_module.h +++ b/modules/audio_coding/include/audio_coding_module.h @@ -78,6 +78,10 @@ class AudioCodingModule { }); } + // Reset encoder and audio coding module. This throws away any audio passed + // and starts fresh. + virtual void Reset() = 0; + // int32_t RegisterTransportCallback() // Register a transport callback which will be called to deliver // the encoded buffers whenever Process() is called and a