From 8734f19c2fdbb5f69360f740def9b3979da7d8b1 Mon Sep 17 00:00:00 2001 From: Eldar Rello Date: Tue, 20 Feb 2024 10:45:02 +0000 Subject: [PATCH] Add jitterBufferTarget attribute to RTCRtpReceiver interface according to the spec. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audio test case in WPT RTCRtpReceiver-jitterBufferTarget-stats.html remains failing due to a problem that without audio tag webrtc receive streams can not be played back and because of that jitter buffer stats are empty. Otherwise it was manually verified that when adding audio tag the test case would start passing. Bug: 324276557 Change-Id: I06cbba95a7f78c20cc7bf803819fcfb5ef2f6be5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5278176 Commit-Queue: Philipp Hancke Reviewed-by: Henrik Boström Reviewed-by: Harald Alvestrand Reviewed-by: Rick Byers Cr-Commit-Position: refs/heads/main@{#1262622} --- AUTHORS | 1 + .../use_counter/metrics/web_feature.mojom | 2 ++ .../peerconnection/rtc_rtp_receiver.cc | 23 ++++++++++++++++ .../modules/peerconnection/rtc_rtp_receiver.h | 3 +++ .../peerconnection/rtc_rtp_receiver.idl | 2 ++ .../platform/runtime_enabled_features.json5 | 6 +++++ ...tpReceiver-jitterBufferTarget-expected.txt | 27 ------------------- ...iver-jitterBufferTarget-stats-expected.txt | 4 +-- .../global-interface-listing-expected.txt | 2 ++ .../global-interface-listing-expected.txt | 2 ++ tools/metrics/histograms/enums.xml | 4 +++ 11 files changed, 46 insertions(+), 30 deletions(-) delete mode 100644 third_party/blink/web_tests/external/wpt/webrtc-extensions/RTCRtpReceiver-jitterBufferTarget-expected.txt diff --git a/AUTHORS b/AUTHORS index 4f252247bf00de..6119bcd752b8b7 100644 --- a/AUTHORS +++ b/AUTHORS @@ -389,6 +389,7 @@ Egor Starkov Ehsan Akhgari Ehsan Akhgari Elan Ruusamäe +Eldar Rello Ely Ronnen Emil Suleymanov Ergun Erdogmus diff --git a/third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom b/third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom index 5c6ed41b641be5..4ddc6b4c699cbd 100644 --- a/third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom +++ b/third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom @@ -4223,6 +4223,8 @@ enum WebFeature { kV8PointerEvent_GetCoalescedEvents_Method = 4859, kCSSFunctions = 4861, kCSSPageRule = 4862, + kV8RTCRtpReceiver_JitterBufferTarget_AttributeGetter = 4863, + kV8RTCRtpReceiver_JitterBufferTarget_AttributeSetter = 4864, // Add new features immediately above this line. Don't change assigned // numbers of any item, and don't reuse removed slots. diff --git a/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.cc b/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.cc index 19fcff243b0bd8..d90124ebbdb9e7 100644 --- a/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.cc +++ b/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.cc @@ -124,6 +124,29 @@ void RTCRtpReceiver::setPlayoutDelayHint(std::optional hint, receiver_->SetJitterBufferMinimumDelay(playout_delay_hint_); } +std::optional RTCRtpReceiver::jitterBufferTarget() const { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + return jitter_buffer_target_; +} + +void RTCRtpReceiver::setJitterBufferTarget(std::optional target, + ExceptionState& exception_state) { + DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); + if (target.has_value() && (target.value() < 0.0 || target.value() > 4000.0)) { + exception_state.ThrowRangeError( + "jitterBufferTarget is out of expected range 0 to 4000 ms"); + return; + } + + jitter_buffer_target_ = target; + if (jitter_buffer_target_.has_value()) { + receiver_->SetJitterBufferMinimumDelay(jitter_buffer_target_.value() / + 1000.0); + } else { + receiver_->SetJitterBufferMinimumDelay(std::nullopt); + } +} + HeapVector> RTCRtpReceiver::getSynchronizationSources(ScriptState* script_state, ExceptionState& exception_state) { diff --git a/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.h b/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.h index 7c3108c94d4eaa..667a266c673b5f 100644 --- a/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.h +++ b/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.h @@ -70,6 +70,8 @@ class RTCRtpReceiver final : public ScriptWrappable, RTCDtlsTransport* rtcpTransport(); std::optional playoutDelayHint() const; void setPlayoutDelayHint(std::optional, ExceptionState&); + std::optional jitterBufferTarget() const; + void setJitterBufferTarget(std::optional, ExceptionState&); RTCRtpReceiveParameters* getParameters(); HeapVector> getSynchronizationSources( ScriptState*, @@ -141,6 +143,7 @@ class RTCRtpReceiver final : public ScriptWrappable, // observed delay may differ depending on the congestion control. |nullopt| // means default value must be used. std::optional playout_delay_hint_; + std::optional jitter_buffer_target_; THREAD_CHECKER(thread_checker_); diff --git a/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.idl b/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.idl index 5619f214531655..d17313927a2d54 100644 --- a/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.idl +++ b/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.idl @@ -10,6 +10,8 @@ interface RTCRtpReceiver { readonly attribute RTCDtlsTransport? rtcpTransport; // https://henbos.github.io/webrtc-extensions/#dom-rtcrtpreceiver-playoutdelayhint [RaisesException=Setter, Measure] attribute double? playoutDelayHint; + // https://w3c.github.io/webrtc-extensions/#dom-rtcrtpreceiver-jitterbuffertarget + [RaisesException=Setter, Measure, RuntimeEnabled=RTCJitterBufferTarget] attribute double? jitterBufferTarget; [CallWith=ScriptState, HighEntropy, Measure] static RTCRtpCapabilities? getCapabilities(DOMString kind); RTCRtpReceiveParameters getParameters(); [CallWith=ScriptState, RaisesException] sequence getSynchronizationSources(); diff --git a/third_party/blink/renderer/platform/runtime_enabled_features.json5 b/third_party/blink/renderer/platform/runtime_enabled_features.json5 index 7fa47372f789bc..90f38c2852e1d4 100644 --- a/third_party/blink/renderer/platform/runtime_enabled_features.json5 +++ b/third_party/blink/renderer/platform/runtime_enabled_features.json5 @@ -3152,6 +3152,12 @@ name: "RTCEncodedVideoFrameAdditionalMetadata", status: "experimental", }, + // Enables the use of jitterBufferTarget attribute in WebRTC. + // Spec: https://w3c.github.io/webrtc-extensions/#dom-rtcrtpreceiver-jitterbuffertarget + { + name: "RTCJitterBufferTarget", + status: "stable", + }, // Legacy callback-based getStats() has limited availability unless this // Deprecation Trial is enabled. // TODO(https://crbug.com/822696): Remove when origin trial ends. diff --git a/third_party/blink/web_tests/external/wpt/webrtc-extensions/RTCRtpReceiver-jitterBufferTarget-expected.txt b/third_party/blink/web_tests/external/wpt/webrtc-extensions/RTCRtpReceiver-jitterBufferTarget-expected.txt deleted file mode 100644 index 33e681f6da0051..00000000000000 --- a/third_party/blink/web_tests/external/wpt/webrtc-extensions/RTCRtpReceiver-jitterBufferTarget-expected.txt +++ /dev/null @@ -1,27 +0,0 @@ -This is a testharness.js-based test. -[FAIL] audio jitterBufferTarget is null by default - assert_equals: expected (object) null but got (undefined) undefined -[FAIL] audio jitterBufferTarget accepts posititve values - assert_equals: expected (object) null but got (undefined) undefined -[FAIL] audio jitterBufferTarget accepts values up to 4000 milliseconds - assert_equals: expected (object) null but got (undefined) undefined -[FAIL] audio jitterBufferTarget returns last valid value on throw - assert_equals: expected (object) null but got (undefined) undefined -[FAIL] audio jitterBufferTarget allows zero value - assert_equals: expected (object) null but got (undefined) undefined -[FAIL] audio jitterBufferTarget allows to reset value to null - assert_equals: expected (object) null but got (undefined) undefined -[FAIL] video jitterBufferTarget is null by default - assert_equals: expected (object) null but got (undefined) undefined -[FAIL] video jitterBufferTarget accepts posititve values - assert_equals: expected (object) null but got (undefined) undefined -[FAIL] video jitterBufferTarget accepts values up to 4000 milliseconds - assert_equals: expected (object) null but got (undefined) undefined -[FAIL] video jitterBufferTarget returns last valid value - assert_equals: expected (object) null but got (undefined) undefined -[FAIL] video jitterBufferTarget allows zero value - assert_equals: expected (object) null but got (undefined) undefined -[FAIL] video jitterBufferTarget allows to reset value to null - assert_equals: expected (object) null but got (undefined) undefined -Harness: the test ran to completion. - diff --git a/third_party/blink/web_tests/external/wpt/webrtc-extensions/RTCRtpReceiver-jitterBufferTarget-stats-expected.txt b/third_party/blink/web_tests/external/wpt/webrtc-extensions/RTCRtpReceiver-jitterBufferTarget-stats-expected.txt index 8e35e4c5b51c38..115f126d682fc0 100644 --- a/third_party/blink/web_tests/external/wpt/webrtc-extensions/RTCRtpReceiver-jitterBufferTarget-stats-expected.txt +++ b/third_party/blink/web_tests/external/wpt/webrtc-extensions/RTCRtpReceiver-jitterBufferTarget-stats-expected.txt @@ -1,7 +1,5 @@ This is a testharness.js-based test. -[FAIL] measure raising and lowering video jitterBufferTarget - assert_equals: jitterBufferTarget supported for video expected (object) null but got (undefined) undefined [FAIL] measure raising and lowering audio jitterBufferTarget - assert_equals: jitterBufferTarget supported for audio expected (object) null but got (undefined) undefined + assert_greater_than: audio increased delay NaN greater than base delay NaN expected a number greater than NaN but got NaN Harness: the test ran to completion. diff --git a/third_party/blink/web_tests/virtual/stable/webexposed/global-interface-listing-expected.txt b/third_party/blink/web_tests/virtual/stable/webexposed/global-interface-listing-expected.txt index 0cacfa1630bc47..dfcc012b016c38 100644 --- a/third_party/blink/web_tests/virtual/stable/webexposed/global-interface-listing-expected.txt +++ b/third_party/blink/web_tests/virtual/stable/webexposed/global-interface-listing-expected.txt @@ -6739,6 +6739,7 @@ interface RTCPeerConnectionIceEvent : Event interface RTCRtpReceiver static method getCapabilities attribute @@toStringTag + getter jitterBufferTarget getter playoutDelayHint getter rtcpTransport getter track @@ -6749,6 +6750,7 @@ interface RTCRtpReceiver method getParameters method getStats method getSynchronizationSources + setter jitterBufferTarget setter playoutDelayHint interface RTCRtpSender static method getCapabilities diff --git a/third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt b/third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt index 266911cf3b2797..4c01a4b8d7f589 100644 --- a/third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt +++ b/third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt @@ -7746,6 +7746,7 @@ interface RTCPeerConnectionIceEvent : Event interface RTCRtpReceiver static method getCapabilities attribute @@toStringTag + getter jitterBufferTarget getter playoutDelayHint getter rtcpTransport getter track @@ -7756,6 +7757,7 @@ interface RTCRtpReceiver method getParameters method getStats method getSynchronizationSources + setter jitterBufferTarget setter playoutDelayHint interface RTCRtpSender static method getCapabilities diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml index df1d7c41a800f0..c5d88e59755680 100644 --- a/tools/metrics/histograms/enums.xml +++ b/tools/metrics/histograms/enums.xml @@ -11240,6 +11240,10 @@ Called by update_use_counter_feature_enum.py.--> + +