From aaedf75520a76680f607c8b183318ae8858d2c65 Mon Sep 17 00:00:00 2001 From: Fredrik Solenberg Date: Mon, 18 Dec 2017 13:09:12 +0100 Subject: [PATCH] Replace VoEBase::[Start/Stop]Send(). The functionality is moved into AudioState. Bug: webrtc:4690 Change-Id: Iee1bfd185566c9507422e8eea8a2cce02baaefe1 Reviewed-on: https://webrtc-review.googlesource.com/33521 Reviewed-by: Henrik Andreassson Commit-Queue: Fredrik Solenberg Cr-Commit-Position: refs/heads/master@{#21324} --- audio/audio_send_stream.cc | 18 +----- audio/audio_state.cc | 31 ++++++++-- audio/audio_state.h | 1 + media/engine/fakewebrtcvoiceengine.h | 3 - test/mock_voe_channel_proxy.h | 2 + test/mock_voice_engine.h | 2 - video/video_quality_test.cc | 2 - voice_engine/channel_proxy.cc | 11 ++++ voice_engine/channel_proxy.h | 2 + voice_engine/include/voe_base.h | 15 ----- voice_engine/shared_data.cc | 13 ---- voice_engine/shared_data.h | 1 - voice_engine/voe_base_impl.cc | 88 ---------------------------- voice_engine/voe_base_impl.h | 11 ---- 14 files changed, 44 insertions(+), 156 deletions(-) diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc index 6aa469d6ca..f0d13e3ee4 100644 --- a/audio/audio_send_stream.cc +++ b/audio/audio_send_stream.cc @@ -16,7 +16,6 @@ #include "audio/audio_state.h" #include "audio/conversion.h" -#include "audio/scoped_voe_interface.h" #include "call/rtp_transport_controller_send_interface.h" #include "modules/audio_coding/codecs/cng/audio_encoder_cng.h" #include "modules/bitrate_controller/include/bitrate_controller.h" @@ -30,7 +29,6 @@ #include "rtc_base/timeutils.h" #include "system_wrappers/include/field_trial.h" #include "voice_engine/channel_proxy.h" -#include "voice_engine/include/voe_base.h" #include "voice_engine/voice_engine_impl.h" namespace webrtc { @@ -245,12 +243,7 @@ void AudioSendStream::Start() { transport_->packet_sender()->SetAccountForAudioPackets(true); ConfigureBitrateObserver(config_.min_bitrate_bps, config_.max_bitrate_bps); } - - ScopedVoEInterface base(voice_engine()); - int error = base->StartSend(config_.voe_channel_id); - if (error != 0) { - RTC_LOG(LS_ERROR) << "AudioSendStream::Start failed with error: " << error; - } + channel_proxy_->StartSend(); sending_ = true; audio_state()->AddSendingStream(this, encoder_sample_rate_hz_, encoder_num_channels_); @@ -263,12 +256,7 @@ void AudioSendStream::Stop() { } RemoveBitrateObserver(); - - ScopedVoEInterface base(voice_engine()); - int error = base->StopSend(config_.voe_channel_id); - if (error != 0) { - RTC_LOG(LS_ERROR) << "AudioSendStream::Stop failed with error: " << error; - } + channel_proxy_->StopSend(); sending_ = false; audio_state()->RemoveSendingStream(this); } @@ -695,7 +683,5 @@ void AudioSendStream::RegisterCngPayloadType(int payload_type, } } } - - } // namespace internal } // namespace webrtc diff --git a/audio/audio_state.cc b/audio/audio_state.cc index a83b681252..ac7eaf7bca 100644 --- a/audio/audio_state.cc +++ b/audio/audio_state.cc @@ -32,6 +32,7 @@ AudioState::AudioState(const AudioState::Config& config) config_.audio_device_module.get()) { process_thread_checker_.DetachFromThread(); RTC_DCHECK(config_.audio_mixer); + RTC_DCHECK(config_.audio_device_module); } AudioState::~AudioState() { @@ -61,6 +62,18 @@ void AudioState::AddSendingStream(webrtc::AudioSendStream* stream, properties.sample_rate_hz = sample_rate_hz; properties.num_channels = num_channels; UpdateAudioTransportWithSendingStreams(); + + // Make sure recording is initialized; start recording if enabled. + auto* adm = config_.audio_device_module.get(); + if (!adm->Recording()) { + if (adm->InitRecording() == 0) { + if (recording_enabled_) { + adm->StartRecording(); + } + } else { + RTC_DLOG_F(LS_ERROR) << "Failed to initialize recording."; + } + } } void AudioState::RemoveSendingStream(webrtc::AudioSendStream* stream) { @@ -68,6 +81,9 @@ void AudioState::RemoveSendingStream(webrtc::AudioSendStream* stream) { auto count = sending_streams_.erase(stream); RTC_DCHECK_EQ(1, count); UpdateAudioTransportWithSendingStreams(); + if (sending_streams_.empty()) { + config_.audio_device_module->StopRecording(); + } } void AudioState::SetPlayout(bool enabled) { @@ -93,11 +109,16 @@ void AudioState::SetPlayout(bool enabled) { void AudioState::SetRecording(bool enabled) { RTC_LOG(INFO) << "SetRecording(" << enabled << ")"; RTC_DCHECK(thread_checker_.CalledOnValidThread()); - // TODO(henrika): keep track of state as in SetPlayout(). - // Will stop/start recording of the underlying device, if necessary, and - // remember the setting for when it receives subsequent calls of - // StartPlayout. - voe_base_->SetRecording(enabled); + if (recording_enabled_ != enabled) { + recording_enabled_ = enabled; + if (enabled) { + if (!sending_streams_.empty()) { + config_.audio_device_module->StartRecording(); + } + } else { + config_.audio_device_module->StopRecording(); + } + } } AudioState::Stats AudioState::GetAudioInputStats() const { diff --git a/audio/audio_state.h b/audio/audio_state.h index 14dc78891b..540a249902 100644 --- a/audio/audio_state.h +++ b/audio/audio_state.h @@ -67,6 +67,7 @@ class AudioState final : public webrtc::AudioState { rtc::ThreadChecker thread_checker_; rtc::ThreadChecker process_thread_checker_; const webrtc::AudioState::Config config_; + bool recording_enabled_ = true; // We hold one interface pointer to the VoE to make sure it is kept alive. ScopedVoEInterface voe_base_; diff --git a/media/engine/fakewebrtcvoiceengine.h b/media/engine/fakewebrtcvoiceengine.h index 163edd8c05..a70975b12d 100644 --- a/media/engine/fakewebrtcvoiceengine.h +++ b/media/engine/fakewebrtcvoiceengine.h @@ -82,11 +82,8 @@ class FakeWebRtcVoiceEngine : public webrtc::VoEBase { return 0; } WEBRTC_STUB(StartPlayout, (int channel)); - WEBRTC_STUB(StartSend, (int channel)); WEBRTC_STUB(StopPlayout, (int channel)); - WEBRTC_STUB(StopSend, (int channel)); WEBRTC_STUB(SetPlayout, (bool enable)); - WEBRTC_STUB(SetRecording, (bool enable)); size_t GetNetEqCapacity() const { auto ch = channels_.find(last_channel_); diff --git a/test/mock_voe_channel_proxy.h b/test/mock_voe_channel_proxy.h index e075124552..ac3ffca817 100644 --- a/test/mock_voe_channel_proxy.h +++ b/test/mock_voe_channel_proxy.h @@ -98,6 +98,8 @@ class MockVoEChannelProxy : public voe::ChannelProxy { MOCK_METHOD1(OnRecoverableUplinkPacketLossRate, void(float recoverable_packet_loss_rate)); MOCK_CONST_METHOD0(GetSources, std::vector()); + MOCK_METHOD0(StartSend, void()); + MOCK_METHOD0(StopSend, void()); }; } // namespace test } // namespace webrtc diff --git a/test/mock_voice_engine.h b/test/mock_voice_engine.h index 5c966bd76c..a31c90fec1 100644 --- a/test/mock_voice_engine.h +++ b/test/mock_voice_engine.h @@ -92,8 +92,6 @@ class MockVoiceEngine : public VoiceEngineImpl { MOCK_METHOD1(DeleteChannel, int(int channel)); MOCK_METHOD1(StartPlayout, int(int channel)); MOCK_METHOD1(StopPlayout, int(int channel)); - MOCK_METHOD1(StartSend, int(int channel)); - MOCK_METHOD1(StopSend, int(int channel)); private: // TODO(ossu): I'm not particularly happy about keeping the decoder factory diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index 0aa12bb905..eaa90999b7 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -2084,7 +2084,6 @@ void VideoQualityTest::RunWithRenderers(const Params& params) { // Start sending audio. audio_send_stream_->Start(); - EXPECT_EQ(0, voe.base->StartSend(voe.send_channel_id)); } }); @@ -2093,7 +2092,6 @@ void VideoQualityTest::RunWithRenderers(const Params& params) { task_queue_.SendTask([&]() { if (params_.audio.enabled) { // Stop sending audio. - EXPECT_EQ(0, voe.base->StopSend(voe.send_channel_id)); audio_send_stream_->Stop(); // Stop receiving audio. diff --git a/voice_engine/channel_proxy.cc b/voice_engine/channel_proxy.cc index 505113dbbb..8adc1e2062 100644 --- a/voice_engine/channel_proxy.cc +++ b/voice_engine/channel_proxy.cc @@ -332,6 +332,17 @@ std::vector ChannelProxy::GetSources() const { return channel()->GetSources(); } +void ChannelProxy::StartSend() { + RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); + int error = channel()->StartSend(); + RTC_DCHECK_EQ(0, error); +} + +void ChannelProxy::StopSend() { + RTC_DCHECK(worker_thread_checker_.CalledOnValidThread()); + channel()->StopSend(); +} + Channel* ChannelProxy::channel() const { RTC_DCHECK(channel_owner_.channel()); return channel_owner_.channel(); diff --git a/voice_engine/channel_proxy.h b/voice_engine/channel_proxy.h index 1dec23a3eb..060f9d5928 100644 --- a/voice_engine/channel_proxy.h +++ b/voice_engine/channel_proxy.h @@ -121,6 +121,8 @@ class ChannelProxy : public RtpPacketSinkInterface { virtual void OnRecoverableUplinkPacketLossRate( float recoverable_packet_loss_rate); virtual std::vector GetSources() const; + virtual void StartSend(); + virtual void StopSend(); private: Channel* channel() const; diff --git a/voice_engine/include/voe_base.h b/voice_engine/include/voe_base.h index 238de1df95..f94279c749 100644 --- a/voice_engine/include/voe_base.h +++ b/voice_engine/include/voe_base.h @@ -117,13 +117,6 @@ class WEBRTC_DLLEXPORT VoEBase { // specified |channel|. virtual int StopPlayout(int channel) = 0; - // Starts sending packets to an already specified IP address and - // port number for a specified |channel|. - virtual int StartSend(int channel) = 0; - - // Stops sending packets from a specified |channel|. - virtual int StopSend(int channel) = 0; - // Enable or disable playout to the underlying device. Takes precedence over // StartPlayout. Though calls to StartPlayout are remembered; if // SetPlayout(true) is called after StartPlayout, playout will be started. @@ -131,14 +124,6 @@ class WEBRTC_DLLEXPORT VoEBase { // By default, playout is enabled. virtual int SetPlayout(bool enabled) = 0; - // Enable or disable recording (which drives sending of encoded audio packtes) - // from the underlying device. Takes precedence over StartSend. Though calls - // to StartSend are remembered; if SetRecording(true) is called after - // StartSend, recording will be started. - // - // By default, recording is enabled. - virtual int SetRecording(bool enabled) = 0; - protected: VoEBase() {} virtual ~VoEBase() {} diff --git a/voice_engine/shared_data.cc b/voice_engine/shared_data.cc index b60633d8cc..ae5407921b 100644 --- a/voice_engine/shared_data.cc +++ b/voice_engine/shared_data.cc @@ -44,19 +44,6 @@ void SharedData::set_audio_device( _audioDevicePtr = audio_device; } -int SharedData::NumOfSendingChannels() { - ChannelManager::Iterator it(&_channelManager); - int sending_channels = 0; - - for (ChannelManager::Iterator it(&_channelManager); it.IsValid(); - it.Increment()) { - if (it.GetChannel()->Sending()) - ++sending_channels; - } - - return sending_channels; -} - int SharedData::NumOfPlayingChannels() { ChannelManager::Iterator it(&_channelManager); int playout_channels = 0; diff --git a/voice_engine/shared_data.h b/voice_engine/shared_data.h index 1a7985a4a4..7c72f914a5 100644 --- a/voice_engine/shared_data.h +++ b/voice_engine/shared_data.h @@ -40,7 +40,6 @@ public: ProcessThread* process_thread() { return _moduleProcessThreadPtr.get(); } rtc::TaskQueue* encoder_queue(); - int NumOfSendingChannels(); int NumOfPlayingChannels(); protected: diff --git a/voice_engine/voe_base_impl.cc b/voice_engine/voe_base_impl.cc index 094288df74..793444b0e6 100644 --- a/voice_engine/voe_base_impl.cc +++ b/voice_engine/voe_base_impl.cc @@ -107,9 +107,6 @@ int VoEBaseImpl::DeleteChannel(int channel) { } shared_->channel_manager().DestroyChannel(channel); - if (StopSend() != 0) { - return -1; - } if (StopPlayout() != 0) { return -1; } @@ -149,36 +146,6 @@ int VoEBaseImpl::StopPlayout(int channel) { return StopPlayout(); } -int VoEBaseImpl::StartSend(int channel) { - rtc::CritScope cs(shared_->crit_sec()); - voe::ChannelOwner ch = shared_->channel_manager().GetChannel(channel); - voe::Channel* channelPtr = ch.channel(); - if (channelPtr == nullptr) { - RTC_LOG(LS_ERROR) << "StartSend() failed to locate channel"; - return -1; - } - if (channelPtr->Sending()) { - return 0; - } - if (StartSend() != 0) { - RTC_LOG(LS_ERROR) << "StartSend() failed to start recording"; - return -1; - } - return channelPtr->StartSend(); -} - -int VoEBaseImpl::StopSend(int channel) { - rtc::CritScope cs(shared_->crit_sec()); - voe::ChannelOwner ch = shared_->channel_manager().GetChannel(channel); - voe::Channel* channelPtr = ch.channel(); - if (channelPtr == nullptr) { - RTC_LOG(LS_ERROR) << "StopSend() failed to locate channel"; - return -1; - } - channelPtr->StopSend(); - return StopSend(); -} - int32_t VoEBaseImpl::StartPlayout() { if (!shared_->audio_device()->Playing()) { if (shared_->audio_device()->InitPlayout() != 0) { @@ -207,35 +174,6 @@ int32_t VoEBaseImpl::StopPlayout() { return 0; } -int32_t VoEBaseImpl::StartSend() { - if (!shared_->audio_device()->Recording()) { - if (shared_->audio_device()->InitRecording() != 0) { - RTC_LOG_F(LS_ERROR) << "Failed to initialize recording"; - return -1; - } - if (recording_enabled_ && shared_->audio_device()->StartRecording() != 0) { - RTC_LOG_F(LS_ERROR) << "Failed to start recording"; - return -1; - } - } - return 0; -} - -int32_t VoEBaseImpl::StopSend() { - if (!recording_enabled_) { - return 0; - } - // Stop audio-device recording if no channel is recording. - if (shared_->NumOfSendingChannels() == 0) { - if (shared_->audio_device()->StopRecording() != 0) { - RTC_LOG(LS_ERROR) << "StopSend() failed to stop recording"; - return -1; - } - } - - return 0; -} - int32_t VoEBaseImpl::SetPlayout(bool enabled) { RTC_LOG(INFO) << "SetPlayout(" << enabled << ")"; if (playout_enabled_ == enabled) { @@ -262,32 +200,6 @@ int32_t VoEBaseImpl::SetPlayout(bool enabled) { return ret; } -int32_t VoEBaseImpl::SetRecording(bool enabled) { - RTC_LOG(INFO) << "SetRecording(" << enabled << ")"; - if (recording_enabled_ == enabled) { - return 0; - } - recording_enabled_ = enabled; - if (shared_->NumOfSendingChannels() == 0) { - // If there are no channels attempting to record out yet, there's nothing to - // be done; we should be in a "not recording" state either way. - return 0; - } - int32_t ret; - if (enabled) { - ret = shared_->audio_device()->StartRecording(); - if (ret != 0) { - RTC_LOG(LS_ERROR) << "SetRecording(true) failed to start recording"; - } - } else { - ret = shared_->audio_device()->StopRecording(); - if (ret != 0) { - RTC_LOG(LS_ERROR) << "SetRecording(false) failed to stop recording"; - } - } - return ret; -} - void VoEBaseImpl::TerminateInternal() { // Delete any remaining channel objects shared_->channel_manager().DestroyAllChannels(); diff --git a/voice_engine/voe_base_impl.h b/voice_engine/voe_base_impl.h index 2649b8c09b..2692c04140 100644 --- a/voice_engine/voe_base_impl.h +++ b/voice_engine/voe_base_impl.h @@ -34,12 +34,9 @@ class VoEBaseImpl : public VoEBase { int DeleteChannel(int channel) override; int StartPlayout(int channel) override; - int StartSend(int channel) override; int StopPlayout(int channel) override; - int StopSend(int channel) override; int SetPlayout(bool enabled) override; - int SetRecording(bool enabled) override; protected: VoEBaseImpl(voe::SharedData* shared); @@ -48,15 +45,8 @@ class VoEBaseImpl : public VoEBase { private: int32_t StartPlayout(); int32_t StopPlayout(); - int32_t StartSend(); - int32_t StopSend(); void TerminateInternal(); - void GetPlayoutData(int sample_rate, size_t number_of_channels, - size_t number_of_frames, bool feed_data_to_apm, - void* audio_data, int64_t* elapsed_time_ms, - int64_t* ntp_time_ms); - // Initialize channel by setting Engine Information then initializing // channel. int InitializeChannel(voe::ChannelOwner* channel_owner); @@ -65,7 +55,6 @@ class VoEBaseImpl : public VoEBase { AudioFrame audioFrame_; voe::SharedData* shared_; bool playout_enabled_ = true; - bool recording_enabled_ = true; }; } // namespace webrtc