diff options
-rw-r--r-- | talk/media/base/codec.cc | 5 | ||||
-rw-r--r-- | talk/media/base/codec.h | 1 | ||||
-rw-r--r-- | talk/media/base/constants.cc | 1 | ||||
-rw-r--r-- | talk/media/base/constants.h | 3 | ||||
-rw-r--r-- | talk/media/webrtc/webrtcvideoengine2.cc | 39 | ||||
-rw-r--r-- | talk/media/webrtc/webrtcvideoengine2.h | 4 | ||||
-rw-r--r-- | talk/media/webrtc/webrtcvideoengine2_unittest.cc | 28 | ||||
-rw-r--r-- | webrtc/video/end_to_end_tests.cc | 24 | ||||
-rw-r--r-- | webrtc/video/rampup_tests.cc | 5 | ||||
-rw-r--r-- | webrtc/video/video_quality_test.cc | 1 | ||||
-rw-r--r-- | webrtc/video/video_receive_stream.cc | 4 | ||||
-rw-r--r-- | webrtc/video_receive_stream.h | 3 |
12 files changed, 99 insertions, 19 deletions
diff --git a/talk/media/base/codec.cc b/talk/media/base/codec.cc index 5b747d1917..5fbae2c45e 100644 --- a/talk/media/base/codec.cc +++ b/talk/media/base/codec.cc @@ -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; } diff --git a/talk/media/base/codec.h b/talk/media/base/codec.h index 3bb08e7c7a..afdf5fdad9 100644 --- a/talk/media/base/codec.h +++ b/talk/media/base/codec.h @@ -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 diff --git a/talk/media/base/constants.cc b/talk/media/base/constants.cc index 4063004968..2361be6f50 100644 --- a/talk/media/base/constants.cc +++ b/talk/media/base/constants.cc @@ -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"; diff --git a/talk/media/base/constants.h b/talk/media/base/constants.h index b6a9e5681f..706a7bdc87 100644 --- a/talk/media/base/constants.h +++ b/talk/media/base/constants.h @@ -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[]; diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc index 1612183c7f..2d648b73a3 100644 --- a/talk/media/webrtc/webrtcvideoengine2.cc +++ b/talk/media/webrtc/webrtcvideoengine2.cc @@ -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(); } diff --git a/talk/media/webrtc/webrtcvideoengine2.h b/talk/media/webrtc/webrtcvideoengine2.h index 24033a1603..85e79281f8 100644 --- a/talk/media/webrtc/webrtcvideoengine2.h +++ b/talk/media/webrtc/webrtcvideoengine2.h @@ -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); diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.cc b/talk/media/webrtc/webrtcvideoengine2_unittest.cc index 87a0ac81a9..832c4d0623 100644 --- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc @@ -75,6 +75,8 @@ void VerifyCodecHasDefaultFeedbackParams(const cricket::VideoCodec& codec) { 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_); diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index e2450d9c7d..e541d370d8 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -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: diff --git a/webrtc/video/rampup_tests.cc b/webrtc/video/rampup_tests.cc index 6450e0290d..c9b8e11122 100644 --- a/webrtc/video/rampup_tests.cc +++ b/webrtc/video/rampup_tests.cc @@ -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]; diff --git a/webrtc/video/video_quality_test.cc b/webrtc/video/video_quality_test.cc index ebceb5642b..a46fe1bd7c 100644 --- a/webrtc/video/video_quality_test.cc +++ b/webrtc/video/video_quality_test.cc @@ -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; } } diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index 287f5355dc..683e89aa97 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -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); diff --git a/webrtc/video_receive_stream.h b/webrtc/video_receive_stream.h index 275162ca1c..6be2c5a86e 100644 --- a/webrtc/video_receive_stream.h +++ b/webrtc/video_receive_stream.h @@ -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; |