From 7ff164e6e16a530fc2bbeeb7f756238d5a8bca78 Mon Sep 17 00:00:00 2001 From: Johannes Kron Date: Thu, 7 Feb 2019 12:50:18 +0100 Subject: [PATCH] Plumbing of feedback on request setting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:10263 Change-Id: I23c09e680d6381598e4172b76025ff84f33aa4de Reviewed-on: https://webrtc-review.googlesource.com/c/121422 Reviewed-by: Sebastian Jansson Reviewed-by: Karl Wiberg Reviewed-by: Stefan Holmer Reviewed-by: Erik Språng Commit-Queue: Johannes Kron Cr-Commit-Position: refs/heads/master@{#26606} --- api/rtp_parameters.cc | 2 ++ api/rtp_parameters.h | 1 + call/call.cc | 14 +++++++++++++- .../include/receive_side_congestion_controller.h | 1 + .../receive_side_congestion_controller.cc | 6 ++++++ .../remote_estimator_proxy.cc | 9 ++++++++- .../remote_estimator_proxy.h | 2 ++ 7 files changed, 33 insertions(+), 2 deletions(-) diff --git a/api/rtp_parameters.cc b/api/rtp_parameters.cc index c4fd1120be..6de14da10f 100644 --- a/api/rtp_parameters.cc +++ b/api/rtp_parameters.cc @@ -108,6 +108,8 @@ const int RtpExtension::kVideoRotationDefaultId = 4; const char RtpExtension::kTransportSequenceNumberUri[] = "http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01"; +const char RtpExtension::kTransportSequenceNumberV2Uri[] = + "http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-02"; const int RtpExtension::kTransportSequenceNumberDefaultId = 5; // This extension allows applications to adaptively limit the playout delay diff --git a/api/rtp_parameters.h b/api/rtp_parameters.h index 25a21c1582..90e4a4cf1c 100644 --- a/api/rtp_parameters.h +++ b/api/rtp_parameters.h @@ -295,6 +295,7 @@ struct RtpExtension { // Header extension for transport sequence number, see url for details: // http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions static const char kTransportSequenceNumberUri[]; + static const char kTransportSequenceNumberV2Uri[]; static const int kTransportSequenceNumberDefaultId; static const char kPlayoutDelayUri[]; diff --git a/call/call.cc b/call/call.cc index f3f6cd314b..5baf4250ef 100644 --- a/call/call.cc +++ b/call/call.cc @@ -68,13 +68,22 @@ namespace webrtc { namespace { +bool SendFeedbackOnRequestOnly(const std::vector& extensions) { + for (const auto& extension : extensions) { + if (extension.uri == RtpExtension::kTransportSequenceNumberV2Uri) + return true; + } + return false; +} + // TODO(nisse): This really begs for a shared context struct. bool UseSendSideBwe(const std::vector& extensions, bool transport_cc) { if (!transport_cc) return false; for (const auto& extension : extensions) { - if (extension.uri == RtpExtension::kTransportSequenceNumberUri) + if (extension.uri == RtpExtension::kTransportSequenceNumberUri || + extension.uri == RtpExtension::kTransportSequenceNumberV2Uri) return true; } return false; @@ -862,6 +871,9 @@ webrtc::VideoReceiveStream* Call::CreateVideoReceiveStream( TRACE_EVENT0("webrtc", "Call::CreateVideoReceiveStream"); RTC_DCHECK_CALLED_SEQUENTIALLY(&configuration_sequence_checker_); + receive_side_cc_.SetSendFeedbackOnRequestOnly( + SendFeedbackOnRequestOnly(configuration.rtp.extensions)); + RegisterRateObserver(); VideoReceiveStream* receive_stream = new VideoReceiveStream( diff --git a/modules/congestion_controller/include/receive_side_congestion_controller.h b/modules/congestion_controller/include/receive_side_congestion_controller.h index b021618912..5532b3c60f 100644 --- a/modules/congestion_controller/include/receive_side_congestion_controller.h +++ b/modules/congestion_controller/include/receive_side_congestion_controller.h @@ -38,6 +38,7 @@ class ReceiveSideCongestionController : public CallStatsObserver, size_t payload_size, const RTPHeader& header); + void SetSendFeedbackOnRequestOnly(bool send_feedback_on_request_only); // TODO(nisse): Delete these methods, design a more specific interface. virtual RemoteBitrateEstimator* GetRemoteBitrateEstimator(bool send_side_bwe); virtual const RemoteBitrateEstimator* GetRemoteBitrateEstimator( diff --git a/modules/congestion_controller/receive_side_congestion_controller.cc b/modules/congestion_controller/receive_side_congestion_controller.cc index 224a49d2f3..1a95f8ce0f 100644 --- a/modules/congestion_controller/receive_side_congestion_controller.cc +++ b/modules/congestion_controller/receive_side_congestion_controller.cc @@ -139,6 +139,12 @@ void ReceiveSideCongestionController::OnReceivedPacket( } } +void ReceiveSideCongestionController::SetSendFeedbackOnRequestOnly( + bool send_feedback_on_request_only) { + remote_estimator_proxy_.SetSendFeedbackOnRequestOnly( + send_feedback_on_request_only); +} + RemoteBitrateEstimator* ReceiveSideCongestionController::GetRemoteBitrateEstimator(bool send_side_bwe) { if (send_side_bwe) { diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc index 9d9d27f7b0..932304428f 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc @@ -41,7 +41,8 @@ RemoteEstimatorProxy::RemoteEstimatorProxy( media_ssrc_(0), feedback_sequence_(0), window_start_seq_(-1), - send_interval_ms_(kDefaultSendIntervalMs) {} + send_interval_ms_(kDefaultSendIntervalMs), + send_feedback_on_request_only_(false) {} RemoteEstimatorProxy::~RemoteEstimatorProxy() {} @@ -110,6 +111,12 @@ void RemoteEstimatorProxy::OnBitrateChanged(int bitrate_bps) { rtc::SafeClamp(0.05 * bitrate_bps, kMinTwccRate, kMaxTwccRate)); } +void RemoteEstimatorProxy::SetSendFeedbackOnRequestOnly( + bool send_feedback_on_request_only) { + rtc::CritScope cs(&lock_); + send_feedback_on_request_only_ = send_feedback_on_request_only; +} + void RemoteEstimatorProxy::OnPacketArrival(uint16_t sequence_number, int64_t arrival_time) { if (arrival_time < 0 || arrival_time > kMaxTimeMs) { diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.h b/modules/remote_bitrate_estimator/remote_estimator_proxy.h index db41a94d23..94b1716004 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy.h +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.h @@ -47,6 +47,7 @@ class RemoteEstimatorProxy : public RemoteBitrateEstimator { int64_t TimeUntilNextProcess() override; void Process() override; void OnBitrateChanged(int bitrate); + void SetSendFeedbackOnRequestOnly(bool send_feedback_on_request_only); static const int kMinSendIntervalMs; static const int kMaxSendIntervalMs; @@ -71,6 +72,7 @@ class RemoteEstimatorProxy : public RemoteBitrateEstimator { // Map unwrapped seq -> time. std::map packet_arrival_times_ RTC_GUARDED_BY(&lock_); int64_t send_interval_ms_ RTC_GUARDED_BY(&lock_); + bool send_feedback_on_request_only_ RTC_GUARDED_BY(&lock_); }; } // namespace webrtc