From a20de2030f7f3a3c5e252ccc76a467109f5a93dc Mon Sep 17 00:00:00 2001 From: mflodman Date: Sun, 18 Oct 2015 22:08:19 -0700 Subject: [PATCH] Move ownership of receive ViEChannel to VideoReceiveStream. This CL changes as little as possible and I'll follow up later with ownership of the other members in ChannelGroup. The next step is to remove the id used for channels. BUG=webrtc:5079 Review URL: https://codereview.webrtc.org/1411723002 Cr-Commit-Position: refs/heads/master@{#10318} --- webrtc/call/call.cc | 8 +- webrtc/video/video_receive_stream.cc | 50 ++++++++++-- webrtc/video/video_receive_stream.h | 7 +- webrtc/video/video_send_stream.cc | 4 +- webrtc/video_engine/vie_channel_group.cc | 96 ++---------------------- webrtc/video_engine/vie_channel_group.h | 22 +----- 6 files changed, 59 insertions(+), 128 deletions(-) diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc index 6d9bf40ae8..ce5bceb359 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc @@ -219,7 +219,7 @@ webrtc::AudioReceiveStream* Call::CreateAudioReceiveStream( const webrtc::AudioReceiveStream::Config& config) { TRACE_EVENT0("webrtc", "Call::CreateAudioReceiveStream"); AudioReceiveStream* receive_stream = new AudioReceiveStream( - channel_group_->GetRemoteBitrateEstimator(), config); + channel_group_->GetRemoteBitrateEstimator(false), config); { WriteLockScoped write_lock(*receive_crit_); RTC_DCHECK(audio_receive_ssrcs_.find(config.rtp.remote_ssrc) == @@ -321,7 +321,7 @@ webrtc::VideoReceiveStream* Call::CreateVideoReceiveStream( VideoReceiveStream* receive_stream = new VideoReceiveStream( num_cpu_cores_, channel_group_.get(), rtc::AtomicOps::Increment(&next_channel_id_), config, - config_.voice_engine); + config_.voice_engine, module_process_thread_.get()); // This needs to be taken before receive_crit_ as both locks need to be held // while changing network state. @@ -382,8 +382,8 @@ Call::Stats Call::GetStats() const { channel_group_->GetBitrateController()->AvailableBandwidth(&send_bandwidth); std::vector ssrcs; uint32_t recv_bandwidth = 0; - channel_group_->GetRemoteBitrateEstimator()->LatestEstimate(&ssrcs, - &recv_bandwidth); + channel_group_->GetRemoteBitrateEstimator(false)->LatestEstimate( + &ssrcs, &recv_bandwidth); stats.send_bandwidth_bps = send_bandwidth; stats.recv_bandwidth_bps = recv_bandwidth; stats.pacer_delay_ms = channel_group_->GetPacerQueuingDelayMs(); diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index 141e918b7f..036f623663 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -20,9 +20,19 @@ #include "webrtc/system_wrappers/interface/logging.h" #include "webrtc/video/receive_statistics_proxy.h" #include "webrtc/video_encoder.h" +#include "webrtc/video_engine/call_stats.h" #include "webrtc/video_receive_stream.h" namespace webrtc { + +static bool UseSendSideBwe(const std::vector& extensions) { + for (const auto& extension : extensions) { + if (extension.name == RtpExtension::kTransportSequenceNumber) + return true; + } + return false; +} + std::string VideoReceiveStream::Decoder::ToString() const { std::stringstream ss; ss << "{decoder: " << (decoder != nullptr ? "(VideoDecoder)" : "nullptr"); @@ -132,18 +142,34 @@ VideoReceiveStream::VideoReceiveStream(int num_cpu_cores, ChannelGroup* channel_group, int channel_id, const VideoReceiveStream::Config& config, - webrtc::VoiceEngine* voice_engine) + webrtc::VoiceEngine* voice_engine, + ProcessThread* process_thread) : transport_adapter_(config.rtcp_send_transport), encoded_frame_proxy_(config.pre_decode_callback), config_(config), clock_(Clock::GetRealTimeClock()), - channel_group_(channel_group), - channel_id_(channel_id) { + channel_group_(channel_group) { LOG(LS_INFO) << "VideoReceiveStream: " << config_.ToString(); - RTC_CHECK(channel_group_->CreateReceiveChannel( - channel_id_, &transport_adapter_, num_cpu_cores, config)); - vie_channel_ = channel_group_->GetChannel(channel_id_); + bool send_side_bwe = UseSendSideBwe(config_.rtp.extensions); + + RemoteBitrateEstimator* bitrate_estimator = + channel_group_->GetRemoteBitrateEstimator(send_side_bwe); + + vie_channel_.reset(new ViEChannel( + num_cpu_cores, &transport_adapter_, process_thread, + channel_group_->GetRtcpIntraFrameObserver(), + channel_group_->GetBitrateController()->CreateRtcpBandwidthObserver(), + nullptr, bitrate_estimator, + channel_group_->GetCallStats()->rtcp_rtt_stats(), channel_group_->pacer(), + channel_group_->packet_router(), 1, false)); + + RTC_CHECK(vie_channel_->Init() == 0); + + // Register the channel to receive stats updates. + channel_group_->GetCallStats()->RegisterStatsObserver( + vie_channel_->GetStatsObserver()); + // TODO(pbos): This is not fine grained enough... vie_channel_->SetProtectionMode(config_.rtp.nack.rtp_history_ms > 0, false, @@ -175,7 +201,8 @@ VideoReceiveStream::VideoReceiveStream(int num_cpu_cores, // TODO(pbos): Remove channel_group_ usage from VideoReceiveStream. This // should be configured in call.cc. - channel_group_->SetChannelRembStatus(false, config_.rtp.remb, vie_channel_); + channel_group_->SetChannelRembStatus(false, config_.rtp.remb, + vie_channel_.get()); for (size_t i = 0; i < config_.rtp.extensions.size(); ++i) { const std::string& extension = config_.rtp.extensions[i].name; @@ -266,7 +293,14 @@ VideoReceiveStream::~VideoReceiveStream() { for (size_t i = 0; i < config_.decoders.size(); ++i) vie_channel_->DeRegisterExternalDecoder(config_.decoders[i].payload_type); - channel_group_->DeleteChannel(channel_id_); + channel_group_->GetCallStats()->DeregisterStatsObserver( + vie_channel_->GetStatsObserver()); + channel_group_->SetChannelRembStatus(false, false, vie_channel_.get()); + + uint32_t remote_ssrc = vie_channel_->GetRemoteSSRC(); + bool send_side_bwe = UseSendSideBwe(config_.rtp.extensions); + channel_group_->GetRemoteBitrateEstimator(send_side_bwe)->RemoveStream( + remote_ssrc); } void VideoReceiveStream::Start() { diff --git a/webrtc/video/video_receive_stream.h b/webrtc/video/video_receive_stream.h index a16eab4529..6d00942df4 100644 --- a/webrtc/video/video_receive_stream.h +++ b/webrtc/video/video_receive_stream.h @@ -41,7 +41,8 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, ChannelGroup* channel_group, int channel_id, const VideoReceiveStream::Config& config, - webrtc::VoiceEngine* voice_engine); + webrtc::VoiceEngine* voice_engine, + ProcessThread* process_thread); ~VideoReceiveStream() override; // webrtc::ReceiveStream implementation. @@ -74,12 +75,10 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, Clock* const clock_; ChannelGroup* const channel_group_; - const int channel_id_; - ViEChannel* vie_channel_; rtc::scoped_ptr incoming_video_stream_; - rtc::scoped_ptr stats_proxy_; + rtc::scoped_ptr vie_channel_; }; } // namespace internal } // namespace webrtc diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index 33605162b4..b53fd9f914 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -150,7 +150,7 @@ VideoSendStream::VideoSendStream( channel_group_->GetRtcpIntraFrameObserver(), channel_group_->GetBitrateController()->CreateRtcpBandwidthObserver(), transport_feedback_observer, - channel_group_->GetRemoteBitrateEstimator(), + channel_group_->GetRemoteBitrateEstimator(false), channel_group_->GetCallStats()->rtcp_rtt_stats(), channel_group_->pacer(), channel_group_->packet_router(), ssrcs.size(), true)); RTC_CHECK(vie_channel_->Init() == 0); @@ -258,7 +258,7 @@ VideoSendStream::~VideoSendStream() { vie_encoder_->StopThreadsAndRemoveSharedMembers(); uint32_t remote_ssrc = vie_channel_->GetRemoteSSRC(); - channel_group_->GetRemoteBitrateEstimator()->RemoveStream(remote_ssrc); + channel_group_->GetRemoteBitrateEstimator(false)->RemoveStream(remote_ssrc); } VideoCaptureInput* VideoSendStream::Input() { diff --git a/webrtc/video_engine/vie_channel_group.cc b/webrtc/video_engine/vie_channel_group.cc index 51ef83cbcb..75865aec80 100644 --- a/webrtc/video_engine/vie_channel_group.cc +++ b/webrtc/video_engine/vie_channel_group.cc @@ -191,83 +191,10 @@ ChannelGroup::~ChannelGroup() { call_stats_->DeregisterStatsObserver(remote_bitrate_estimator_.get()); if (transport_feedback_adapter_.get()) call_stats_->DeregisterStatsObserver(transport_feedback_adapter_.get()); - RTC_DCHECK(channel_map_.empty()); RTC_DCHECK(!remb_->InUse()); RTC_DCHECK(encoders_.empty()); } -bool ChannelGroup::CreateReceiveChannel( - int channel_id, - Transport* transport, - int number_of_cores, - const VideoReceiveStream::Config& config) { - bool send_side_bwe = false; - for (const RtpExtension& extension : config.rtp.extensions) { - if (extension.name == RtpExtension::kTransportSequenceNumber) { - send_side_bwe = true; - break; - } - } - - RemoteBitrateEstimator* bitrate_estimator; - if (send_side_bwe) { - bitrate_estimator = remote_estimator_proxy_.get(); - } else { - bitrate_estimator = remote_bitrate_estimator_.get(); - } - return CreateChannel(channel_id, transport, number_of_cores, 1, false, - bitrate_estimator, nullptr); -} - -bool ChannelGroup::CreateChannel(int channel_id, - Transport* transport, - int number_of_cores, - size_t max_rtp_streams, - bool sender, - RemoteBitrateEstimator* bitrate_estimator, - TransportFeedbackObserver* feedback_observer) { - rtc::scoped_ptr channel(new ViEChannel( - number_of_cores, transport, process_thread_, - encoder_state_feedback_->GetRtcpIntraFrameObserver(), - bitrate_controller_->CreateRtcpBandwidthObserver(), feedback_observer, - bitrate_estimator, call_stats_->rtcp_rtt_stats(), pacer_.get(), - packet_router_.get(), max_rtp_streams, sender)); - if (channel->Init() != 0) { - return false; - } - - // Register the channel to receive stats updates. - call_stats_->RegisterStatsObserver(channel->GetStatsObserver()); - - // Store the channel and add it to the channel group. - channel_map_[channel_id] = channel.release(); - return true; -} - -void ChannelGroup::DeleteChannel(int channel_id) { - ViEChannel* vie_channel = PopChannel(channel_id); - - call_stats_->DeregisterStatsObserver(vie_channel->GetStatsObserver()); - SetChannelRembStatus(false, false, vie_channel); - - unsigned int remote_ssrc = vie_channel->GetRemoteSSRC(); - channel_map_.erase(channel_id); - remote_bitrate_estimator_->RemoveStream(remote_ssrc); - - delete vie_channel; - - LOG(LS_VERBOSE) << "Channel deleted " << channel_id; -} - -ViEChannel* ChannelGroup::GetChannel(int channel_id) const { - ChannelMap::const_iterator it = channel_map_.find(channel_id); - if (it == channel_map_.end()) { - LOG(LS_ERROR) << "Channel doesn't exist " << channel_id; - return NULL; - } - return it->second; -} - void ChannelGroup::AddEncoder(const std::vector& ssrcs, ViEEncoder* encoder) { encoder_state_feedback_->AddEncoder(ssrcs, encoder); @@ -286,20 +213,6 @@ void ChannelGroup::RemoveEncoder(ViEEncoder* encoder) { } } -ViEChannel* ChannelGroup::PopChannel(int channel_id) { - ChannelMap::iterator c_it = channel_map_.find(channel_id); - RTC_DCHECK(c_it != channel_map_.end()); - ViEChannel* channel = c_it->second; - channel_map_.erase(c_it); - - return channel; -} - -void ChannelGroup::SetSyncInterface(VoEVideoSync* sync_interface) { - for (auto channel : channel_map_) - channel.second->SetVoiceChannel(-1, sync_interface); -} - void ChannelGroup::SetBweBitrates(int min_bitrate_bps, int start_bitrate_bps, int max_bitrate_bps) { @@ -318,8 +231,13 @@ BitrateController* ChannelGroup::GetBitrateController() const { return bitrate_controller_.get(); } -RemoteBitrateEstimator* ChannelGroup::GetRemoteBitrateEstimator() const { - return remote_bitrate_estimator_.get(); +RemoteBitrateEstimator* ChannelGroup::GetRemoteBitrateEstimator( + bool send_side_bwe) const { + + if (send_side_bwe) + return remote_estimator_proxy_.get(); + else + return remote_bitrate_estimator_.get(); } CallStats* ChannelGroup::GetCallStats() const { diff --git a/webrtc/video_engine/vie_channel_group.h b/webrtc/video_engine/vie_channel_group.h index 90c6bdd535..528a3a6bd9 100644 --- a/webrtc/video_engine/vie_channel_group.h +++ b/webrtc/video_engine/vie_channel_group.h @@ -40,7 +40,6 @@ class TransportFeedbackAdapter; class ViEChannel; class ViEEncoder; class VieRemb; -class VoEVideoSync; // Channel group contains data common for several channels. All channels in the // group are assumed to send/receive data to the same end-point. @@ -48,15 +47,8 @@ class ChannelGroup : public BitrateObserver { public: explicit ChannelGroup(ProcessThread* process_thread); ~ChannelGroup(); - bool CreateReceiveChannel(int channel_id, - Transport* transport, - int number_of_cores, - const VideoReceiveStream::Config& config); - void DeleteChannel(int channel_id); - ViEChannel* GetChannel(int channel_id) const; void AddEncoder(const std::vector& ssrcs, ViEEncoder* encoder); void RemoveEncoder(ViEEncoder* encoder); - void SetSyncInterface(VoEVideoSync* sync_interface); void SetBweBitrates(int min_bitrate_bps, int start_bitrate_bps, int max_bitrate_bps); @@ -66,7 +58,7 @@ class ChannelGroup : public BitrateObserver { void SignalNetworkState(NetworkState state); BitrateController* GetBitrateController() const; - RemoteBitrateEstimator* GetRemoteBitrateEstimator() const; + RemoteBitrateEstimator* GetRemoteBitrateEstimator(bool send_side_bwe) const; CallStats* GetCallStats() const; int64_t GetPacerQueuingDelayMs() const; PacedSender* pacer() const { return pacer_.get(); } @@ -84,17 +76,6 @@ class ChannelGroup : public BitrateObserver { void OnSentPacket(const rtc::SentPacket& sent_packet); private: - typedef std::map ChannelMap; - - bool CreateChannel(int channel_id, - Transport* transport, - int number_of_cores, - size_t max_rtp_streams, - bool sender, - RemoteBitrateEstimator* bitrate_estimator, - TransportFeedbackObserver* feedback_observer); - ViEChannel* PopChannel(int channel_id); - rtc::scoped_ptr remb_; rtc::scoped_ptr bitrate_allocator_; rtc::scoped_ptr call_stats_; @@ -103,7 +84,6 @@ class ChannelGroup : public BitrateObserver { rtc::scoped_ptr remote_bitrate_estimator_; rtc::scoped_ptr remote_estimator_proxy_; rtc::scoped_ptr encoder_state_feedback_; - ChannelMap channel_map_; mutable rtc::CriticalSection encoder_crit_; std::vector encoders_ GUARDED_BY(encoder_crit_);