Require negotiation to send transport cc feedback over RTCP.

BUG=4312

Review URL: https://codereview.webrtc.org/1452883002

Cr-Commit-Position: refs/heads/master@{#10735}
This commit is contained in:
stefan
2015-11-20 18:05:48 -08:00
committed by Commit bot
parent bd13838ccc
commit 43edf0ffb9
12 changed files with 99 additions and 19 deletions

View File

@ -334,6 +334,11 @@ bool HasRemb(const VideoCodec& codec) {
FeedbackParam(kRtcpFbParamRemb, kParamValueEmpty));
}
bool HasTransportCc(const VideoCodec& codec) {
return codec.HasFeedbackParam(
FeedbackParam(kRtcpFbParamTransportCc, kParamValueEmpty));
}
bool CodecNamesEq(const std::string& name1, const std::string& name2) {
return _stricmp(name1.c_str(), name2.c_str()) == 0;
}

View File

@ -271,6 +271,7 @@ bool FindCodecById(const std::vector<Codec>& codecs,
bool CodecNamesEq(const std::string& name1, const std::string& name2);
bool HasNack(const VideoCodec& codec);
bool HasRemb(const VideoCodec& codec);
bool HasTransportCc(const VideoCodec& codec);
} // namespace cricket

View File

@ -90,6 +90,7 @@ const int kPreferredUseInbandFec = 0;
const char kRtcpFbParamNack[] = "nack";
const char kRtcpFbNackParamPli[] = "pli";
const char kRtcpFbParamRemb[] = "goog-remb";
const char kRtcpFbParamTransportCc[] = "transport-cc";
const char kRtcpFbParamCcm[] = "ccm";
const char kRtcpFbCcmParamFir[] = "fir";

View File

@ -107,6 +107,9 @@ extern const char kRtcpFbNackParamPli[];
// rtcp-fb messages according to
// http://tools.ietf.org/html/draft-alvestrand-rmcat-remb-00
extern const char kRtcpFbParamRemb[];
// rtcp-fb messages according to
// https://tools.ietf.org/html/draft-holmer-rmcat-transport-wide-cc-extensions-01
extern const char kRtcpFbParamTransportCc[];
// ccm submessages according to RFC 5104
extern const char kRtcpFbParamCcm[];
extern const char kRtcpFbCcmParamFir[];

View File

@ -166,6 +166,8 @@ void AddDefaultFeedbackParams(VideoCodec* codec) {
codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamNack, kParamValueEmpty));
codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamNack, kRtcpFbNackParamPli));
codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamRemb, kParamValueEmpty));
codec->AddFeedbackParam(
FeedbackParam(kRtcpFbParamTransportCc, kParamValueEmpty));
}
static VideoCodec MakeVideoCodecWithDefaultFeedbackParams(int payload_type,
@ -960,12 +962,15 @@ bool WebRtcVideoChannel2::SetSendCodecs(const std::vector<VideoCodec>& codecs) {
RTC_DCHECK(kv.second != nullptr);
kv.second->SetCodec(supported_codecs.front());
}
LOG(LS_INFO) << "SetNackAndRemb on all the receive streams because the send "
"codec has changed.";
LOG(LS_INFO)
<< "SetFeedbackOptions on all the receive streams because the send "
"codec has changed.";
for (auto& kv : receive_streams_) {
RTC_DCHECK(kv.second != nullptr);
kv.second->SetNackAndRemb(HasNack(supported_codecs.front().codec),
HasRemb(supported_codecs.front().codec));
kv.second->SetFeedbackParameters(
HasNack(supported_codecs.front().codec),
HasRemb(supported_codecs.front().codec),
HasTransportCc(supported_codecs.front().codec));
}
// TODO(holmer): Changing the codec parameters shouldn't necessarily mean that
@ -1215,6 +1220,8 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp,
config.sync_group = sp.sync_label;
config.rtp.remb = send_codec_ ? HasRemb(send_codec_->codec) : false;
config.rtp.transport_cc =
send_codec_ ? HasTransportCc(send_codec_->codec) : false;
receive_streams_[ssrc] = new WebRtcVideoReceiveStream(
call_, sp, config, external_decoder_factory_, default_stream,
@ -2465,20 +2472,28 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetLocalSsrc(
RecreateWebRtcStream();
}
void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetNackAndRemb(
bool nack_enabled, bool remb_enabled) {
void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetFeedbackParameters(
bool nack_enabled,
bool remb_enabled,
bool transport_cc_enabled) {
int nack_history_ms = nack_enabled ? kNackHistoryMs : 0;
if (config_.rtp.nack.rtp_history_ms == nack_history_ms &&
config_.rtp.remb == remb_enabled) {
LOG(LS_INFO) << "Ignoring call to SetNackAndRemb because parameters are "
"unchanged; nack=" << nack_enabled
<< ", remb=" << remb_enabled;
config_.rtp.remb == remb_enabled &&
config_.rtp.transport_cc == transport_cc_enabled) {
LOG(LS_INFO)
<< "Ignoring call to SetFeedbackParameters because parameters are "
"unchanged; nack="
<< nack_enabled << ", remb=" << remb_enabled
<< ", transport_cc=" << transport_cc_enabled;
return;
}
config_.rtp.remb = remb_enabled;
config_.rtp.nack.rtp_history_ms = nack_history_ms;
LOG(LS_INFO) << "RecreateWebRtcStream (recv) because of SetNackAndRemb; nack="
<< nack_enabled << ", remb=" << remb_enabled;
config_.rtp.transport_cc = transport_cc_enabled;
LOG(LS_INFO)
<< "RecreateWebRtcStream (recv) because of SetFeedbackParameters; nack="
<< nack_enabled << ", remb=" << remb_enabled
<< ", transport_cc=" << transport_cc_enabled;
RecreateWebRtcStream();
}

View File

@ -400,7 +400,9 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler,
const std::vector<uint32_t>& GetSsrcs() const;
void SetLocalSsrc(uint32_t local_ssrc);
void SetNackAndRemb(bool nack_enabled, bool remb_enabled);
void SetFeedbackParameters(bool nack_enabled,
bool remb_enabled,
bool transport_cc_enabled);
void SetRecvCodecs(const std::vector<VideoCodecSettings>& recv_codecs);
void SetRtpExtensions(const std::vector<webrtc::RtpExtension>& extensions);

View File

@ -74,6 +74,8 @@ void VerifyCodecHasDefaultFeedbackParams(const cricket::VideoCodec& codec) {
cricket::kRtcpFbParamNack, cricket::kRtcpFbNackParamPli)));
EXPECT_TRUE(codec.HasFeedbackParam(cricket::FeedbackParam(
cricket::kRtcpFbParamRemb, cricket::kParamValueEmpty)));
EXPECT_TRUE(codec.HasFeedbackParam(cricket::FeedbackParam(
cricket::kRtcpFbParamTransportCc, cricket::kParamValueEmpty)));
EXPECT_TRUE(codec.HasFeedbackParam(cricket::FeedbackParam(
cricket::kRtcpFbParamCcm, cricket::kRtcpFbCcmParamFir)));
}
@ -1451,6 +1453,11 @@ TEST_F(WebRtcVideoChannel2Test, RembIsEnabledByDefault) {
EXPECT_TRUE(stream->GetConfig().rtp.remb);
}
TEST_F(WebRtcVideoChannel2Test, TransportCcIsEnabledByDefault) {
FakeVideoReceiveStream* stream = AddRecvStream();
EXPECT_TRUE(stream->GetConfig().rtp.transport_cc);
}
TEST_F(WebRtcVideoChannel2Test, RembCanBeEnabledAndDisabled) {
FakeVideoReceiveStream* stream = AddRecvStream();
EXPECT_TRUE(stream->GetConfig().rtp.remb);
@ -1471,6 +1478,27 @@ TEST_F(WebRtcVideoChannel2Test, RembCanBeEnabledAndDisabled) {
EXPECT_TRUE(stream->GetConfig().rtp.remb);
}
TEST_F(WebRtcVideoChannel2Test, TransportCcCanBeEnabledAndDisabled) {
FakeVideoReceiveStream* stream = AddRecvStream();
EXPECT_TRUE(stream->GetConfig().rtp.transport_cc);
// Verify that transport cc feedback is turned off when send(!) codecs without
// transport cc feedback are set.
cricket::VideoSendParameters parameters;
parameters.codecs.push_back(kVp8Codec);
EXPECT_TRUE(parameters.codecs[0].feedback_params.params().empty());
EXPECT_TRUE(channel_->SetSendParameters(parameters));
stream = fake_call_->GetVideoReceiveStreams()[0];
EXPECT_FALSE(stream->GetConfig().rtp.transport_cc);
// Verify that transport cc feedback is turned on when setting default codecs
// since the default codecs have transport cc feedback enabled.
parameters.codecs = engine_.codecs();
EXPECT_TRUE(channel_->SetSendParameters(parameters));
stream = fake_call_->GetVideoReceiveStreams()[0];
EXPECT_TRUE(stream->GetConfig().rtp.transport_cc);
}
TEST_F(WebRtcVideoChannel2Test, NackIsEnabledByDefault) {
VerifyCodecHasDefaultFeedbackParams(default_codec_);

View File

@ -1533,9 +1533,8 @@ TEST_F(EndToEndTest, AssignsTransportSequenceNumbers) {
tester.RunTest();
}
TEST_F(EndToEndTest, ReceivesTransportFeedback) {
void TransportFeedbackTest(bool feedback_enabled) {
static const int kExtensionId = 5;
class TransportFeedbackObserver : public test::DirectTransport {
public:
TransportFeedbackObserver(Call* receiver_call, rtc::Event* done_event)
@ -1563,12 +1562,16 @@ TEST_F(EndToEndTest, ReceivesTransportFeedback) {
class TransportFeedbackTester : public MultiStreamTest {
public:
TransportFeedbackTester() : done_(false, false) {}
explicit TransportFeedbackTester(bool feedback_enabled)
: feedback_enabled_(feedback_enabled), done_(false, false) {}
virtual ~TransportFeedbackTester() {}
protected:
void Wait() override {
EXPECT_TRUE(done_.Wait(CallTest::kDefaultTimeoutMs));
const int64_t kDisabledFeedbackTimeoutMs = 5000;
EXPECT_EQ(feedback_enabled_, done_.Wait(feedback_enabled_
? test::CallTest::kDefaultTimeoutMs
: kDisabledFeedbackTimeoutMs));
}
void UpdateSendConfig(
@ -1585,6 +1588,7 @@ TEST_F(EndToEndTest, ReceivesTransportFeedback) {
VideoReceiveStream::Config* receive_config) override {
receive_config->rtp.extensions.push_back(
RtpExtension(RtpExtension::kTransportSequenceNumber, kExtensionId));
receive_config->rtp.transport_cc = feedback_enabled_;
}
test::DirectTransport* CreateReceiveTransport(
@ -1593,10 +1597,20 @@ TEST_F(EndToEndTest, ReceivesTransportFeedback) {
}
private:
const bool feedback_enabled_;
rtc::Event done_;
} tester;
} tester(feedback_enabled);
tester.RunTest();
}
TEST_F(EndToEndTest, ReceivesTransportFeedback) {
TransportFeedbackTest(true);
}
TEST_F(EndToEndTest, TransportFeedbackNotConfigured) {
TransportFeedbackTest(false);
}
TEST_F(EndToEndTest, ObserversEncodedFrames) {
class EncodedFrameTestObserver : public EncodedFrameObserver {
public:

View File

@ -124,16 +124,20 @@ void RampUpTester::ModifyConfigs(
send_config->rtp.extensions.clear();
bool remb;
bool transport_cc;
if (extension_type_ == RtpExtension::kAbsSendTime) {
remb = true;
transport_cc = false;
send_config->rtp.extensions.push_back(
RtpExtension(extension_type_.c_str(), kAbsSendTimeExtensionId));
} else if (extension_type_ == RtpExtension::kTransportSequenceNumber) {
remb = false;
transport_cc = true;
send_config->rtp.extensions.push_back(RtpExtension(
extension_type_.c_str(), kTransportSequenceNumberExtensionId));
} else {
remb = true;
transport_cc = false;
send_config->rtp.extensions.push_back(RtpExtension(
extension_type_.c_str(), kTransmissionTimeOffsetExtensionId));
}
@ -153,6 +157,7 @@ void RampUpTester::ModifyConfigs(
size_t i = 0;
for (VideoReceiveStream::Config& recv_config : *receive_configs) {
recv_config.rtp.remb = remb;
recv_config.rtp.transport_cc = transport_cc;
recv_config.rtp.extensions = send_config->rtp.extensions;
recv_config.rtp.remote_ssrc = ssrcs_[i];

View File

@ -828,6 +828,7 @@ void VideoQualityTest::SetupCommon(Transport* send_transport,
receive_configs_[i].rtp.rtx[kSendRtxPayloadType].ssrc = kSendRtxSsrcs[i];
receive_configs_[i].rtp.rtx[kSendRtxPayloadType].payload_type =
kSendRtxPayloadType;
receive_configs_[i].rtp.transport_cc = params_.common.send_side_bwe;
}
}

View File

@ -81,6 +81,7 @@ std::string VideoReceiveStream::Config::Rtp::ToString() const {
<< (rtcp_xr.receiver_reference_time_report ? "on" : "off");
ss << '}';
ss << ", remb: " << (remb ? "on" : "off");
ss << ", transport_cc: " << (transport_cc ? "on" : "off");
ss << ", nack: {rtp_history_ms: " << nack.rtp_history_ms << '}';
ss << ", fec: " << fec.ToString();
ss << ", rtx: {";
@ -153,7 +154,8 @@ VideoReceiveStream::VideoReceiveStream(
call_stats_(call_stats) {
LOG(LS_INFO) << "VideoReceiveStream: " << config_.ToString();
bool send_side_bwe = UseSendSideBwe(config_.rtp.extensions);
bool send_side_bwe =
config.rtp.transport_cc && UseSendSideBwe(config_.rtp.extensions);
RemoteBitrateEstimator* bitrate_estimator =
congestion_controller_->GetRemoteBitrateEstimator(send_side_bwe);

View File

@ -113,6 +113,9 @@ class VideoReceiveStream : public ReceiveStream {
// See draft-alvestrand-rmcat-remb for information.
bool remb = false;
// See draft-holmer-rmcat-transport-wide-cc-extensions for details.
bool transport_cc = false;
// See NackConfig for description.
NackConfig nack;