From 709ed67c38d0a942f3bf3e68e337cc27a27bc353 Mon Sep 17 00:00:00 2001 From: Fredrik Solenberg Date: Tue, 15 Sep 2015 12:26:33 +0200 Subject: [PATCH] Move instantiation of webrtc::Call into a MediaController class so that it can be used for both audio and video media channels. I'm not super happy with the GetVoE() function added on MediaEngineInterface, but this will eventually be gone, once webrtc::Call owns the shared VoE state (or initially, maps ADM* to an implicitly created VoE). BUG=webrtc:4690 R=pbos@webrtc.org, pthatcher@webrtc.org Review URL: https://codereview.webrtc.org/1269863005 . Cr-Commit-Position: refs/heads/master@{#9939} --- talk/app/webrtc/mediacontroller.cc | 62 +++++- talk/app/webrtc/mediacontroller.h | 24 ++- talk/app/webrtc/webrtcsession.cc | 14 +- talk/app/webrtc/webrtcsession.h | 3 +- talk/libjingle.gyp | 2 + talk/media/base/fakemediaengine.h | 13 +- talk/media/base/mediachannel.h | 2 - talk/media/base/mediaengine.h | 32 ++- talk/media/base/videoengine_unittest.h | 11 +- talk/media/webrtc/webrtcvideoengine2.cc | 75 ++----- talk/media/webrtc/webrtcvideoengine2.h | 38 +--- .../webrtc/webrtcvideoengine2_unittest.cc | 109 ++++------ talk/media/webrtc/webrtcvoiceengine.cc | 92 ++++----- talk/media/webrtc/webrtcvoiceengine.h | 18 +- .../webrtc/webrtcvoiceengine_unittest.cc | 195 ++++++------------ talk/session/media/channelmanager.cc | 63 +++--- talk/session/media/channelmanager.h | 58 +++--- talk/session/media/channelmanager_unittest.cc | 35 +++- 18 files changed, 380 insertions(+), 466 deletions(-) diff --git a/talk/app/webrtc/mediacontroller.cc b/talk/app/webrtc/mediacontroller.cc index 33db3ac8f2..d8c9b52095 100644 --- a/talk/app/webrtc/mediacontroller.cc +++ b/talk/app/webrtc/mediacontroller.cc @@ -25,7 +25,63 @@ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -// Place holder file to be able to update Chrome's libjingle.gyp before the real -// implementation goes in. - #include "talk/app/webrtc/mediacontroller.h" + +#include "webrtc/base/bind.h" +#include "webrtc/base/checks.h" +#include "webrtc/call.h" + +namespace { + +const int kMinBandwidthBps = 30000; +const int kStartBandwidthBps = 300000; +const int kMaxBandwidthBps = 2000000; + +class MediaController : public webrtc::MediaControllerInterface { + public: + MediaController(rtc::Thread* worker_thread, + webrtc::VoiceEngine* voice_engine) + : worker_thread_(worker_thread) { + DCHECK(nullptr != worker_thread); + worker_thread_->Invoke( + rtc::Bind(&MediaController::Construct_w, this, voice_engine)); + } + ~MediaController() override { + worker_thread_->Invoke( + rtc::Bind(&MediaController::Destruct_w, this)); + } + + webrtc::Call* call_w() override { + DCHECK(worker_thread_->IsCurrent()); + return call_.get(); + } + + private: + void Construct_w(webrtc::VoiceEngine* voice_engine) { + DCHECK(worker_thread_->IsCurrent()); + webrtc::Call::Config config; + config.voice_engine = voice_engine; + config.bitrate_config.min_bitrate_bps = kMinBandwidthBps; + config.bitrate_config.start_bitrate_bps = kStartBandwidthBps; + config.bitrate_config.max_bitrate_bps = kMaxBandwidthBps; + call_.reset(webrtc::Call::Create(config)); + } + void Destruct_w() { + DCHECK(worker_thread_->IsCurrent()); + call_.reset(nullptr); + } + + rtc::Thread* worker_thread_; + rtc::scoped_ptr call_; + + DISALLOW_IMPLICIT_CONSTRUCTORS(MediaController); +}; +} // namespace { + +namespace webrtc { + +MediaControllerInterface* MediaControllerInterface::Create( + rtc::Thread* worker_thread, webrtc::VoiceEngine* voice_engine) { + return new MediaController(worker_thread, voice_engine); +} +} // namespace webrtc diff --git a/talk/app/webrtc/mediacontroller.h b/talk/app/webrtc/mediacontroller.h index 1191206e18..68798515d0 100644 --- a/talk/app/webrtc/mediacontroller.h +++ b/talk/app/webrtc/mediacontroller.h @@ -25,5 +25,25 @@ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -// Place holder file to be able to update Chrome's libjingle.gyp before the real -// implementation goes in. +#ifndef TALK_APP_WEBRTC_MEDIACONTROLLER_H_ +#define TALK_APP_WEBRTC_MEDIACONTROLLER_H_ + +#include "webrtc/base/thread.h" + +namespace webrtc { +class Call; +class VoiceEngine; + +// The MediaController currently owns shared state between media channels, but +// in the future will create and own RtpSenders and RtpReceivers. +class MediaControllerInterface { + public: + static MediaControllerInterface* Create(rtc::Thread* worker_thread, + webrtc::VoiceEngine* voice_engine); + + virtual ~MediaControllerInterface() {} + virtual webrtc::Call* call_w() = 0; +}; +} // namespace webrtc + +#endif // TALK_APP_WEBRTC_MEDIACONTROLLER_H_ diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc index 179d793f5a..26a9505e89 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -570,7 +570,7 @@ WebRtcSession::~WebRtcSession() { } if (voice_channel_) { SignalVoiceChannelDestroyed(); - channel_manager_->DestroyVoiceChannel(voice_channel_.release(), nullptr); + channel_manager_->DestroyVoiceChannel(voice_channel_.release()); } if (data_channel_) { SignalDataChannelDestroyed(); @@ -783,6 +783,9 @@ bool WebRtcSession::Initialize( cricket::PORTALLOCATOR_ENABLE_LOCALHOST_CANDIDATE); } + media_controller_.reset(MediaControllerInterface::Create( + worker_thread(), channel_manager_->media_engine()->GetVoE())); + return true; } @@ -1690,8 +1693,7 @@ void WebRtcSession::RemoveUnusedChannelsAndTransports( mediastream_signaling_->OnAudioChannelClose(); SignalVoiceChannelDestroyed(); const std::string content_name = voice_channel_->content_name(); - channel_manager_->DestroyVoiceChannel(voice_channel_.release(), - video_channel_.get()); + channel_manager_->DestroyVoiceChannel(voice_channel_.release()); DestroyTransportProxy(content_name); } @@ -1706,7 +1708,7 @@ void WebRtcSession::RemoveUnusedChannelsAndTransports( } } -// TODO(mallinath) - Add a correct error code if the channels are not creatued +// TODO(mallinath) - Add a correct error code if the channels are not created // due to BUNDLE is enabled but rtcp-mux is disabled. bool WebRtcSession::CreateChannels(const SessionDescription* desc) { // Creating the media channels and transport proxies. @@ -1766,7 +1768,7 @@ bool WebRtcSession::CreateChannels(const SessionDescription* desc) { bool WebRtcSession::CreateVoiceChannel(const cricket::ContentInfo* content) { voice_channel_.reset(channel_manager_->CreateVoiceChannel( - this, content->name, true, audio_options_)); + media_controller_.get(), this, content->name, true, audio_options_)); if (!voice_channel_) { return false; } @@ -1778,7 +1780,7 @@ bool WebRtcSession::CreateVoiceChannel(const cricket::ContentInfo* content) { bool WebRtcSession::CreateVideoChannel(const cricket::ContentInfo* content) { video_channel_.reset(channel_manager_->CreateVideoChannel( - this, content->name, true, video_options_, voice_channel_.get())); + media_controller_.get(), this, content->name, true, video_options_)); if (!video_channel_) { return false; } diff --git a/talk/app/webrtc/webrtcsession.h b/talk/app/webrtc/webrtcsession.h index bb405c5da4..3c70b02e0e 100644 --- a/talk/app/webrtc/webrtcsession.h +++ b/talk/app/webrtc/webrtcsession.h @@ -32,6 +32,7 @@ #include "talk/app/webrtc/datachannel.h" #include "talk/app/webrtc/dtmfsender.h" +#include "talk/app/webrtc/mediacontroller.h" #include "talk/app/webrtc/mediastreamprovider.h" #include "talk/app/webrtc/peerconnectioninterface.h" #include "talk/app/webrtc/statstypes.h" @@ -195,7 +196,6 @@ class WebRtcSession : public cricket::BaseSession, virtual bool GetLocalTrackIdBySsrc(uint32 ssrc, std::string* track_id); virtual bool GetRemoteTrackIdBySsrc(uint32 ssrc, std::string* track_id); - // AudioMediaProviderInterface implementation. void SetAudioPlayout(uint32 ssrc, bool enable, @@ -370,6 +370,7 @@ class WebRtcSession : public cricket::BaseSession, void ReportNegotiatedCiphers(const cricket::TransportStats& stats); + rtc::scoped_ptr media_controller_; rtc::scoped_ptr voice_channel_; rtc::scoped_ptr video_channel_; rtc::scoped_ptr data_channel_; diff --git a/talk/libjingle.gyp b/talk/libjingle.gyp index f36f7b8241..a71be2b20a 100755 --- a/talk/libjingle.gyp +++ b/talk/libjingle.gyp @@ -725,6 +725,8 @@ 'app/webrtc/localaudiosource.h', 'app/webrtc/mediaconstraintsinterface.cc', 'app/webrtc/mediaconstraintsinterface.h', + 'app/webrtc/mediacontroller.cc', + 'app/webrtc/mediacontroller.h', 'app/webrtc/mediastream.cc', 'app/webrtc/mediastream.h', 'app/webrtc/mediastreamhandler.cc', diff --git a/talk/media/base/fakemediaengine.h b/talk/media/base/fakemediaengine.h index 56986c5fb1..d4dcefb37c 100644 --- a/talk/media/base/fakemediaengine.h +++ b/talk/media/base/fakemediaengine.h @@ -523,7 +523,6 @@ class FakeVideoMediaChannel : public RtpHelper { return RtpHelper::RemoveSendStream(ssrc); } - void DetachVoiceChannel() override {} virtual bool SetRecvCodecs(const std::vector& codecs) { if (fail_set_recv_codecs()) { // Fake the failure in SetRecvCodecs. @@ -765,6 +764,7 @@ class FakeVoiceEngine : public FakeBaseEngine { bool Init(rtc::Thread* worker_thread) { return true; } void Terminate() {} int GetCapabilities() { return AUDIO_SEND | AUDIO_RECV; } + webrtc::VoiceEngine* GetVoE() { return nullptr; } AudioOptions GetOptions() const { return options_; } @@ -774,7 +774,8 @@ class FakeVoiceEngine : public FakeBaseEngine { return true; } - VoiceMediaChannel* CreateChannel(const AudioOptions& options) { + VoiceMediaChannel* CreateChannel(webrtc::Call* call, + const AudioOptions& options) { if (fail_create_channel_) { return nullptr; } @@ -865,9 +866,7 @@ class FakeVoiceEngine : public FakeBaseEngine { class FakeVideoEngine : public FakeBaseEngine { public: - FakeVideoEngine() : FakeVideoEngine(nullptr) {} - explicit FakeVideoEngine(FakeVoiceEngine* voice) - : capture_(false) { + FakeVideoEngine() : capture_(false) { // Add a fake video codec. Note that the name must not be "" as there are // sanity checks against that. codecs_.push_back(VideoCodec(0, "fake_video_codec", 0, 0, 0, 0)); @@ -887,8 +886,8 @@ class FakeVideoEngine : public FakeBaseEngine { return default_encoder_config_; } - VideoMediaChannel* CreateChannel(const VideoOptions& options, - VoiceMediaChannel* channel) { + VideoMediaChannel* CreateChannel(webrtc::Call* call, + const VideoOptions& options) { if (fail_create_channel_) { return NULL; } diff --git a/talk/media/base/mediachannel.h b/talk/media/base/mediachannel.h index cfeb2d8c91..1ba89dccff 100644 --- a/talk/media/base/mediachannel.h +++ b/talk/media/base/mediachannel.h @@ -1164,8 +1164,6 @@ class VideoMediaChannel : public MediaChannel { VideoMediaChannel() : renderer_(NULL) {} virtual ~VideoMediaChannel() {} - // Allow video channel to unhook itself from an associated voice channel. - virtual void DetachVoiceChannel() = 0; // TODO(pthatcher): Remove SetSendCodecs, // SetSendRtpHeaderExtensions, SetMaxSendBandwidth, and SetOptions // once all implementations implement SetSendParameters. diff --git a/talk/media/base/mediaengine.h b/talk/media/base/mediaengine.h index 339251f906..08addc4640 100644 --- a/talk/media/base/mediaengine.h +++ b/talk/media/base/mediaengine.h @@ -32,8 +32,6 @@ #include #endif -#include - #include #include @@ -51,6 +49,11 @@ #define DISABLE_MEDIA_ENGINE_FACTORY #endif +namespace webrtc { +class Call; +class VoiceEngine; +} + namespace cricket { class VideoCapturer; @@ -73,15 +76,19 @@ class MediaEngineInterface { virtual void Terminate() = 0; // Returns what the engine is capable of, as a set of Capabilities, above. virtual int GetCapabilities() = 0; + // TODO(solenberg): Remove once VoE API refactoring is done. + virtual webrtc::VoiceEngine* GetVoE() = 0; // MediaChannel creation // Creates a voice media channel. Returns NULL on failure. - virtual VoiceMediaChannel* CreateChannel(const AudioOptions& options) = 0; + virtual VoiceMediaChannel* CreateChannel( + webrtc::Call* call, + const AudioOptions& options) = 0; // Creates a video media channel, paired with the specified voice channel. // Returns NULL on failure. virtual VideoMediaChannel* CreateVideoChannel( - const VideoOptions& options, - VoiceMediaChannel* voice_media_channel) = 0; + webrtc::Call* call, + const VideoOptions& options) = 0; // Configuration // Gets global audio options. @@ -162,7 +169,6 @@ class MediaEngineFactory { template class CompositeMediaEngine : public MediaEngineInterface { public: - CompositeMediaEngine() : video_(&voice_) {} virtual ~CompositeMediaEngine() {} virtual bool Init(rtc::Thread* worker_thread) { if (!voice_.Init(worker_thread)) @@ -177,12 +183,16 @@ class CompositeMediaEngine : public MediaEngineInterface { virtual int GetCapabilities() { return (voice_.GetCapabilities() | video_.GetCapabilities()); } - virtual VoiceMediaChannel* CreateChannel(const AudioOptions& options) { - return voice_.CreateChannel(options); + virtual webrtc::VoiceEngine* GetVoE() { + return voice_.GetVoE(); } - virtual VideoMediaChannel* CreateVideoChannel(const VideoOptions& options, - VoiceMediaChannel* channel) { - return video_.CreateChannel(options, channel); + virtual VoiceMediaChannel* CreateChannel(webrtc::Call* call, + const AudioOptions& options) { + return voice_.CreateChannel(call, options); + } + virtual VideoMediaChannel* CreateVideoChannel(webrtc::Call* call, + const VideoOptions& options) { + return video_.CreateChannel(call, options); } virtual AudioOptions GetAudioOptions() const { diff --git a/talk/media/base/videoengine_unittest.h b/talk/media/base/videoengine_unittest.h index c74479c7da..9e6b227011 100644 --- a/talk/media/base/videoengine_unittest.h +++ b/talk/media/base/videoengine_unittest.h @@ -36,9 +36,11 @@ #include "talk/media/base/fakevideorenderer.h" #include "talk/media/base/mediachannel.h" #include "talk/media/base/streamparams.h" +#include "talk/media/webrtc/fakewebrtccall.h" #include "webrtc/base/bytebuffer.h" #include "webrtc/base/gunit.h" #include "webrtc/base/timeutils.h" +#include "webrtc/call.h" #define EXPECT_FRAME_WAIT(c, w, h, t) \ EXPECT_EQ_WAIT((c), renderer_.num_rendered_frames(), (t)); \ @@ -96,7 +98,7 @@ inline int TimeBetweenSend(const cricket::VideoCodec& codec) { template class VideoEngineOverride : public T { public: - VideoEngineOverride() : T(nullptr) { + VideoEngineOverride() : T() { } virtual ~VideoEngineOverride() { } @@ -448,6 +450,9 @@ template class VideoMediaChannelTest : public testing::Test, public sigslot::has_slots<> { protected: + VideoMediaChannelTest() + : call_(webrtc::Call::Create(webrtc::Call::Config())) {} + virtual cricket::VideoCodec DefaultCodec() = 0; virtual cricket::StreamParams DefaultSendStreamParams() { @@ -457,7 +462,8 @@ class VideoMediaChannelTest : public testing::Test, virtual void SetUp() { cricket::Device device("test", "device"); engine_.Init(); - channel_.reset(engine_.CreateChannel(cricket::VideoOptions(), NULL)); + channel_.reset( + engine_.CreateChannel(call_.get(), cricket::VideoOptions())); EXPECT_TRUE(channel_.get() != NULL); ConnectVideoChannelError(); network_interface_.SetDestination(channel_.get()); @@ -1867,6 +1873,7 @@ class VideoMediaChannelTest : public testing::Test, EXPECT_TRUE(channel_->RemoveRecvStream(kSsrc + 2)); } + const rtc::scoped_ptr call_; VideoEngineOverride engine_; rtc::scoped_ptr video_capturer_; rtc::scoped_ptr video_capturer_2_; diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc index 917efc8c50..cde449e262 100644 --- a/talk/media/webrtc/webrtcvideoengine2.cc +++ b/talk/media/webrtc/webrtcvideoengine2.cc @@ -340,10 +340,6 @@ static const int kDefaultQpMax = 56; static const int kDefaultRtcpReceiverReportSsrc = 1; -const int kMinBandwidthBps = 30000; -const int kStartBandwidthBps = 300000; -const int kMaxBandwidthBps = 2000000; - std::vector DefaultVideoCodecList() { std::vector codecs; if (CodecIsInternallySupported(kVp9CodecName)) { @@ -537,13 +533,6 @@ UnsignalledSsrcHandler::Action DefaultUnsignalledSsrcHandler::OnUnsignalledSsrc( return kDeliverPacket; } -WebRtcCallFactory::~WebRtcCallFactory() { -} -webrtc::Call* WebRtcCallFactory::CreateCall( - const webrtc::Call::Config& config) { - return webrtc::Call::Create(config); -} - VideoRenderer* DefaultUnsignalledSsrcHandler::GetDefaultRenderer() const { return default_renderer_; } @@ -557,10 +546,8 @@ void DefaultUnsignalledSsrcHandler::SetDefaultRenderer( } } -WebRtcVideoEngine2::WebRtcVideoEngine2(WebRtcVoiceEngine* voice_engine) - : voice_engine_(voice_engine), - initialized_(false), - call_factory_(&default_call_factory_), +WebRtcVideoEngine2::WebRtcVideoEngine2() + : initialized_(false), external_decoder_factory_(NULL), external_encoder_factory_(NULL) { LOG(LS_INFO) << "WebRtcVideoEngine2::WebRtcVideoEngine2()"; @@ -580,11 +567,6 @@ WebRtcVideoEngine2::~WebRtcVideoEngine2() { LOG(LS_INFO) << "WebRtcVideoEngine2::~WebRtcVideoEngine2"; } -void WebRtcVideoEngine2::SetCallFactory(WebRtcCallFactory* call_factory) { - DCHECK(!initialized_); - call_factory_ = call_factory; -} - void WebRtcVideoEngine2::Init() { LOG(LS_INFO) << "WebRtcVideoEngine2::Init"; initialized_ = true; @@ -616,20 +598,12 @@ bool WebRtcVideoEngine2::SetDefaultEncoderConfig( } WebRtcVideoChannel2* WebRtcVideoEngine2::CreateChannel( - const VideoOptions& options, - VoiceMediaChannel* voice_channel) { + webrtc::Call* call, + const VideoOptions& options) { DCHECK(initialized_); - LOG(LS_INFO) << "CreateChannel: " - << (voice_channel != NULL ? "With" : "Without") - << " voice channel. Options: " << options.ToString(); - WebRtcVideoChannel2* channel = - new WebRtcVideoChannel2(call_factory_, voice_engine_, - static_cast(voice_channel), options, - external_encoder_factory_, external_decoder_factory_); - if (!channel->Init()) { - delete channel; - return NULL; - } + LOG(LS_INFO) << "CreateChannel. Options: " << options.ToString(); + WebRtcVideoChannel2* channel = new WebRtcVideoChannel2(call, options, + external_encoder_factory_, external_decoder_factory_); channel->SetRecvCodecs(video_codecs_); return channel; } @@ -788,32 +762,18 @@ std::vector WebRtcVideoEngine2::GetSupportedCodecs() const { } WebRtcVideoChannel2::WebRtcVideoChannel2( - WebRtcCallFactory* call_factory, - WebRtcVoiceEngine* voice_engine, - WebRtcVoiceMediaChannel* voice_channel, + webrtc::Call* call, const VideoOptions& options, WebRtcVideoEncoderFactory* external_encoder_factory, WebRtcVideoDecoderFactory* external_decoder_factory) - : unsignalled_ssrc_handler_(&default_unsignalled_ssrc_handler_), - voice_channel_(voice_channel), - voice_channel_id_(voice_channel ? voice_channel->voe_channel() : -1), + : call_(call), + unsignalled_ssrc_handler_(&default_unsignalled_ssrc_handler_), external_encoder_factory_(external_encoder_factory), external_decoder_factory_(external_decoder_factory) { DCHECK(thread_checker_.CalledOnValidThread()); SetDefaultOptions(); options_.SetAll(options); options_.cpu_overuse_detection.Get(&signal_cpu_adaptation_); - webrtc::Call::Config config; - if (voice_engine != NULL) { - config.voice_engine = voice_engine->voe()->engine(); - } - config.bitrate_config.min_bitrate_bps = kMinBandwidthBps; - config.bitrate_config.start_bitrate_bps = kStartBandwidthBps; - config.bitrate_config.max_bitrate_bps = kMaxBandwidthBps; - call_.reset(call_factory->CreateCall(config)); - if (voice_channel_) { - voice_channel_->SetCall(call_.get()); - } rtcp_receiver_report_ssrc_ = kDefaultRtcpReceiverReportSsrc; sending_ = false; default_send_ssrc_ = 0; @@ -828,23 +788,12 @@ void WebRtcVideoChannel2::SetDefaultOptions() { } WebRtcVideoChannel2::~WebRtcVideoChannel2() { - DetachVoiceChannel(); for (auto& kv : send_streams_) delete kv.second; for (auto& kv : receive_streams_) delete kv.second; } -bool WebRtcVideoChannel2::Init() { return true; } - -void WebRtcVideoChannel2::DetachVoiceChannel() { - DCHECK(thread_checker_.CalledOnValidThread()); - if (voice_channel_) { - voice_channel_->SetCall(nullptr); - voice_channel_ = nullptr; - } -} - bool WebRtcVideoChannel2::CodecIsExternallySupported( const std::string& name) const { if (external_encoder_factory_ == NULL) { @@ -1149,7 +1098,7 @@ bool WebRtcVideoChannel2::AddSendStream(const StreamParams& sp) { config.overuse_callback = this; WebRtcVideoSendStream* stream = - new WebRtcVideoSendStream(call_.get(), + new WebRtcVideoSendStream(call_, sp, config, external_encoder_factory_, @@ -1272,7 +1221,7 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp, } receive_streams_[ssrc] = new WebRtcVideoReceiveStream( - call_.get(), sp, config, external_decoder_factory_, default_stream, + call_, sp, config, external_decoder_factory_, default_stream, recv_codecs_); return true; diff --git a/talk/media/webrtc/webrtcvideoengine2.h b/talk/media/webrtc/webrtcvideoengine2.h index f27099bf97..144d1c4cee 100644 --- a/talk/media/webrtc/webrtcvideoengine2.h +++ b/talk/media/webrtc/webrtcvideoengine2.h @@ -103,22 +103,11 @@ class DefaultUnsignalledSsrcHandler : public UnsignalledSsrcHandler { VideoRenderer* default_renderer_; }; -// CallFactory, overridden for testing to verify that webrtc::Call is configured -// properly. -class WebRtcCallFactory { - public: - virtual ~WebRtcCallFactory(); - virtual webrtc::Call* CreateCall(const webrtc::Call::Config& config); -}; - // WebRtcVideoEngine2 is used for the new native WebRTC Video API (webrtc:1667). -class WebRtcVideoEngine2 : public sigslot::has_slots<> { +class WebRtcVideoEngine2 { public: - explicit WebRtcVideoEngine2(WebRtcVoiceEngine* voice_engine); - virtual ~WebRtcVideoEngine2(); - - // Used for testing to be able to check and use the webrtc::Call config. - void SetCallFactory(WebRtcCallFactory* call_factory); + WebRtcVideoEngine2(); + ~WebRtcVideoEngine2(); // Basic video engine implementation. void Init(); @@ -126,8 +115,8 @@ class WebRtcVideoEngine2 : public sigslot::has_slots<> { int GetCapabilities(); bool SetDefaultEncoderConfig(const VideoEncoderConfig& config); - WebRtcVideoChannel2* CreateChannel(const VideoOptions& options, - VoiceMediaChannel* voice_channel); + WebRtcVideoChannel2* CreateChannel(webrtc::Call* call, + const VideoOptions& options); const std::vector& codecs() const; const std::vector& rtp_header_extensions() const; @@ -155,15 +144,11 @@ class WebRtcVideoEngine2 : public sigslot::has_slots<> { private: std::vector GetSupportedCodecs() const; - WebRtcVoiceEngine* voice_engine_; std::vector video_codecs_; std::vector rtp_header_extensions_; bool initialized_; - WebRtcCallFactory default_call_factory_; - WebRtcCallFactory* call_factory_; - WebRtcVideoDecoderFactory* external_decoder_factory_; WebRtcVideoEncoderFactory* external_encoder_factory_; rtc::scoped_ptr simulcast_encoder_factory_; @@ -174,17 +159,13 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler, public webrtc::newapi::Transport, public webrtc::LoadObserver { public: - WebRtcVideoChannel2(WebRtcCallFactory* call_factory, - WebRtcVoiceEngine* voice_engine, - WebRtcVoiceMediaChannel* voice_channel, + WebRtcVideoChannel2(webrtc::Call* call, const VideoOptions& options, WebRtcVideoEncoderFactory* external_encoder_factory, WebRtcVideoDecoderFactory* external_decoder_factory); - ~WebRtcVideoChannel2(); - bool Init(); + ~WebRtcVideoChannel2() override; // VideoMediaChannel implementation - void DetachVoiceChannel() override; bool SetSendParameters(const VideoSendParameters& params) override; bool SetRecvParameters(const VideoRecvParameters& params) override; bool SetRecvCodecs(const std::vector& codecs) override; @@ -507,8 +488,7 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler, uint32_t rtcp_receiver_report_ssrc_; bool sending_; - rtc::scoped_ptr call_; - WebRtcCallFactory* call_factory_; + webrtc::Call* const call_; uint32_t default_send_ssrc_; @@ -534,8 +514,6 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler, Settable send_codec_; std::vector send_rtp_extensions_; - WebRtcVoiceMediaChannel* voice_channel_; - const int voice_channel_id_; WebRtcVideoEncoderFactory* const external_encoder_factory_; WebRtcVideoDecoderFactory* const external_decoder_factory_; std::vector recv_codecs_; diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.cc b/talk/media/webrtc/webrtcvideoengine2_unittest.cc index 52acec4082..5a7a0d1c0e 100644 --- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc @@ -110,7 +110,8 @@ class WebRtcVideoEngine2Test : public ::testing::Test { public: WebRtcVideoEngine2Test() : WebRtcVideoEngine2Test(nullptr) {} WebRtcVideoEngine2Test(WebRtcVoiceEngine* voice_engine) - : engine_(voice_engine) { + : call_(webrtc::Call::Create(webrtc::Call::Config())), + engine_() { std::vector engine_codecs = engine_.codecs(); DCHECK(!engine_codecs.empty()); bool codec_set = false; @@ -135,21 +136,6 @@ class WebRtcVideoEngine2Test : public ::testing::Test { } protected: - class FakeCallFactory : public WebRtcCallFactory { - public: - FakeCallFactory() : fake_call_(NULL) {} - FakeCall* GetCall() { return fake_call_; } - - private: - webrtc::Call* CreateCall(const webrtc::Call::Config& config) override { - DCHECK(fake_call_ == NULL); - fake_call_ = new FakeCall(config); - return fake_call_; - } - - FakeCall* fake_call_; - }; - VideoMediaChannel* SetUpForExternalEncoderFactory( cricket::WebRtcVideoEncoderFactory* encoder_factory, const std::vector& codecs); @@ -160,6 +146,7 @@ class WebRtcVideoEngine2Test : public ::testing::Test { // Used in WebRtcVideoEngine2VoiceTest, but defined here so it's properly // initialized when the constructor is called. + rtc::scoped_ptr call_; WebRtcVoiceEngine voice_engine_; WebRtcVideoEngine2 engine_; VideoCodec default_codec_; @@ -350,7 +337,7 @@ TEST_F(WebRtcVideoEngine2Test, CVOSetHeaderExtensionAfterCapturer) { TEST_F(WebRtcVideoEngine2Test, SetSendFailsBeforeSettingCodecs) { engine_.Init(); rtc::scoped_ptr channel( - engine_.CreateChannel(cricket::VideoOptions(), NULL)); + engine_.CreateChannel(call_.get(), cricket::VideoOptions())); EXPECT_TRUE(channel->AddSendStream(StreamParams::CreateLegacy(123))); @@ -363,7 +350,7 @@ TEST_F(WebRtcVideoEngine2Test, SetSendFailsBeforeSettingCodecs) { TEST_F(WebRtcVideoEngine2Test, GetStatsWithoutSendCodecsSetDoesNotCrash) { engine_.Init(); rtc::scoped_ptr channel( - engine_.CreateChannel(cricket::VideoOptions(), NULL)); + engine_.CreateChannel(call_.get(), cricket::VideoOptions())); EXPECT_TRUE(channel->AddSendStream(StreamParams::CreateLegacy(123))); VideoMediaInfo info; channel->GetStats(&info); @@ -425,8 +412,8 @@ TEST_F(WebRtcVideoEngine2Test, PropagatesInputFrameTimestamp) { std::vector codecs; codecs.push_back(kVp8Codec); - FakeCallFactory factory; - engine_.SetCallFactory(&factory); + FakeCall* fake_call = new FakeCall(webrtc::Call::Config()); + call_.reset(fake_call); rtc::scoped_ptr channel( SetUpForExternalEncoderFactory(&encoder_factory, codecs)); @@ -440,8 +427,7 @@ TEST_F(WebRtcVideoEngine2Test, PropagatesInputFrameTimestamp) { cricket::FOURCC_I420)); channel->SetSend(true); - FakeVideoSendStream* stream = factory.GetCall()->GetVideoSendStreams()[0]; - + FakeVideoSendStream* stream = fake_call->GetVideoSendStreams()[0]; EXPECT_TRUE(capturer.CaptureFrame()); int64_t last_timestamp = stream->GetLastTimestamp(); @@ -486,15 +472,15 @@ TEST_F(WebRtcVideoEngine2Test, std::vector codecs; codecs.push_back(kVp8Codec); - FakeCallFactory factory; - engine_.SetCallFactory(&factory); + FakeCall* fake_call = new FakeCall(webrtc::Call::Config()); + call_.reset(fake_call); rtc::scoped_ptr channel( SetUpForExternalEncoderFactory(&encoder_factory, codecs)); EXPECT_TRUE( channel->AddSendStream(cricket::StreamParams::CreateLegacy(kSsrc))); channel->SetSend(true); - FakeVideoSendStream* stream = factory.GetCall()->GetVideoSendStreams()[0]; + FakeVideoSendStream* stream = fake_call->GetVideoSendStreams()[0]; FakeVideoCapturer capturer1; EXPECT_TRUE(channel->SetCapturer(kSsrc, &capturer1)); @@ -544,7 +530,7 @@ VideoMediaChannel* WebRtcVideoEngine2Test::SetUpForExternalEncoderFactory( engine_.Init(); VideoMediaChannel* channel = - engine_.CreateChannel(cricket::VideoOptions(), NULL); + engine_.CreateChannel(call_.get(), cricket::VideoOptions()); EXPECT_TRUE(channel->SetSendCodecs(codecs)); return channel; @@ -557,7 +543,7 @@ VideoMediaChannel* WebRtcVideoEngine2Test::SetUpForExternalDecoderFactory( engine_.Init(); VideoMediaChannel* channel = - engine_.CreateChannel(cricket::VideoOptions(), NULL); + engine_.CreateChannel(call_.get(), cricket::VideoOptions()); EXPECT_TRUE(channel->SetRecvCodecs(codecs)); return channel; @@ -908,15 +894,14 @@ TEST_F(WebRtcVideoChannel2BaseTest, DISABLED_SendVp8HdAndReceiveAdaptedVp8Vga) { EXPECT_FRAME_WAIT(1, codec.width, codec.height, kTimeout); } -class WebRtcVideoChannel2Test : public WebRtcVideoEngine2Test, - public WebRtcCallFactory { +class WebRtcVideoChannel2Test : public WebRtcVideoEngine2Test { public: - WebRtcVideoChannel2Test() : fake_call_(NULL), last_ssrc_(0) {} + WebRtcVideoChannel2Test() : last_ssrc_(0) {} void SetUp() override { - engine_.SetCallFactory(this); + fake_call_.reset(new FakeCall(webrtc::Call::Config())); engine_.Init(); - channel_.reset(engine_.CreateChannel(cricket::VideoOptions(), NULL)); - ASSERT_TRUE(fake_call_ != NULL) << "Call not created through factory."; + channel_.reset( + engine_.CreateChannel(fake_call_.get(), cricket::VideoOptions())); last_ssrc_ = 123; ASSERT_TRUE(channel_->SetSendCodecs(engine_.codecs())); } @@ -926,12 +911,6 @@ class WebRtcVideoChannel2Test : public WebRtcVideoEngine2Test, return engine_.codecs(); } - webrtc::Call* CreateCall(const webrtc::Call::Config& config) override { - DCHECK(fake_call_ == NULL); - fake_call_ = new FakeCall(config); - return fake_call_; - } - FakeVideoSendStream* AddSendStream() { return AddSendStream(StreamParams::CreateLegacy(++last_ssrc_)); } @@ -985,7 +964,6 @@ class WebRtcVideoChannel2Test : public WebRtcVideoEngine2Test, void TestSetSendRtpHeaderExtensions(const std::string& cricket_ext, const std::string& webrtc_ext) { - FakeCall* call = fake_call_; // Enable extension. const int id = 1; std::vector extensions; @@ -1010,14 +988,14 @@ class WebRtcVideoChannel2Test : public WebRtcVideoEngine2Test, // Verify that existing RTP header extensions can be removed. std::vector empty_extensions; EXPECT_TRUE(channel_->SetSendRtpHeaderExtensions(empty_extensions)); - ASSERT_EQ(1u, call->GetVideoSendStreams().size()); - send_stream = call->GetVideoSendStreams()[0]; + ASSERT_EQ(1u, fake_call_->GetVideoSendStreams().size()); + send_stream = fake_call_->GetVideoSendStreams()[0]; EXPECT_TRUE(send_stream->GetConfig().rtp.extensions.empty()); // Verify that adding receive RTP header extensions adds them for existing // streams. EXPECT_TRUE(channel_->SetSendRtpHeaderExtensions(extensions)); - send_stream = call->GetVideoSendStreams()[0]; + send_stream = fake_call_->GetVideoSendStreams()[0]; ASSERT_EQ(1u, send_stream->GetConfig().rtp.extensions.size()); EXPECT_EQ(id, send_stream->GetConfig().rtp.extensions[0].id); EXPECT_EQ(webrtc_ext, send_stream->GetConfig().rtp.extensions[0].name); @@ -1025,7 +1003,6 @@ class WebRtcVideoChannel2Test : public WebRtcVideoEngine2Test, void TestSetRecvRtpHeaderExtensions(const std::string& cricket_ext, const std::string& webrtc_ext) { - FakeCall* call = fake_call_; // Enable extension. const int id = 1; std::vector extensions; @@ -1051,14 +1028,14 @@ class WebRtcVideoChannel2Test : public WebRtcVideoEngine2Test, // Verify that existing RTP header extensions can be removed. std::vector empty_extensions; EXPECT_TRUE(channel_->SetRecvRtpHeaderExtensions(empty_extensions)); - ASSERT_EQ(1u, call->GetVideoReceiveStreams().size()); - recv_stream = call->GetVideoReceiveStreams()[0]; + ASSERT_EQ(1u, fake_call_->GetVideoReceiveStreams().size()); + recv_stream = fake_call_->GetVideoReceiveStreams()[0]; EXPECT_TRUE(recv_stream->GetConfig().rtp.extensions.empty()); // Verify that adding receive RTP header extensions adds them for existing // streams. EXPECT_TRUE(channel_->SetRecvRtpHeaderExtensions(extensions)); - recv_stream = call->GetVideoReceiveStreams()[0]; + recv_stream = fake_call_->GetVideoReceiveStreams()[0]; ASSERT_EQ(1u, recv_stream->GetConfig().rtp.extensions.size()); EXPECT_EQ(id, recv_stream->GetConfig().rtp.extensions[0].id); EXPECT_EQ(webrtc_ext, recv_stream->GetConfig().rtp.extensions[0].name); @@ -1096,7 +1073,7 @@ class WebRtcVideoChannel2Test : public WebRtcVideoEngine2Test, return AddSendStream(CreateSimStreamParams("cname", ssrcs)); } - FakeCall* fake_call_; + rtc::scoped_ptr fake_call_; rtc::scoped_ptr channel_; uint32 last_ssrc_; }; @@ -2833,13 +2810,7 @@ TEST_F(WebRtcVideoChannel2Test, ConfiguresLocalSsrcOnExistingReceivers) { TestReceiverLocalSsrcConfiguration(true); } -class WebRtcVideoEngine2SimulcastTest : public testing::Test { - public: - WebRtcVideoEngine2SimulcastTest() : engine_(nullptr) {} - - protected: - WebRtcVideoEngine2 engine_; -}; +class WebRtcVideoEngine2SimulcastTest : public testing::Test {}; // Test that if we add a stream with RTX SSRC's, SSRC's get set correctly. TEST_F(WebRtcVideoEngine2SimulcastTest, DISABLED_TestStreamWithRtx) { @@ -2986,26 +2957,17 @@ TEST_F(WebRtcVideoEngine2SimulcastTest, FAIL() << "Not implemented."; } -class WebRtcVideoChannel2SimulcastTest : public WebRtcVideoEngine2SimulcastTest, - public WebRtcCallFactory { +class WebRtcVideoChannel2SimulcastTest : public testing::Test { public: - WebRtcVideoChannel2SimulcastTest() : fake_call_(NULL) {} + WebRtcVideoChannel2SimulcastTest() : fake_call_(webrtc::Call::Config()) {} void SetUp() override { - engine_.SetCallFactory(this); engine_.Init(); - channel_.reset(engine_.CreateChannel(VideoOptions(), NULL)); - ASSERT_TRUE(fake_call_ != NULL) << "Call not created through factory."; + channel_.reset(engine_.CreateChannel(&fake_call_, VideoOptions())); last_ssrc_ = 123; } protected: - webrtc::Call* CreateCall(const webrtc::Call::Config& config) override { - DCHECK(fake_call_ == NULL); - fake_call_ = new FakeCall(config); - return fake_call_; - } - void VerifySimulcastSettings(const VideoCodec& codec, VideoOptions::HighestBitrate bitrate_mode, size_t num_configured_streams, @@ -3099,16 +3061,16 @@ class WebRtcVideoChannel2SimulcastTest : public WebRtcVideoEngine2SimulcastTest, FakeVideoSendStream* AddSendStream(const StreamParams& sp) { size_t num_streams = - fake_call_->GetVideoSendStreams().size(); + fake_call_.GetVideoSendStreams().size(); EXPECT_TRUE(channel_->AddSendStream(sp)); std::vector streams = - fake_call_->GetVideoSendStreams(); + fake_call_.GetVideoSendStreams(); EXPECT_EQ(num_streams + 1, streams.size()); return streams[streams.size() - 1]; } std::vector GetFakeSendStreams() { - return fake_call_->GetVideoSendStreams(); + return fake_call_.GetVideoSendStreams(); } FakeVideoReceiveStream* AddRecvStream() { @@ -3117,15 +3079,16 @@ class WebRtcVideoChannel2SimulcastTest : public WebRtcVideoEngine2SimulcastTest, FakeVideoReceiveStream* AddRecvStream(const StreamParams& sp) { size_t num_streams = - fake_call_->GetVideoReceiveStreams().size(); + fake_call_.GetVideoReceiveStreams().size(); EXPECT_TRUE(channel_->AddRecvStream(sp)); std::vector streams = - fake_call_->GetVideoReceiveStreams(); + fake_call_.GetVideoReceiveStreams(); EXPECT_EQ(num_streams + 1, streams.size()); return streams[streams.size() - 1]; } - FakeCall* fake_call_; + FakeCall fake_call_; + WebRtcVideoEngine2 engine_; rtc::scoped_ptr channel_; uint32 last_ssrc_; }; diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc index 43cbc2e929..b01bfab3d8 100644 --- a/talk/media/webrtc/webrtcvoiceengine.cc +++ b/talk/media/webrtc/webrtcvoiceengine.cc @@ -579,9 +579,9 @@ int WebRtcVoiceEngine::GetCapabilities() { return AUDIO_SEND | AUDIO_RECV; } -VoiceMediaChannel* WebRtcVoiceEngine::CreateChannel( +VoiceMediaChannel* WebRtcVoiceEngine::CreateChannel(webrtc::Call* call, const AudioOptions& options) { - WebRtcVoiceMediaChannel* ch = new WebRtcVoiceMediaChannel(this); + WebRtcVoiceMediaChannel* ch = new WebRtcVoiceMediaChannel(this, call); if (!ch->valid()) { delete ch; return nullptr; @@ -1689,7 +1689,8 @@ class WebRtcVoiceMediaChannel::WebRtcVoiceChannelRenderer }; // WebRtcVoiceMediaChannel -WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel(WebRtcVoiceEngine* engine) +WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel(WebRtcVoiceEngine* engine, + webrtc::Call* call) : engine_(engine), voe_channel_(engine->CreateMediaVoiceChannel()), send_bitrate_setting_(false), @@ -1702,18 +1703,18 @@ WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel(WebRtcVoiceEngine* engine) typing_noise_detected_(false), desired_send_(SEND_NOTHING), send_(SEND_NOTHING), - call_(nullptr), + call_(call), default_receive_ssrc_(0) { engine->RegisterChannel(this); LOG(LS_VERBOSE) << "WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel " << voe_channel(); + DCHECK(nullptr != call); ConfigureSendChannel(voe_channel()); } WebRtcVoiceMediaChannel::~WebRtcVoiceMediaChannel() { LOG(LS_VERBOSE) << "WebRtcVoiceMediaChannel::~WebRtcVoiceMediaChannel " << voe_channel(); - DCHECK(receive_streams_.empty() || call_); // Remove any remaining send streams, the default channel will be deleted // later. @@ -1833,7 +1834,7 @@ bool WebRtcVoiceMediaChannel::SetOptions(const AudioOptions& options) { } } - SetCall(call_); + RecreateAudioReceiveStreams(); LOG(LS_INFO) << "Set voice channel options. Current options: " << options_.ToString(); @@ -2202,7 +2203,7 @@ bool WebRtcVoiceMediaChannel::SetRecvRtpHeaderExtensions( } recv_rtp_extensions_.swap(exts); - SetCall(call_); + RecreateAudioReceiveStreams(); } return true; @@ -2561,7 +2562,7 @@ bool WebRtcVoiceMediaChannel::AddRecvStream(const StreamParams& sp) { new WebRtcVoiceChannelRenderer(voe_channel(), audio_transport); receive_channels_.insert(std::make_pair(ssrc, channel_renderer)); receive_stream_params_[ssrc] = sp; - TryAddAudioRecvStream(ssrc); + AddAudioReceiveStream(ssrc); return SetPlayout(voe_channel(), playout_); } @@ -2581,7 +2582,7 @@ bool WebRtcVoiceMediaChannel::AddRecvStream(const StreamParams& sp) { new WebRtcVoiceChannelRenderer(channel, audio_transport); receive_channels_.insert(std::make_pair(ssrc, channel_renderer)); receive_stream_params_[ssrc] = sp; - TryAddAudioRecvStream(ssrc); + AddAudioReceiveStream(ssrc); LOG(LS_INFO) << "New audio stream " << ssrc << " registered to VoiceEngine channel #" @@ -2670,7 +2671,7 @@ bool WebRtcVoiceMediaChannel::RemoveRecvStream(uint32 ssrc) { return false; } - TryRemoveAudioRecvStream(ssrc); + RemoveAudioReceiveStream(ssrc); receive_stream_params_.erase(ssrc); // Delete the WebRtcVoiceChannelRenderer object connected to the channel, this @@ -2964,15 +2965,12 @@ void WebRtcVoiceMediaChannel::OnPacketReceived( rtc::Buffer* packet, const rtc::PacketTime& packet_time) { DCHECK(thread_checker_.CalledOnValidThread()); - // If hooked up to a "Call", forward packet there too. - if (call_) { - const webrtc::PacketTime webrtc_packet_time(packet_time.timestamp, - packet_time.not_before); - call_->Receiver()->DeliverPacket( - webrtc::MediaType::AUDIO, - reinterpret_cast(packet->data()), packet->size(), - webrtc_packet_time); - } + // Forward packet to Call as well. + const webrtc::PacketTime webrtc_packet_time(packet_time.timestamp, + packet_time.not_before); + call_->Receiver()->DeliverPacket(webrtc::MediaType::AUDIO, + reinterpret_cast(packet->data()), packet->size(), + webrtc_packet_time); // Pick which channel to send this packet to. If this packet doesn't match // any multiplexed streams, just send it to the default channel. Otherwise, @@ -3009,15 +3007,12 @@ void WebRtcVoiceMediaChannel::OnRtcpReceived( rtc::Buffer* packet, const rtc::PacketTime& packet_time) { DCHECK(thread_checker_.CalledOnValidThread()); - // If hooked up to a "Call", forward packet there too. - if (call_) { - const webrtc::PacketTime webrtc_packet_time(packet_time.timestamp, - packet_time.not_before); - call_->Receiver()->DeliverPacket( - webrtc::MediaType::AUDIO, - reinterpret_cast(packet->data()), packet->size(), - webrtc_packet_time); - } + // Forward packet to Call as well. + const webrtc::PacketTime webrtc_packet_time(packet_time.timestamp, + packet_time.not_before); + call_->Receiver()->DeliverPacket(webrtc::MediaType::AUDIO, + reinterpret_cast(packet->data()), packet->size(), + webrtc_packet_time); // Sending channels need all RTCP packets with feedback information. // Even sender reports can contain attached report blocks. @@ -3402,17 +3397,6 @@ int WebRtcVoiceMediaChannel::GetSendChannelNum(uint32 ssrc) const { return -1; } -void WebRtcVoiceMediaChannel::SetCall(webrtc::Call* call) { - DCHECK(thread_checker_.CalledOnValidThread()); - for (const auto& it : receive_channels_) { - TryRemoveAudioRecvStream(it.first); - } - call_ = call; - for (const auto& it : receive_channels_) { - TryAddAudioRecvStream(it.first); - } -} - bool WebRtcVoiceMediaChannel::GetRedSendCodec(const AudioCodec& red_codec, const std::vector& all_codecs, webrtc::CodecInst* send_codec) { // Get the RED encodings from the parameter with no name. This may @@ -3559,15 +3543,21 @@ bool WebRtcVoiceMediaChannel::SetHeaderExtension(ExtensionSetterFunction setter, return true; } -void WebRtcVoiceMediaChannel::TryAddAudioRecvStream(uint32 ssrc) { +void WebRtcVoiceMediaChannel::RecreateAudioReceiveStreams() { + DCHECK(thread_checker_.CalledOnValidThread()); + for (const auto& it : receive_channels_) { + RemoveAudioReceiveStream(it.first); + } + for (const auto& it : receive_channels_) { + AddAudioReceiveStream(it.first); + } +} + +void WebRtcVoiceMediaChannel::AddAudioReceiveStream(uint32 ssrc) { DCHECK(thread_checker_.CalledOnValidThread()); WebRtcVoiceChannelRenderer* channel = receive_channels_[ssrc]; DCHECK(channel != nullptr); DCHECK(receive_streams_.find(ssrc) == receive_streams_.end()); - // If we are hooked up to a webrtc::Call, create an AudioReceiveStream too. - if (!call_) { - return; - } webrtc::AudioReceiveStream::Config config; config.rtp.remote_ssrc = ssrc; // Only add RTP extensions if we support combined A/V BWE. @@ -3580,16 +3570,12 @@ void WebRtcVoiceMediaChannel::TryAddAudioRecvStream(uint32 ssrc) { receive_streams_.insert(std::make_pair(ssrc, s)); } -void WebRtcVoiceMediaChannel::TryRemoveAudioRecvStream(uint32 ssrc) { +void WebRtcVoiceMediaChannel::RemoveAudioReceiveStream(uint32 ssrc) { DCHECK(thread_checker_.CalledOnValidThread()); - // If we are hooked up to a webrtc::Call, assume there is an - // AudioReceiveStream to destroy too. - if (call_) { - auto stream_it = receive_streams_.find(ssrc); - if (stream_it != receive_streams_.end()) { - call_->DestroyAudioReceiveStream(stream_it->second); - receive_streams_.erase(stream_it); - } + auto stream_it = receive_streams_.find(ssrc); + if (stream_it != receive_streams_.end()) { + call_->DestroyAudioReceiveStream(stream_it->second); + receive_streams_.erase(stream_it); } } diff --git a/talk/media/webrtc/webrtcvoiceengine.h b/talk/media/webrtc/webrtcvoiceengine.h index 951c878161..e8ae157596 100644 --- a/talk/media/webrtc/webrtcvoiceengine.h +++ b/talk/media/webrtc/webrtcvoiceengine.h @@ -100,7 +100,9 @@ class WebRtcVoiceEngine void Terminate(); int GetCapabilities(); - VoiceMediaChannel* CreateChannel(const AudioOptions& options); + webrtc::VoiceEngine* GetVoE() { return voe()->engine(); } + VoiceMediaChannel* CreateChannel(webrtc::Call* call, + const AudioOptions& options); AudioOptions GetOptions() const { return options_; } bool SetOptions(const AudioOptions& options); @@ -280,7 +282,8 @@ class WebRtcVoiceEngine class WebRtcVoiceMediaChannel : public VoiceMediaChannel, public webrtc::Transport { public: - explicit WebRtcVoiceMediaChannel(WebRtcVoiceEngine *engine); + explicit WebRtcVoiceMediaChannel(WebRtcVoiceEngine* engine, + webrtc::Call* call); ~WebRtcVoiceMediaChannel() override; int voe_channel() const { return voe_channel_; } @@ -356,8 +359,6 @@ class WebRtcVoiceMediaChannel : public VoiceMediaChannel, int GetReceiveChannelNum(uint32 ssrc) const; int GetSendChannelNum(uint32 ssrc) const; - void SetCall(webrtc::Call* call); - private: bool SetLocalRenderer(uint32 ssrc, AudioRenderer* renderer); bool MuteStream(uint32 ssrc, bool mute); @@ -402,8 +403,9 @@ class WebRtcVoiceMediaChannel : public VoiceMediaChannel, bool SetHeaderExtension(ExtensionSetterFunction setter, int channel_id, const RtpHeaderExtension* extension); - void TryAddAudioRecvStream(uint32 ssrc); - void TryRemoveAudioRecvStream(uint32 ssrc); + void RecreateAudioReceiveStreams(); + void AddAudioReceiveStream(uint32 ssrc); + void RemoveAudioReceiveStream(uint32 ssrc); bool SetRecvCodecsInternal(const std::vector& new_codecs); bool SetChannelRecvRtpHeaderExtensions( @@ -415,7 +417,7 @@ class WebRtcVoiceMediaChannel : public VoiceMediaChannel, rtc::ThreadChecker thread_checker_; - WebRtcVoiceEngine* engine_; + WebRtcVoiceEngine* const engine_; const int voe_channel_; rtc::scoped_ptr ringback_tone_; std::set ringback_channels_; // channels playing ringback @@ -432,7 +434,7 @@ class WebRtcVoiceMediaChannel : public VoiceMediaChannel, bool typing_noise_detected_; SendFlags desired_send_; SendFlags send_; - webrtc::Call* call_; + webrtc::Call* const call_; // send_channels_ contains the channels which are being used for sending. // When the default channel (voe_channel) is used for sending, it is diff --git a/talk/media/webrtc/webrtcvoiceengine_unittest.cc b/talk/media/webrtc/webrtcvoiceengine_unittest.cc index 18b318a6ef..27a9c0271f 100644 --- a/talk/media/webrtc/webrtcvoiceengine_unittest.cc +++ b/talk/media/webrtc/webrtcvoiceengine_unittest.cc @@ -27,6 +27,7 @@ #include "webrtc/base/byteorder.h" #include "webrtc/base/gunit.h" +#include "webrtc/call.h" #include "talk/media/base/constants.h" #include "talk/media/base/fakemediaengine.h" #include "talk/media/base/fakemediaprocessor.h" @@ -122,7 +123,8 @@ class WebRtcVoiceEngineTestFake : public testing::Test { }; WebRtcVoiceEngineTestFake() - : voe_(kAudioCodecs, ARRAY_SIZE(kAudioCodecs)), + : call_(webrtc::Call::Config()), + voe_(kAudioCodecs, ARRAY_SIZE(kAudioCodecs)), trace_wrapper_(new FakeVoETraceWrapper()), engine_(new FakeVoEWrapper(&voe_), trace_wrapper_), channel_(nullptr) { @@ -133,7 +135,7 @@ class WebRtcVoiceEngineTestFake : public testing::Test { if (!engine_.Init(rtc::Thread::Current())) { return false; } - channel_ = engine_.CreateChannel(cricket::AudioOptions()); + channel_ = engine_.CreateChannel(&call_, cricket::AudioOptions()); return (channel_ != nullptr); } bool SetupEngine() { @@ -166,7 +168,7 @@ class WebRtcVoiceEngineTestFake : public testing::Test { void TestInsertDtmf(uint32 ssrc, bool caller) { EXPECT_TRUE(engine_.Init(rtc::Thread::Current())); - channel_ = engine_.CreateChannel(cricket::AudioOptions()); + channel_ = engine_.CreateChannel(&call_, cricket::AudioOptions()); EXPECT_TRUE(channel_ != nullptr); if (caller) { // if this is a caller, local description will be applied and add the @@ -325,6 +327,7 @@ class WebRtcVoiceEngineTestFake : public testing::Test { } protected: + cricket::FakeCall call_; cricket::FakeWebRtcVoiceEngine voe_; FakeVoETraceWrapper* trace_wrapper_; cricket::WebRtcVoiceEngine engine_; @@ -346,7 +349,7 @@ 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(cricket::AudioOptions()); + channel_ = engine_.CreateChannel(&call_, cricket::AudioOptions()); EXPECT_TRUE(channel_ != nullptr); } @@ -354,7 +357,7 @@ TEST_F(WebRtcVoiceEngineTestFake, CreateChannel) { TEST_F(WebRtcVoiceEngineTestFake, CreateChannelFail) { voe_.set_fail_create_channel(true); EXPECT_TRUE(engine_.Init(rtc::Thread::Current())); - channel_ = engine_.CreateChannel(cricket::AudioOptions()); + channel_ = engine_.CreateChannel(&call_, cricket::AudioOptions()); EXPECT_TRUE(channel_ == nullptr); } @@ -663,7 +666,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetMaxSendBandwidthFixedRateAsCaller) { TEST_F(WebRtcVoiceEngineTestFake, SetMaxSendBandwidthMultiRateAsCallee) { EXPECT_TRUE(engine_.Init(rtc::Thread::Current())); - channel_ = engine_.CreateChannel(cricket::AudioOptions()); + channel_ = engine_.CreateChannel(&call_, cricket::AudioOptions()); EXPECT_TRUE(channel_ != nullptr); EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs())); @@ -1037,7 +1040,7 @@ 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(cricket::AudioOptions()); + channel_ = engine_.CreateChannel(&call_, cricket::AudioOptions()); EXPECT_TRUE(channel_ != nullptr); int channel_num = voe_.GetLastChannel(); @@ -1617,7 +1620,7 @@ 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(cricket::AudioOptions()); + channel_ = engine_.CreateChannel(&call_, cricket::AudioOptions()); EXPECT_TRUE(channel_ != nullptr); int channel_num = voe_.GetLastChannel(); @@ -1734,7 +1737,7 @@ 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(cricket::AudioOptions()); + channel_ = engine_.CreateChannel(&call_, cricket::AudioOptions()); EXPECT_TRUE(channel_ != nullptr); int channel_num = voe_.GetLastChannel(); @@ -2423,7 +2426,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(cricket::AudioOptions()); + channel_ = engine_.CreateChannel(&call_, cricket::AudioOptions()); EXPECT_TRUE(channel_->SetOptions(options_conference_)); EXPECT_TRUE(channel_->AddRecvStream(cricket::StreamParams::CreateLegacy(1))); @@ -2994,10 +2997,10 @@ TEST_F(WebRtcVoiceEngineTestFake, SetOptionOverridesViaChannels) { EXPECT_TRUE(SetupEngine()); rtc::scoped_ptr channel1( static_cast( - engine_.CreateChannel(cricket::AudioOptions()))); + engine_.CreateChannel(&call_, cricket::AudioOptions()))); rtc::scoped_ptr channel2( static_cast( - engine_.CreateChannel(cricket::AudioOptions()))); + engine_.CreateChannel(&call_, cricket::AudioOptions()))); // Have to add a stream to make SetSend work. cricket::StreamParams stream1; @@ -3109,7 +3112,7 @@ TEST_F(WebRtcVoiceEngineTestFake, SetOptionOverridesViaChannels) { TEST_F(WebRtcVoiceEngineTestFake, TestSetDscpOptions) { EXPECT_TRUE(SetupEngine()); rtc::scoped_ptr channel( - engine_.CreateChannel(cricket::AudioOptions())); + engine_.CreateChannel(&call_, cricket::AudioOptions())); rtc::scoped_ptr network_interface( new cricket::FakeNetworkInterface); channel->SetInterface(network_interface.get()); @@ -3180,14 +3183,10 @@ TEST_F(WebRtcVoiceEngineTestFake, SetOutputScaling) { } TEST_F(WebRtcVoiceEngineTestFake, SetsSyncGroupFromSyncLabel) { - cricket::FakeCall call((webrtc::Call::Config())); const uint32 kAudioSsrc = 123; const std::string kSyncLabel = "AvSyncLabel"; EXPECT_TRUE(SetupEngine()); - cricket::WebRtcVoiceMediaChannel* media_channel = - static_cast(channel_); - media_channel->SetCall(&call); cricket::StreamParams sp = cricket::StreamParams::CreateLegacy(kAudioSsrc); sp.sync_label = kSyncLabel; // Creating two channels to make sure that sync label is set properly for both @@ -3196,116 +3195,66 @@ TEST_F(WebRtcVoiceEngineTestFake, SetsSyncGroupFromSyncLabel) { sp.ssrcs[0] += 1; EXPECT_TRUE(channel_->AddRecvStream(sp)); - ASSERT_EQ(2, call.GetAudioReceiveStreams().size()); + ASSERT_EQ(2, call_.GetAudioReceiveStreams().size()); EXPECT_EQ(kSyncLabel, - call.GetAudioReceiveStream(kAudioSsrc)->GetConfig().sync_group) + call_.GetAudioReceiveStream(kAudioSsrc)->GetConfig().sync_group) << "SyncGroup should be set based on sync_label"; EXPECT_EQ(kSyncLabel, - call.GetAudioReceiveStream(kAudioSsrc + 1)->GetConfig().sync_group) + call_.GetAudioReceiveStream(kAudioSsrc + 1)->GetConfig().sync_group) << "SyncGroup should be set based on sync_label"; - - media_channel->SetCall(nullptr); } TEST_F(WebRtcVoiceEngineTestFake, CanChangeCombinedBweOption) { // Test that changing the combined_audio_video_bwe option results in the // expected state changes on an associated Call. - cricket::FakeCall call((webrtc::Call::Config())); - const uint32 kAudioSsrc1 = 223; - const uint32 kAudioSsrc2 = 224; + std::vector ssrcs; + ssrcs.push_back(223); + ssrcs.push_back(224); EXPECT_TRUE(SetupEngine()); cricket::WebRtcVoiceMediaChannel* media_channel = static_cast(channel_); - const auto& rtp_extensions = engine_.rtp_header_extensions(); - media_channel->SetRecvRtpHeaderExtensions(rtp_extensions); - media_channel->SetCall(&call); - EXPECT_TRUE(media_channel->AddRecvStream( - cricket::StreamParams::CreateLegacy(kAudioSsrc1))); - EXPECT_TRUE(media_channel->AddRecvStream( - cricket::StreamParams::CreateLegacy(kAudioSsrc2))); + for (uint32 ssrc : ssrcs) { + EXPECT_TRUE(media_channel->AddRecvStream( + cricket::StreamParams::CreateLegacy(ssrc))); + } + EXPECT_EQ(2, call_.GetAudioReceiveStreams().size()); - // Combined BWE should not be set up yet. - EXPECT_EQ(2, call.GetAudioReceiveStreams().size()); - EXPECT_FALSE(call.GetAudioReceiveStream(kAudioSsrc1) - ->GetConfig() - .combined_audio_video_bwe); - EXPECT_FALSE(call.GetAudioReceiveStream(kAudioSsrc2) - ->GetConfig() - .combined_audio_video_bwe); + // Combined BWE should be disabled. + for (uint32 ssrc : ssrcs) { + const auto* s = call_.GetAudioReceiveStream(ssrc); + EXPECT_NE(nullptr, s); + EXPECT_EQ(false, s->GetConfig().combined_audio_video_bwe); + } // Enable combined BWE option - now it should be set up. cricket::AudioOptions options; options.combined_audio_video_bwe.Set(true); EXPECT_TRUE(media_channel->SetOptions(options)); - EXPECT_EQ(2, call.GetAudioReceiveStreams().size()); - EXPECT_TRUE(call.GetAudioReceiveStream(kAudioSsrc1) - ->GetConfig() - .combined_audio_video_bwe); - EXPECT_TRUE(call.GetAudioReceiveStream(kAudioSsrc2) - ->GetConfig() - .combined_audio_video_bwe); + for (uint32 ssrc : ssrcs) { + const auto* s = call_.GetAudioReceiveStream(ssrc); + EXPECT_NE(nullptr, s); + EXPECT_EQ(true, s->GetConfig().combined_audio_video_bwe); + } // Disable combined BWE option - should be disabled again. options.combined_audio_video_bwe.Set(false); EXPECT_TRUE(media_channel->SetOptions(options)); - EXPECT_EQ(2, call.GetAudioReceiveStreams().size()); - EXPECT_FALSE(call.GetAudioReceiveStream(kAudioSsrc1) - ->GetConfig() - .combined_audio_video_bwe); - EXPECT_FALSE(call.GetAudioReceiveStream(kAudioSsrc2) - ->GetConfig() - .combined_audio_video_bwe); + for (uint32 ssrc : ssrcs) { + const auto* s = call_.GetAudioReceiveStream(ssrc); + EXPECT_NE(nullptr, s); + EXPECT_EQ(false, s->GetConfig().combined_audio_video_bwe); + } - media_channel->SetCall(nullptr); -} - -TEST_F(WebRtcVoiceEngineTestFake, SetCallConfiguresAudioReceiveChannels) { - // Test that calling SetCall() on the voice media channel results in the - // expected state changes in Call. - cricket::FakeCall call((webrtc::Call::Config())); - cricket::FakeCall call2((webrtc::Call::Config())); - const uint32 kAudioSsrc1 = 223; - const uint32 kAudioSsrc2 = 224; - - EXPECT_TRUE(SetupEngine()); - cricket::WebRtcVoiceMediaChannel* media_channel = - static_cast(channel_); - EXPECT_TRUE(media_channel->AddRecvStream( - cricket::StreamParams::CreateLegacy(kAudioSsrc1))); - EXPECT_TRUE(media_channel->AddRecvStream( - cricket::StreamParams::CreateLegacy(kAudioSsrc2))); - - // Combined BWE should not be set up yet. - EXPECT_EQ(0, call.GetAudioReceiveStreams().size()); - - // Register - should be enabled. - media_channel->SetCall(&call); - EXPECT_EQ(2, call.GetAudioReceiveStreams().size()); - EXPECT_NE(nullptr, call.GetAudioReceiveStream(kAudioSsrc1)); - EXPECT_NE(nullptr, call.GetAudioReceiveStream(kAudioSsrc2)); - - // Re-register - should now be enabled on new call. - media_channel->SetCall(&call2); - EXPECT_EQ(0, call.GetAudioReceiveStreams().size()); - EXPECT_EQ(2, call2.GetAudioReceiveStreams().size()); - EXPECT_NE(nullptr, call2.GetAudioReceiveStream(kAudioSsrc1)); - EXPECT_NE(nullptr, call2.GetAudioReceiveStream(kAudioSsrc2)); - - // Unregister - should be disabled again. - media_channel->SetCall(nullptr); - EXPECT_EQ(0, call.GetAudioReceiveStreams().size()); + EXPECT_EQ(2, call_.GetAudioReceiveStreams().size()); } TEST_F(WebRtcVoiceEngineTestFake, ConfigureCombinedBweForNewRecvStreams) { // Test that adding receive streams after enabling combined bandwidth // estimation will correctly configure each channel. - cricket::FakeCall call((webrtc::Call::Config())); - EXPECT_TRUE(SetupEngine()); cricket::WebRtcVoiceMediaChannel* media_channel = static_cast(channel_); - media_channel->SetCall(&call); cricket::AudioOptions options; options.combined_audio_video_bwe.Set(true); EXPECT_TRUE(media_channel->SetOptions(options)); @@ -3314,21 +3263,14 @@ TEST_F(WebRtcVoiceEngineTestFake, ConfigureCombinedBweForNewRecvStreams) { for (unsigned int i = 0; i < ARRAY_SIZE(kSsrcs); ++i) { EXPECT_TRUE(media_channel->AddRecvStream( cricket::StreamParams::CreateLegacy(kSsrcs[i]))); - EXPECT_NE(nullptr, call.GetAudioReceiveStream(kSsrcs[i])); - EXPECT_TRUE(call.GetAudioReceiveStream(kSsrcs[i]) - ->GetConfig() - .combined_audio_video_bwe); + EXPECT_NE(nullptr, call_.GetAudioReceiveStream(kSsrcs[i])); } - EXPECT_EQ(ARRAY_SIZE(kSsrcs), call.GetAudioReceiveStreams().size()); - - media_channel->SetCall(nullptr); - EXPECT_EQ(0, call.GetAudioReceiveStreams().size()); + EXPECT_EQ(ARRAY_SIZE(kSsrcs), call_.GetAudioReceiveStreams().size()); } TEST_F(WebRtcVoiceEngineTestFake, ConfiguresAudioReceiveStreamRtpExtensions) { // Test that setting the header extensions results in the expected state // changes on an associated Call. - cricket::FakeCall call((webrtc::Call::Config())); std::vector ssrcs; ssrcs.push_back(223); ssrcs.push_back(224); @@ -3336,16 +3278,18 @@ TEST_F(WebRtcVoiceEngineTestFake, ConfiguresAudioReceiveStreamRtpExtensions) { EXPECT_TRUE(SetupEngine()); cricket::WebRtcVoiceMediaChannel* media_channel = static_cast(channel_); - media_channel->SetCall(&call); + cricket::AudioOptions options; + options.combined_audio_video_bwe.Set(true); + EXPECT_TRUE(media_channel->SetOptions(options)); for (uint32 ssrc : ssrcs) { EXPECT_TRUE(media_channel->AddRecvStream( cricket::StreamParams::CreateLegacy(ssrc))); } // Combined BWE should be set up, but with no configured extensions. - EXPECT_EQ(2, call.GetAudioReceiveStreams().size()); + EXPECT_EQ(2, call_.GetAudioReceiveStreams().size()); for (uint32 ssrc : ssrcs) { - const auto* s = call.GetAudioReceiveStream(ssrc); + const auto* s = call_.GetAudioReceiveStream(ssrc); EXPECT_NE(nullptr, s); EXPECT_EQ(0, s->GetConfig().rtp.extensions.size()); } @@ -3353,9 +3297,9 @@ TEST_F(WebRtcVoiceEngineTestFake, ConfiguresAudioReceiveStreamRtpExtensions) { // Set up receive extensions. const auto& e_exts = engine_.rtp_header_extensions(); channel_->SetRecvRtpHeaderExtensions(e_exts); - EXPECT_EQ(2, call.GetAudioReceiveStreams().size()); + EXPECT_EQ(2, call_.GetAudioReceiveStreams().size()); for (uint32 ssrc : ssrcs) { - const auto* s = call.GetAudioReceiveStream(ssrc); + const auto* s = call_.GetAudioReceiveStream(ssrc); EXPECT_NE(nullptr, s); const auto& s_exts = s->GetConfig().rtp.extensions; EXPECT_EQ(e_exts.size(), s_exts.size()); @@ -3372,17 +3316,14 @@ TEST_F(WebRtcVoiceEngineTestFake, ConfiguresAudioReceiveStreamRtpExtensions) { std::vector extensions; channel_->SetRecvRtpHeaderExtensions(extensions); for (uint32 ssrc : ssrcs) { - const auto* s = call.GetAudioReceiveStream(ssrc); + const auto* s = call_.GetAudioReceiveStream(ssrc); EXPECT_NE(nullptr, s); EXPECT_EQ(0, s->GetConfig().rtp.extensions.size()); } - - media_channel->SetCall(nullptr); } TEST_F(WebRtcVoiceEngineTestFake, DeliverAudioPacket_Call) { // Test that packets are forwarded to the Call when configured accordingly. - cricket::FakeCall call((webrtc::Call::Config())); const uint32 kAudioSsrc = 1; rtc::Buffer kPcmuPacket(kPcmuFrame, sizeof(kPcmuFrame)); static const unsigned char kRtcp[] = { @@ -3402,24 +3343,14 @@ TEST_F(WebRtcVoiceEngineTestFake, DeliverAudioPacket_Call) { EXPECT_TRUE(media_channel->AddRecvStream( cricket::StreamParams::CreateLegacy(kAudioSsrc))); - // Call not set on media channel, so no packets can be forwarded. - EXPECT_EQ(0, call.GetAudioReceiveStreams().size()); - channel_->OnPacketReceived(&kPcmuPacket, rtc::PacketTime()); - channel_->OnRtcpReceived(&kRtcpPacket, rtc::PacketTime()); - EXPECT_EQ(0, call.GetAudioReceiveStreams().size()); - - // Set Call, now there should be a receive stream which is forwarded packets. - media_channel->SetCall(&call); - EXPECT_EQ(1, call.GetAudioReceiveStreams().size()); + EXPECT_EQ(1, call_.GetAudioReceiveStreams().size()); const cricket::FakeAudioReceiveStream* s = - call.GetAudioReceiveStream(kAudioSsrc); + call_.GetAudioReceiveStream(kAudioSsrc); EXPECT_EQ(0, s->received_packets()); channel_->OnPacketReceived(&kPcmuPacket, rtc::PacketTime()); EXPECT_EQ(1, s->received_packets()); channel_->OnRtcpReceived(&kRtcpPacket, rtc::PacketTime()); EXPECT_EQ(2, s->received_packets()); - - media_channel->SetCall(nullptr); } // Associate channel should not set on 1:1 call, since the receive channel also @@ -3506,8 +3437,10 @@ TEST(WebRtcVoiceEngineTest, TestDefaultOptionsBeforeInit) { TEST(WebRtcVoiceEngineTest, StartupShutdown) { cricket::WebRtcVoiceEngine engine; EXPECT_TRUE(engine.Init(rtc::Thread::Current())); + rtc::scoped_ptr call( + webrtc::Call::Create(webrtc::Call::Config())); cricket::VoiceMediaChannel* channel = - engine.CreateChannel(cricket::AudioOptions()); + engine.CreateChannel(call.get(), cricket::AudioOptions()); EXPECT_TRUE(channel != nullptr); delete channel; engine.Terminate(); @@ -3601,16 +3534,16 @@ TEST(WebRtcVoiceEngineTest, HasCorrectCodecs) { TEST(WebRtcVoiceEngineTest, Has32Channels) { cricket::WebRtcVoiceEngine engine; EXPECT_TRUE(engine.Init(rtc::Thread::Current())); + rtc::scoped_ptr call( + webrtc::Call::Create(webrtc::Call::Config())); cricket::VoiceMediaChannel* channels[32]; int num_channels = 0; - while (num_channels < ARRAY_SIZE(channels)) { cricket::VoiceMediaChannel* channel = - engine.CreateChannel(cricket::AudioOptions()); + engine.CreateChannel(call.get(), cricket::AudioOptions()); if (!channel) break; - channels[num_channels++] = channel; } @@ -3628,6 +3561,8 @@ TEST(WebRtcVoiceEngineTest, Has32Channels) { TEST(WebRtcVoiceEngineTest, SetRecvCodecs) { cricket::WebRtcVoiceEngine engine; EXPECT_TRUE(engine.Init(rtc::Thread::Current())); - cricket::WebRtcVoiceMediaChannel channel(&engine); + rtc::scoped_ptr call( + webrtc::Call::Create(webrtc::Call::Config())); + cricket::WebRtcVoiceMediaChannel channel(&engine, call.get()); EXPECT_TRUE(channel.SetRecvCodecs(engine.codecs())); } diff --git a/talk/session/media/channelmanager.cc b/talk/session/media/channelmanager.cc index f39e966e51..c109d60388 100644 --- a/talk/session/media/channelmanager.cc +++ b/talk/session/media/channelmanager.cc @@ -33,6 +33,7 @@ #include +#include "talk/app/webrtc/mediacontroller.h" #include "talk/media/base/capturemanager.h" #include "talk/media/base/hybriddataengine.h" #include "talk/media/base/rtpdataengine.h" @@ -308,7 +309,7 @@ void ChannelManager::Terminate_w() { DestroyVideoChannel_w(video_channels_.back()); } while (!voice_channels_.empty()) { - DestroyVoiceChannel_w(voice_channels_.back(), nullptr); + DestroyVoiceChannel_w(voice_channels_.back()); } if (!SetCaptureDevice_w(NULL)) { LOG(LS_WARNING) << "failed to delete video capturer"; @@ -317,23 +318,32 @@ void ChannelManager::Terminate_w() { } VoiceChannel* ChannelManager::CreateVoiceChannel( + webrtc::MediaControllerInterface* media_controller, 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, options)); + Bind(&ChannelManager::CreateVoiceChannel_w, + this, + media_controller, + session, + content_name, + rtcp, + options)); } VoiceChannel* ChannelManager::CreateVoiceChannel_w( + webrtc::MediaControllerInterface* media_controller, 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(options); + ASSERT(nullptr != media_controller); + VoiceMediaChannel* media_channel = + media_engine_->CreateChannel(media_controller->call_w(), options); if (!media_channel) return nullptr; @@ -348,17 +358,14 @@ VoiceChannel* ChannelManager::CreateVoiceChannel_w( return voice_channel; } -void ChannelManager::DestroyVoiceChannel(VoiceChannel* voice_channel, - VideoChannel* video_channel) { +void ChannelManager::DestroyVoiceChannel(VoiceChannel* voice_channel) { if (voice_channel) { worker_thread_->Invoke( - Bind(&ChannelManager::DestroyVoiceChannel_w, this, voice_channel, - video_channel)); + Bind(&ChannelManager::DestroyVoiceChannel_w, this, voice_channel)); } } -void ChannelManager::DestroyVoiceChannel_w(VoiceChannel* voice_channel, - VideoChannel* video_channel) { +void ChannelManager::DestroyVoiceChannel_w(VoiceChannel* voice_channel) { // Destroy voice channel. ASSERT(initialized_); ASSERT(worker_thread_ == rtc::Thread::Current()); @@ -367,57 +374,37 @@ void ChannelManager::DestroyVoiceChannel_w(VoiceChannel* voice_channel, ASSERT(it != voice_channels_.end()); if (it == voice_channels_.end()) return; - - if (video_channel) { - video_channel->media_channel()->DetachVoiceChannel(); - } voice_channels_.erase(it); delete voice_channel; } VideoChannel* ChannelManager::CreateVideoChannel( + webrtc::MediaControllerInterface* media_controller, BaseSession* session, const std::string& content_name, bool rtcp, - VoiceChannel* voice_channel) { + const VideoOptions& options) { return worker_thread_->Invoke( Bind(&ChannelManager::CreateVideoChannel_w, this, + media_controller, session, content_name, rtcp, - VideoOptions(), - voice_channel)); -} - -VideoChannel* ChannelManager::CreateVideoChannel( - BaseSession* session, - const std::string& content_name, - bool rtcp, - const VideoOptions& options, - VoiceChannel* voice_channel) { - return worker_thread_->Invoke( - Bind(&ChannelManager::CreateVideoChannel_w, - this, - session, - content_name, - rtcp, - options, - voice_channel)); + options)); } VideoChannel* ChannelManager::CreateVideoChannel_w( + webrtc::MediaControllerInterface* media_controller, BaseSession* session, const std::string& content_name, bool rtcp, - const VideoOptions& options, - VoiceChannel* voice_channel) { + const VideoOptions& options) { ASSERT(initialized_); ASSERT(worker_thread_ == rtc::Thread::Current()); + ASSERT(nullptr != media_controller); VideoMediaChannel* media_channel = - // voice_channel can be NULL in case of NullVoiceEngine. - media_engine_->CreateVideoChannel( - options, voice_channel ? voice_channel->media_channel() : NULL); + media_engine_->CreateVideoChannel(media_controller->call_w(), options); if (media_channel == NULL) return NULL; diff --git a/talk/session/media/channelmanager.h b/talk/session/media/channelmanager.h index 81cb3eed61..3bfef84de2 100644 --- a/talk/session/media/channelmanager.h +++ b/talk/session/media/channelmanager.h @@ -40,6 +40,9 @@ #include "webrtc/base/sigslotrepeater.h" #include "webrtc/base/thread.h" +namespace webrtc { +class MediaControllerInterface; +} namespace cricket { const int kDefaultAudioDelayOffset = 0; @@ -81,6 +84,8 @@ class ChannelManager : public rtc::MessageHandler, return true; } + MediaEngineInterface* media_engine() { return media_engine_.get(); } + // Gets capabilities. Can be called prior to starting the media engine. int GetCapabilities(); @@ -102,25 +107,22 @@ 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, - const AudioOptions& options); + VoiceChannel* CreateVoiceChannel( + webrtc::MediaControllerInterface* media_controller, + 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); - // TODO(pbos): Remove as soon as all call sites specify VideoOptions. - VideoChannel* CreateVideoChannel(BaseSession* session, - const std::string& content_name, - bool rtcp, - VoiceChannel* voice_channel); + void DestroyVoiceChannel(VoiceChannel* voice_channel); // Creates a video channel, synced with the specified voice channel, and // associated with the specified session. - VideoChannel* CreateVideoChannel(BaseSession* session, - const std::string& content_name, - bool rtcp, - const VideoOptions& options, - VoiceChannel* voice_channel); + VideoChannel* CreateVideoChannel( + webrtc::MediaControllerInterface* media_controller, + BaseSession* session, + const std::string& content_name, + bool rtcp, + const VideoOptions& options); // Destroys a video channel created with the Create API. void DestroyVideoChannel(VideoChannel* video_channel); DataChannel* CreateDataChannel( @@ -247,17 +249,19 @@ 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, - const AudioOptions& options); - void DestroyVoiceChannel_w(VoiceChannel* voice_channel, - VideoChannel* video_channel); - VideoChannel* CreateVideoChannel_w(BaseSession* session, - const std::string& content_name, - bool rtcp, - const VideoOptions& options, - VoiceChannel* voice_channel); + VoiceChannel* CreateVoiceChannel_w( + webrtc::MediaControllerInterface* media_controller, + BaseSession* session, + const std::string& content_name, + bool rtcp, + const AudioOptions& options); + void DestroyVoiceChannel_w(VoiceChannel* voice_channel); + VideoChannel* CreateVideoChannel_w( + webrtc::MediaControllerInterface* media_controller, + BaseSession* session, + const std::string& content_name, + bool rtcp, + const VideoOptions& options); void DestroyVideoChannel_w(VideoChannel* video_channel); DataChannel* CreateDataChannel_w( BaseSession* session, const std::string& content_name, diff --git a/talk/session/media/channelmanager_unittest.cc b/talk/session/media/channelmanager_unittest.cc index 1ffdaf2836..e48fd74181 100644 --- a/talk/session/media/channelmanager_unittest.cc +++ b/talk/session/media/channelmanager_unittest.cc @@ -25,11 +25,13 @@ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ +#include "talk/app/webrtc/mediacontroller.h" #include "talk/media/base/fakecapturemanager.h" #include "talk/media/base/fakemediaengine.h" #include "talk/media/base/fakemediaprocessor.h" #include "talk/media/base/testutils.h" #include "talk/media/devices/fakedevicemanager.h" +#include "talk/media/webrtc/fakewebrtccall.h" #include "webrtc/p2p/base/fakesession.h" #include "talk/session/media/channelmanager.h" #include "webrtc/base/gunit.h" @@ -49,10 +51,21 @@ static const VideoCodec kVideoCodecs[] = { VideoCodec(96, "rtx", 100, 200, 300, 0), }; +class FakeMediaController : public webrtc::MediaControllerInterface { + public: + explicit FakeMediaController(webrtc::Call* call) : call_(call) { + DCHECK(nullptr != call); + } + ~FakeMediaController() override {} + webrtc::Call* call_w() override { return call_; } + private: + webrtc::Call* call_; +}; + class ChannelManagerTest : public testing::Test { protected: - ChannelManagerTest() : fme_(NULL), fdm_(NULL), fcm_(NULL), cm_(NULL) { - } + ChannelManagerTest() : fake_call_(webrtc::Call::Config()), + fake_mc_(&fake_call_), fme_(NULL), fdm_(NULL), fcm_(NULL), cm_(NULL) {} virtual void SetUp() { fme_ = new cricket::FakeMediaEngine(); @@ -88,6 +101,8 @@ class ChannelManagerTest : public testing::Test { } rtc::Thread worker_; + cricket::FakeCall fake_call_; + cricket::FakeMediaController fake_mc_; cricket::FakeMediaEngine* fme_; cricket::FakeDataEngine* fdme_; cricket::FakeDeviceManager* fdm_; @@ -125,17 +140,17 @@ TEST_F(ChannelManagerTest, StartupShutdownOnThread) { TEST_F(ChannelManagerTest, CreateDestroyChannels) { EXPECT_TRUE(cm_->Init()); cricket::VoiceChannel* voice_channel = cm_->CreateVoiceChannel( - session_, cricket::CN_AUDIO, false, AudioOptions()); + &fake_mc_, 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); + &fake_mc_, session_, cricket::CN_VIDEO, false, VideoOptions()); EXPECT_TRUE(video_channel != nullptr); cricket::DataChannel* data_channel = cm_->CreateDataChannel(session_, cricket::CN_DATA, false, cricket::DCT_RTP); EXPECT_TRUE(data_channel != nullptr); cm_->DestroyVideoChannel(video_channel); - cm_->DestroyVoiceChannel(voice_channel, nullptr); + cm_->DestroyVoiceChannel(voice_channel); cm_->DestroyDataChannel(data_channel); cm_->Terminate(); } @@ -148,17 +163,17 @@ TEST_F(ChannelManagerTest, CreateDestroyChannelsOnThread) { delete session_; session_ = new cricket::FakeSession(&worker_, true); cricket::VoiceChannel* voice_channel = cm_->CreateVoiceChannel( - session_, cricket::CN_AUDIO, false, AudioOptions()); + &fake_mc_, 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); + &fake_mc_, session_, cricket::CN_VIDEO, false, VideoOptions()); EXPECT_TRUE(video_channel != nullptr); cricket::DataChannel* data_channel = cm_->CreateDataChannel(session_, cricket::CN_DATA, false, cricket::DCT_RTP); EXPECT_TRUE(data_channel != nullptr); cm_->DestroyVideoChannel(video_channel); - cm_->DestroyVoiceChannel(voice_channel, nullptr); + cm_->DestroyVoiceChannel(voice_channel); cm_->DestroyDataChannel(data_channel); cm_->Terminate(); } @@ -174,10 +189,10 @@ TEST_F(ChannelManagerTest, NoTransportChannelTest) { "audio", cricket::ICE_CANDIDATE_COMPONENT_RTP) == nullptr); cricket::VoiceChannel* voice_channel = cm_->CreateVoiceChannel( - session_, cricket::CN_AUDIO, false, AudioOptions()); + &fake_mc_, 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); + &fake_mc_, session_, cricket::CN_VIDEO, false, VideoOptions()); EXPECT_TRUE(video_channel == nullptr); cricket::DataChannel* data_channel = cm_->CreateDataChannel(session_, cricket::CN_DATA,