From 358c94f5ddd9460ebb009ffb3f9a1a31ace35136 Mon Sep 17 00:00:00 2001 From: Jonas Oreland Date: Thu, 20 Feb 2025 20:39:38 +0100 Subject: [PATCH] AudioState extensions This patch modifies AudioState to always call InitRecording before StartRecording(). This makes it possible to do SetRecording(false) + SetRecording(true), which before this patch would not actually work if there was sending streams. The only way was to add/remove streams...via SDP operations, puh :(. Bonus: We also needed to modifu AndroidAudioDeviceModule (which is a thin wrapper) so that StopRecording() will call AudioInput->StopRecording() even when recording is not enabled. BUG=b/397376626 Change-Id: I954b5caab11225b544c3e6a78c5dde357d4eedb5 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/378140 Commit-Queue: Jonas Oreland Auto-Submit: Jonas Oreland Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#43946} --- audio/audio_state.cc | 7 +++-- audio/audio_state_unittest.cc | 31 +++++++++++++++++++ .../jni/audio_device/audio_device_module.cc | 2 -- 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/audio/audio_state.cc b/audio/audio_state.cc index a0861eef6b..fb0a140360 100644 --- a/audio/audio_state.cc +++ b/audio/audio_state.cc @@ -141,13 +141,16 @@ void AudioState::SetRecording(bool enabled) { RTC_LOG(LS_INFO) << "SetRecording(" << enabled << ")"; RTC_DCHECK_RUN_ON(&thread_checker_); if (recording_enabled_ != enabled) { + auto* adm = config_.audio_device_module.get(); recording_enabled_ = enabled; if (enabled) { if (!sending_streams_.empty()) { - config_.audio_device_module->StartRecording(); + if (adm->InitRecording() == 0) { + adm->StartRecording(); + } } } else { - config_.audio_device_module->StopRecording(); + adm->StopRecording(); } } } diff --git a/audio/audio_state_unittest.cc b/audio/audio_state_unittest.cc index 954e6c19c3..63a722948f 100644 --- a/audio/audio_state_unittest.cc +++ b/audio/audio_state_unittest.cc @@ -26,6 +26,7 @@ namespace test { namespace { using ::testing::_; +using ::testing::InSequence; using ::testing::Matcher; using ::testing::NiceMock; using ::testing::StrictMock; @@ -357,6 +358,36 @@ TEST_P(AudioStateTest, audio_buffer, n_samples_out, &elapsed_time_ms, &ntp_time_ms); } +TEST_P(AudioStateTest, AlwaysCallInitRecordingBeforeStartRecording) { + ConfigHelper helper(GetParam()); + rtc::scoped_refptr audio_state( + rtc::make_ref_counted(helper.config())); + + auto* adm = reinterpret_cast( + helper.config().audio_device_module.get()); + + MockAudioSendStream stream; + { + InSequence s; + EXPECT_CALL(*adm, InitRecording()); + EXPECT_CALL(*adm, StartRecording()); + audio_state->AddSendingStream(&stream, kSampleRate, kNumberOfChannels); + } + + EXPECT_CALL(*adm, StopRecording()); + audio_state->SetRecording(false); + + { + InSequence s; + EXPECT_CALL(*adm, InitRecording()); + EXPECT_CALL(*adm, StartRecording()); + audio_state->SetRecording(true); + } + + EXPECT_CALL(*adm, StopRecording()); + audio_state->RemoveSendingStream(&stream); +} + INSTANTIATE_TEST_SUITE_P(AudioStateTest, AudioStateTest, Values(ConfigHelper::Params({false, false}), diff --git a/sdk/android/src/jni/audio_device/audio_device_module.cc b/sdk/android/src/jni/audio_device/audio_device_module.cc index dafe18010f..2c571185fa 100644 --- a/sdk/android/src/jni/audio_device/audio_device_module.cc +++ b/sdk/android/src/jni/audio_device/audio_device_module.cc @@ -298,8 +298,6 @@ class AndroidAudioDeviceModule : public AudioDeviceModule { RTC_DLOG(LS_INFO) << __FUNCTION__; if (!initialized_) return -1; - if (!Recording()) - return 0; audio_device_buffer_->StopRecording(); int32_t result = input_->StopRecording(); RTC_DLOG(LS_INFO) << "output: " << result;