ChannelGroup cleanup.
Move CallStats to Call, EncoderStateFeedback to VideoSendStream and
remove last ViEChannel dependency from ChannelGroup.
BUG=webrtc:5079
R=pbos@webrtc.org, stefan@webrtc.org
Review URL: https://codereview.webrtc.org/1418613002 .
Cr-Commit-Position: refs/heads/master@{#10355}
diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc
index 4b59258..eff441c 100644
--- a/webrtc/call/call.cc
+++ b/webrtc/call/call.cc
@@ -34,6 +34,7 @@
#include "webrtc/system_wrappers/interface/trace.h"
#include "webrtc/video/video_receive_stream.h"
#include "webrtc/video/video_send_stream.h"
+#include "webrtc/video_engine/call_stats.h"
#include "webrtc/voice_engine/include/voe_codec.h"
namespace webrtc {
@@ -94,6 +95,7 @@
const int num_cpu_cores_;
const rtc::scoped_ptr<ProcessThread> module_process_thread_;
+ const rtc::scoped_ptr<CallStats> call_stats_;
const rtc::scoped_ptr<ChannelGroup> channel_group_;
Call::Config config_;
rtc::ThreadChecker configuration_thread_checker_;
@@ -138,7 +140,9 @@
Call::Call(const Call::Config& config)
: num_cpu_cores_(CpuInfo::DetectNumberOfCores()),
module_process_thread_(ProcessThread::Create("ModuleProcessThread")),
- channel_group_(new ChannelGroup(module_process_thread_.get())),
+ call_stats_(new CallStats()),
+ channel_group_(new ChannelGroup(module_process_thread_.get(),
+ call_stats_.get())),
config_(config),
network_enabled_(true),
receive_crit_(RWLockWrapper::CreateRWLock()),
@@ -162,6 +166,7 @@
Trace::CreateTrace();
module_process_thread_->Start();
+ module_process_thread_->RegisterModule(call_stats_.get());
channel_group_->SetBweBitrates(config_.bitrate_config.min_bitrate_bps,
config_.bitrate_config.start_bitrate_bps,
@@ -177,6 +182,7 @@
RTC_CHECK(video_receive_ssrcs_.empty());
RTC_CHECK(video_receive_streams_.empty());
+ module_process_thread_->DeRegisterModule(call_stats_.get());
module_process_thread_->Stop();
Trace::ReturnTrace();
}
@@ -272,8 +278,8 @@
// TODO(mflodman): Base the start bitrate on a current bandwidth estimate, if
// the call has already started.
VideoSendStream* send_stream = new VideoSendStream(num_cpu_cores_,
- module_process_thread_.get(), channel_group_.get(), config,
- encoder_config, suspended_video_send_ssrcs_);
+ module_process_thread_.get(), call_stats_.get(), channel_group_.get(),
+ config, encoder_config, suspended_video_send_ssrcs_);
// This needs to be taken before send_crit_ as both locks need to be held
// while changing network state.
@@ -333,7 +339,7 @@
RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
VideoReceiveStream* receive_stream = new VideoReceiveStream(
num_cpu_cores_, channel_group_.get(), config, config_.voice_engine,
- module_process_thread_.get());
+ module_process_thread_.get(), call_stats_.get());
// This needs to be taken before receive_crit_ as both locks need to be held
// while changing network state.
diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc
index e892989..380466a 100644
--- a/webrtc/video/video_receive_stream.cc
+++ b/webrtc/video/video_receive_stream.cc
@@ -141,12 +141,14 @@
ChannelGroup* channel_group,
const VideoReceiveStream::Config& config,
webrtc::VoiceEngine* voice_engine,
- ProcessThread* process_thread)
+ ProcessThread* process_thread,
+ CallStats* call_stats)
: transport_adapter_(config.rtcp_send_transport),
encoded_frame_proxy_(config.pre_decode_callback),
config_(config),
clock_(Clock::GetRealTimeClock()),
- channel_group_(channel_group) {
+ channel_group_(channel_group),
+ call_stats_(call_stats) {
LOG(LS_INFO) << "VideoReceiveStream: " << config_.ToString();
bool send_side_bwe = UseSendSideBwe(config_.rtp.extensions);
@@ -155,18 +157,15 @@
channel_group_->GetRemoteBitrateEstimator(send_side_bwe);
vie_channel_.reset(new ViEChannel(
- num_cpu_cores, &transport_adapter_, process_thread,
- channel_group_->GetRtcpIntraFrameObserver(),
+ num_cpu_cores, &transport_adapter_, process_thread, nullptr,
channel_group_->GetBitrateController()->CreateRtcpBandwidthObserver(),
- nullptr, bitrate_estimator,
- channel_group_->GetCallStats()->rtcp_rtt_stats(), channel_group_->pacer(),
- channel_group_->packet_router(), 1, false));
+ nullptr, bitrate_estimator, call_stats_->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());
+ call_stats_->RegisterStatsObserver(vie_channel_->GetStatsObserver());
// TODO(pbos): This is not fine grained enough...
vie_channel_->SetProtectionMode(config_.rtp.nack.rtp_history_ms > 0, false,
@@ -196,10 +195,8 @@
vie_channel_->SetUseRtxPayloadMappingOnRestore(
config_.rtp.use_rtx_payload_mapping_on_restore);
- // TODO(pbos): Remove channel_group_ usage from VideoReceiveStream. This
- // should be configured in call.cc.
channel_group_->SetChannelRembStatus(false, config_.rtp.remb,
- vie_channel_.get());
+ vie_channel_->rtp_rtcp());
for (size_t i = 0; i < config_.rtp.extensions.size(); ++i) {
const std::string& extension = config_.rtp.extensions[i].name;
@@ -289,9 +286,8 @@
for (size_t i = 0; i < config_.decoders.size(); ++i)
vie_channel_->DeRegisterExternalDecoder(config_.decoders[i].payload_type);
- channel_group_->GetCallStats()->DeregisterStatsObserver(
- vie_channel_->GetStatsObserver());
- channel_group_->SetChannelRembStatus(false, false, vie_channel_.get());
+ call_stats_->DeregisterStatsObserver(vie_channel_->GetStatsObserver());
+ channel_group_->SetChannelRembStatus(false, false, vie_channel_->rtp_rtcp());
uint32_t remote_ssrc = vie_channel_->GetRemoteSSRC();
bool send_side_bwe = UseSendSideBwe(config_.rtp.extensions);
diff --git a/webrtc/video/video_receive_stream.h b/webrtc/video/video_receive_stream.h
index fbea604..06adb72 100644
--- a/webrtc/video/video_receive_stream.h
+++ b/webrtc/video/video_receive_stream.h
@@ -30,6 +30,7 @@
namespace webrtc {
+class CallStats;
class VoiceEngine;
namespace internal {
@@ -43,7 +44,8 @@
ChannelGroup* channel_group,
const VideoReceiveStream::Config& config,
webrtc::VoiceEngine* voice_engine,
- ProcessThread* process_thread);
+ ProcessThread* process_thread,
+ CallStats* call_stats);
~VideoReceiveStream() override;
// webrtc::ReceiveStream implementation.
@@ -81,6 +83,7 @@
Clock* const clock_;
ChannelGroup* const channel_group_;
+ CallStats* const call_stats_;
rtc::scoped_ptr<IncomingVideoStream> incoming_video_stream_;
rtc::scoped_ptr<ReceiveStatisticsProxy> stats_proxy_;
diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc
index 8e4ccbc..b9edbba 100644
--- a/webrtc/video/video_send_stream.cc
+++ b/webrtc/video/video_send_stream.cc
@@ -22,6 +22,7 @@
#include "webrtc/system_wrappers/interface/logging.h"
#include "webrtc/video/video_capture_input.h"
#include "webrtc/video_engine/call_stats.h"
+#include "webrtc/video_engine/encoder_state_feedback.h"
#include "webrtc/video_engine/payload_router.h"
#include "webrtc/video_engine/vie_channel.h"
#include "webrtc/video_engine/vie_channel_group.h"
@@ -111,6 +112,7 @@
VideoSendStream::VideoSendStream(
int num_cpu_cores,
ProcessThread* module_process_thread,
+ CallStats* call_stats,
ChannelGroup* channel_group,
const VideoSendStream::Config& config,
const VideoEncoderConfig& encoder_config,
@@ -120,7 +122,9 @@
config_(config),
suspended_ssrcs_(suspended_ssrcs),
module_process_thread_(module_process_thread),
+ call_stats_(call_stats),
channel_group_(channel_group),
+ encoder_feedback_(new EncoderStateFeedback()),
use_config_bitrate_(true),
stats_proxy_(Clock::GetRealTimeClock(), config) {
LOG(LS_INFO) << "VideoSendStream: " << config_.ToString();
@@ -146,16 +150,15 @@
vie_channel_.reset(new ViEChannel(
num_cpu_cores, config.send_transport, module_process_thread_,
- channel_group_->GetRtcpIntraFrameObserver(),
+ encoder_feedback_->GetRtcpIntraFrameObserver(),
channel_group_->GetBitrateController()->CreateRtcpBandwidthObserver(),
transport_feedback_observer,
channel_group_->GetRemoteBitrateEstimator(false),
- channel_group_->GetCallStats()->rtcp_rtt_stats(), channel_group_->pacer(),
+ call_stats_->rtcp_rtt_stats(), channel_group_->pacer(),
channel_group_->packet_router(), ssrcs.size(), true));
RTC_CHECK(vie_channel_->Init() == 0);
- channel_group_->GetCallStats()->RegisterStatsObserver(
- vie_channel_->GetStatsObserver());
+ call_stats_->RegisterStatsObserver(vie_channel_->GetStatsObserver());
vie_encoder_->StartThreadsAndSetSharedMembers(
vie_channel_->send_payload_router(),
@@ -183,8 +186,7 @@
}
}
- // TODO(pbos): Consider configuring REMB in Call.
- channel_group_->SetChannelRembStatus(true, false, vie_channel_.get());
+ channel_group_->SetChannelRembStatus(true, false, vie_channel_->rtp_rtcp());
// Enable NACK, FEC or both.
const bool enable_protection_nack = config_.rtp.nack.rtp_history_ms > 0;
@@ -226,7 +228,8 @@
if (config_.suspend_below_min_bitrate)
vie_encoder_->SuspendBelowMinBitrate();
- channel_group_->AddEncoder(ssrcs, vie_encoder_.get());
+ channel_group_->AddEncoder(vie_encoder_.get());
+ encoder_feedback_->AddEncoder(ssrcs, vie_encoder_.get());
vie_channel_->RegisterSendChannelRtcpStatisticsCallback(&stats_proxy_);
vie_channel_->RegisterSendChannelRtpStatisticsCallback(&stats_proxy_);
@@ -250,13 +253,13 @@
vie_encoder_->DeRegisterExternalEncoder(
config_.encoder_settings.payload_type);
- channel_group_->GetCallStats()->DeregisterStatsObserver(
- vie_channel_->GetStatsObserver());
- channel_group_->SetChannelRembStatus(false, false, vie_channel_.get());
+ call_stats_->DeregisterStatsObserver(vie_channel_->GetStatsObserver());
+ channel_group_->SetChannelRembStatus(false, false, vie_channel_->rtp_rtcp());
// Remove the feedback, stop all encoding threads and processing. This must be
// done before deleting the channel.
channel_group_->RemoveEncoder(vie_encoder_.get());
+ encoder_feedback_->RemoveEncoder(vie_encoder_.get());
vie_encoder_->StopThreadsAndRemoveSharedMembers();
uint32_t remote_ssrc = vie_channel_->GetRemoteSSRC();
diff --git a/webrtc/video/video_send_stream.h b/webrtc/video/video_send_stream.h
index 68473b1..d61e50f 100644
--- a/webrtc/video/video_send_stream.h
+++ b/webrtc/video/video_send_stream.h
@@ -27,7 +27,9 @@
namespace webrtc {
+class CallStats;
class ChannelGroup;
+class EncoderStateFeedback;
class ProcessThread;
class ViEChannel;
class ViEEncoder;
@@ -39,6 +41,7 @@
public:
VideoSendStream(int num_cpu_cores,
ProcessThread* module_process_thread,
+ CallStats* call_stats,
ChannelGroup* channel_group,
const VideoSendStream::Config& config,
const VideoEncoderConfig& encoder_config,
@@ -76,11 +79,13 @@
std::map<uint32_t, RtpState> suspended_ssrcs_;
ProcessThread* const module_process_thread_;
+ CallStats* const call_stats_;
ChannelGroup* const channel_group_;
rtc::scoped_ptr<VideoCaptureInput> input_;
rtc::scoped_ptr<ViEChannel> vie_channel_;
rtc::scoped_ptr<ViEEncoder> vie_encoder_;
+ rtc::scoped_ptr<EncoderStateFeedback> encoder_feedback_;
// Used as a workaround to indicate that we should be using the configured
// start bitrate initially, instead of the one reported by VideoEngine (which
diff --git a/webrtc/video_engine/vie_channel.cc b/webrtc/video_engine/vie_channel.cc
index ba6d524..9a73711 100644
--- a/webrtc/video_engine/vie_channel.cc
+++ b/webrtc/video_engine/vie_channel.cc
@@ -609,10 +609,6 @@
return target_delay_ms * 40 * 30 / 1000;
}
-void ViEChannel::EnableRemb(bool enable) {
- rtp_rtcp_modules_[0]->SetREMBStatus(enable);
-}
-
int ViEChannel::SetSendTimestampOffsetStatus(bool enable, int id) {
// Disable any previous registrations of this extension to avoid errors.
for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) {
diff --git a/webrtc/video_engine/vie_channel.h b/webrtc/video_engine/vie_channel.h
index cd0646b..3e95cb1 100644
--- a/webrtc/video_engine/vie_channel.h
+++ b/webrtc/video_engine/vie_channel.h
@@ -109,7 +109,6 @@
bool IsSendingFecEnabled();
int SetSenderBufferingMode(int target_delay_ms);
int SetReceiverBufferingMode(int target_delay_ms);
- void EnableRemb(bool enable);
int SetSendTimestampOffsetStatus(bool enable, int id);
int SetReceiveTimestampOffsetStatus(bool enable, int id);
int SetSendAbsoluteSendTimeStatus(bool enable, int id);
diff --git a/webrtc/video_engine/vie_channel_group.cc b/webrtc/video_engine/vie_channel_group.cc
index 75865ae..62d6040 100644
--- a/webrtc/video_engine/vie_channel_group.cc
+++ b/webrtc/video_engine/vie_channel_group.cc
@@ -25,9 +25,7 @@
#include "webrtc/system_wrappers/interface/critical_section_wrapper.h"
#include "webrtc/system_wrappers/interface/logging.h"
#include "webrtc/video_engine/call_stats.h"
-#include "webrtc/video_engine/encoder_state_feedback.h"
#include "webrtc/video_engine/payload_router.h"
-#include "webrtc/video_engine/vie_channel.h"
#include "webrtc/video_engine/vie_encoder.h"
#include "webrtc/video_engine/vie_remb.h"
#include "webrtc/voice_engine/include/voe_video_sync.h"
@@ -145,10 +143,10 @@
} // namespace
-ChannelGroup::ChannelGroup(ProcessThread* process_thread)
+ChannelGroup::ChannelGroup(ProcessThread* process_thread,
+ CallStats* call_stats)
: remb_(new VieRemb()),
bitrate_allocator_(new BitrateAllocator()),
- call_stats_(new CallStats()),
packet_router_(new PacketRouter()),
pacer_(new PacedSender(Clock::GetRealTimeClock(),
packet_router_.get(),
@@ -161,8 +159,8 @@
remote_estimator_proxy_(
new RemoteEstimatorProxy(Clock::GetRealTimeClock(),
packet_router_.get())),
- encoder_state_feedback_(new EncoderStateFeedback()),
process_thread_(process_thread),
+ call_stats_(call_stats),
pacer_thread_(ProcessThread::Create("PacerThread")),
// Constructed last as this object calls the provided callback on
// construction.
@@ -177,7 +175,6 @@
process_thread->RegisterModule(remote_estimator_proxy_.get());
process_thread->RegisterModule(remote_bitrate_estimator_.get());
- process_thread->RegisterModule(call_stats_.get());
process_thread->RegisterModule(bitrate_controller_.get());
}
@@ -185,7 +182,6 @@
pacer_thread_->Stop();
pacer_thread_->DeRegisterModule(pacer_.get());
process_thread_->DeRegisterModule(bitrate_controller_.get());
- process_thread_->DeRegisterModule(call_stats_.get());
process_thread_->DeRegisterModule(remote_bitrate_estimator_.get());
process_thread_->DeRegisterModule(remote_estimator_proxy_.get());
call_stats_->DeregisterStatsObserver(remote_bitrate_estimator_.get());
@@ -195,15 +191,12 @@
RTC_DCHECK(encoders_.empty());
}
-void ChannelGroup::AddEncoder(const std::vector<uint32_t>& ssrcs,
- ViEEncoder* encoder) {
- encoder_state_feedback_->AddEncoder(ssrcs, encoder);
+void ChannelGroup::AddEncoder(ViEEncoder* encoder) {
rtc::CritScope lock(&encoder_crit_);
encoders_.push_back(encoder);
}
void ChannelGroup::RemoveEncoder(ViEEncoder* encoder) {
- encoder_state_feedback_->RemoveEncoder(encoder);
rtc::CritScope lock(&encoder_crit_);
for (auto it = encoders_.begin(); it != encoders_.end(); ++it) {
if (*it == encoder) {
@@ -240,10 +233,6 @@
return remote_bitrate_estimator_.get();
}
-CallStats* ChannelGroup::GetCallStats() const {
- return call_stats_.get();
-}
-
TransportFeedbackObserver* ChannelGroup::GetTransportFeedbackObserver() {
if (transport_feedback_adapter_.get() == nullptr) {
transport_feedback_adapter_.reset(new TransportFeedbackAdapter(
@@ -259,21 +248,15 @@
return transport_feedback_adapter_.get();
}
-RtcpIntraFrameObserver* ChannelGroup::GetRtcpIntraFrameObserver() const {
- return encoder_state_feedback_->GetRtcpIntraFrameObserver();
-}
-
int64_t ChannelGroup::GetPacerQueuingDelayMs() const {
return pacer_->QueueInMs();
}
+// TODO(mflodman): Move out of this class.
void ChannelGroup::SetChannelRembStatus(bool sender,
bool receiver,
- ViEChannel* channel) {
- // Update the channel state.
- channel->EnableRemb(sender || receiver);
- // Update the REMB instance with necessary RTP modules.
- RtpRtcp* rtp_module = channel->rtp_rtcp();
+ RtpRtcp* rtp_module) {
+ rtp_module->SetREMBStatus(sender || receiver);
if (sender) {
remb_->AddRembSender(rtp_module);
} else {
diff --git a/webrtc/video_engine/vie_channel_group.h b/webrtc/video_engine/vie_channel_group.h
index 528a3a6..d26ee10 100644
--- a/webrtc/video_engine/vie_channel_group.h
+++ b/webrtc/video_engine/vie_channel_group.h
@@ -28,16 +28,14 @@
class BitrateAllocator;
class CallStats;
class Config;
-class EncoderStateFeedback;
-class I420FrameCallback;
class PacedSender;
class PacketRouter;
class ProcessThread;
class RemoteBitrateEstimator;
class RemoteEstimatorProxy;
+class RtpRtcp;
class SendStatisticsProxy;
class TransportFeedbackAdapter;
-class ViEChannel;
class ViEEncoder;
class VieRemb;
@@ -45,21 +43,20 @@
// group are assumed to send/receive data to the same end-point.
class ChannelGroup : public BitrateObserver {
public:
- explicit ChannelGroup(ProcessThread* process_thread);
+ ChannelGroup(ProcessThread* process_thread, CallStats* call_stats);
~ChannelGroup();
- void AddEncoder(const std::vector<uint32_t>& ssrcs, ViEEncoder* encoder);
+ void AddEncoder(ViEEncoder* encoder);
void RemoveEncoder(ViEEncoder* encoder);
void SetBweBitrates(int min_bitrate_bps,
int start_bitrate_bps,
int max_bitrate_bps);
- void SetChannelRembStatus(bool sender, bool receiver, ViEChannel* channel);
+ void SetChannelRembStatus(bool sender, bool receiver, RtpRtcp* rtp_module);
void SignalNetworkState(NetworkState state);
BitrateController* GetBitrateController() const;
RemoteBitrateEstimator* GetRemoteBitrateEstimator(bool send_side_bwe) const;
- CallStats* GetCallStats() const;
int64_t GetPacerQueuingDelayMs() const;
PacedSender* pacer() const { return pacer_.get(); }
PacketRouter* packet_router() const { return packet_router_.get(); }
@@ -78,18 +75,18 @@
private:
rtc::scoped_ptr<VieRemb> remb_;
rtc::scoped_ptr<BitrateAllocator> bitrate_allocator_;
- rtc::scoped_ptr<CallStats> call_stats_;
rtc::scoped_ptr<PacketRouter> packet_router_;
rtc::scoped_ptr<PacedSender> pacer_;
rtc::scoped_ptr<RemoteBitrateEstimator> remote_bitrate_estimator_;
rtc::scoped_ptr<RemoteEstimatorProxy> remote_estimator_proxy_;
- rtc::scoped_ptr<EncoderStateFeedback> encoder_state_feedback_;
mutable rtc::CriticalSection encoder_crit_;
std::vector<ViEEncoder*> encoders_ GUARDED_BY(encoder_crit_);
// Registered at construct time and assumed to outlive this class.
ProcessThread* const process_thread_;
+ CallStats* const call_stats_;
+
rtc::scoped_ptr<ProcessThread> pacer_thread_;
rtc::scoped_ptr<BitrateController> bitrate_controller_;