From d6fc47ea9585fa405d8617ccc0b7704d2c9e8a0a Mon Sep 17 00:00:00 2001 From: pbos Date: Thu, 23 Jul 2015 06:58:33 -0700 Subject: [PATCH] Remove base channel for video receivers. BUG=webrtc:1695 R=stefan@webrtc.org Review URL: https://codereview.webrtc.org/1251163002 Cr-Commit-Position: refs/heads/master@{#9624} --- webrtc/video/call.cc | 13 +- webrtc/video/video_receive_stream.cc | 6 +- webrtc/video/video_receive_stream.h | 1 - webrtc/video/video_send_stream.cc | 3 +- webrtc/video_engine/vie_channel.cc | 22 +--- webrtc/video_engine/vie_channel.h | 6 +- webrtc/video_engine/vie_channel_group.cc | 156 ++++------------------- webrtc/video_engine/vie_channel_group.h | 27 +--- 8 files changed, 36 insertions(+), 198 deletions(-) diff --git a/webrtc/video/call.cc b/webrtc/video/call.cc index adfaa53b45..a2f07aa590 100644 --- a/webrtc/video/call.cc +++ b/webrtc/video/call.cc @@ -115,7 +115,6 @@ class Call : public webrtc::Call, public PacketReceiver { const int num_cpu_cores_; const rtc::scoped_ptr module_process_thread_; const rtc::scoped_ptr channel_group_; - const int base_channel_id_; volatile int next_channel_id_; Call::Config config_; @@ -158,8 +157,7 @@ Call::Call(const Call::Config& config) : num_cpu_cores_(CpuInfo::DetectNumberOfCores()), module_process_thread_(ProcessThread::Create()), channel_group_(new ChannelGroup(module_process_thread_.get())), - base_channel_id_(0), - next_channel_id_(base_channel_id_ + 1), + next_channel_id_(0), config_(config), network_enabled_(true), transport_adapter_(nullptr), @@ -178,12 +176,6 @@ Call::Call(const Call::Config& config) Trace::CreateTrace(); module_process_thread_->Start(); - // TODO(pbos): Remove base channel when CreateReceiveChannel no longer - // requires one. - CHECK(channel_group_->CreateSendChannel(base_channel_id_, 0, - &transport_adapter_, num_cpu_cores_, - std::vector(), true)); - if (config.overuse_callback) { overuse_observer_proxy_.reset( new CpuOveruseObserverProxy(config.overuse_callback)); @@ -199,7 +191,6 @@ Call::~Call() { CHECK_EQ(0u, video_receive_ssrcs_.size()); CHECK_EQ(0u, video_receive_streams_.size()); - channel_group_->DeleteChannel(base_channel_id_); module_process_thread_->Stop(); Trace::ReturnTrace(); } @@ -320,7 +311,7 @@ webrtc::VideoReceiveStream* Call::CreateVideoReceiveStream( TRACE_EVENT0("webrtc", "Call::CreateVideoReceiveStream"); LOG(LS_INFO) << "CreateVideoReceiveStream: " << config.ToString(); VideoReceiveStream* receive_stream = new VideoReceiveStream( - num_cpu_cores_, base_channel_id_, channel_group_.get(), + num_cpu_cores_, channel_group_.get(), rtc::AtomicOps::Increment(&next_channel_id_), config, config_.send_transport, config_.voice_engine); diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index d770f58969..57ddee07d1 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -125,7 +125,6 @@ VideoCodec CreateDecoderVideoCodec(const VideoReceiveStream::Decoder& decoder) { } // namespace VideoReceiveStream::VideoReceiveStream(int num_cpu_cores, - int base_channel_id, ChannelGroup* channel_group, int channel_id, const VideoReceiveStream::Config& config, @@ -137,9 +136,8 @@ VideoReceiveStream::VideoReceiveStream(int num_cpu_cores, clock_(Clock::GetRealTimeClock()), channel_group_(channel_group), channel_id_(channel_id) { - CHECK(channel_group_->CreateReceiveChannel(channel_id_, 0, base_channel_id, - &transport_adapter_, num_cpu_cores, - true)); + CHECK(channel_group_->CreateReceiveChannel( + channel_id_, 0, &transport_adapter_, num_cpu_cores)); vie_channel_ = channel_group_->GetChannel(channel_id_); diff --git a/webrtc/video/video_receive_stream.h b/webrtc/video/video_receive_stream.h index 717622d0fa..24b99ff651 100644 --- a/webrtc/video/video_receive_stream.h +++ b/webrtc/video/video_receive_stream.h @@ -38,7 +38,6 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, public VideoRenderCallback { public: VideoReceiveStream(int num_cpu_cores, - int base_channel_id, ChannelGroup* channel_group, int channel_id, const VideoReceiveStream::Config& config, diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index 7cd1f9eab0..f51e0e251d 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -120,8 +120,7 @@ VideoSendStream::VideoSendStream( stats_proxy_(Clock::GetRealTimeClock(), config) { DCHECK(!config_.rtp.ssrcs.empty()); CHECK(channel_group->CreateSendChannel(channel_id_, 0, &transport_adapter_, - num_cpu_cores, config_.rtp.ssrcs, - true)); + num_cpu_cores, config_.rtp.ssrcs)); vie_channel_ = channel_group_->GetChannel(channel_id_); vie_encoder_ = channel_group_->GetEncoder(channel_id_); diff --git a/webrtc/video_engine/vie_channel.cc b/webrtc/video_engine/vie_channel.cc index d0465b209d..8f4d2c6d59 100644 --- a/webrtc/video_engine/vie_channel.cc +++ b/webrtc/video_engine/vie_channel.cc @@ -91,8 +91,7 @@ ViEChannel::ViEChannel(int32_t channel_id, PacedSender* paced_sender, PacketRouter* packet_router, size_t max_rtp_streams, - bool sender, - bool disable_default_encoder) + bool sender) : channel_id_(channel_id), engine_id_(engine_id), number_of_cores_(number_of_cores), @@ -116,7 +115,6 @@ ViEChannel::ViEChannel(int32_t channel_id, packet_router_(packet_router), bandwidth_observer_(bandwidth_observer), decoder_reset_(true), - disable_default_encoder_(disable_default_encoder), nack_history_size_sender_(kSendSidePacketHistorySize), max_nack_reordering_threshold_(kMaxPacketAgeToNack), pre_render_callback_(NULL), @@ -172,24 +170,6 @@ int32_t ViEChannel::Init() { module_process_thread_->RegisterModule(vcm_); module_process_thread_->RegisterModule(&vie_sync_); -#ifdef VIDEOCODEC_VP8 - if (!disable_default_encoder_) { - VideoCodec video_codec; - if (vcm_->Codec(kVideoCodecVP8, &video_codec) == VCM_OK) { - rtp_rtcp_modules_[0]->RegisterSendPayload(video_codec); - // TODO(holmer): Can we call SetReceiveCodec() here instead? - if (!vie_receiver_.RegisterPayload(video_codec)) { - return -1; - } - vcm_->RegisterReceiveCodec(&video_codec, number_of_cores_); - vcm_->RegisterSendCodec(&video_codec, number_of_cores_, - rtp_rtcp_modules_[0]->MaxDataPayloadLength()); - } else { - RTC_NOTREACHED(); - } - } -#endif - return 0; } diff --git a/webrtc/video_engine/vie_channel.h b/webrtc/video_engine/vie_channel.h index 188fe3c3a4..f18ada5d44 100644 --- a/webrtc/video_engine/vie_channel.h +++ b/webrtc/video_engine/vie_channel.h @@ -109,8 +109,7 @@ class ViEChannel : public VCMFrameTypeCallback, PacedSender* paced_sender, PacketRouter* packet_router, size_t max_rtp_streams, - bool sender, - bool disable_default_encoder); + bool sender); ~ViEChannel(); int32_t Init(); @@ -478,9 +477,6 @@ class ViEChannel : public VCMFrameTypeCallback, VideoCodec receive_codec_ GUARDED_BY(crit_); rtc::scoped_ptr decode_thread_; - // Used to skip default encoder in the new API. - const bool disable_default_encoder_; - int nack_history_size_sender_; int max_nack_reordering_threshold_; I420FrameCallback* pre_render_callback_ GUARDED_BY(crit_); diff --git a/webrtc/video_engine/vie_channel_group.cc b/webrtc/video_engine/vie_channel_group.cc index cb041bda2e..4a088c836a 100644 --- a/webrtc/video_engine/vie_channel_group.cc +++ b/webrtc/video_engine/vie_channel_group.cc @@ -153,7 +153,6 @@ ChannelGroup::ChannelGroup(ProcessThread* process_thread) PacedSender::kDefaultPaceMultiplier * BitrateController::kDefaultStartBitrateKbps, 0)), - encoder_map_cs_(CriticalSectionWrapper::CreateCriticalSection()), config_(new Config), process_thread_(process_thread), pacer_thread_(ProcessThread::Create()), @@ -182,19 +181,16 @@ ChannelGroup::~ChannelGroup() { process_thread_->DeRegisterModule(call_stats_.get()); process_thread_->DeRegisterModule(remote_bitrate_estimator_.get()); call_stats_->DeregisterStatsObserver(remote_bitrate_estimator_.get()); - DCHECK(channels_.empty()); DCHECK(channel_map_.empty()); DCHECK(!remb_->InUse()); DCHECK(vie_encoder_map_.empty()); - DCHECK(send_encoders_.empty()); } bool ChannelGroup::CreateSendChannel(int channel_id, int engine_id, Transport* transport, int number_of_cores, - const std::vector& ssrcs, - bool disable_default_encoder) { + const std::vector& ssrcs) { // TODO(pbos): Remove checks for empty ssrcs and add this check when there's // no base channel. // DCHECK(!ssrcs.empty()); @@ -208,7 +204,7 @@ bool ChannelGroup::CreateSendChannel(int channel_id, ViEEncoder* encoder = vie_encoder.get(); if (!CreateChannel(channel_id, engine_id, transport, number_of_cores, vie_encoder.release(), ssrcs.empty() ? 1 : ssrcs.size(), - true, disable_default_encoder)) { + true)) { return false; } ViEChannel* channel = channel_map_[channel_id]; @@ -226,13 +222,10 @@ bool ChannelGroup::CreateSendChannel(int channel_id, bool ChannelGroup::CreateReceiveChannel(int channel_id, int engine_id, - int base_channel_id, Transport* transport, - int number_of_cores, - bool disable_default_encoder) { - ViEEncoder* encoder = GetEncoder(base_channel_id); + int number_of_cores) { return CreateChannel(channel_id, engine_id, transport, number_of_cores, - encoder, 1, false, disable_default_encoder); + nullptr, 1, false); } bool ChannelGroup::CreateChannel(int channel_id, @@ -241,40 +234,25 @@ bool ChannelGroup::CreateChannel(int channel_id, int number_of_cores, ViEEncoder* vie_encoder, size_t max_rtp_streams, - bool sender, - bool disable_default_encoder) { - DCHECK(vie_encoder); - + bool sender) { rtc::scoped_ptr channel(new ViEChannel( channel_id, engine_id, number_of_cores, *config_.get(), transport, process_thread_, encoder_state_feedback_->GetRtcpIntraFrameObserver(), bitrate_controller_->CreateRtcpBandwidthObserver(), remote_bitrate_estimator_.get(), call_stats_->rtcp_rtt_stats(), - pacer_.get(), packet_router_.get(), max_rtp_streams, sender, - disable_default_encoder)); + pacer_.get(), packet_router_.get(), max_rtp_streams, sender)); if (channel->Init() != 0) { return false; } - if (!disable_default_encoder) { - VideoCodec encoder; - if (vie_encoder->GetEncoder(&encoder) != 0) { - return false; - } - if (sender && channel->SetSendCodec(encoder) != 0) { - return false; - } - } // Register the channel to receive stats updates. call_stats_->RegisterStatsObserver(channel->GetStatsObserver()); // Store the channel, add it to the channel group and save the vie_encoder. channel_map_[channel_id] = channel.release(); - { - CriticalSectionScoped lock(encoder_map_cs_.get()); + if (vie_encoder) { + rtc::CritScope lock(&encoder_map_crit_); vie_encoder_map_[channel_id] = vie_encoder; - if (sender) - send_encoders_[channel_id] = vie_encoder; } return true; @@ -284,63 +262,35 @@ void ChannelGroup::DeleteChannel(int channel_id) { ViEChannel* vie_channel = PopChannel(channel_id); ViEEncoder* vie_encoder = GetEncoder(channel_id); - DCHECK(vie_encoder != NULL); call_stats_->DeregisterStatsObserver(vie_channel->GetStatsObserver()); SetChannelRembStatus(false, false, vie_channel); - // If we're owning the encoder, remove the feedback and stop all encoding - // threads and processing. This must be done before deleting the channel. - if (vie_encoder->channel_id() == channel_id) { + // If we're a sender, remove the feedback and stop all encoding threads and + // processing. This must be done before deleting the channel. + if (vie_encoder) { encoder_state_feedback_->RemoveEncoder(vie_encoder); vie_encoder->StopThreadsAndRemoveSharedMembers(); } unsigned int remote_ssrc = 0; vie_channel->GetRemoteSSRC(&remote_ssrc); - RemoveChannel(channel_id); + channel_map_.erase(channel_id); remote_bitrate_estimator_->RemoveStream(remote_ssrc); - // Check if other channels are using the same encoder. - if (OtherChannelsUsingEncoder(channel_id)) { - vie_encoder = NULL; - } else { - // Delete later when we've released the critsect. - } - - // We can't erase the item before we've checked for other channels using - // same ViEEncoder. - PopEncoder(channel_id); - delete vie_channel; - // Leave the write critsect before deleting the objects. - // Deleting a channel can cause other objects, such as renderers, to be - // deleted, which might take time. - // If statment just to show that this object is not always deleted. + if (vie_encoder) { - LOG(LS_VERBOSE) << "ViEEncoder deleted for channel " << channel_id; + { + rtc::CritScope lock(&encoder_map_crit_); + vie_encoder_map_.erase(vie_encoder_map_.find(channel_id)); + } delete vie_encoder; } LOG(LS_VERBOSE) << "Channel deleted " << channel_id; } -void ChannelGroup::AddChannel(int channel_id) { - channels_.insert(channel_id); -} - -void ChannelGroup::RemoveChannel(int channel_id) { - channels_.erase(channel_id); -} - -bool ChannelGroup::HasChannel(int channel_id) const { - return channels_.find(channel_id) != channels_.end(); -} - -bool ChannelGroup::Empty() const { - return channels_.empty(); -} - ViEChannel* ChannelGroup::GetChannel(int channel_id) const { ChannelMap::const_iterator it = channel_map_.find(channel_id); if (it == channel_map_.end()) { @@ -351,11 +301,10 @@ ViEChannel* ChannelGroup::GetChannel(int channel_id) const { } ViEEncoder* ChannelGroup::GetEncoder(int channel_id) const { - CriticalSectionScoped lock(encoder_map_cs_.get()); + rtc::CritScope lock(&encoder_map_crit_); EncoderMap::const_iterator it = vie_encoder_map_.find(channel_id); - if (it == vie_encoder_map_.end()) { - return NULL; - } + if (it == vie_encoder_map_.end()) + return nullptr; return it->second; } @@ -368,68 +317,9 @@ ViEChannel* ChannelGroup::PopChannel(int channel_id) { return channel; } -ViEEncoder* ChannelGroup::PopEncoder(int channel_id) { - CriticalSectionScoped lock(encoder_map_cs_.get()); - auto it = vie_encoder_map_.find(channel_id); - DCHECK(it != vie_encoder_map_.end()); - ViEEncoder* encoder = it->second; - vie_encoder_map_.erase(it); - - it = send_encoders_.find(channel_id); - if (it != send_encoders_.end()) - send_encoders_.erase(it); - - return encoder; -} - -std::vector ChannelGroup::GetChannelIds() const { - std::vector ids; - for (auto channel : channel_map_) - ids.push_back(channel.first); - return ids; -} - -bool ChannelGroup::OtherChannelsUsingEncoder(int channel_id) const { - CriticalSectionScoped lock(encoder_map_cs_.get()); - EncoderMap::const_iterator orig_it = vie_encoder_map_.find(channel_id); - if (orig_it == vie_encoder_map_.end()) { - // No ViEEncoder for this channel. - return false; - } - - // Loop through all other channels to see if anyone points at the same - // ViEEncoder. - for (EncoderMap::const_iterator comp_it = vie_encoder_map_.begin(); - comp_it != vie_encoder_map_.end(); ++comp_it) { - // Make sure we're not comparing the same channel with itself. - if (comp_it->first != channel_id) { - if (comp_it->second == orig_it->second) { - return true; - } - } - } - return false; -} - void ChannelGroup::SetSyncInterface(VoEVideoSync* sync_interface) { - for (auto channel : channel_map_) { + for (auto channel : channel_map_) channel.second->SetVoiceChannel(-1, sync_interface); - } -} - -void ChannelGroup::GetChannelsUsingEncoder(int channel_id, - ChannelList* channels) const { - CriticalSectionScoped lock(encoder_map_cs_.get()); - EncoderMap::const_iterator orig_it = vie_encoder_map_.find(channel_id); - - for (ChannelMap::const_iterator c_it = channel_map_.begin(); - c_it != channel_map_.end(); ++c_it) { - EncoderMap::const_iterator comp_it = vie_encoder_map_.find(c_it->first); - DCHECK(comp_it != vie_encoder_map_.end()); - if (comp_it->second == orig_it->second) { - channels->push_back(c_it->second); - } - } } BitrateController* ChannelGroup::GetBitrateController() const { @@ -477,8 +367,8 @@ void ChannelGroup::OnNetworkChanged(uint32_t target_bitrate_bps, bitrate_allocator_->OnNetworkChanged(target_bitrate_bps, fraction_loss, rtt); int pad_up_to_bitrate_bps = 0; { - CriticalSectionScoped lock(encoder_map_cs_.get()); - for (const auto& encoder : send_encoders_) + rtc::CritScope lock(&encoder_map_crit_); + for (const auto& encoder : vie_encoder_map_) pad_up_to_bitrate_bps += encoder.second->GetPaddingNeededBps(); } pacer_->UpdateBitrate( diff --git a/webrtc/video_engine/vie_channel_group.h b/webrtc/video_engine/vie_channel_group.h index ff001f97d8..499ec7305d 100644 --- a/webrtc/video_engine/vie_channel_group.h +++ b/webrtc/video_engine/vie_channel_group.h @@ -16,6 +16,7 @@ #include #include +#include "webrtc/base/criticalsection.h" #include "webrtc/base/scoped_ptr.h" #include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" @@ -46,25 +47,14 @@ class ChannelGroup : public BitrateObserver { int engine_id, Transport* transport, int number_of_cores, - const std::vector& ssrcs, - bool disable_default_encoder); + const std::vector& ssrcs); bool CreateReceiveChannel(int channel_id, int engine_id, - int base_channel_id, Transport* transport, - int number_of_cores, - bool disable_default_encoder); + int number_of_cores); void DeleteChannel(int channel_id); - void AddChannel(int channel_id); - void RemoveChannel(int channel_id); - bool HasChannel(int channel_id) const; - bool Empty() const; ViEChannel* GetChannel(int channel_id) const; ViEEncoder* GetEncoder(int channel_id) const; - std::vector GetChannelIds() const; - bool OtherChannelsUsingEncoder(int channel_id) const; - void GetChannelsUsingEncoder(int channel_id, ChannelList* channels) const; - void SetSyncInterface(VoEVideoSync* sync_interface); void SetChannelRembStatus(bool sender, bool receiver, ViEChannel* channel); @@ -82,7 +72,6 @@ class ChannelGroup : public BitrateObserver { private: typedef std::map ChannelMap; - typedef std::set ChannelSet; typedef std::map EncoderMap; bool CreateChannel(int channel_id, @@ -91,10 +80,8 @@ class ChannelGroup : public BitrateObserver { int number_of_cores, ViEEncoder* vie_encoder, size_t max_rtp_streams, - bool sender, - bool disable_default_encoder); + bool sender); ViEChannel* PopChannel(int channel_id); - ViEEncoder* PopEncoder(int channel_id); rtc::scoped_ptr remb_; rtc::scoped_ptr bitrate_allocator_; @@ -103,12 +90,10 @@ class ChannelGroup : public BitrateObserver { rtc::scoped_ptr encoder_state_feedback_; rtc::scoped_ptr packet_router_; rtc::scoped_ptr pacer_; - ChannelSet channels_; ChannelMap channel_map_; // Maps Channel id -> ViEEncoder. - EncoderMap vie_encoder_map_; - EncoderMap send_encoders_; - rtc::scoped_ptr encoder_map_cs_; + mutable rtc::CriticalSection encoder_map_crit_; + EncoderMap vie_encoder_map_ GUARDED_BY(encoder_map_crit_); const rtc::scoped_ptr config_;