Move RemoteBitrateEstimator::RemoveStream calls from receive streams to Call.
We can then drop the CongestionController and RemoteBitrateEstimator
completely from the receive streams.
BUG=webrtc:6847
Review-Url: https://codereview.webrtc.org/2669463006
Cr-Commit-Position: refs/heads/master@{#16459}
diff --git a/webrtc/audio/audio_receive_stream.cc b/webrtc/audio/audio_receive_stream.cc
index 8228a60..05d6edf 100644
--- a/webrtc/audio/audio_receive_stream.cc
+++ b/webrtc/audio/audio_receive_stream.cc
@@ -63,18 +63,15 @@
namespace internal {
AudioReceiveStream::AudioReceiveStream(
PacketRouter* packet_router,
- RemoteBitrateEstimator* remote_bitrate_estimator,
const webrtc::AudioReceiveStream::Config& config,
const rtc::scoped_refptr<webrtc::AudioState>& audio_state,
webrtc::RtcEventLog* event_log)
- : remote_bitrate_estimator_(remote_bitrate_estimator),
- config_(config),
+ : config_(config),
audio_state_(audio_state) {
LOG(LS_INFO) << "AudioReceiveStream: " << config_.ToString();
RTC_DCHECK_NE(config_.voe_channel_id, -1);
RTC_DCHECK(audio_state_.get());
RTC_DCHECK(packet_router);
- RTC_DCHECK(remote_bitrate_estimator);
module_process_thread_checker_.DetachFromThread();
@@ -125,7 +122,6 @@
channel_proxy_->DeRegisterExternalTransport();
channel_proxy_->ResetCongestionControlObjects();
channel_proxy_->SetRtcEventLog(nullptr);
- remote_bitrate_estimator_->RemoveStream(config_.rtp.remote_ssrc);
}
void AudioReceiveStream::Start() {
diff --git a/webrtc/audio/audio_receive_stream.h b/webrtc/audio/audio_receive_stream.h
index 13869c4..0792e24 100644
--- a/webrtc/audio/audio_receive_stream.h
+++ b/webrtc/audio/audio_receive_stream.h
@@ -22,7 +22,6 @@
namespace webrtc {
class PacketRouter;
-class RemoteBitrateEstimator;
class RtcEventLog;
namespace voe {
@@ -37,7 +36,6 @@
public Syncable {
public:
AudioReceiveStream(PacketRouter* packet_router,
- RemoteBitrateEstimator* remote_bitrate_estimator,
const webrtc::AudioReceiveStream::Config& config,
const rtc::scoped_refptr<webrtc::AudioState>& audio_state,
webrtc::RtcEventLog* event_log);
@@ -77,7 +75,6 @@
rtc::ThreadChecker worker_thread_checker_;
rtc::ThreadChecker module_process_thread_checker_;
- RemoteBitrateEstimator* const remote_bitrate_estimator_;
const webrtc::AudioReceiveStream::Config config_;
rtc::scoped_refptr<webrtc::AudioState> audio_state_;
std::unique_ptr<voe::ChannelProxy> channel_proxy_;
diff --git a/webrtc/audio/audio_receive_stream_unittest.cc b/webrtc/audio/audio_receive_stream_unittest.cc
index 70e6ac1..2f5998c 100644
--- a/webrtc/audio/audio_receive_stream_unittest.cc
+++ b/webrtc/audio/audio_receive_stream_unittest.cc
@@ -18,7 +18,6 @@
#include "webrtc/modules/audio_coding/codecs/mock/mock_audio_decoder_factory.h"
#include "webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h"
#include "webrtc/modules/pacing/packet_router.h"
-#include "webrtc/modules/remote_bitrate_estimator/include/mock/mock_remote_bitrate_estimator.h"
#include "webrtc/modules/rtp_rtcp/source/byte_io.h"
#include "webrtc/test/gtest.h"
#include "webrtc/test/mock_voe_channel_proxy.h"
@@ -126,9 +125,6 @@
}
PacketRouter* packet_router() { return &packet_router_; }
- MockRemoteBitrateEstimator* remote_bitrate_estimator() {
- return &remote_bitrate_estimator_;
- }
MockRtcEventLog* event_log() { return &event_log_; }
AudioReceiveStream::Config& config() { return stream_config_; }
rtc::scoped_refptr<AudioState> audio_state() { return audio_state_; }
@@ -136,11 +132,6 @@
MockVoiceEngine& voice_engine() { return voice_engine_; }
MockVoEChannelProxy* channel_proxy() { return channel_proxy_; }
- void SetupMockForBweFeedback(bool send_side_bwe) {
- EXPECT_CALL(remote_bitrate_estimator_,
- RemoveStream(stream_config_.rtp.remote_ssrc));
- }
-
void SetupMockForGetStats() {
using testing::DoAll;
using testing::SetArgPointee;
@@ -163,7 +154,6 @@
private:
PacketRouter packet_router_;
rtc::scoped_refptr<AudioDecoderFactory> decoder_factory_;
- MockRemoteBitrateEstimator remote_bitrate_estimator_;
MockRtcEventLog event_log_;
testing::StrictMock<MockVoiceEngine> voice_engine_;
rtc::scoped_refptr<AudioState> audio_state_;
@@ -243,17 +233,14 @@
ConfigHelper helper;
internal::AudioReceiveStream recv_stream(
helper.packet_router(),
- helper.remote_bitrate_estimator(),
helper.config(), helper.audio_state(), helper.event_log());
}
TEST(AudioReceiveStreamTest, ReceiveRtpPacket) {
ConfigHelper helper;
helper.config().rtp.transport_cc = true;
- helper.SetupMockForBweFeedback(true);
internal::AudioReceiveStream recv_stream(
helper.packet_router(),
- helper.remote_bitrate_estimator(),
helper.config(), helper.audio_state(), helper.event_log());
const int kTransportSequenceNumberValue = 1234;
std::vector<uint8_t> rtp_packet = CreateRtpHeaderWithOneByteExtension(
@@ -271,10 +258,8 @@
TEST(AudioReceiveStreamTest, ReceiveRtcpPacket) {
ConfigHelper helper;
helper.config().rtp.transport_cc = true;
- helper.SetupMockForBweFeedback(true);
internal::AudioReceiveStream recv_stream(
helper.packet_router(),
- helper.remote_bitrate_estimator(),
helper.config(), helper.audio_state(), helper.event_log());
std::vector<uint8_t> rtcp_packet = CreateRtcpSenderReport();
@@ -288,7 +273,6 @@
ConfigHelper helper;
internal::AudioReceiveStream recv_stream(
helper.packet_router(),
- helper.remote_bitrate_estimator(),
helper.config(), helper.audio_state(), helper.event_log());
helper.SetupMockForGetStats();
AudioReceiveStream::Stats stats = recv_stream.GetStats();
@@ -334,7 +318,6 @@
ConfigHelper helper;
internal::AudioReceiveStream recv_stream(
helper.packet_router(),
- helper.remote_bitrate_estimator(),
helper.config(), helper.audio_state(), helper.event_log());
EXPECT_CALL(*helper.channel_proxy(),
SetChannelOutputVolumeScaling(FloatEq(0.765f)));
@@ -345,7 +328,6 @@
ConfigHelper helper;
internal::AudioReceiveStream recv_stream(
helper.packet_router(),
- helper.remote_bitrate_estimator(),
helper.config(), helper.audio_state(), helper.event_log());
EXPECT_CALL(helper.voice_engine(), StartPlayout(_)).WillOnce(Return(-1));
@@ -358,7 +340,6 @@
ConfigHelper helper;
internal::AudioReceiveStream recv_stream(
helper.packet_router(),
- helper.remote_bitrate_estimator(),
helper.config(), helper.audio_state(), helper.event_log());
EXPECT_CALL(helper.voice_engine(), StartPlayout(_)).WillOnce(Return(0));
diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc
index 5e21fe5..e21b076 100644
--- a/webrtc/call/call.cc
+++ b/webrtc/call/call.cc
@@ -60,6 +60,34 @@
const int Call::Config::kDefaultStartBitrateBps = 300000;
+namespace {
+
+// TODO(nisse): This really begs for a shared context struct.
+bool UseSendSideBwe(const std::vector<RtpExtension>& extensions,
+ bool transport_cc) {
+ if (!transport_cc)
+ return false;
+ for (const auto& extension : extensions) {
+ if (extension.uri == RtpExtension::kTransportSequenceNumberUri)
+ return true;
+ }
+ return false;
+}
+
+bool UseSendSideBwe(const VideoReceiveStream::Config& config) {
+ return UseSendSideBwe(config.rtp.extensions, config.rtp.transport_cc);
+}
+
+bool UseSendSideBwe(const AudioReceiveStream::Config& config) {
+ return UseSendSideBwe(config.rtp.extensions, config.rtp.transport_cc);
+}
+
+bool UseSendSideBwe(const FlexfecReceiveStream::Config& config) {
+ return UseSendSideBwe(config.rtp_header_extensions, config.transport_cc);
+}
+
+} // namespace
+
namespace internal {
class Call : public webrtc::Call,
@@ -199,16 +227,17 @@
struct ReceiveRtpConfig {
ReceiveRtpConfig() = default; // Needed by std::map
ReceiveRtpConfig(const std::vector<RtpExtension>& extensions,
- bool transport_cc)
- : extensions(extensions), transport_cc(transport_cc) {}
+ bool use_send_side_bwe)
+ : extensions(extensions), use_send_side_bwe(use_send_side_bwe) {}
// Registered RTP header extensions for each stream. Note that RTP header
// extensions are negotiated per track ("m= line") in the SDP, but we have
// no notion of tracks at the Call level. We therefore store the RTP header
// extensions per SSRC instead, which leads to some storage overhead.
RtpHeaderExtensionMap extensions;
- // Set if the RTCP feedback message needed for send side BWE was negotiated.
- bool transport_cc = false;
+ // Set if both RTP extension the RTCP feedback message needed for
+ // send side BWE are negotiated.
+ bool use_send_side_bwe = false;
};
std::map<uint32_t, ReceiveRtpConfig> receive_rtp_config_
GUARDED_BY(receive_crit_);
@@ -525,8 +554,7 @@
RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
event_log_->LogAudioReceiveStreamConfig(config);
AudioReceiveStream* receive_stream = new AudioReceiveStream(
- &packet_router_,
- congestion_controller_->GetRemoteBitrateEstimator(true), config,
+ &packet_router_, config,
config_.audio_state, event_log_);
{
WriteLockScoped write_lock(*receive_crit_);
@@ -534,7 +562,7 @@
audio_receive_ssrcs_.end());
audio_receive_ssrcs_[config.rtp.remote_ssrc] = receive_stream;
receive_rtp_config_[config.rtp.remote_ssrc] =
- ReceiveRtpConfig(config.rtp.extensions, config.rtp.transport_cc);
+ ReceiveRtpConfig(config.rtp.extensions, UseSendSideBwe(config));
ConfigureSync(config.sync_group);
}
@@ -559,8 +587,10 @@
static_cast<webrtc::internal::AudioReceiveStream*>(receive_stream);
{
WriteLockScoped write_lock(*receive_crit_);
- uint32_t ssrc = audio_receive_stream->config().rtp.remote_ssrc;
-
+ const AudioReceiveStream::Config& config = audio_receive_stream->config();
+ uint32_t ssrc = config.rtp.remote_ssrc;
+ congestion_controller_->GetRemoteBitrateEstimator(UseSendSideBwe(config))
+ ->RemoveStream(ssrc);
size_t num_deleted = audio_receive_ssrcs_.erase(ssrc);
RTC_DCHECK(num_deleted == 1);
const std::string& sync_group = audio_receive_stream->config().sync_group;
@@ -658,13 +688,13 @@
flexfec_receive_ssrcs_media_.end();
}
VideoReceiveStream* receive_stream = new VideoReceiveStream(
- num_cpu_cores_, protected_by_flexfec, congestion_controller_.get(),
+ num_cpu_cores_, protected_by_flexfec,
&packet_router_, std::move(configuration), module_process_thread_.get(),
call_stats_.get(), &remb_);
const webrtc::VideoReceiveStream::Config& config = receive_stream->config();
ReceiveRtpConfig receive_config(config.rtp.extensions,
- config.rtp.transport_cc);
+ UseSendSideBwe(config));
{
WriteLockScoped write_lock(*receive_crit_);
RTC_DCHECK(video_receive_ssrcs_.find(config.rtp.remote_ssrc) ==
@@ -714,6 +744,11 @@
RTC_CHECK(receive_stream_impl != nullptr);
ConfigureSync(receive_stream_impl->config().sync_group);
}
+ const VideoReceiveStream::Config& config = receive_stream_impl->config();
+
+ congestion_controller_->GetRemoteBitrateEstimator(UseSendSideBwe(config))
+ ->RemoveStream(config.rtp.remote_ssrc);
+
UpdateAggregateNetworkState();
delete receive_stream_impl;
}
@@ -745,7 +780,7 @@
RTC_DCHECK(receive_rtp_config_.find(config.remote_ssrc) ==
receive_rtp_config_.end());
receive_rtp_config_[config.remote_ssrc] =
- ReceiveRtpConfig(config.rtp_header_extensions, config.transport_cc);
+ ReceiveRtpConfig(config.rtp_header_extensions, UseSendSideBwe(config));
}
// TODO(brandtr): Store config in RtcEventLog here.
@@ -765,7 +800,9 @@
{
WriteLockScoped write_lock(*receive_crit_);
- uint32_t ssrc = receive_stream_impl->GetConfig().remote_ssrc;
+ const FlexfecReceiveStream::Config& config =
+ receive_stream_impl->GetConfig();
+ uint32_t ssrc = config.remote_ssrc;
receive_rtp_config_.erase(ssrc);
// Remove all SSRCs pointing to the FlexfecReceiveStreamImpl to be
@@ -785,6 +822,9 @@
++media_it;
}
+ congestion_controller_->GetRemoteBitrateEstimator(UseSendSideBwe(config))
+ ->RemoveStream(ssrc);
+
flexfec_receive_streams_.erase(receive_stream_impl);
}
@@ -1234,13 +1274,13 @@
void Call::NotifyBweOfReceivedPacket(const RtpPacketReceived& packet,
MediaType media_type) {
auto it = receive_rtp_config_.find(packet.Ssrc());
- bool transport_cc =
- (it != receive_rtp_config_.end()) && it->second.transport_cc;
+ bool use_send_side_bwe =
+ (it != receive_rtp_config_.end()) && it->second.use_send_side_bwe;
RTPHeader header;
packet.GetHeader(&header);
- if (!transport_cc && header.extension.hasTransportSequenceNumber) {
+ if (!use_send_side_bwe && header.extension.hasTransportSequenceNumber) {
// Inconsistent configuration of send side BWE. Do nothing.
// TODO(nisse): Without this check, we may produce RTCP feedback
// packets even when not negotiated. But it would be cleaner to
@@ -1255,7 +1295,7 @@
// FakeNetworkPipe::Process. We need to treat that as video. Tests
// should be fixed to use the same MediaType as the production code.
if (media_type != MediaType::AUDIO ||
- (transport_cc && header.extension.hasTransportSequenceNumber)) {
+ (use_send_side_bwe && header.extension.hasTransportSequenceNumber)) {
congestion_controller_->OnReceivedPacket(
packet.arrival_time_ms(), packet.payload_size() + packet.padding_size(),
header);
diff --git a/webrtc/video/rtp_stream_receiver.cc b/webrtc/video/rtp_stream_receiver.cc
index 50c50d0..af12b51 100644
--- a/webrtc/video/rtp_stream_receiver.cc
+++ b/webrtc/video/rtp_stream_receiver.cc
@@ -50,7 +50,6 @@
Transport* outgoing_transport,
RtcpRttStats* rtt_stats,
RtcpPacketTypeCounterObserver* rtcp_packet_type_counter_observer,
- RemoteBitrateEstimator* remote_bitrate_estimator,
TransportSequenceNumberAllocator* transport_sequence_number_allocator) {
RtpRtcp::Configuration configuration;
configuration.audio = false;
@@ -81,7 +80,6 @@
static const int kPacketLogIntervalMs = 10000;
RtpStreamReceiver::RtpStreamReceiver(
- RemoteBitrateEstimator* remote_bitrate_estimator,
Transport* transport,
RtcpRttStats* rtt_stats,
PacketRouter* packet_router,
@@ -95,7 +93,6 @@
VCMTiming* timing)
: clock_(Clock::GetRealTimeClock()),
config_(*config),
- remote_bitrate_estimator_(remote_bitrate_estimator),
packet_router_(packet_router),
remb_(remb),
process_thread_(process_thread),
@@ -114,7 +111,6 @@
transport,
rtt_stats,
receive_stats_proxy,
- remote_bitrate_estimator_,
packet_router)),
complete_frame_callback_(complete_frame_callback),
keyframe_request_sender_(keyframe_request_sender),
@@ -309,7 +305,6 @@
bool RtpStreamReceiver::DeliverRtp(const uint8_t* rtp_packet,
size_t rtp_packet_length,
const PacketTime& packet_time) {
- RTC_DCHECK(remote_bitrate_estimator_);
{
rtc::CritScope lock(&receive_cs_);
if (!receiving_) {
diff --git a/webrtc/video/rtp_stream_receiver.h b/webrtc/video/rtp_stream_receiver.h
index 7cd60cd..2e29585 100644
--- a/webrtc/video/rtp_stream_receiver.h
+++ b/webrtc/video/rtp_stream_receiver.h
@@ -41,7 +41,6 @@
class ProcessThread;
class ReceiveStatistics;
class ReceiveStatisticsProxy;
-class RemoteBitrateEstimator;
class RemoteNtpTimeEstimator;
class RtcpRttStats;
class RtpHeaderParser;
@@ -65,7 +64,6 @@
public CallStatsObserver {
public:
RtpStreamReceiver(
- RemoteBitrateEstimator* remote_bitrate_estimator,
Transport* transport,
RtcpRttStats* rtt_stats,
PacketRouter* packet_router,
@@ -161,7 +159,6 @@
Clock* const clock_;
// Ownership of this object lies with VideoReceiveStream, which owns |this|.
const VideoReceiveStream::Config& config_;
- RemoteBitrateEstimator* const remote_bitrate_estimator_;
PacketRouter* const packet_router_;
VieRemb* const remb_;
ProcessThread* const process_thread_;
diff --git a/webrtc/video/rtp_stream_receiver_unittest.cc b/webrtc/video/rtp_stream_receiver_unittest.cc
index 859c1f5..4eb5b6e 100644
--- a/webrtc/video/rtp_stream_receiver_unittest.cc
+++ b/webrtc/video/rtp_stream_receiver_unittest.cc
@@ -104,7 +104,7 @@
void SetUp() {
rtp_stream_receiver_.reset(new RtpStreamReceiver(
- nullptr, &mock_transport_, nullptr, &packet_router_, nullptr, &config_,
+ &mock_transport_, nullptr, &packet_router_, nullptr, &config_,
nullptr, process_thread_.get(), &mock_nack_sender_,
&mock_key_frame_request_sender_, &mock_on_complete_frame_callback_,
&timing_));
diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc
index 9fe84a9..3d9fa07 100644
--- a/webrtc/video/video_receive_stream.cc
+++ b/webrtc/video/video_receive_stream.cc
@@ -21,7 +21,6 @@
#include "webrtc/base/optional.h"
#include "webrtc/common_video/h264/profile_level_id.h"
#include "webrtc/common_video/libyuv/include/webrtc_libyuv.h"
-#include "webrtc/modules/congestion_controller/include/congestion_controller.h"
#include "webrtc/modules/rtp_rtcp/include/rtp_receiver.h"
#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h"
#include "webrtc/modules/utility/include/process_thread.h"
@@ -38,16 +37,6 @@
namespace webrtc {
-static bool UseSendSideBwe(const VideoReceiveStream::Config& config) {
- if (!config.rtp.transport_cc)
- return false;
- for (const auto& extension : config.rtp.extensions) {
- if (extension.uri == RtpExtension::kTransportSequenceNumberUri)
- return true;
- }
- return false;
-}
-
std::string VideoReceiveStream::Decoder::ToString() const {
std::stringstream ss;
ss << "{decoder: " << (decoder ? "(VideoDecoder)" : "nullptr");
@@ -188,7 +177,6 @@
VideoReceiveStream::VideoReceiveStream(
int num_cpu_cores,
bool protected_by_flexfec,
- CongestionController* congestion_controller,
PacketRouter* packet_router,
VideoReceiveStream::Config config,
ProcessThread* process_thread,
@@ -201,14 +189,11 @@
process_thread_(process_thread),
clock_(Clock::GetRealTimeClock()),
decode_thread_(DecodeThreadFunction, this, "DecodingThread"),
- congestion_controller_(congestion_controller),
call_stats_(call_stats),
timing_(new VCMTiming(clock_)),
video_receiver_(clock_, nullptr, this, timing_.get(), this, this),
stats_proxy_(&config_, clock_),
- rtp_stream_receiver_(congestion_controller_->GetRemoteBitrateEstimator(
- UseSendSideBwe(config_)),
- &transport_adapter_,
+ rtp_stream_receiver_(&transport_adapter_,
call_stats_->rtcp_rtt_stats(),
packet_router,
remb,
@@ -223,7 +208,6 @@
LOG(LS_INFO) << "VideoReceiveStream: " << config_.ToString();
RTC_DCHECK(process_thread_);
- RTC_DCHECK(congestion_controller_);
RTC_DCHECK(call_stats_);
module_process_thread_checker_.DetachFromThread();
@@ -256,9 +240,6 @@
process_thread_->DeRegisterModule(&rtp_stream_sync_);
process_thread_->DeRegisterModule(&video_receiver_);
-
- congestion_controller_->GetRemoteBitrateEstimator(UseSendSideBwe(config_))
- ->RemoveStream(rtp_stream_receiver_.GetRemoteSsrc());
}
void VideoReceiveStream::SignalNetworkState(NetworkState state) {
diff --git a/webrtc/video/video_receive_stream.h b/webrtc/video/video_receive_stream.h
index 6aa847c..f2d9755 100644
--- a/webrtc/video/video_receive_stream.h
+++ b/webrtc/video/video_receive_stream.h
@@ -32,7 +32,6 @@
namespace webrtc {
class CallStats;
-class CongestionController;
class IvfFileWriter;
class ProcessThread;
class RTPFragmentationHeader;
@@ -52,7 +51,6 @@
public:
VideoReceiveStream(int num_cpu_cores,
bool protected_by_flexfec,
- CongestionController* congestion_controller,
PacketRouter* packet_router,
VideoReceiveStream::Config config,
ProcessThread* process_thread,
@@ -127,7 +125,6 @@
rtc::PlatformThread decode_thread_;
- CongestionController* const congestion_controller_;
CallStats* const call_stats_;
std::unique_ptr<VCMTiming> timing_; // Jitter buffer experiment.