From c28a896a7bbd8a1ffef44a1f66ac67c43b4eeada Mon Sep 17 00:00:00 2001 From: Jelena Marusic Date: Fri, 29 May 2015 15:05:44 +0200 Subject: [PATCH] VoE: Initialize WebRtcVoiceMediaChannel with AudioOptions during creation BUG=4690 Changes: 1. In MediaEngineInterface changed CreateChannel() to CreateChannel(const AudioOptions&). Plan is to eventually remove Get/SetAudioOptions and the cousins SetDelayOffset and SetDevices. 2. In ChannelManager changed CreateVoiceChannel(...) to CreateVoiceChannel(..., const AudioOptions&). 3. In ChannelManager removed SetEngineAudioOptions, because it is not used and we want to eventually remove SetAudioOptions. 4. Updated MediaEngineInterface implementations and unit tests accordingly. 5. In WebRtcVoiceEngine changed access of Set/ClearOptionOverrides to protected. These are only used by WebRtcVoiceMediaChannel (now a friend). Plan is to rethink the logic behind option overrides. 6. Cosmetics: replaced NULL with nullptr in touched code R=solenberg@google.com, tommi@webrtc.org Review URL: https://webrtc-codereview.appspot.com/56499004 Cr-Commit-Position: refs/heads/master@{#9330} --- talk/app/webrtc/webrtcsession.cc | 3 +- talk/media/base/fakemediaengine.h | 5 +- talk/media/base/filemediaengine.cc | 18 ++++--- talk/media/base/filemediaengine.h | 2 +- talk/media/base/filemediaengine_unittest.cc | 8 +-- talk/media/base/mediaengine.h | 10 ++-- .../webrtc/webrtcvideoengine2_unittest.cc | 8 +-- talk/media/webrtc/webrtcvoiceengine.cc | 8 ++- talk/media/webrtc/webrtcvoiceengine.h | 30 ++++++----- .../webrtc/webrtcvoiceengine_unittest.cc | 53 ++++++++++--------- talk/session/media/channelmanager.cc | 45 +++++----------- talk/session/media/channelmanager.h | 15 +++--- talk/session/media/channelmanager_unittest.cc | 45 +++++----------- webrtc/libjingle/session/media/call.cc | 2 +- 14 files changed, 113 insertions(+), 139 deletions(-) diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc index a058129ca..766c5d418 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -1633,12 +1633,11 @@ bool WebRtcSession::CreateChannels(const SessionDescription* desc) { bool WebRtcSession::CreateVoiceChannel(const cricket::ContentInfo* content) { voice_channel_.reset(channel_manager_->CreateVoiceChannel( - this, content->name, true)); + this, content->name, true, audio_options_)); if (!voice_channel_) { return false; } - voice_channel_->SetChannelOptions(audio_options_); voice_channel_->SignalDtlsSetupFailure.connect( this, &WebRtcSession::OnDtlsSetupFailure); return true; diff --git a/talk/media/base/fakemediaengine.h b/talk/media/base/fakemediaengine.h index 70f89c673..15fcb202f 100644 --- a/talk/media/base/fakemediaengine.h +++ b/talk/media/base/fakemediaengine.h @@ -763,12 +763,13 @@ class FakeVoiceEngine : public FakeBaseEngine { return true; } - VoiceMediaChannel* CreateChannel() { + VoiceMediaChannel* CreateChannel(const AudioOptions& options) { if (fail_create_channel_) { - return NULL; + return nullptr; } FakeVoiceMediaChannel* ch = new FakeVoiceMediaChannel(this); + ch->SetOptions(options); channels_.push_back(ch); return ch; } diff --git a/talk/media/base/filemediaengine.cc b/talk/media/base/filemediaengine.cc index 0fc8d5610..5f4c83996 100644 --- a/talk/media/base/filemediaengine.cc +++ b/talk/media/base/filemediaengine.cc @@ -61,18 +61,18 @@ int FileMediaEngine::GetCapabilities() { return capabilities; } -VoiceMediaChannel* FileMediaEngine::CreateChannel() { - rtc::FileStream* input_file_stream = NULL; - rtc::FileStream* output_file_stream = NULL; +VoiceMediaChannel* FileMediaEngine::CreateChannel(const AudioOptions& options) { + rtc::FileStream* input_file_stream = nullptr; + rtc::FileStream* output_file_stream = nullptr; if (voice_input_filename_.empty() && voice_output_filename_.empty()) - return NULL; + return nullptr; if (!voice_input_filename_.empty()) { input_file_stream = rtc::Filesystem::OpenFile( rtc::Pathname(voice_input_filename_), "rb"); if (!input_file_stream) { LOG(LS_ERROR) << "Not able to open the input audio stream file."; - return NULL; + return nullptr; } } @@ -82,12 +82,14 @@ VoiceMediaChannel* FileMediaEngine::CreateChannel() { if (!output_file_stream) { delete input_file_stream; LOG(LS_ERROR) << "Not able to open the output audio stream file."; - return NULL; + return nullptr; } } - return new FileVoiceChannel(input_file_stream, output_file_stream, - rtp_sender_thread_); + FileVoiceChannel* channel = new FileVoiceChannel( + input_file_stream, output_file_stream, rtp_sender_thread_); + channel->SetOptions(options); + return channel; } VideoMediaChannel* FileMediaEngine::CreateVideoChannel( diff --git a/talk/media/base/filemediaengine.h b/talk/media/base/filemediaengine.h index 90425ad84..88f179f1b 100644 --- a/talk/media/base/filemediaengine.h +++ b/talk/media/base/filemediaengine.h @@ -85,7 +85,7 @@ class FileMediaEngine : public MediaEngineInterface { } virtual void Terminate() {} virtual int GetCapabilities(); - virtual VoiceMediaChannel* CreateChannel(); + virtual VoiceMediaChannel* CreateChannel(const AudioOptions& options); virtual VideoMediaChannel* CreateVideoChannel(const VideoOptions& options, VoiceMediaChannel* voice_ch); virtual AudioOptions GetAudioOptions() const { return AudioOptions(); } diff --git a/talk/media/base/filemediaengine_unittest.cc b/talk/media/base/filemediaengine_unittest.cc index e54aa9c7c..9b26c3b13 100644 --- a/talk/media/base/filemediaengine_unittest.cc +++ b/talk/media/base/filemediaengine_unittest.cc @@ -136,8 +136,8 @@ class FileMediaEngineTest : public testing::Test { engine_->set_video_output_filename(video_out); engine_->set_rtp_sender_thread(rtc::Thread::Current()); - voice_channel_.reset(engine_->CreateChannel()); - video_channel_.reset(engine_->CreateVideoChannel(VideoOptions(), NULL)); + voice_channel_.reset(engine_->CreateChannel(AudioOptions())); + video_channel_.reset(engine_->CreateVideoChannel(VideoOptions(), nullptr)); } bool GetTempFilename(std::string* filename) { @@ -240,8 +240,8 @@ TEST_F(FileMediaEngineTest, TestBadFilePath) { engine_.reset(new FileMediaEngine); engine_->set_voice_input_filename(kFakeFileName); engine_->set_video_input_filename(kFakeFileName); - EXPECT_TRUE(engine_->CreateChannel() == NULL); - EXPECT_TRUE(engine_->CreateVideoChannel(VideoOptions(), NULL) == NULL); + EXPECT_TRUE(engine_->CreateChannel(AudioOptions()) == nullptr); + EXPECT_TRUE(engine_->CreateVideoChannel(VideoOptions(), nullptr) == nullptr); } TEST_F(FileMediaEngineTest, TestCodecs) { diff --git a/talk/media/base/mediaengine.h b/talk/media/base/mediaengine.h index 439622c5d..501333f94 100644 --- a/talk/media/base/mediaengine.h +++ b/talk/media/base/mediaengine.h @@ -77,7 +77,7 @@ class MediaEngineInterface { // MediaChannel creation // Creates a voice media channel. Returns NULL on failure. - virtual VoiceMediaChannel *CreateChannel() = 0; + virtual VoiceMediaChannel* CreateChannel(const AudioOptions& options) = 0; // Creates a video media channel, paired with the specified voice channel. // Returns NULL on failure. virtual VideoMediaChannel* CreateVideoChannel( @@ -178,8 +178,8 @@ class CompositeMediaEngine : public MediaEngineInterface { virtual int GetCapabilities() { return (voice_.GetCapabilities() | video_.GetCapabilities()); } - virtual VoiceMediaChannel *CreateChannel() { - return voice_.CreateChannel(); + virtual VoiceMediaChannel* CreateChannel(const AudioOptions& options) { + return voice_.CreateChannel(options); } virtual VideoMediaChannel* CreateVideoChannel(const VideoOptions& options, VoiceMediaChannel* channel) { @@ -265,8 +265,8 @@ class NullVoiceEngine { void Terminate() {} int GetCapabilities() { return 0; } // If you need this to return an actual channel, use FakeMediaEngine instead. - VoiceMediaChannel* CreateChannel() { - return NULL; + VoiceMediaChannel* CreateChannel(const AudioOptions& options) { + return nullptr; } bool SetDelayOffset(int offset) { return true; } AudioOptions GetOptions() const { return AudioOptions(); } diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.cc b/talk/media/webrtc/webrtcvideoengine2_unittest.cc index f7269b1cc..afce214f0 100644 --- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc @@ -177,8 +177,8 @@ TEST_F(WebRtcVideoEngine2VoiceTest, ConfiguresAvSyncForFirstReceiveChannel) { engine_.Init(); rtc::scoped_ptr voice_channel( - voice_engine_.CreateChannel()); - ASSERT_TRUE(voice_channel.get() != NULL); + voice_engine_.CreateChannel(cricket::AudioOptions())); + ASSERT_TRUE(voice_channel.get() != nullptr); WebRtcVoiceMediaChannel* webrtc_voice_channel = static_cast(voice_channel.get()); ASSERT_NE(webrtc_voice_channel->voe_channel(), -1); @@ -186,11 +186,11 @@ TEST_F(WebRtcVideoEngine2VoiceTest, ConfiguresAvSyncForFirstReceiveChannel) { engine_.CreateChannel(cricket::VideoOptions(), voice_channel.get())); FakeCall* fake_call = call_factory.GetCall(); - ASSERT_TRUE(fake_call != NULL); + ASSERT_TRUE(fake_call != nullptr); webrtc::Call::Config call_config = fake_call->GetConfig(); - ASSERT_TRUE(voice_engine_.voe()->engine() != NULL); + ASSERT_TRUE(voice_engine_.voe()->engine() != nullptr); ASSERT_EQ(voice_engine_.voe()->engine(), call_config.voice_engine); EXPECT_TRUE(channel->AddRecvStream(StreamParams::CreateLegacy(kSsrc))); diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc index 57405ca74..230953e88 100644 --- a/talk/media/webrtc/webrtcvoiceengine.cc +++ b/talk/media/webrtc/webrtcvoiceengine.cc @@ -584,11 +584,15 @@ int WebRtcVoiceEngine::GetCapabilities() { return AUDIO_SEND | AUDIO_RECV; } -VoiceMediaChannel *WebRtcVoiceEngine::CreateChannel() { +VoiceMediaChannel* WebRtcVoiceEngine::CreateChannel( + const AudioOptions& options) { WebRtcVoiceMediaChannel* ch = new WebRtcVoiceMediaChannel(this); if (!ch->valid()) { delete ch; - ch = NULL; + return nullptr; + } + if (!ch->SetOptions(options)) { + LOG(LS_WARNING) << "Failed to set options while creating channel."; } return ch; } diff --git a/talk/media/webrtc/webrtcvoiceengine.h b/talk/media/webrtc/webrtcvoiceengine.h index 076922b06..65dde0854 100644 --- a/talk/media/webrtc/webrtcvoiceengine.h +++ b/talk/media/webrtc/webrtcvoiceengine.h @@ -89,6 +89,8 @@ class WebRtcVoiceEngine : public webrtc::VoiceEngineObserver, public webrtc::TraceCallback, public webrtc::VoEMediaProcess { + friend class WebRtcVoiceMediaChannel; + public: WebRtcVoiceEngine(); // Dependency injection for testing. @@ -98,23 +100,10 @@ class WebRtcVoiceEngine void Terminate(); int GetCapabilities(); - VoiceMediaChannel* CreateChannel(); + VoiceMediaChannel* CreateChannel(const AudioOptions& options); AudioOptions GetOptions() const { return options_; } bool SetOptions(const AudioOptions& options); - // Overrides, when set, take precedence over the options on a - // per-option basis. For example, if AGC is set in options and AEC - // is set in overrides, AGC and AEC will be both be set. Overrides - // can also turn off options. For example, if AGC is set to "on" in - // options and AGC is set to "off" in overrides, the result is that - // AGC will be off until different overrides are applied or until - // the overrides are cleared. Only one set of overrides is present - // at a time (they do not "stack"). And when the overrides are - // cleared, the media engine's state reverts back to the options set - // via SetOptions. This allows us to have both "persistent options" - // (the normal options) and "temporary options" (overrides). - bool SetOptionOverrides(const AudioOptions& options); - bool ClearOptionOverrides(); bool SetDelayOffset(int offset); bool SetDevices(const Device* in_device, const Device* out_device); bool GetOutputVolume(int* level); @@ -186,6 +175,19 @@ class WebRtcVoiceEngine // allows us to selectively turn on and off different options easily // at any time. bool ApplyOptions(const AudioOptions& options); + // Overrides, when set, take precedence over the options on a + // per-option basis. For example, if AGC is set in options and AEC + // is set in overrides, AGC and AEC will be both be set. Overrides + // can also turn off options. For example, if AGC is set to "on" in + // options and AGC is set to "off" in overrides, the result is that + // AGC will be off until different overrides are applied or until + // the overrides are cleared. Only one set of overrides is present + // at a time (they do not "stack"). And when the overrides are + // cleared, the media engine's state reverts back to the options set + // via SetOptions. This allows us to have both "persistent options" + // (the normal options) and "temporary options" (overrides). + bool SetOptionOverrides(const AudioOptions& options); + bool ClearOptionOverrides(); // webrtc::TraceCallback: void Print(webrtc::TraceLevel level, const char* trace, int length) override; diff --git a/talk/media/webrtc/webrtcvoiceengine_unittest.cc b/talk/media/webrtc/webrtcvoiceengine_unittest.cc index 7d1fb500c..658001104 100644 --- a/talk/media/webrtc/webrtcvoiceengine_unittest.cc +++ b/talk/media/webrtc/webrtcvoiceengine_unittest.cc @@ -129,9 +129,8 @@ class WebRtcVoiceEngineTestFake : public testing::Test { WebRtcVoiceEngineTestFake() : voe_(kAudioCodecs, ARRAY_SIZE(kAudioCodecs)), trace_wrapper_(new FakeVoETraceWrapper()), - engine_(new FakeVoEWrapper(&voe_), - trace_wrapper_), - channel_(NULL) { + engine_(new FakeVoEWrapper(&voe_), trace_wrapper_), + channel_(nullptr) { options_conference_.conference_mode.Set(true); options_adjust_agc_.adjust_agc_delta.Set(-10); } @@ -139,8 +138,8 @@ class WebRtcVoiceEngineTestFake : public testing::Test { if (!engine_.Init(rtc::Thread::Current())) { return false; } - channel_ = engine_.CreateChannel(); - return (channel_ != NULL); + channel_ = engine_.CreateChannel(cricket::AudioOptions()); + return (channel_ != nullptr); } bool SetupEngine() { if (!SetupEngineWithoutStream()) { @@ -172,8 +171,8 @@ class WebRtcVoiceEngineTestFake : public testing::Test { void TestInsertDtmf(uint32 ssrc, bool caller) { EXPECT_TRUE(engine_.Init(rtc::Thread::Current())); - channel_ = engine_.CreateChannel(); - EXPECT_TRUE(channel_ != NULL); + channel_ = engine_.CreateChannel(cricket::AudioOptions()); + EXPECT_TRUE(channel_ != nullptr); if (caller) { // if this is a caller, local description will be applied and add the // send stream. @@ -352,16 +351,16 @@ TEST_F(WebRtcVoiceEngineTestFake, StartupShutdown) { // Tests that we can create and destroy a channel. TEST_F(WebRtcVoiceEngineTestFake, CreateChannel) { EXPECT_TRUE(engine_.Init(rtc::Thread::Current())); - channel_ = engine_.CreateChannel(); - EXPECT_TRUE(channel_ != NULL); + channel_ = engine_.CreateChannel(cricket::AudioOptions()); + EXPECT_TRUE(channel_ != nullptr); } // Tests that we properly handle failures in CreateChannel. TEST_F(WebRtcVoiceEngineTestFake, CreateChannelFail) { voe_.set_fail_create_channel(true); EXPECT_TRUE(engine_.Init(rtc::Thread::Current())); - channel_ = engine_.CreateChannel(); - EXPECT_TRUE(channel_ == NULL); + channel_ = engine_.CreateChannel(cricket::AudioOptions()); + EXPECT_TRUE(channel_ == nullptr); } // Tests that the list of supported codecs is created properly and ordered @@ -669,8 +668,8 @@ TEST_F(WebRtcVoiceEngineTestFake, SetMaxSendBandwidthFixedRateAsCaller) { TEST_F(WebRtcVoiceEngineTestFake, SetMaxSendBandwidthMultiRateAsCallee) { EXPECT_TRUE(engine_.Init(rtc::Thread::Current())); - channel_ = engine_.CreateChannel(); - EXPECT_TRUE(channel_ != NULL); + channel_ = engine_.CreateChannel(cricket::AudioOptions()); + EXPECT_TRUE(channel_ != nullptr); EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs())); int desired_bitrate = 128000; @@ -1043,8 +1042,8 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecEnableNackAsCaller) { // Test that we can enable NACK with opus as callee. TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecEnableNackAsCallee) { EXPECT_TRUE(engine_.Init(rtc::Thread::Current())); - channel_ = engine_.CreateChannel(); - EXPECT_TRUE(channel_ != NULL); + channel_ = engine_.CreateChannel(cricket::AudioOptions()); + EXPECT_TRUE(channel_ != nullptr); int channel_num = voe_.GetLastChannel(); std::vector codecs; @@ -1623,8 +1622,8 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsCNandDTMFAsCaller) { // Test that we set VAD and DTMF types correctly as callee. TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsCNandDTMFAsCallee) { EXPECT_TRUE(engine_.Init(rtc::Thread::Current())); - channel_ = engine_.CreateChannel(); - EXPECT_TRUE(channel_ != NULL); + channel_ = engine_.CreateChannel(cricket::AudioOptions()); + EXPECT_TRUE(channel_ != nullptr); int channel_num = voe_.GetLastChannel(); std::vector codecs; @@ -1740,8 +1739,8 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsREDAsCaller) { // Test that we set up RED correctly as callee. TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecsREDAsCallee) { EXPECT_TRUE(engine_.Init(rtc::Thread::Current())); - channel_ = engine_.CreateChannel(); - EXPECT_TRUE(channel_ != NULL); + channel_ = engine_.CreateChannel(cricket::AudioOptions()); + EXPECT_TRUE(channel_ != nullptr); int channel_num = voe_.GetLastChannel(); std::vector codecs; @@ -2423,7 +2422,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendSsrcWithMultipleStreams) { // receive channel is created before the send channel. TEST_F(WebRtcVoiceEngineTestFake, SetSendSsrcAfterCreatingReceiveChannel) { EXPECT_TRUE(engine_.Init(rtc::Thread::Current())); - channel_ = engine_.CreateChannel(); + channel_ = engine_.CreateChannel(cricket::AudioOptions()); EXPECT_TRUE(channel_->SetOptions(options_conference_)); EXPECT_TRUE(channel_->AddRecvStream(cricket::StreamParams::CreateLegacy(1))); @@ -2991,9 +2990,9 @@ TEST_F(WebRtcVoiceEngineTestFake, InitDoesNotOverwriteDefaultAgcConfig) { TEST_F(WebRtcVoiceEngineTestFake, SetOptionOverridesViaChannels) { EXPECT_TRUE(SetupEngine()); rtc::scoped_ptr channel1( - engine_.CreateChannel()); + engine_.CreateChannel(cricket::AudioOptions())); rtc::scoped_ptr channel2( - engine_.CreateChannel()); + engine_.CreateChannel(cricket::AudioOptions())); // Have to add a stream to make SetSend work. cricket::StreamParams stream1; @@ -3111,7 +3110,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetOptionOverridesViaChannels) { TEST_F(WebRtcVoiceEngineTestFake, TestSetDscpOptions) { EXPECT_TRUE(SetupEngine()); rtc::scoped_ptr channel( - engine_.CreateChannel()); + engine_.CreateChannel(cricket::AudioOptions())); rtc::scoped_ptr network_interface( new cricket::FakeNetworkInterface); channel->SetInterface(network_interface.get()); @@ -3193,8 +3192,9 @@ TEST_F(WebRtcVoiceEngineTestFake, SetOutputScaling) { TEST(WebRtcVoiceEngineTest, StartupShutdown) { cricket::WebRtcVoiceEngine engine; EXPECT_TRUE(engine.Init(rtc::Thread::Current())); - cricket::VoiceMediaChannel* channel = engine.CreateChannel(); - EXPECT_TRUE(channel != NULL); + cricket::VoiceMediaChannel* channel = + engine.CreateChannel(cricket::AudioOptions()); + EXPECT_TRUE(channel != nullptr); delete channel; engine.Terminate(); @@ -3292,7 +3292,8 @@ TEST(WebRtcVoiceEngineTest, Has32Channels) { int num_channels = 0; while (num_channels < ARRAY_SIZE(channels)) { - cricket::VoiceMediaChannel* channel = engine.CreateChannel(); + cricket::VoiceMediaChannel* channel = + engine.CreateChannel(cricket::AudioOptions()); if (!channel) break; diff --git a/talk/session/media/channelmanager.cc b/talk/session/media/channelmanager.cc index 987cec709..52116ddb3 100644 --- a/talk/session/media/channelmanager.cc +++ b/talk/session/media/channelmanager.cc @@ -317,26 +317,32 @@ void ChannelManager::Terminate_w() { } VoiceChannel* ChannelManager::CreateVoiceChannel( - BaseSession* session, const std::string& content_name, bool rtcp) { + BaseSession* session, + const std::string& content_name, + bool rtcp, + const AudioOptions& options) { return worker_thread_->Invoke( - Bind(&ChannelManager::CreateVoiceChannel_w, this, - session, content_name, rtcp)); + Bind(&ChannelManager::CreateVoiceChannel_w, this, session, content_name, + rtcp, options)); } VoiceChannel* ChannelManager::CreateVoiceChannel_w( - BaseSession* session, const std::string& content_name, bool rtcp) { + BaseSession* session, + const std::string& content_name, + bool rtcp, + const AudioOptions& options) { ASSERT(initialized_); ASSERT(worker_thread_ == rtc::Thread::Current()); - VoiceMediaChannel* media_channel = media_engine_->CreateChannel(); - if (media_channel == NULL) - return NULL; + VoiceMediaChannel* media_channel = media_engine_->CreateChannel(options); + if (!media_channel) + return nullptr; VoiceChannel* voice_channel = new VoiceChannel( worker_thread_, media_engine_.get(), media_channel, session, content_name, rtcp); if (!voice_channel->Init()) { delete voice_channel; - return NULL; + return nullptr; } voice_channels_.push_back(voice_channel); return voice_channel; @@ -572,29 +578,6 @@ bool ChannelManager::SetAudioOptions_w( return ret; } -// Sets Engine-specific audio options according to enabled experiments. -bool ChannelManager::SetEngineAudioOptions(const AudioOptions& options) { - // If we're initialized, pass the settings to the media engine. - bool ret = false; - if (initialized_) { - ret = worker_thread_->Invoke( - Bind(&ChannelManager::SetEngineAudioOptions_w, this, options)); - } - - // If all worked well, save the audio options. - if (ret) { - audio_options_ = options; - } - return ret; -} - -bool ChannelManager::SetEngineAudioOptions_w(const AudioOptions& options) { - ASSERT(worker_thread_ == rtc::Thread::Current()); - ASSERT(initialized_); - - return media_engine_->SetAudioOptions(options); -} - bool ChannelManager::GetOutputVolume(int* level) { if (!initialized_) { return false; diff --git a/talk/session/media/channelmanager.h b/talk/session/media/channelmanager.h index ba9338947..898ae1cb2 100644 --- a/talk/session/media/channelmanager.h +++ b/talk/session/media/channelmanager.h @@ -103,8 +103,10 @@ class ChannelManager : public rtc::MessageHandler, // The operations below all occur on the worker thread. // Creates a voice channel, to be associated with the specified session. - VoiceChannel* CreateVoiceChannel( - BaseSession* session, const std::string& content_name, bool rtcp); + VoiceChannel* CreateVoiceChannel(BaseSession* session, + const std::string& content_name, + bool rtcp, + const AudioOptions& options); // Destroys a voice channel created with the Create API. void DestroyVoiceChannel(VoiceChannel* voice_channel, VideoChannel* video_channel); @@ -141,8 +143,6 @@ class ChannelManager : public rtc::MessageHandler, bool SetAudioOptions(const std::string& wave_in_device, const std::string& wave_out_device, const AudioOptions& options); - // Sets Engine-specific audio options according to enabled experiments. - bool SetEngineAudioOptions(const AudioOptions& options); bool GetOutputVolume(int* level); bool SetOutputVolume(int level); bool IsSameCapturer(const std::string& capturer_name, @@ -255,8 +255,10 @@ class ChannelManager : public rtc::MessageHandler, bool InitMediaEngine_w(); void DestructorDeletes_w(); void Terminate_w(); - VoiceChannel* CreateVoiceChannel_w( - BaseSession* session, const std::string& content_name, bool rtcp); + VoiceChannel* CreateVoiceChannel_w(BaseSession* session, + const std::string& content_name, + bool rtcp, + const AudioOptions& options); void DestroyVoiceChannel_w(VoiceChannel* voice_channel, VideoChannel* video_channel); VideoChannel* CreateVideoChannel_w(BaseSession* session, @@ -271,7 +273,6 @@ class ChannelManager : public rtc::MessageHandler, void DestroyDataChannel_w(DataChannel* data_channel); bool SetAudioOptions_w(const AudioOptions& options, int delay_offset, const Device* in_dev, const Device* out_dev); - bool SetEngineAudioOptions_w(const AudioOptions& options); bool SetCaptureDevice_w(const Device* cam_device); void OnVideoCaptureStateChange(VideoCapturer* capturer, CaptureState result); diff --git a/talk/session/media/channelmanager_unittest.cc b/talk/session/media/channelmanager_unittest.cc index b0abf0487..1ffdaf283 100644 --- a/talk/session/media/channelmanager_unittest.cc +++ b/talk/session/media/channelmanager_unittest.cc @@ -125,15 +125,15 @@ TEST_F(ChannelManagerTest, StartupShutdownOnThread) { TEST_F(ChannelManagerTest, CreateDestroyChannels) { EXPECT_TRUE(cm_->Init()); cricket::VoiceChannel* voice_channel = cm_->CreateVoiceChannel( - session_, cricket::CN_AUDIO, false); - EXPECT_TRUE(voice_channel != NULL); + session_, cricket::CN_AUDIO, false, AudioOptions()); + EXPECT_TRUE(voice_channel != nullptr); cricket::VideoChannel* video_channel = cm_->CreateVideoChannel( session_, cricket::CN_VIDEO, false, VideoOptions(), voice_channel); - EXPECT_TRUE(video_channel != NULL); + EXPECT_TRUE(video_channel != nullptr); cricket::DataChannel* data_channel = cm_->CreateDataChannel(session_, cricket::CN_DATA, false, cricket::DCT_RTP); - EXPECT_TRUE(data_channel != NULL); + EXPECT_TRUE(data_channel != nullptr); cm_->DestroyVideoChannel(video_channel); cm_->DestroyVoiceChannel(voice_channel, nullptr); cm_->DestroyDataChannel(data_channel); @@ -148,15 +148,15 @@ TEST_F(ChannelManagerTest, CreateDestroyChannelsOnThread) { delete session_; session_ = new cricket::FakeSession(&worker_, true); cricket::VoiceChannel* voice_channel = cm_->CreateVoiceChannel( - session_, cricket::CN_AUDIO, false); - EXPECT_TRUE(voice_channel != NULL); + session_, cricket::CN_AUDIO, false, AudioOptions()); + EXPECT_TRUE(voice_channel != nullptr); cricket::VideoChannel* video_channel = cm_->CreateVideoChannel( session_, cricket::CN_VIDEO, false, VideoOptions(), voice_channel); - EXPECT_TRUE(video_channel != NULL); + EXPECT_TRUE(video_channel != nullptr); cricket::DataChannel* data_channel = cm_->CreateDataChannel(session_, cricket::CN_DATA, false, cricket::DCT_RTP); - EXPECT_TRUE(data_channel != NULL); + EXPECT_TRUE(data_channel != nullptr); cm_->DestroyVideoChannel(video_channel); cm_->DestroyVoiceChannel(voice_channel, nullptr); cm_->DestroyDataChannel(data_channel); @@ -171,18 +171,18 @@ TEST_F(ChannelManagerTest, NoTransportChannelTest) { // The test is useless unless the session does not fail creating // cricket::TransportChannel. ASSERT_TRUE(session_->CreateChannel( - "audio", cricket::ICE_CANDIDATE_COMPONENT_RTP) == NULL); + "audio", cricket::ICE_CANDIDATE_COMPONENT_RTP) == nullptr); cricket::VoiceChannel* voice_channel = cm_->CreateVoiceChannel( - session_, cricket::CN_AUDIO, false); - EXPECT_TRUE(voice_channel == NULL); + session_, cricket::CN_AUDIO, false, AudioOptions()); + EXPECT_TRUE(voice_channel == nullptr); cricket::VideoChannel* video_channel = cm_->CreateVideoChannel( session_, cricket::CN_VIDEO, false, VideoOptions(), voice_channel); - EXPECT_TRUE(video_channel == NULL); + EXPECT_TRUE(video_channel == nullptr); cricket::DataChannel* data_channel = cm_->CreateDataChannel(session_, cricket::CN_DATA, false, cricket::DCT_RTP); - EXPECT_TRUE(data_channel == NULL); + EXPECT_TRUE(data_channel == nullptr); cm_->Terminate(); } @@ -309,25 +309,6 @@ TEST_F(ChannelManagerTest, SetAudioOptions) { EXPECT_FALSE(cm_->SetAudioOptions("audio-in9", "audio-out2", options)); } -TEST_F(ChannelManagerTest, SetEngineAudioOptions) { - EXPECT_TRUE(cm_->Init()); - // Test setting specific values. - AudioOptions options; - options.experimental_ns.Set(true); - EXPECT_TRUE(cm_->SetEngineAudioOptions(options)); - bool experimental_ns = false; - EXPECT_TRUE(fme_->audio_options().experimental_ns.Get(&experimental_ns)); - EXPECT_TRUE(experimental_ns); -} - -TEST_F(ChannelManagerTest, SetEngineAudioOptionsBeforeInitFails) { - // Test that values that we set before Init are not applied. - AudioOptions options; - options.experimental_ns.Set(true); - EXPECT_FALSE(cm_->SetEngineAudioOptions(options)); - EXPECT_FALSE(fme_->audio_options().experimental_ns.IsSet()); -} - TEST_F(ChannelManagerTest, SetCaptureDeviceBeforeInit) { // Test that values that we set before Init are applied. EXPECT_TRUE(cm_->SetCaptureDevice("video-in2")); diff --git a/webrtc/libjingle/session/media/call.cc b/webrtc/libjingle/session/media/call.cc index c75c5d438..6a5102e5a 100644 --- a/webrtc/libjingle/session/media/call.cc +++ b/webrtc/libjingle/session/media/call.cc @@ -269,7 +269,7 @@ bool Call::AddSession(Session* session, const SessionDescription* offer) { // Create voice channel and start a media monitor. media_session.voice_channel = session_client_->channel_manager()->CreateVoiceChannel( - session, audio_offer->name, has_video_); + session, audio_offer->name, has_video_, AudioOptions()); // voice_channel can be NULL in case of NullVoiceEngine. if (media_session.voice_channel) { media_session.voice_channel->SignalMediaMonitor.connect(