Enable send-side BWE by default for video in call tests.

Also fixes a bug where RTCP transport feedback was sent even though RTCP was disabled.

May affect perf numbers since the behavior of the send-side BWE differs a lot from the recv-side BWE.

BUG=webrtc:7111

Review-Url: https://codereview.webrtc.org/2669413003
Cr-Commit-Position: refs/heads/master@{#16451}
This commit is contained in:
stefan 2017-02-06 06:29:38 -08:00 committed by Commit bot
parent fd8d2654d7
commit b77c716d8a
4 changed files with 88 additions and 81 deletions

View File

@ -1049,6 +1049,12 @@ bool RTCPSender::SendFeedbackPacket(const rtcp::TransportFeedback& packet) {
// but we can't because of an incorrect warning (C4822) in MVS 2013.
} sender(transport_, event_log_);
{
rtc::CritScope lock(&critical_section_rtcp_sender_);
if (method_ == RtcpMode::kOff)
return false;
}
RTC_DCHECK_LE(max_packet_size_, IP_PACKET_SIZE);
uint8_t buffer[IP_PACKET_SIZE];
return packet.BuildExternalBuffer(buffer, max_packet_size_, &sender) &&

View File

@ -200,7 +200,8 @@ void CallTest::CreateSendConfig(size_t num_video_streams,
video_send_config_.encoder_settings.payload_type =
kFakeVideoSendPayloadType;
video_send_config_.rtp.extensions.push_back(
RtpExtension(RtpExtension::kAbsSendTimeUri, kAbsSendTimeExtensionId));
RtpExtension(RtpExtension::kTransportSequenceNumberUri,
kTransportSequenceNumberExtensionId));
FillEncoderConfiguration(num_video_streams, &video_encoder_config_);
for (size_t i = 0; i < num_video_streams; ++i)
@ -231,7 +232,8 @@ void CallTest::CreateMatchingReceiveConfigs(Transport* rtcp_send_transport) {
if (num_video_streams_ > 0) {
RTC_DCHECK(!video_send_config_.rtp.ssrcs.empty());
VideoReceiveStream::Config video_config(rtcp_send_transport);
video_config.rtp.remb = true;
video_config.rtp.remb = false;
video_config.rtp.transport_cc = true;
video_config.rtp.local_ssrc = kReceiverLocalVideoSsrc;
for (const RtpExtension& extension : video_send_config_.rtp.extensions)
video_config.rtp.extensions.push_back(extension);

View File

@ -1308,6 +1308,7 @@ void EndToEndTest::RespectsRtcpMode(RtcpMode rtcp_mode) {
private:
Action OnSendRtp(const uint8_t* packet, size_t length) override {
rtc::CritScope lock(&crit_);
if (++sent_rtp_ % 3 == 0)
return DROP_PACKET;
@ -1315,6 +1316,7 @@ void EndToEndTest::RespectsRtcpMode(RtcpMode rtcp_mode) {
}
Action OnReceiveRtcp(const uint8_t* packet, size_t length) override {
rtc::CritScope lock(&crit_);
++sent_rtcp_;
test::RtcpPacketParser parser;
EXPECT_TRUE(parser.Parse(packet, length));
@ -1323,7 +1325,10 @@ void EndToEndTest::RespectsRtcpMode(RtcpMode rtcp_mode) {
switch (rtcp_mode_) {
case RtcpMode::kCompound:
if (parser.receiver_report()->num_packets() == 0) {
// TODO(holmer): We shouldn't send transport feedback alone if
// compound RTCP is negotiated.
if (parser.receiver_report()->num_packets() == 0 &&
parser.transport_feedback()->num_packets() == 0) {
ADD_FAILURE() << "Received RTCP packet without receiver report for "
"RtcpMode::kCompound.";
observation_complete_.Set();
@ -1362,8 +1367,11 @@ void EndToEndTest::RespectsRtcpMode(RtcpMode rtcp_mode) {
}
RtcpMode rtcp_mode_;
int sent_rtp_;
int sent_rtcp_;
rtc::CriticalSection crit_;
// Must be protected since RTCP can be sent by both the process thread
// and the pacer thread.
int sent_rtp_ GUARDED_BY(&crit_);
int sent_rtcp_ GUARDED_BY(&crit_);
} test(rtcp_mode);
RunBaseTest(&test);
@ -1803,10 +1811,6 @@ class TransportFeedbackTester : public test::EndToEndTest {
VideoSendStream::Config* send_config,
std::vector<VideoReceiveStream::Config>* receive_configs,
VideoEncoderConfig* encoder_config) override {
send_config->rtp.extensions.clear();
send_config->rtp.extensions.push_back(
RtpExtension(RtpExtension::kTransportSequenceNumberUri, kExtensionId));
(*receive_configs)[0].rtp.extensions = send_config->rtp.extensions;
(*receive_configs)[0].rtp.transport_cc = feedback_enabled_;
}
@ -1934,6 +1938,17 @@ TEST_P(EndToEndTest, ReceiveStreamSendsRemb) {
public:
RembObserver() : EndToEndTest(kDefaultTimeoutMs) {}
void ModifyVideoConfigs(
VideoSendStream::Config* send_config,
std::vector<VideoReceiveStream::Config>* receive_configs,
VideoEncoderConfig* encoder_config) override {
send_config->rtp.extensions.clear();
send_config->rtp.extensions.push_back(RtpExtension(
RtpExtension::kAbsSendTimeUri, test::kAbsSendTimeExtensionId));
(*receive_configs)[0].rtp.remb = true;
(*receive_configs)[0].rtp.transport_cc = false;
}
Action OnReceiveRtcp(const uint8_t* packet, size_t length) override {
test::RtcpPacketParser parser;
EXPECT_TRUE(parser.Parse(packet, length));
@ -1958,46 +1973,66 @@ TEST_P(EndToEndTest, ReceiveStreamSendsRemb) {
RunBaseTest(&test);
}
TEST_P(EndToEndTest, VerifyBandwidthStats) {
class RtcpObserver : public test::EndToEndTest {
public:
RtcpObserver()
: EndToEndTest(kDefaultTimeoutMs),
sender_call_(nullptr),
receiver_call_(nullptr),
has_seen_pacer_delay_(false) {}
class BandwidthStatsTest : public test::EndToEndTest {
public:
explicit BandwidthStatsTest(bool send_side_bwe)
: EndToEndTest(test::CallTest::kDefaultTimeoutMs),
sender_call_(nullptr),
receiver_call_(nullptr),
has_seen_pacer_delay_(false),
send_side_bwe_(send_side_bwe) {}
Action OnSendRtp(const uint8_t* packet, size_t length) override {
Call::Stats sender_stats = sender_call_->GetStats();
Call::Stats receiver_stats = receiver_call_->GetStats();
if (!has_seen_pacer_delay_)
has_seen_pacer_delay_ = sender_stats.pacer_delay_ms > 0;
if (sender_stats.send_bandwidth_bps > 0 &&
receiver_stats.recv_bandwidth_bps > 0 && has_seen_pacer_delay_) {
void ModifyVideoConfigs(
VideoSendStream::Config* send_config,
std::vector<VideoReceiveStream::Config>* receive_configs,
VideoEncoderConfig* encoder_config) override {
if (!send_side_bwe_) {
send_config->rtp.extensions.clear();
send_config->rtp.extensions.push_back(RtpExtension(
RtpExtension::kAbsSendTimeUri, test::kAbsSendTimeExtensionId));
(*receive_configs)[0].rtp.remb = true;
(*receive_configs)[0].rtp.transport_cc = false;
}
}
Action OnSendRtp(const uint8_t* packet, size_t length) override {
Call::Stats sender_stats = sender_call_->GetStats();
Call::Stats receiver_stats = receiver_call_->GetStats();
if (!has_seen_pacer_delay_)
has_seen_pacer_delay_ = sender_stats.pacer_delay_ms > 0;
if (sender_stats.send_bandwidth_bps > 0 && has_seen_pacer_delay_) {
if (send_side_bwe_ || receiver_stats.recv_bandwidth_bps > 0)
observation_complete_.Set();
}
return SEND_PACKET;
}
return SEND_PACKET;
}
void OnCallsCreated(Call* sender_call, Call* receiver_call) override {
sender_call_ = sender_call;
receiver_call_ = receiver_call;
}
void OnCallsCreated(Call* sender_call, Call* receiver_call) override {
sender_call_ = sender_call;
receiver_call_ = receiver_call;
}
void PerformTest() override {
EXPECT_TRUE(Wait()) << "Timed out while waiting for "
"non-zero bandwidth stats.";
}
void PerformTest() override {
EXPECT_TRUE(Wait()) << "Timed out while waiting for "
"non-zero bandwidth stats.";
}
private:
Call* sender_call_;
Call* receiver_call_;
bool has_seen_pacer_delay_;
} test;
private:
Call* sender_call_;
Call* receiver_call_;
bool has_seen_pacer_delay_;
const bool send_side_bwe_;
};
TEST_P(EndToEndTest, VerifySendSideBweStats) {
BandwidthStatsTest test(true);
RunBaseTest(&test);
}
TEST_P(EndToEndTest, VerifyRecvSideBweStats) {
BandwidthStatsTest test(false);
RunBaseTest(&test);
}
// Verifies that it's possible to limit the send BWE by sending a REMB.
// This is verified by allowing the send BWE to ramp-up to >1000 kbps,
@ -2042,18 +2077,11 @@ TEST_P(EndToEndTest, RembWithSendSideBwe) {
std::vector<VideoReceiveStream::Config>* receive_configs,
VideoEncoderConfig* encoder_config) override {
ASSERT_EQ(1u, send_config->rtp.ssrcs.size());
send_config->rtp.extensions.clear();
send_config->rtp.extensions.push_back(
RtpExtension(RtpExtension::kTransportSequenceNumberUri,
test::kTransportSequenceNumberExtensionId));
sender_ssrc_ = send_config->rtp.ssrcs[0];
encoder_config->max_bitrate_bps = 2000000;
ASSERT_EQ(1u, receive_configs->size());
(*receive_configs)[0].rtp.remb = false;
(*receive_configs)[0].rtp.transport_cc = true;
(*receive_configs)[0].rtp.extensions = send_config->rtp.extensions;
RtpRtcp::Configuration config;
config.receiver_only = true;
config.clock = clock_;
@ -2307,11 +2335,6 @@ void EndToEndTest::VerifyHistogramStats(bool use_rtx,
VideoSendStream::Config* send_config,
std::vector<VideoReceiveStream::Config>* receive_configs,
VideoEncoderConfig* encoder_config) override {
static const int kExtensionId = 8;
send_config->rtp.extensions.push_back(RtpExtension(
RtpExtension::kTransportSequenceNumberUri, kExtensionId));
(*receive_configs)[0].rtp.extensions.push_back(RtpExtension(
RtpExtension::kTransportSequenceNumberUri, kExtensionId));
// NACK
send_config->rtp.nack.rtp_history_ms = kNackRtpHistoryMs;
(*receive_configs)[0].rtp.nack.rtp_history_ms = kNackRtpHistoryMs;
@ -3974,16 +3997,6 @@ TEST_P(EndToEndTest, TransportSeqNumOnAudioAndVideo) {
size_t GetNumVideoStreams() const override { return 1; }
size_t GetNumAudioStreams() const override { return 1; }
void ModifyVideoConfigs(
VideoSendStream::Config* send_config,
std::vector<VideoReceiveStream::Config>* receive_configs,
VideoEncoderConfig* encoder_config) override {
send_config->rtp.extensions.clear();
send_config->rtp.extensions.push_back(RtpExtension(
RtpExtension::kTransportSequenceNumberUri, kExtensionId));
(*receive_configs)[0].rtp.extensions = send_config->rtp.extensions;
}
void ModifyAudioConfigs(
AudioSendStream::Config* send_config,
std::vector<AudioReceiveStream::Config>* receive_configs) override {

View File

@ -240,9 +240,6 @@ TEST_F(VideoSendStreamTest, SupportsTransportWideSequenceNumbers) {
std::vector<VideoReceiveStream::Config>* receive_configs,
VideoEncoderConfig* encoder_config) override {
send_config->encoder_settings.encoder = &encoder_;
send_config->rtp.extensions.clear();
send_config->rtp.extensions.push_back(RtpExtension(
RtpExtension::kTransportSequenceNumberUri, kExtensionId));
}
void PerformTest() override {
@ -460,12 +457,12 @@ class UlpfecObserver : public test::EndToEndTest {
VideoSendStreamTest::kRedPayloadType;
send_config->rtp.ulpfec.ulpfec_payload_type =
VideoSendStreamTest::kUlpfecPayloadType;
if (header_extensions_enabled_) {
EXPECT_FALSE(send_config->rtp.extensions.empty());
if (!header_extensions_enabled_) {
send_config->rtp.extensions.clear();
} else {
send_config->rtp.extensions.push_back(RtpExtension(
RtpExtension::kAbsSendTimeUri, test::kAbsSendTimeExtensionId));
send_config->rtp.extensions.push_back(
RtpExtension(RtpExtension::kTransportSequenceNumberUri,
test::kTransportSequenceNumberExtensionId));
}
(*receive_configs)[0].rtp.ulpfec.red_payload_type =
send_config->rtp.ulpfec.red_payload_type;
@ -614,9 +611,8 @@ class FlexfecObserver : public test::EndToEndTest {
RtpExtension::kAbsSendTimeUri, test::kAbsSendTimeExtensionId));
send_config->rtp.extensions.push_back(RtpExtension(
RtpExtension::kTimestampOffsetUri, test::kTOffsetExtensionId));
send_config->rtp.extensions.push_back(
RtpExtension(RtpExtension::kTransportSequenceNumberUri,
test::kTransportSequenceNumberExtensionId));
} else {
send_config->rtp.extensions.clear();
}
}
@ -1272,19 +1268,9 @@ TEST_F(VideoSendStreamTest, PaddingIsPrimarilyRetransmissions) {
VideoSendStream::Config* send_config,
std::vector<VideoReceiveStream::Config>* receive_configs,
VideoEncoderConfig* encoder_config) override {
send_config->rtp.extensions.clear();
send_config->rtp.extensions.push_back(
RtpExtension(RtpExtension::kTransportSequenceNumberUri,
test::kTransportSequenceNumberExtensionId));
// Turn on RTX.
send_config->rtp.rtx.payload_type = kFakeVideoSendPayloadType;
send_config->rtp.rtx.ssrcs.push_back(kVideoSendSsrcs[0]);
(*receive_configs)[0].rtp.extensions.clear();
(*receive_configs)[0].rtp.extensions.push_back(
RtpExtension(RtpExtension::kTransportSequenceNumberUri,
test::kTransportSequenceNumberExtensionId));
(*receive_configs)[0].rtp.transport_cc = true;
}
void PerformTest() override {