From 4b60c73e74d62beff484b7f54d8f3267cb66274f Mon Sep 17 00:00:00 2001 From: Fredrik Solenberg Date: Thu, 7 May 2015 14:07:48 +0200 Subject: [PATCH] Hook up libjingle WebRtcVoiceEngine to Call API for combined A/V BWE. BUG=4574,3109 R=pbos@webrtc.org, tommi@webrtc.org Review URL: https://webrtc-codereview.appspot.com/49269004 Cr-Commit-Position: refs/heads/master@{#9150} --- talk/app/webrtc/webrtcsession.cc | 5 +- talk/media/base/fakemediaengine.h | 1 + talk/media/base/filemediaengine.h | 2 +- talk/media/base/mediachannel.h | 2 + talk/media/webrtc/fakewebrtccall.cc | 102 ++++++--- talk/media/webrtc/fakewebrtccall.h | 24 ++- talk/media/webrtc/webrtcvideoengine2.cc | 33 +-- talk/media/webrtc/webrtcvideoengine2.h | 8 +- talk/media/webrtc/webrtcvoiceengine.cc | 98 ++++++++- talk/media/webrtc/webrtcvoiceengine.h | 12 ++ .../webrtc/webrtcvoiceengine_unittest.cc | 201 ++++++++++++++++++ talk/session/media/channel.h | 11 +- talk/session/media/channelmanager.cc | 20 +- talk/session/media/channelmanager.h | 6 +- talk/session/media/channelmanager_unittest.cc | 4 +- 15 files changed, 465 insertions(+), 64 deletions(-) diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc index f235cd5eb..dd5d858ea 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -505,7 +505,7 @@ WebRtcSession::~WebRtcSession() { } if (voice_channel_) { SignalVoiceChannelDestroyed(); - channel_manager_->DestroyVoiceChannel(voice_channel_.release()); + channel_manager_->DestroyVoiceChannel(voice_channel_.release(), nullptr); } if (data_channel_) { SignalDataChannelDestroyed(); @@ -1553,7 +1553,8 @@ void WebRtcSession::RemoveUnusedChannelsAndTransports( mediastream_signaling_->OnAudioChannelClose(); SignalVoiceChannelDestroyed(); const std::string content_name = voice_channel_->content_name(); - channel_manager_->DestroyVoiceChannel(voice_channel_.release()); + channel_manager_->DestroyVoiceChannel(voice_channel_.release(), + video_channel_.get()); DestroyTransportProxy(content_name); } diff --git a/talk/media/base/fakemediaengine.h b/talk/media/base/fakemediaengine.h index 7deaed735..ead9f79e3 100644 --- a/talk/media/base/fakemediaengine.h +++ b/talk/media/base/fakemediaengine.h @@ -516,6 +516,7 @@ 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. diff --git a/talk/media/base/filemediaengine.h b/talk/media/base/filemediaengine.h index 1cd9901c8..c2e80c383 100644 --- a/talk/media/base/filemediaengine.h +++ b/talk/media/base/filemediaengine.h @@ -262,8 +262,8 @@ class FileVideoChannel : public VideoMediaChannel { rtc::StreamInterface* output_file_stream, rtc::Thread* rtp_sender_thread); virtual ~FileVideoChannel(); - // Implement pure virtual methods of VideoMediaChannel. + void DetachVoiceChannel() override {} virtual bool SetRecvCodecs(const std::vector& codecs) { return true; } diff --git a/talk/media/base/mediachannel.h b/talk/media/base/mediachannel.h index 12bb519dc..dd78f0e04 100644 --- a/talk/media/base/mediachannel.h +++ b/talk/media/base/mediachannel.h @@ -1084,6 +1084,8 @@ class VideoMediaChannel : public MediaChannel { VideoMediaChannel() : renderer_(NULL) {} virtual ~VideoMediaChannel() {} + // Allow video channel to unhook itself from an associated voice channel. + virtual void DetachVoiceChannel() = 0; // Sets the codecs/payload types to be used for incoming media. virtual bool SetRecvCodecs(const std::vector& codecs) = 0; // Sets the codecs/payload types to be used for outgoing media. diff --git a/talk/media/webrtc/fakewebrtccall.cc b/talk/media/webrtc/fakewebrtccall.cc index d5201edbd..257c01850 100644 --- a/talk/media/webrtc/fakewebrtccall.cc +++ b/talk/media/webrtc/fakewebrtccall.cc @@ -27,10 +27,26 @@ #include "talk/media/webrtc/fakewebrtccall.h" +#include + #include "talk/media/base/rtputils.h" #include "webrtc/base/gunit.h" namespace cricket { +FakeAudioReceiveStream::FakeAudioReceiveStream( + const webrtc::AudioReceiveStream::Config& config) + : config_(config), received_packets_(0) { +} + +const webrtc::AudioReceiveStream::Config& + FakeAudioReceiveStream::GetConfig() const { + return config_; +} + +void FakeAudioReceiveStream::IncrementReceivedPackets() { + received_packets_++; +} + FakeVideoSendStream::FakeVideoSendStream( const webrtc::VideoSendStream::Config& config, const webrtc::VideoEncoderConfig& encoder_config) @@ -181,30 +197,56 @@ FakeCall::FakeCall(const webrtc::Call::Config& config) FakeCall::~FakeCall() { EXPECT_EQ(0u, video_send_streams_.size()); EXPECT_EQ(0u, video_receive_streams_.size()); + EXPECT_EQ(0u, audio_receive_streams_.size()); } webrtc::Call::Config FakeCall::GetConfig() const { return config_; } -std::vector FakeCall::GetVideoSendStreams() { +const std::vector& FakeCall::GetVideoSendStreams() { return video_send_streams_; } -std::vector FakeCall::GetVideoReceiveStreams() { +const std::vector& FakeCall::GetVideoReceiveStreams() { return video_receive_streams_; } +const std::vector& FakeCall::GetAudioReceiveStreams() { + return audio_receive_streams_; +} + +const FakeAudioReceiveStream* FakeCall::GetAudioReceiveStream(uint32_t ssrc) { + for (const auto p : GetAudioReceiveStreams()) { + if (p->GetConfig().rtp.remote_ssrc == ssrc) { + return p; + } + } + return nullptr; +} + webrtc::Call::NetworkState FakeCall::GetNetworkState() const { return network_state_; } webrtc::AudioReceiveStream* FakeCall::CreateAudioReceiveStream( const webrtc::AudioReceiveStream::Config& config) { - return nullptr; + audio_receive_streams_.push_back(new FakeAudioReceiveStream(config)); + ++num_created_receive_streams_; + return audio_receive_streams_.back(); } + void FakeCall::DestroyAudioReceiveStream( webrtc::AudioReceiveStream* receive_stream) { + auto it = std::find(audio_receive_streams_.begin(), + audio_receive_streams_.end(), + static_cast(receive_stream)); + if (it == audio_receive_streams_.end()) { + ADD_FAILURE() << "DestroyAudioReceiveStream called with unknown paramter."; + } else { + delete *it; + audio_receive_streams_.erase(it); + } } webrtc::VideoSendStream* FakeCall::CreateVideoSendStream( @@ -218,37 +260,35 @@ webrtc::VideoSendStream* FakeCall::CreateVideoSendStream( } void FakeCall::DestroyVideoSendStream(webrtc::VideoSendStream* send_stream) { - FakeVideoSendStream* fake_stream = - static_cast(send_stream); - for (size_t i = 0; i < video_send_streams_.size(); ++i) { - if (video_send_streams_[i] == fake_stream) { - delete video_send_streams_[i]; - video_send_streams_.erase(video_send_streams_.begin() + i); - return; - } + auto it = std::find(video_send_streams_.begin(), + video_send_streams_.end(), + static_cast(send_stream)); + if (it == video_send_streams_.end()) { + ADD_FAILURE() << "DestroyVideoSendStream called with unknown paramter."; + } else { + delete *it; + video_send_streams_.erase(it); } - ADD_FAILURE() << "DestroyVideoSendStream called with unknown paramter."; } webrtc::VideoReceiveStream* FakeCall::CreateVideoReceiveStream( const webrtc::VideoReceiveStream::Config& config) { video_receive_streams_.push_back(new FakeVideoReceiveStream(config)); ++num_created_receive_streams_; - return video_receive_streams_[video_receive_streams_.size() - 1]; + return video_receive_streams_.back(); } void FakeCall::DestroyVideoReceiveStream( webrtc::VideoReceiveStream* receive_stream) { - FakeVideoReceiveStream* fake_stream = - static_cast(receive_stream); - for (size_t i = 0; i < video_receive_streams_.size(); ++i) { - if (video_receive_streams_[i] == fake_stream) { - delete video_receive_streams_[i]; - video_receive_streams_.erase(video_receive_streams_.begin() + i); - return; - } + auto it = std::find(video_receive_streams_.begin(), + video_receive_streams_.end(), + static_cast(receive_stream)); + if (it == video_receive_streams_.end()) { + ADD_FAILURE() << "DestroyVideoReceiveStream called with unknown paramter."; + } else { + delete *it; + video_receive_streams_.erase(it); } - ADD_FAILURE() << "DestroyVideoReceiveStream called with unknown paramter."; } webrtc::PacketReceiver* FakeCall::Receiver() { @@ -258,16 +298,26 @@ webrtc::PacketReceiver* FakeCall::Receiver() { FakeCall::DeliveryStatus FakeCall::DeliverPacket(webrtc::MediaType media_type, const uint8_t* packet, size_t length) { - EXPECT_TRUE(media_type == webrtc::MediaType::ANY || - media_type == webrtc::MediaType::VIDEO); EXPECT_GE(length, 12u); uint32_t ssrc; if (!GetRtpSsrc(packet, length, &ssrc)) return DELIVERY_PACKET_ERROR; - for (auto receiver : video_receive_streams_) { - if (receiver->GetConfig().rtp.remote_ssrc == ssrc) + if (media_type == webrtc::MediaType::ANY || + media_type == webrtc::MediaType::VIDEO) { + for (auto receiver : video_receive_streams_) { + if (receiver->GetConfig().rtp.remote_ssrc == ssrc) return DELIVERY_OK; + } + } + if (media_type == webrtc::MediaType::ANY || + media_type == webrtc::MediaType::AUDIO) { + for (auto receiver : audio_receive_streams_) { + if (receiver->GetConfig().rtp.remote_ssrc == ssrc) { + receiver->IncrementReceivedPackets(); + return DELIVERY_OK; + } + } } return DELIVERY_UNKNOWN_SSRC; } diff --git a/talk/media/webrtc/fakewebrtccall.h b/talk/media/webrtc/fakewebrtccall.h index 1588de492..e4b00d251 100644 --- a/talk/media/webrtc/fakewebrtccall.h +++ b/talk/media/webrtc/fakewebrtccall.h @@ -31,11 +31,27 @@ #include #include "webrtc/call.h" +#include "webrtc/audio_receive_stream.h" #include "webrtc/video_frame.h" #include "webrtc/video_receive_stream.h" #include "webrtc/video_send_stream.h" namespace cricket { +class FakeAudioReceiveStream : public webrtc::AudioReceiveStream { + public: + explicit FakeAudioReceiveStream( + const webrtc::AudioReceiveStream::Config& config); + + const webrtc::AudioReceiveStream::Config& GetConfig() const; + + int received_packets() const { return received_packets_; } + void IncrementReceivedPackets(); + + private: + webrtc::AudioReceiveStream::Config config_; + int received_packets_; +}; + class FakeVideoSendStream : public webrtc::VideoSendStream, public webrtc::VideoSendStreamInput { public: @@ -109,8 +125,11 @@ class FakeCall : public webrtc::Call, public webrtc::PacketReceiver { ~FakeCall(); webrtc::Call::Config GetConfig() const; - std::vector GetVideoSendStreams(); - std::vector GetVideoReceiveStreams(); + const std::vector& GetVideoSendStreams(); + const std::vector& GetVideoReceiveStreams(); + + const std::vector& GetAudioReceiveStreams(); + const FakeAudioReceiveStream* GetAudioReceiveStream(uint32_t ssrc); webrtc::Call::NetworkState GetNetworkState() const; int GetNumCreatedSendStreams() const; @@ -148,6 +167,7 @@ class FakeCall : public webrtc::Call, public webrtc::PacketReceiver { webrtc::Call::Stats stats_; std::vector video_send_streams_; std::vector video_receive_streams_; + std::vector audio_receive_streams_; int num_created_send_streams_; int num_created_receive_streams_; diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc index 93bd447aa..fbb5fc212 100644 --- a/talk/media/webrtc/webrtcvideoengine2.cc +++ b/talk/media/webrtc/webrtcvideoengine2.cc @@ -610,12 +610,9 @@ WebRtcVideoChannel2* WebRtcVideoEngine2::CreateChannel( << (voice_channel != NULL ? "With" : "Without") << " voice channel. Options: " << options.ToString(); WebRtcVideoChannel2* channel = - new WebRtcVideoChannel2(call_factory_, - voice_engine_, - voice_channel, - options, - external_encoder_factory_, - external_decoder_factory_); + new WebRtcVideoChannel2(call_factory_, voice_engine_, + static_cast(voice_channel), options, + external_encoder_factory_, external_decoder_factory_); if (!channel->Init()) { delete channel; return NULL; @@ -780,17 +777,16 @@ std::vector WebRtcVideoEngine2::GetSupportedCodecs() const { WebRtcVideoChannel2::WebRtcVideoChannel2( WebRtcCallFactory* call_factory, WebRtcVoiceEngine* voice_engine, - VoiceMediaChannel* voice_channel, + WebRtcVoiceMediaChannel* voice_channel, const VideoOptions& options, WebRtcVideoEncoderFactory* external_encoder_factory, WebRtcVideoDecoderFactory* external_decoder_factory) : unsignalled_ssrc_handler_(&default_unsignalled_ssrc_handler_), - voice_channel_id_(voice_channel != nullptr - ? static_cast( - voice_channel)->voe_channel() - : -1), + voice_channel_(voice_channel), + voice_channel_id_(voice_channel ? voice_channel->voe_channel() : -1), 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_); @@ -803,7 +799,9 @@ WebRtcVideoChannel2::WebRtcVideoChannel2( 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; @@ -818,6 +816,7 @@ void WebRtcVideoChannel2::SetDefaultOptions() { } WebRtcVideoChannel2::~WebRtcVideoChannel2() { + DetachVoiceChannel(); for (auto& kv : send_streams_) delete kv.second; for (auto& kv : receive_streams_) @@ -826,6 +825,14 @@ WebRtcVideoChannel2::~WebRtcVideoChannel2() { 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) { @@ -1121,6 +1128,8 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp) { bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp, bool default_stream) { + DCHECK(thread_checker_.CalledOnValidThread()); + LOG(LS_INFO) << "AddRecvStream" << (default_stream ? " (default stream)" : "") << ": " << sp.ToString(); if (!ValidateStreamParams(sp)) diff --git a/talk/media/webrtc/webrtcvideoengine2.h b/talk/media/webrtc/webrtcvideoengine2.h index a0fde8f9b..b7775dbfb 100644 --- a/talk/media/webrtc/webrtcvideoengine2.h +++ b/talk/media/webrtc/webrtcvideoengine2.h @@ -40,6 +40,7 @@ #include "webrtc/base/criticalsection.h" #include "webrtc/base/scoped_ptr.h" #include "webrtc/base/thread_annotations.h" +#include "webrtc/base/thread_checker.h" #include "webrtc/call.h" #include "webrtc/transport.h" #include "webrtc/video_frame.h" @@ -71,6 +72,7 @@ class WebRtcRenderAdapter; class WebRtcVideoChannelRecvInfo; class WebRtcVideoChannelSendInfo; class WebRtcVoiceEngine; +class WebRtcVoiceMediaChannel; struct CapturedFrame; struct Device; @@ -178,7 +180,7 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler, public: WebRtcVideoChannel2(WebRtcCallFactory* call_factory, WebRtcVoiceEngine* voice_engine, - VoiceMediaChannel* voice_channel, + WebRtcVoiceMediaChannel* voice_channel, const VideoOptions& options, WebRtcVideoEncoderFactory* external_encoder_factory, WebRtcVideoDecoderFactory* external_decoder_factory); @@ -186,6 +188,7 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler, bool Init(); // VideoMediaChannel implementation + void DetachVoiceChannel() override; bool SetRecvCodecs(const std::vector& codecs) override; bool SetSendCodecs(const std::vector& codecs) override; bool GetSendCodec(VideoCodec* send_codec) override; @@ -484,6 +487,8 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler, void FillBandwidthEstimationStats(const webrtc::Call::Stats& stats, VideoMediaInfo* info); + rtc::ThreadChecker thread_checker_; + uint32_t rtcp_receiver_report_ssrc_; bool sending_; rtc::scoped_ptr call_; @@ -513,6 +518,7 @@ 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_; diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc index 74a4b2560..ac36ed5ac 100644 --- a/talk/media/webrtc/webrtcvoiceengine.cc +++ b/talk/media/webrtc/webrtcvoiceengine.cc @@ -1889,11 +1889,11 @@ WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel(WebRtcVoiceEngine *engine) send_(SEND_NOTHING), shared_bwe_vie_(NULL), shared_bwe_vie_channel_(-1), + call_(nullptr), default_receive_ssrc_(0) { engine->RegisterChannel(this); LOG(LS_VERBOSE) << "WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel " << voe_channel(); - ConfigureSendChannel(voe_channel()); } @@ -1901,6 +1901,7 @@ WebRtcVoiceMediaChannel::~WebRtcVoiceMediaChannel() { LOG(LS_VERBOSE) << "WebRtcVoiceMediaChannel::~WebRtcVoiceMediaChannel " << voe_channel(); SetupSharedBandwidthEstimation(NULL, -1); + DCHECK(receive_streams_.empty() || call_); // Remove any remaining send streams, the default channel will be deleted // later. @@ -1913,6 +1914,7 @@ WebRtcVoiceMediaChannel::~WebRtcVoiceMediaChannel() { while (!receive_channels_.empty()) { RemoveRecvStream(receive_channels_.begin()->first); } + DCHECK(receive_streams_.empty()); // Delete the default channel. DeleteChannel(voe_channel()); @@ -2006,6 +2008,7 @@ bool WebRtcVoiceMediaChannel::SetOptions(const AudioOptions& options) { shared_bwe_vie_channel_)) { return false; } + SetCall(call_); LOG(LS_INFO) << "Set voice channel options. Current options: " << options_.ToString(); @@ -2398,6 +2401,29 @@ bool WebRtcVoiceMediaChannel::SetRecvRtpHeaderExtensions( } receive_extensions_ = extensions; + + // Recreate AudioReceiveStream:s. + { + std::vector exts; + + const RtpHeaderExtension* audio_level_extension = + FindHeaderExtension(extensions, kRtpAudioLevelHeaderExtension); + if (audio_level_extension) { + exts.push_back({ + kRtpAudioLevelHeaderExtension, audio_level_extension->id}); + } + + const RtpHeaderExtension* send_time_extension = + FindHeaderExtension(extensions, kRtpAbsoluteSenderTimeHeaderExtension); + if (send_time_extension) { + exts.push_back({ + kRtpAbsoluteSenderTimeHeaderExtension, send_time_extension->id}); + } + + recv_rtp_extensions_.swap(exts); + SetCall(call_); + } + return true; } @@ -2418,6 +2444,7 @@ bool WebRtcVoiceMediaChannel::SetChannelRecvRtpHeaderExtensions( send_time_extension)) { return false; } + return true; } @@ -2711,6 +2738,7 @@ bool WebRtcVoiceMediaChannel::RemoveSendStream(uint32 ssrc) { } bool WebRtcVoiceMediaChannel::AddRecvStream(const StreamParams& sp) { + DCHECK(thread_checker_.CalledOnValidThread()); rtc::CritScope lock(&receive_channels_cs_); if (!VERIFY(sp.ssrcs.size() == 1)) @@ -2727,14 +2755,15 @@ bool WebRtcVoiceMediaChannel::AddRecvStream(const StreamParams& sp) { return false; } + TryAddAudioRecvStream(ssrc); + // Reuse default channel for recv stream in non-conference mode call // when the default channel is not being used. webrtc::AudioTransport* audio_transport = engine()->voe()->base()->audio_transport(); if (!InConferenceMode() && default_receive_ssrc_ == 0) { - LOG(LS_INFO) << "Recv stream " << sp.first_ssrc() - << " reuse default channel"; - default_receive_ssrc_ = sp.first_ssrc(); + LOG(LS_INFO) << "Recv stream " << ssrc << " reuse default channel"; + default_receive_ssrc_ = ssrc; receive_channels_.insert(std::make_pair( default_receive_ssrc_, new WebRtcVoiceChannelRenderer(voe_channel(), audio_transport))); @@ -2837,6 +2866,7 @@ bool WebRtcVoiceMediaChannel::ConfigureRecvChannel(int channel) { } bool WebRtcVoiceMediaChannel::RemoveRecvStream(uint32 ssrc) { + DCHECK(thread_checker_.CalledOnValidThread()); rtc::CritScope lock(&receive_channels_cs_); ChannelMap::iterator it = receive_channels_.find(ssrc); if (it == receive_channels_.end()) { @@ -2845,6 +2875,8 @@ bool WebRtcVoiceMediaChannel::RemoveRecvStream(uint32 ssrc) { return false; } + TryRemoveAudioRecvStream(ssrc); + // Delete the WebRtcVoiceChannelRenderer object connected to the channel, this // will disconnect the audio renderer with the receive channel. // Cache the channel before the deletion. @@ -3173,6 +3205,14 @@ bool WebRtcVoiceMediaChannel::InsertDtmf(uint32 ssrc, int event, 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_) { + call_->Receiver()->DeliverPacket(webrtc::MediaType::AUDIO, + reinterpret_cast(packet->data()), packet->size()); + } + // 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, // send it to the specific decoder instance for that stream. @@ -3206,6 +3246,14 @@ void WebRtcVoiceMediaChannel::OnPacketReceived( 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_) { + call_->Receiver()->DeliverPacket(webrtc::MediaType::AUDIO, + reinterpret_cast(packet->data()), packet->size()); + } + // Sending channels need all RTCP packets with feedback information. // Even sender reports can contain attached report blocks. // Receiving channels need sender reports in order to create @@ -3608,6 +3656,22 @@ bool WebRtcVoiceMediaChannel::SetupSharedBandwidthEstimation( return true; } +void WebRtcVoiceMediaChannel::SetCall(webrtc::Call* call) { + DCHECK(thread_checker_.CalledOnValidThread()); + DCHECK(!call || !shared_bwe_vie_); + DCHECK(!call || shared_bwe_vie_channel_ == -1); + + 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 @@ -3776,6 +3840,32 @@ bool WebRtcVoiceMediaChannel::SetupSharedBweOnChannel(int voe_channel) { return true; } +void WebRtcVoiceMediaChannel::TryAddAudioRecvStream(uint32 ssrc) { + DCHECK(thread_checker_.CalledOnValidThread()); + // If we are hooked up to a webrtc::Call, create an AudioReceiveStream too. + if (call_ && options_.combined_audio_video_bwe.GetWithDefaultIfUnset(false)) { + DCHECK(receive_streams_.find(ssrc) == receive_streams_.end()); + webrtc::AudioReceiveStream::Config config; + config.rtp.remote_ssrc = ssrc; + config.rtp.extensions = recv_rtp_extensions_; + webrtc::AudioReceiveStream* s = call_->CreateAudioReceiveStream(config); + receive_streams_.insert(std::make_pair(ssrc, s)); + } +} + +void WebRtcVoiceMediaChannel::TryRemoveAudioRecvStream(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); + } + } +} + int WebRtcSoundclipStream::Read(void *buf, size_t len) { size_t res = 0; mem_.Read(buf, len, &res, NULL); diff --git a/talk/media/webrtc/webrtcvoiceengine.h b/talk/media/webrtc/webrtcvoiceengine.h index 56665de7b..29a565688 100644 --- a/talk/media/webrtc/webrtcvoiceengine.h +++ b/talk/media/webrtc/webrtcvoiceengine.h @@ -43,6 +43,8 @@ #include "webrtc/base/logging.h" #include "webrtc/base/scoped_ptr.h" #include "webrtc/base/stream.h" +#include "webrtc/base/thread_checker.h" +#include "webrtc/call.h" #include "webrtc/common.h" #if !defined(LIBPEERCONNECTION_LIB) && \ @@ -394,6 +396,7 @@ class WebRtcVoiceMediaChannel bool SetupSharedBandwidthEstimation(webrtc::VideoEngine* vie, int vie_channel); + void SetCall(webrtc::Call* call); protected: int GetLastEngineError() { return engine()->GetLastEngineError(); } int GetOutputLevel(int channel); @@ -438,6 +441,9 @@ class WebRtcVoiceMediaChannel const RtpHeaderExtension* extension); bool SetupSharedBweOnChannel(int voe_channel); + void TryAddAudioRecvStream(uint32 ssrc); + void TryRemoveAudioRecvStream(uint32 ssrc); + bool SetChannelRecvRtpHeaderExtensions( int channel_id, const std::vector& extensions); @@ -445,6 +451,8 @@ class WebRtcVoiceMediaChannel int channel_id, const std::vector& extensions); + rtc::ThreadChecker thread_checker_; + rtc::scoped_ptr ringback_tone_; std::set ringback_channels_; // channels playing ringback std::vector recv_codecs_; @@ -465,6 +473,7 @@ class WebRtcVoiceMediaChannel // to for Bandwidth Estimation purposes. webrtc::VideoEngine* shared_bwe_vie_; int shared_bwe_vie_channel_; + webrtc::Call* call_; // send_channels_ contains the channels which are being used for sending. // When the default channel (voe_channel) is used for sending, it is @@ -476,11 +485,14 @@ class WebRtcVoiceMediaChannel // receive_channels_ and send_channels_ in non-conference mode and in that // case it will only be there if a non-zero default_receive_ssrc_ is set. ChannelMap receive_channels_; // for multiple sources + std::map receive_streams_; // receive_channels_ can be read from WebRtc callback thread. Access from // the WebRtc thread must be synchronized with edits on the worker thread. // Reads on the worker thread are ok. // std::vector receive_extensions_; + std::vector recv_rtp_extensions_; + // Do not lock this on the VoE media processor thread; potential for deadlock // exists. mutable rtc::CriticalSection receive_channels_cs_; diff --git a/talk/media/webrtc/webrtcvoiceengine_unittest.cc b/talk/media/webrtc/webrtcvoiceengine_unittest.cc index f0e450328..0a3e91cdc 100644 --- a/talk/media/webrtc/webrtcvoiceengine_unittest.cc +++ b/talk/media/webrtc/webrtcvoiceengine_unittest.cc @@ -37,6 +37,7 @@ #include "talk/media/base/fakemediaprocessor.h" #include "talk/media/base/fakenetworkinterface.h" #include "talk/media/base/fakertp.h" +#include "talk/media/webrtc/fakewebrtccall.h" #include "talk/media/webrtc/fakewebrtcvoiceengine.h" #include "talk/media/webrtc/webrtcvie.h" #include "talk/media/webrtc/webrtcvoiceengine.h" @@ -3518,3 +3519,203 @@ TEST_F(WebRtcVoiceEngineTestFake, ConfigureCombinedBweForNewRecvStreams) { EXPECT_EQ(-1, voe_.GetVideoChannel(voe_channels[i])); } } + +TEST_F(WebRtcVoiceEngineTestFake, ChangeCombinedBweOption_Call) { + // 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(nullptr)); + const uint32 kAudioSsrc1 = 223; + const uint32 kAudioSsrc2 = 224; + + EXPECT_TRUE(SetupEngine()); + cricket::WebRtcVoiceMediaChannel* media_channel = + static_cast(channel_); + media_channel->SetCall(&call); + 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()); + + // 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_NE(nullptr, call.GetAudioReceiveStream(kAudioSsrc1)); + EXPECT_NE(nullptr, call.GetAudioReceiveStream(kAudioSsrc2)); + + // Disable combined BWE option - should be disabled again. + options.combined_audio_video_bwe.Set(false); + EXPECT_TRUE(media_channel->SetOptions(options)); + EXPECT_EQ(0, call.GetAudioReceiveStreams().size()); + + media_channel->SetCall(nullptr); +} + +TEST_F(WebRtcVoiceEngineTestFake, ConfigureCombinedBwe_Call) { + // Test that calling SetCall() on the voice media channel results in the + // expected state changes in Call. + cricket::FakeCall call(webrtc::Call::Config(nullptr)); + cricket::FakeCall call2(webrtc::Call::Config(nullptr)); + const uint32 kAudioSsrc1 = 223; + const uint32 kAudioSsrc2 = 224; + + EXPECT_TRUE(SetupEngine()); + cricket::WebRtcVoiceMediaChannel* media_channel = + static_cast(channel_); + cricket::AudioOptions options; + options.combined_audio_video_bwe.Set(true); + EXPECT_TRUE(media_channel->SetOptions(options)); + 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()); +} + +TEST_F(WebRtcVoiceEngineTestFake, ConfigureCombinedBweForNewRecvStreams_Call) { + // Test that adding receive streams after enabling combined bandwidth + // estimation will correctly configure each channel. + cricket::FakeCall call(webrtc::Call::Config(nullptr)); + + 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)); + + static const uint32 kSsrcs[] = {1, 2, 3, 4}; + 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_EQ(ARRAY_SIZE(kSsrcs), call.GetAudioReceiveStreams().size()); + + media_channel->SetCall(nullptr); + EXPECT_EQ(0, call.GetAudioReceiveStreams().size()); +} + +TEST_F(WebRtcVoiceEngineTestFake, ConfigureCombinedBweExtensions_Call) { + // Test that setting the header extensions results in the expected state + // changes on an associated Call. + cricket::FakeCall call(webrtc::Call::Config(nullptr)); + std::vector ssrcs; + ssrcs.push_back(223); + ssrcs.push_back(224); + + 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()); + for (uint32 ssrc : ssrcs) { + const auto* s = call.GetAudioReceiveStream(ssrc); + EXPECT_NE(nullptr, s); + EXPECT_EQ(0, s->GetConfig().rtp.extensions.size()); + } + + // Set up receive extensions. + const auto& e_exts = engine_.rtp_header_extensions(); + channel_->SetRecvRtpHeaderExtensions(e_exts); + EXPECT_EQ(2, call.GetAudioReceiveStreams().size()); + for (uint32 ssrc : ssrcs) { + 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()); + for (const auto& e_ext : e_exts) { + for (const auto& s_ext : s_exts) { + if (e_ext.id == s_ext.id) { + EXPECT_EQ(e_ext.uri, s_ext.name); + } + } + } + } + + // Disable receive extensions. + std::vector extensions; + channel_->SetRecvRtpHeaderExtensions(extensions); + for (uint32 ssrc : ssrcs) { + 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(nullptr)); + const uint32 kAudioSsrc = 1; + rtc::Buffer kPcmuPacket(kPcmuFrame, sizeof(kPcmuFrame)); + static const unsigned char kRtcp[] = { + 0x80, 0xc9, 0x00, 0x01, 0x00, 0x00, 0x00, 0x02, + 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 + }; + rtc::Buffer kRtcpPacket(kRtcp, sizeof(kRtcp)); + + EXPECT_TRUE(SetupEngine()); + cricket::WebRtcVoiceMediaChannel* media_channel = + static_cast(channel_); + cricket::AudioOptions options; + options.combined_audio_video_bwe.Set(true); + EXPECT_TRUE(media_channel->SetOptions(options)); + 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()); + const cricket::FakeAudioReceiveStream* s = + 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); +} diff --git a/talk/session/media/channel.h b/talk/session/media/channel.h index 14dae3b14..441fe64b6 100644 --- a/talk/session/media/channel.h +++ b/talk/session/media/channel.h @@ -526,6 +526,11 @@ class VideoChannel : public BaseChannel { ~VideoChannel(); bool Init(); + // downcasts a MediaChannel + virtual VideoMediaChannel* media_channel() const { + return static_cast(BaseChannel::media_channel()); + } + bool SetRenderer(uint32 ssrc, VideoRenderer* renderer); bool ApplyViewRequest(const ViewRequest& request); @@ -559,12 +564,6 @@ class VideoChannel : public BaseChannel { // Configuration and setting. bool SetChannelOptions(const VideoOptions& options); - protected: - // downcasts a MediaChannel - virtual VideoMediaChannel* media_channel() const { - return static_cast(BaseChannel::media_channel()); - } - private: typedef std::map ScreencastMap; struct ScreencastDetailsData; diff --git a/talk/session/media/channelmanager.cc b/talk/session/media/channelmanager.cc index 9c30a9748..5013fc48b 100644 --- a/talk/session/media/channelmanager.cc +++ b/talk/session/media/channelmanager.cc @@ -309,7 +309,7 @@ void ChannelManager::Terminate_w() { DestroyVideoChannel_w(video_channels_.back()); } while (!voice_channels_.empty()) { - DestroyVoiceChannel_w(voice_channels_.back()); + DestroyVoiceChannel_w(voice_channels_.back(), nullptr); } while (!soundclips_.empty()) { DestroySoundclip_w(soundclips_.back()); @@ -329,8 +329,8 @@ VoiceChannel* ChannelManager::CreateVoiceChannel( VoiceChannel* ChannelManager::CreateVoiceChannel_w( BaseSession* session, const std::string& content_name, bool rtcp) { - // This is ok to alloc from a thread other than the worker thread ASSERT(initialized_); + ASSERT(worker_thread_ == rtc::Thread::Current()); VoiceMediaChannel* media_channel = media_engine_->CreateChannel(); if (media_channel == NULL) return NULL; @@ -346,22 +346,29 @@ VoiceChannel* ChannelManager::CreateVoiceChannel_w( return voice_channel; } -void ChannelManager::DestroyVoiceChannel(VoiceChannel* voice_channel) { +void ChannelManager::DestroyVoiceChannel(VoiceChannel* voice_channel, + VideoChannel* video_channel) { if (voice_channel) { worker_thread_->Invoke( - Bind(&ChannelManager::DestroyVoiceChannel_w, this, voice_channel)); + Bind(&ChannelManager::DestroyVoiceChannel_w, this, voice_channel, + video_channel)); } } -void ChannelManager::DestroyVoiceChannel_w(VoiceChannel* voice_channel) { +void ChannelManager::DestroyVoiceChannel_w(VoiceChannel* voice_channel, + VideoChannel* video_channel) { // Destroy voice channel. ASSERT(initialized_); + ASSERT(worker_thread_ == rtc::Thread::Current()); VoiceChannels::iterator it = std::find(voice_channels_.begin(), voice_channels_.end(), 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; } @@ -403,8 +410,8 @@ VideoChannel* ChannelManager::CreateVideoChannel_w( bool rtcp, const VideoOptions& options, VoiceChannel* voice_channel) { - // This is ok to alloc from a thread other than the worker thread ASSERT(initialized_); + ASSERT(worker_thread_ == rtc::Thread::Current()); VideoMediaChannel* media_channel = // voice_channel can be NULL in case of NullVoiceEngine. media_engine_->CreateVideoChannel( @@ -433,6 +440,7 @@ void ChannelManager::DestroyVideoChannel(VideoChannel* video_channel) { void ChannelManager::DestroyVideoChannel_w(VideoChannel* video_channel) { // Destroy video channel. ASSERT(initialized_); + ASSERT(worker_thread_ == rtc::Thread::Current()); VideoChannels::iterator it = std::find(video_channels_.begin(), video_channels_.end(), video_channel); ASSERT(it != video_channels_.end()); diff --git a/talk/session/media/channelmanager.h b/talk/session/media/channelmanager.h index a8eb88d5d..27f874b59 100644 --- a/talk/session/media/channelmanager.h +++ b/talk/session/media/channelmanager.h @@ -107,7 +107,8 @@ class ChannelManager : public rtc::MessageHandler, VoiceChannel* CreateVoiceChannel( BaseSession* session, const std::string& content_name, bool rtcp); // Destroys a voice channel created with the Create API. - void DestroyVoiceChannel(VoiceChannel* voice_channel); + 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, @@ -264,7 +265,8 @@ class ChannelManager : public rtc::MessageHandler, void Terminate_w(); VoiceChannel* CreateVoiceChannel_w( BaseSession* session, const std::string& content_name, bool rtcp); - void DestroyVoiceChannel_w(VoiceChannel* voice_channel); + void DestroyVoiceChannel_w(VoiceChannel* voice_channel, + VideoChannel* video_channel); VideoChannel* CreateVideoChannel_w(BaseSession* session, const std::string& content_name, bool rtcp, diff --git a/talk/session/media/channelmanager_unittest.cc b/talk/session/media/channelmanager_unittest.cc index e57305c48..b0abf0487 100644 --- a/talk/session/media/channelmanager_unittest.cc +++ b/talk/session/media/channelmanager_unittest.cc @@ -135,7 +135,7 @@ TEST_F(ChannelManagerTest, CreateDestroyChannels) { false, cricket::DCT_RTP); EXPECT_TRUE(data_channel != NULL); cm_->DestroyVideoChannel(video_channel); - cm_->DestroyVoiceChannel(voice_channel); + cm_->DestroyVoiceChannel(voice_channel, nullptr); cm_->DestroyDataChannel(data_channel); cm_->Terminate(); } @@ -158,7 +158,7 @@ TEST_F(ChannelManagerTest, CreateDestroyChannelsOnThread) { false, cricket::DCT_RTP); EXPECT_TRUE(data_channel != NULL); cm_->DestroyVideoChannel(video_channel); - cm_->DestroyVoiceChannel(voice_channel); + cm_->DestroyVoiceChannel(voice_channel, nullptr); cm_->DestroyDataChannel(data_channel); cm_->Terminate(); }