From 4bac79ece2c3ddfa06c69e7d190044a90645ae68 Mon Sep 17 00:00:00 2001 From: Ruslan Burakov Date: Wed, 3 Apr 2019 19:55:33 +0200 Subject: [PATCH] Add SetJitterBufferMinimumDelay method to RtpReceiverInterface. This change is required to allow modification of Jitter Buffer delay in javascript via Origin Trial Experiment. Link to experiment description: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Tgm4qiNepJc Bug: webrtc:10287 Change-Id: I4f21380aad5982a4a60c55683b5173ce72ce0392 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/131144 Commit-Queue: Ruslan Burakov Reviewed-by: Steve Anton Cr-Commit-Position: refs/heads/master@{#27444} --- api/rtp_receiver_interface.cc | 3 +++ api/rtp_receiver_interface.h | 9 +++++++++ api/test/mock_rtpreceiver.h | 1 + pc/audio_rtp_receiver.cc | 9 +++++++++ pc/audio_rtp_receiver.h | 3 +++ pc/playout_latency.cc | 9 +++++---- pc/playout_latency_unittest.cc | 15 +++++++++++++++ pc/rtp_sender_receiver_unittest.cc | 25 +++++++++++++++++++++++++ pc/test/mock_rtp_receiver_internal.h | 1 + pc/video_rtp_receiver.cc | 9 +++++++++ pc/video_rtp_receiver.h | 3 +++ 11 files changed, 83 insertions(+), 4 deletions(-) diff --git a/api/rtp_receiver_interface.cc b/api/rtp_receiver_interface.cc index 52f72df45d..615a31dd10 100644 --- a/api/rtp_receiver_interface.cc +++ b/api/rtp_receiver_interface.cc @@ -58,4 +58,7 @@ RtpReceiverInterface::dtls_transport() const { return nullptr; } +void RtpReceiverInterface::SetJitterBufferMinimumDelay( + absl::optional delay_seconds) {} + } // namespace webrtc diff --git a/api/rtp_receiver_interface.h b/api/rtp_receiver_interface.h index 7d9870b8f1..d1ef137f0a 100644 --- a/api/rtp_receiver_interface.h +++ b/api/rtp_receiver_interface.h @@ -128,6 +128,14 @@ class RtpReceiverInterface : public rtc::RefCountInterface { // Must call SetObserver(nullptr) before the observer is destroyed. virtual void SetObserver(RtpReceiverObserverInterface* observer) = 0; + // Sets the jitter buffer minimum delay until media playout. Actual observed + // delay may differ depending on the congestion control. |delay_seconds| is a + // positive value including 0.0 measured in seconds. |nullopt| means default + // value must be used. TODO(kuddai): remove the default implmenetation once + // the subclasses in Chromium implement this. + virtual void SetJitterBufferMinimumDelay( + absl::optional delay_seconds); + // TODO(zhihuang): Remove the default implementation once the subclasses // implement this. Currently, the only relevant subclass is the // content::FakeRtpReceiver in Chromium. @@ -163,6 +171,7 @@ PROXY_CONSTMETHOD0(std::string, id) PROXY_CONSTMETHOD0(RtpParameters, GetParameters) PROXY_METHOD1(bool, SetParameters, const RtpParameters&) PROXY_METHOD1(void, SetObserver, RtpReceiverObserverInterface*) +PROXY_METHOD1(void, SetJitterBufferMinimumDelay, absl::optional) PROXY_CONSTMETHOD0(std::vector, GetSources) PROXY_METHOD1(void, SetFrameDecryptor, diff --git a/api/test/mock_rtpreceiver.h b/api/test/mock_rtpreceiver.h index c585cbf1cc..710f8c551a 100644 --- a/api/test/mock_rtpreceiver.h +++ b/api/test/mock_rtpreceiver.h @@ -30,6 +30,7 @@ class MockRtpReceiver : public rtc::RefCountedObject { MOCK_CONST_METHOD0(GetParameters, RtpParameters()); MOCK_METHOD1(SetParameters, bool(const RtpParameters&)); MOCK_METHOD1(SetObserver, void(RtpReceiverObserverInterface*)); + MOCK_METHOD1(SetJitterBufferMinimumDelay, void(absl::optional)); MOCK_CONST_METHOD0(GetSources, std::vector()); }; diff --git a/pc/audio_rtp_receiver.cc b/pc/audio_rtp_receiver.cc index 4602fdae25..61ea3409e6 100644 --- a/pc/audio_rtp_receiver.cc +++ b/pc/audio_rtp_receiver.cc @@ -25,6 +25,10 @@ namespace webrtc { +namespace { +constexpr double kDefaultLatency = 0.0; +} // namespace + AudioRtpReceiver::AudioRtpReceiver(rtc::Thread* worker_thread, std::string receiver_id, std::vector stream_ids) @@ -232,6 +236,11 @@ void AudioRtpReceiver::SetObserver(RtpReceiverObserverInterface* observer) { } } +void AudioRtpReceiver::SetJitterBufferMinimumDelay( + absl::optional delay_seconds) { + source_->SetLatency(delay_seconds.value_or(kDefaultLatency)); +} + void AudioRtpReceiver::SetMediaChannel(cricket::MediaChannel* media_channel) { RTC_DCHECK(media_channel == nullptr || media_channel->media_type() == media_type()); diff --git a/pc/audio_rtp_receiver.h b/pc/audio_rtp_receiver.h index 82c8d88140..9ad5c61bbd 100644 --- a/pc/audio_rtp_receiver.h +++ b/pc/audio_rtp_receiver.h @@ -95,6 +95,9 @@ class AudioRtpReceiver : public ObserverInterface, streams) override; void SetObserver(RtpReceiverObserverInterface* observer) override; + void SetJitterBufferMinimumDelay( + absl::optional delay_seconds) override; + void SetMediaChannel(cricket::MediaChannel* media_channel) override; std::vector GetSources() const override; diff --git a/pc/playout_latency.cc b/pc/playout_latency.cc index be78439a90..aa7f950527 100644 --- a/pc/playout_latency.cc +++ b/pc/playout_latency.cc @@ -10,8 +10,6 @@ #include "pc/playout_latency.h" -#include "iostream" - #include "rtc_base/checks.h" #include "rtc_base/location.h" #include "rtc_base/logging.h" @@ -21,6 +19,7 @@ namespace { constexpr int kDefaultLatency = 0; +constexpr int kMaximumDelayMs = 10000; constexpr int kRoundToZeroThresholdMs = 10; } // namespace @@ -52,8 +51,6 @@ void PlayoutLatency::OnStop() { void PlayoutLatency::SetLatency(double latency) { RTC_DCHECK_RUN_ON(worker_thread_); - RTC_DCHECK_GE(latency, 0); - RTC_DCHECK_LE(latency, 10); int delay_ms = rtc::dchecked_cast(latency * 1000); // In JitterBuffer 0 delay has special meaning of being unconstrained value @@ -63,6 +60,10 @@ void PlayoutLatency::SetLatency(double latency) { delay_ms = 0; } + if (delay_ms > kMaximumDelayMs) { + delay_ms = kMaximumDelayMs; + } + cached_latency_ = latency; if (media_channel_ && ssrc_) { media_channel_->SetBaseMinimumPlayoutDelayMs(ssrc_.value(), delay_ms); diff --git a/pc/playout_latency_unittest.cc b/pc/playout_latency_unittest.cc index 0a671618c2..d603477379 100644 --- a/pc/playout_latency_unittest.cc +++ b/pc/playout_latency_unittest.cc @@ -100,4 +100,19 @@ TEST_F(PlayoutLatencyTest, Rounding) { latency_->SetLatency(0.005); } +TEST_F(PlayoutLatencyTest, Clamping) { + latency_->OnStart(&delayable_, kSsrc); + + // In current Jitter Buffer implementation (Audio or Video) maximum supported + // value is 10000 milliseconds. + EXPECT_CALL(delayable_, SetBaseMinimumPlayoutDelayMs(kSsrc, 10000)) + .WillOnce(Return(true)); + latency_->SetLatency(10.5); + + EXPECT_CALL(delayable_, SetBaseMinimumPlayoutDelayMs(kSsrc, 0)) + .WillOnce(Return(true)); + + latency_->SetLatency(-2.0); +} + } // namespace webrtc diff --git a/pc/rtp_sender_receiver_unittest.cc b/pc/rtp_sender_receiver_unittest.cc index c8d84f685e..e6d008378e 100644 --- a/pc/rtp_sender_receiver_unittest.cc +++ b/pc/rtp_sender_receiver_unittest.cc @@ -491,6 +491,17 @@ class RtpSenderReceiverTest EXPECT_DOUBLE_EQ(latency_s, delay_ms.value_or(0) / 1000.0); } + // Check that minimum Jitter Buffer delay is propagated to the underlying + // |media_channel|. + void VerifyRtpReceiverDelayBehaviour(cricket::Delayable* media_channel, + RtpReceiverInterface* receiver, + uint32_t ssrc) { + receiver->SetJitterBufferMinimumDelay(/*delay_seconds=*/0.5); + absl::optional delay_ms = + media_channel->GetBaseMinimumPlayoutDelayMs(ssrc); // In milliseconds. + EXPECT_DOUBLE_EQ(0.5, delay_ms.value_or(0) / 1000.0); + } + protected: rtc::Thread* const network_thread_; rtc::Thread* const worker_thread_; @@ -703,6 +714,20 @@ TEST_F(RtpSenderReceiverTest, RemoteVideoTrackLatency) { video_track_->GetSource(), kVideoSsrc); } +TEST_F(RtpSenderReceiverTest, AudioRtpReceiverDelay) { + CreateAudioRtpReceiver(); + VerifyRtpReceiverDelayBehaviour(voice_media_channel_, + audio_rtp_receiver_.get(), kAudioSsrc); + VerifyTrackLatencyBehaviour(voice_media_channel_, audio_track_.get(), + audio_track_->GetSource(), kAudioSsrc); +} + +TEST_F(RtpSenderReceiverTest, VideoRtpReceiverDelay) { + CreateVideoRtpReceiver(); + VerifyRtpReceiverDelayBehaviour(video_media_channel_, + video_rtp_receiver_.get(), kVideoSsrc); +} + // Test that the media channel isn't enabled for sending if the audio sender // doesn't have both a track and SSRC. TEST_F(RtpSenderReceiverTest, AudioSenderWithoutTrackAndSsrc) { diff --git a/pc/test/mock_rtp_receiver_internal.h b/pc/test/mock_rtp_receiver_internal.h index f948d83f2e..0838adabf1 100644 --- a/pc/test/mock_rtp_receiver_internal.h +++ b/pc/test/mock_rtp_receiver_internal.h @@ -35,6 +35,7 @@ class MockRtpReceiverInternal : public RtpReceiverInternal { MOCK_CONST_METHOD0(GetParameters, RtpParameters()); MOCK_METHOD1(SetParameters, bool(const RtpParameters&)); MOCK_METHOD1(SetObserver, void(RtpReceiverObserverInterface*)); + MOCK_METHOD1(SetJitterBufferMinimumDelay, void(absl::optional)); MOCK_CONST_METHOD0(GetSources, std::vector()); MOCK_METHOD1(SetFrameDecryptor, void(rtc::scoped_refptr)); diff --git a/pc/video_rtp_receiver.cc b/pc/video_rtp_receiver.cc index b112f048a9..5a2e77841f 100644 --- a/pc/video_rtp_receiver.cc +++ b/pc/video_rtp_receiver.cc @@ -26,6 +26,10 @@ namespace webrtc { +namespace { +constexpr double kDefaultLatency = 0.0; +} // namespace + VideoRtpReceiver::VideoRtpReceiver(rtc::Thread* worker_thread, std::string receiver_id, std::vector stream_ids) @@ -192,6 +196,11 @@ void VideoRtpReceiver::SetObserver(RtpReceiverObserverInterface* observer) { } } +void VideoRtpReceiver::SetJitterBufferMinimumDelay( + absl::optional delay_seconds) { + source_->SetLatency(delay_seconds.value_or(kDefaultLatency)); +} + void VideoRtpReceiver::SetMediaChannel(cricket::MediaChannel* media_channel) { RTC_DCHECK(media_channel == nullptr || media_channel->media_type() == media_type()); diff --git a/pc/video_rtp_receiver.h b/pc/video_rtp_receiver.h index 2795e7a2bf..9ad4fe7356 100644 --- a/pc/video_rtp_receiver.h +++ b/pc/video_rtp_receiver.h @@ -99,6 +99,9 @@ class VideoRtpReceiver : public rtc::RefCountedObject { void SetObserver(RtpReceiverObserverInterface* observer) override; + void SetJitterBufferMinimumDelay( + absl::optional delay_seconds) override; + void SetMediaChannel(cricket::MediaChannel* media_channel) override; int AttachmentId() const override { return attachment_id_; }