From 428dcb2517a71b271a791629826b61a89b1b32ee Mon Sep 17 00:00:00 2001 From: Ruslan Burakov Date: Thu, 18 Apr 2019 17:49:49 +0200 Subject: [PATCH] Remove SetLatency/GetLatency from MediaSourceInterface API level Bug: webrtc:10287 Change-Id: I74fad31db98b75791085688438064f9510b0b6fa Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/133165 Commit-Queue: Ruslan Burakov Reviewed-by: Steve Anton Cr-Commit-Position: refs/heads/master@{#27692} --- api/media_stream_interface.cc | 4 - api/media_stream_interface.h | 7 -- api/rtp_receiver_interface.cc | 3 - api/rtp_receiver_interface.h | 5 +- api/video_track_source_proxy.h | 2 - pc/BUILD.gn | 10 +- pc/audio_rtp_receiver.cc | 16 ++- pc/audio_rtp_receiver.h | 4 + ...yout_latency.cc => jitter_buffer_delay.cc} | 42 +++---- ...layout_latency.h => jitter_buffer_delay.h} | 28 ++--- ...face.h => jitter_buffer_delay_interface.h} | 24 ++-- ...cy_proxy.h => jitter_buffer_delay_proxy.h} | 13 +- pc/jitter_buffer_delay_unittest.cc | 90 ++++++++++++++ pc/playout_latency_unittest.cc | 113 ------------------ pc/remote_audio_source.cc | 21 +--- pc/remote_audio_source.h | 6 - pc/rtp_sender_receiver_unittest.cc | 69 ----------- pc/video_rtp_receiver.cc | 20 ++-- pc/video_rtp_receiver.h | 29 +---- 19 files changed, 173 insertions(+), 333 deletions(-) rename pc/{playout_latency.cc => jitter_buffer_delay.cc} (58%) rename pc/{playout_latency.h => jitter_buffer_delay.h} (59%) rename pc/{playout_latency_interface.h => jitter_buffer_delay_interface.h} (54%) rename pc/{playout_latency_proxy.h => jitter_buffer_delay_proxy.h} (70%) create mode 100644 pc/jitter_buffer_delay_unittest.cc delete mode 100644 pc/playout_latency_unittest.cc diff --git a/api/media_stream_interface.cc b/api/media_stream_interface.cc index cf994cbeaa..73566c4a2f 100644 --- a/api/media_stream_interface.cc +++ b/api/media_stream_interface.cc @@ -32,8 +32,4 @@ const cricket::AudioOptions AudioSourceInterface::options() const { return {}; } -double MediaSourceInterface::GetLatency() const { - return 0.0; -} - } // namespace webrtc diff --git a/api/media_stream_interface.h b/api/media_stream_interface.h index ccacb4ca56..f4ea4a6ade 100644 --- a/api/media_stream_interface.h +++ b/api/media_stream_interface.h @@ -62,13 +62,6 @@ class RTC_EXPORT MediaSourceInterface : public rtc::RefCountInterface, virtual bool remote() const = 0; - // Sets the minimum latency of the remote source until audio playout. Actual - // observered latency may differ depending on the source. |latency| is in the - // range of [0.0, 10.0] seconds. - // TODO(kuddai) make pure virtual once not only remote tracks support latency. - virtual void SetLatency(double latency) {} - virtual double GetLatency() const; - protected: ~MediaSourceInterface() override = default; }; diff --git a/api/rtp_receiver_interface.cc b/api/rtp_receiver_interface.cc index 615a31dd10..52f72df45d 100644 --- a/api/rtp_receiver_interface.cc +++ b/api/rtp_receiver_interface.cc @@ -58,7 +58,4 @@ 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 d1ef137f0a..3934215cce 100644 --- a/api/rtp_receiver_interface.h +++ b/api/rtp_receiver_interface.h @@ -131,10 +131,9 @@ class RtpReceiverInterface : public rtc::RefCountInterface { // 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. + // value must be used. virtual void SetJitterBufferMinimumDelay( - absl::optional delay_seconds); + absl::optional delay_seconds) = 0; // TODO(zhihuang): Remove the default implementation once the subclasses // implement this. Currently, the only relevant subclass is the diff --git a/api/video_track_source_proxy.h b/api/video_track_source_proxy.h index eb11befa32..820cdcb286 100644 --- a/api/video_track_source_proxy.h +++ b/api/video_track_source_proxy.h @@ -32,8 +32,6 @@ PROXY_WORKER_METHOD2(void, rtc::VideoSinkInterface*, const rtc::VideoSinkWants&) PROXY_WORKER_METHOD1(void, RemoveSink, rtc::VideoSinkInterface*) -PROXY_WORKER_METHOD1(void, SetLatency, double) -PROXY_WORKER_CONSTMETHOD0(double, GetLatency) PROXY_METHOD1(void, RegisterObserver, ObserverInterface*) PROXY_METHOD1(void, UnregisterObserver, ObserverInterface*) END_PROXY_MAP() diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 5ff4b16a23..16973e47f8 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -135,6 +135,10 @@ rtc_static_library("peerconnection") { "dtmf_sender.h", "ice_server_parsing.cc", "ice_server_parsing.h", + "jitter_buffer_delay.cc", + "jitter_buffer_delay.h", + "jitter_buffer_delay_interface.h", + "jitter_buffer_delay_proxy.h", "jsep_ice_candidate.cc", "jsep_session_description.cc", "local_audio_source.cc", @@ -149,10 +153,6 @@ rtc_static_library("peerconnection") { "peer_connection_factory.cc", "peer_connection_factory.h", "peer_connection_internal.h", - "playout_latency.cc", - "playout_latency.h", - "playout_latency_interface.h", - "playout_latency_proxy.h", "remote_audio_source.cc", "remote_audio_source.h", "rtc_stats_collector.cc", @@ -447,6 +447,7 @@ if (rtc_include_tests) { "data_channel_unittest.cc", "dtmf_sender_unittest.cc", "ice_server_parsing_unittest.cc", + "jitter_buffer_delay_unittest.cc", "jsep_session_description_unittest.cc", "local_audio_source_unittest.cc", "media_stream_unittest.cc", @@ -466,7 +467,6 @@ if (rtc_include_tests) { "peer_connection_simulcast_unittest.cc", "peer_connection_wrapper.cc", "peer_connection_wrapper.h", - "playout_latency_unittest.cc", "proxy_unittest.cc", "rtc_stats_collector_unittest.cc", "rtc_stats_integrationtest.cc", diff --git a/pc/audio_rtp_receiver.cc b/pc/audio_rtp_receiver.cc index 61ea3409e6..a470707383 100644 --- a/pc/audio_rtp_receiver.cc +++ b/pc/audio_rtp_receiver.cc @@ -17,6 +17,8 @@ #include "api/media_stream_proxy.h" #include "api/media_stream_track_proxy.h" #include "pc/audio_track.h" +#include "pc/jitter_buffer_delay.h" +#include "pc/jitter_buffer_delay_proxy.h" #include "pc/media_stream.h" #include "rtc_base/checks.h" #include "rtc_base/location.h" @@ -25,10 +27,6 @@ namespace webrtc { -namespace { -constexpr double kDefaultLatency = 0.0; -} // namespace - AudioRtpReceiver::AudioRtpReceiver(rtc::Thread* worker_thread, std::string receiver_id, std::vector stream_ids) @@ -46,7 +44,11 @@ AudioRtpReceiver::AudioRtpReceiver( track_(AudioTrackProxy::Create(rtc::Thread::Current(), AudioTrack::Create(receiver_id, source_))), cached_track_enabled_(track_->enabled()), - attachment_id_(GenerateUniqueId()) { + attachment_id_(GenerateUniqueId()), + delay_(JitterBufferDelayProxy::Create( + rtc::Thread::Current(), + worker_thread_, + new rtc::RefCountedObject(worker_thread))) { RTC_DCHECK(worker_thread_); RTC_DCHECK(track_->GetSource()->remote()); track_->RegisterObserver(this); @@ -162,9 +164,11 @@ void AudioRtpReceiver::SetupMediaChannel(uint32_t ssrc) { } if (ssrc_) { source_->Stop(media_channel_, *ssrc_); + delay_->OnStop(); } ssrc_ = ssrc; source_->Start(media_channel_, *ssrc_); + delay_->OnStart(media_channel_, *ssrc_); Reconfigure(); } @@ -238,7 +242,7 @@ void AudioRtpReceiver::SetObserver(RtpReceiverObserverInterface* observer) { void AudioRtpReceiver::SetJitterBufferMinimumDelay( absl::optional delay_seconds) { - source_->SetLatency(delay_seconds.value_or(kDefaultLatency)); + delay_->Set(delay_seconds); } void AudioRtpReceiver::SetMediaChannel(cricket::MediaChannel* media_channel) { diff --git a/pc/audio_rtp_receiver.h b/pc/audio_rtp_receiver.h index 9ad5c61bbd..bc80fbe841 100644 --- a/pc/audio_rtp_receiver.h +++ b/pc/audio_rtp_receiver.h @@ -22,6 +22,7 @@ #include "api/rtp_parameters.h" #include "api/scoped_refptr.h" #include "media/base/media_channel.h" +#include "pc/jitter_buffer_delay_interface.h" #include "pc/remote_audio_source.h" #include "pc/rtp_receiver.h" #include "rtc_base/ref_counted_object.h" @@ -122,6 +123,9 @@ class AudioRtpReceiver : public ObserverInterface, int attachment_id_ = 0; rtc::scoped_refptr frame_decryptor_; rtc::scoped_refptr dtls_transport_; + // Allows to thread safely change playout delay. Handles caching cases if + // |SetJitterBufferMinimumDelay| is called before start. + rtc::scoped_refptr delay_; }; } // namespace webrtc diff --git a/pc/playout_latency.cc b/pc/jitter_buffer_delay.cc similarity index 58% rename from pc/playout_latency.cc rename to pc/jitter_buffer_delay.cc index 063f2323a7..c9506b3c59 100644 --- a/pc/playout_latency.cc +++ b/pc/jitter_buffer_delay.cc @@ -8,7 +8,7 @@ * be found in the AUTHORS file in the root of the source tree. */ -#include "pc/playout_latency.h" +#include "pc/jitter_buffer_delay.h" #include "rtc_base/checks.h" #include "rtc_base/location.h" @@ -19,61 +19,49 @@ #include "rtc_base/thread_checker.h" namespace { -constexpr int kDefaultLatency = 0; +constexpr int kDefaultDelay = 0; constexpr int kMaximumDelayMs = 10000; } // namespace namespace webrtc { -PlayoutLatency::PlayoutLatency(rtc::Thread* worker_thread) +JitterBufferDelay::JitterBufferDelay(rtc::Thread* worker_thread) : signaling_thread_(rtc::Thread::Current()), worker_thread_(worker_thread) { RTC_DCHECK(worker_thread_); } -void PlayoutLatency::OnStart(cricket::Delayable* media_channel, uint32_t ssrc) { +void JitterBufferDelay::OnStart(cricket::Delayable* media_channel, + uint32_t ssrc) { RTC_DCHECK_RUN_ON(signaling_thread_); media_channel_ = media_channel; ssrc_ = ssrc; - // Trying to apply cached latency for the audio stream. - if (cached_latency_) { - SetLatency(cached_latency_.value()); + // Trying to apply cached delay for the audio stream. + if (cached_delay_seconds_) { + Set(cached_delay_seconds_.value()); } } -void PlayoutLatency::OnStop() { +void JitterBufferDelay::OnStop() { RTC_DCHECK_RUN_ON(signaling_thread_); - // Assume that audio stream is no longer present for latency calls. + // Assume that audio stream is no longer present. media_channel_ = nullptr; ssrc_ = absl::nullopt; } -void PlayoutLatency::SetLatency(double latency) { +void JitterBufferDelay::Set(absl::optional delay_seconds) { RTC_DCHECK_RUN_ON(worker_thread_); - int delay_ms = rtc::dchecked_cast(latency * 1000); + // TODO(kuddai) propagate absl::optional deeper down as default preference. + int delay_ms = + rtc::saturated_cast(delay_seconds.value_or(kDefaultDelay) * 1000); delay_ms = rtc::SafeClamp(delay_ms, 0, kMaximumDelayMs); - cached_latency_ = latency; + cached_delay_seconds_ = delay_seconds; if (media_channel_ && ssrc_) { media_channel_->SetBaseMinimumPlayoutDelayMs(ssrc_.value(), delay_ms); } } -double PlayoutLatency::GetLatency() const { - RTC_DCHECK_RUN_ON(worker_thread_); - - absl::optional delay_ms; - if (media_channel_ && ssrc_) { - delay_ms = media_channel_->GetBaseMinimumPlayoutDelayMs(ssrc_.value()); - } - - if (delay_ms) { - return delay_ms.value() / 1000.0; - } else { - return cached_latency_.value_or(kDefaultLatency); - } -} - } // namespace webrtc diff --git a/pc/playout_latency.h b/pc/jitter_buffer_delay.h similarity index 59% rename from pc/playout_latency.h rename to pc/jitter_buffer_delay.h index f51a927f6c..8edfc6ce20 100644 --- a/pc/playout_latency.h +++ b/pc/jitter_buffer_delay.h @@ -8,35 +8,33 @@ * be found in the AUTHORS file in the root of the source tree. */ -#ifndef PC_PLAYOUT_LATENCY_H_ -#define PC_PLAYOUT_LATENCY_H_ +#ifndef PC_JITTER_BUFFER_DELAY_H_ +#define PC_JITTER_BUFFER_DELAY_H_ #include #include "absl/types/optional.h" #include "media/base/delayable.h" -#include "pc/playout_latency_interface.h" +#include "pc/jitter_buffer_delay_interface.h" #include "rtc_base/thread.h" namespace webrtc { -// PlayoutLatency converts latency measured in seconds to delay measured in -// milliseconds for the underlying media channel. It also handles cases when -// user sets Latency before the start of media_channel by caching its request. -// Note, this class is not thread safe. Its thread safe version is defined in -// pc/playout_latency_proxy.h -class PlayoutLatency : public PlayoutLatencyInterface { +// JitterBufferDelay converts delay from seconds to milliseconds for the +// underlying media channel. It also handles cases when user sets delay before +// the start of media_channel by caching its request. Note, this class is not +// thread safe. Its thread safe version is defined in +// pc/jitter_buffer_delay_proxy.h +class JitterBufferDelay : public JitterBufferDelayInterface { public: // Must be called on signaling thread. - explicit PlayoutLatency(rtc::Thread* worker_thread); + explicit JitterBufferDelay(rtc::Thread* worker_thread); void OnStart(cricket::Delayable* media_channel, uint32_t ssrc) override; void OnStop() override; - void SetLatency(double latency) override; - - double GetLatency() const override; + void Set(absl::optional delay_seconds) override; private: // Throughout webrtc source, sometimes it is also called as |main_thread_|. @@ -45,9 +43,9 @@ class PlayoutLatency : public PlayoutLatencyInterface { // Media channel and ssrc together uniqely identify audio stream. cricket::Delayable* media_channel_ = nullptr; absl::optional ssrc_; - absl::optional cached_latency_; + absl::optional cached_delay_seconds_; }; } // namespace webrtc -#endif // PC_PLAYOUT_LATENCY_H_ +#endif // PC_JITTER_BUFFER_DELAY_H_ diff --git a/pc/playout_latency_interface.h b/pc/jitter_buffer_delay_interface.h similarity index 54% rename from pc/playout_latency_interface.h rename to pc/jitter_buffer_delay_interface.h index e9957337fe..f2132d318d 100644 --- a/pc/playout_latency_interface.h +++ b/pc/jitter_buffer_delay_interface.h @@ -8,36 +8,32 @@ * be found in the AUTHORS file in the root of the source tree. */ -#ifndef PC_PLAYOUT_LATENCY_INTERFACE_H_ -#define PC_PLAYOUT_LATENCY_INTERFACE_H_ +#ifndef PC_JITTER_BUFFER_DELAY_INTERFACE_H_ +#define PC_JITTER_BUFFER_DELAY_INTERFACE_H_ #include +#include "absl/types/optional.h" #include "media/base/delayable.h" #include "rtc_base/ref_count.h" namespace webrtc { -// PlayoutLatency delivers user's latency queries to the underlying media -// channel. It can describe either video or audio latency for receiving stream. -// "Interface" suffix in the interface name is required to be compatible with -// api/proxy.cc -class PlayoutLatencyInterface : public rtc::RefCountInterface { +// JitterBufferDelay delivers user's queries to the underlying media channel. It +// can describe either video or audio delay for receiving stream. "Interface" +// suffix in the interface name is required to be compatible with api/proxy.cc +class JitterBufferDelayInterface : public rtc::RefCountInterface { public: // OnStart allows to uniqely identify to which receiving stream playout - // latency must correpond through |media_channel| and |ssrc| pair. + // delay must correpond through |media_channel| and |ssrc| pair. virtual void OnStart(cricket::Delayable* media_channel, uint32_t ssrc) = 0; // Indicates that underlying receiving stream is stopped. virtual void OnStop() = 0; - // Sets latency in seconds. - virtual void SetLatency(double latency) = 0; - - // Returns latency in seconds. - virtual double GetLatency() const = 0; + virtual void Set(absl::optional delay_seconds) = 0; }; } // namespace webrtc -#endif // PC_PLAYOUT_LATENCY_INTERFACE_H_ +#endif // PC_JITTER_BUFFER_DELAY_INTERFACE_H_ diff --git a/pc/playout_latency_proxy.h b/pc/jitter_buffer_delay_proxy.h similarity index 70% rename from pc/playout_latency_proxy.h rename to pc/jitter_buffer_delay_proxy.h index 22f02c5622..b3380fd258 100644 --- a/pc/playout_latency_proxy.h +++ b/pc/jitter_buffer_delay_proxy.h @@ -8,25 +8,24 @@ * be found in the AUTHORS file in the root of the source tree. */ -#ifndef PC_PLAYOUT_LATENCY_PROXY_H_ -#define PC_PLAYOUT_LATENCY_PROXY_H_ +#ifndef PC_JITTER_BUFFER_DELAY_PROXY_H_ +#define PC_JITTER_BUFFER_DELAY_PROXY_H_ #include #include "api/proxy.h" #include "media/base/delayable.h" -#include "pc/playout_latency_interface.h" +#include "pc/jitter_buffer_delay_interface.h" namespace webrtc { -BEGIN_PROXY_MAP(PlayoutLatency) +BEGIN_PROXY_MAP(JitterBufferDelay) PROXY_SIGNALING_THREAD_DESTRUCTOR() PROXY_METHOD2(void, OnStart, cricket::Delayable*, uint32_t) PROXY_METHOD0(void, OnStop) -PROXY_WORKER_METHOD1(void, SetLatency, double) -PROXY_WORKER_CONSTMETHOD0(double, GetLatency) +PROXY_WORKER_METHOD1(void, Set, absl::optional) END_PROXY_MAP() } // namespace webrtc -#endif // PC_PLAYOUT_LATENCY_PROXY_H_ +#endif // PC_JITTER_BUFFER_DELAY_PROXY_H_ diff --git a/pc/jitter_buffer_delay_unittest.cc b/pc/jitter_buffer_delay_unittest.cc new file mode 100644 index 0000000000..383a4b7382 --- /dev/null +++ b/pc/jitter_buffer_delay_unittest.cc @@ -0,0 +1,90 @@ +/* + * Copyright 2019 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include + +#include "absl/types/optional.h" +#include "api/scoped_refptr.h" +#include "pc/jitter_buffer_delay.h" +#include "pc/test/mock_delayable.h" +#include "rtc_base/ref_counted_object.h" +#include "rtc_base/thread.h" +#include "test/gmock.h" +#include "test/gtest.h" + +using ::testing::Return; + +namespace { +constexpr int kSsrc = 1234; +} // namespace + +namespace webrtc { + +class JitterBufferDelayTest : public ::testing::Test { + public: + JitterBufferDelayTest() + : delay_(new rtc::RefCountedObject( + rtc::Thread::Current())) {} + + protected: + rtc::scoped_refptr delay_; + MockDelayable delayable_; +}; + +TEST_F(JitterBufferDelayTest, Set) { + delay_->OnStart(&delayable_, kSsrc); + + EXPECT_CALL(delayable_, SetBaseMinimumPlayoutDelayMs(kSsrc, 3000)) + .WillOnce(Return(true)); + + // Delay in seconds. + delay_->Set(3.0); +} + +TEST_F(JitterBufferDelayTest, Caching) { + // Check that value is cached before start. + delay_->Set(4.0); + + // Check that cached value applied on the start. + EXPECT_CALL(delayable_, SetBaseMinimumPlayoutDelayMs(kSsrc, 4000)) + .WillOnce(Return(true)); + delay_->OnStart(&delayable_, kSsrc); +} + +TEST_F(JitterBufferDelayTest, Clamping) { + delay_->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)); + delay_->Set(10.5); + + // Test int overflow. + EXPECT_CALL(delayable_, SetBaseMinimumPlayoutDelayMs(kSsrc, 10000)) + .WillOnce(Return(true)); + delay_->Set(21474836470.0); + + EXPECT_CALL(delayable_, SetBaseMinimumPlayoutDelayMs(kSsrc, 0)) + .WillOnce(Return(true)); + delay_->Set(-21474836470.0); + + // Boundary value in seconds to milliseconds conversion. + EXPECT_CALL(delayable_, SetBaseMinimumPlayoutDelayMs(kSsrc, 0)) + .WillOnce(Return(true)); + delay_->Set(0.0009); + + EXPECT_CALL(delayable_, SetBaseMinimumPlayoutDelayMs(kSsrc, 0)) + .WillOnce(Return(true)); + + delay_->Set(-2.0); +} + +} // namespace webrtc diff --git a/pc/playout_latency_unittest.cc b/pc/playout_latency_unittest.cc deleted file mode 100644 index db139f4e76..0000000000 --- a/pc/playout_latency_unittest.cc +++ /dev/null @@ -1,113 +0,0 @@ -/* - * Copyright 2019 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include - -#include "absl/types/optional.h" -#include "api/scoped_refptr.h" -#include "pc/playout_latency.h" -#include "pc/test/mock_delayable.h" -#include "rtc_base/ref_counted_object.h" -#include "rtc_base/thread.h" -#include "test/gmock.h" -#include "test/gtest.h" - -using ::testing::Return; - -namespace { -constexpr int kSsrc = 1234; -} // namespace - -namespace webrtc { - -class PlayoutLatencyTest : public ::testing::Test { - public: - PlayoutLatencyTest() - : latency_( - new rtc::RefCountedObject(rtc::Thread::Current())) { - } - - protected: - rtc::scoped_refptr latency_; - MockDelayable delayable_; -}; - -TEST_F(PlayoutLatencyTest, DefaultValue) { - EXPECT_DOUBLE_EQ(0.0, latency_->GetLatency()); -} - -TEST_F(PlayoutLatencyTest, GetLatency) { - latency_->OnStart(&delayable_, kSsrc); - - EXPECT_CALL(delayable_, GetBaseMinimumPlayoutDelayMs(kSsrc)) - .WillOnce(Return(2000)); - // Latency in seconds. - EXPECT_DOUBLE_EQ(2.0, latency_->GetLatency()); - - EXPECT_CALL(delayable_, GetBaseMinimumPlayoutDelayMs(kSsrc)) - .WillOnce(Return(absl::nullopt)); - // When no value is returned by GetBaseMinimumPlayoutDelayMs, and there are - // no caching, then return default value. - EXPECT_DOUBLE_EQ(0.0, latency_->GetLatency()); -} - -TEST_F(PlayoutLatencyTest, SetLatency) { - latency_->OnStart(&delayable_, kSsrc); - - EXPECT_CALL(delayable_, SetBaseMinimumPlayoutDelayMs(kSsrc, 3000)) - .WillOnce(Return(true)); - - // Latency in seconds. - latency_->SetLatency(3.0); -} - -TEST_F(PlayoutLatencyTest, Caching) { - // Check that value is cached before start. - latency_->SetLatency(4.0); - // Latency in seconds. - EXPECT_DOUBLE_EQ(4.0, latency_->GetLatency()); - - // Check that cached value applied on the start. - EXPECT_CALL(delayable_, SetBaseMinimumPlayoutDelayMs(kSsrc, 4000)) - .WillOnce(Return(true)); - latency_->OnStart(&delayable_, kSsrc); - - EXPECT_CALL(delayable_, GetBaseMinimumPlayoutDelayMs(kSsrc)) - .WillOnce(Return(absl::nullopt)); - // On false the latest cached value is returned. - EXPECT_DOUBLE_EQ(4.0, latency_->GetLatency()); - - latency_->OnStop(); - - // Check that after stop it returns last cached value. - EXPECT_DOUBLE_EQ(4.0, latency_->GetLatency()); -} - -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); - - // Boundary value in seconds to milliseconds conversion. - EXPECT_CALL(delayable_, SetBaseMinimumPlayoutDelayMs(kSsrc, 0)) - .WillOnce(Return(true)); - latency_->SetLatency(0.0009); - - EXPECT_CALL(delayable_, SetBaseMinimumPlayoutDelayMs(kSsrc, 0)) - .WillOnce(Return(true)); - - latency_->SetLatency(-2.0); -} - -} // namespace webrtc diff --git a/pc/remote_audio_source.cc b/pc/remote_audio_source.cc index aba44006bf..ed8ef2de52 100644 --- a/pc/remote_audio_source.cc +++ b/pc/remote_audio_source.cc @@ -16,8 +16,6 @@ #include "absl/algorithm/container.h" #include "absl/memory/memory.h" #include "api/scoped_refptr.h" -#include "pc/playout_latency.h" -#include "pc/playout_latency_proxy.h" #include "rtc_base/checks.h" #include "rtc_base/constructor_magic.h" #include "rtc_base/location.h" @@ -52,11 +50,7 @@ class RemoteAudioSource::AudioDataProxy : public AudioSinkInterface { RemoteAudioSource::RemoteAudioSource(rtc::Thread* worker_thread) : main_thread_(rtc::Thread::Current()), worker_thread_(worker_thread), - state_(MediaSourceInterface::kLive), - latency_(PlayoutLatencyProxy::Create( - main_thread_, - worker_thread_, - new rtc::RefCountedObject(worker_thread))) { + state_(MediaSourceInterface::kLive) { RTC_DCHECK(main_thread_); RTC_DCHECK(worker_thread_); } @@ -79,9 +73,6 @@ void RemoteAudioSource::Start(cricket::VoiceMediaChannel* media_channel, media_channel->SetRawAudioSink(ssrc, absl::make_unique(this)); }); - - // Apply latency to the audio stream if |SetLatency| was called before. - latency_->OnStart(media_channel, ssrc); } void RemoteAudioSource::Stop(cricket::VoiceMediaChannel* media_channel, @@ -89,8 +80,6 @@ void RemoteAudioSource::Stop(cricket::VoiceMediaChannel* media_channel, RTC_DCHECK_RUN_ON(main_thread_); RTC_DCHECK(media_channel); - latency_->OnStop(); - worker_thread_->Invoke( RTC_FROM_HERE, [&] { media_channel->SetRawAudioSink(ssrc, nullptr); }); } @@ -113,14 +102,6 @@ void RemoteAudioSource::SetVolume(double volume) { } } -void RemoteAudioSource::SetLatency(double latency) { - latency_->SetLatency(latency); -} - -double RemoteAudioSource::GetLatency() const { - return latency_->GetLatency(); -} - void RemoteAudioSource::RegisterAudioObserver(AudioObserver* observer) { RTC_DCHECK(observer != NULL); RTC_DCHECK(!absl::c_linear_search(audio_observers_, observer)); diff --git a/pc/remote_audio_source.h b/pc/remote_audio_source.h index f4a5ec8257..399e7e3a44 100644 --- a/pc/remote_audio_source.h +++ b/pc/remote_audio_source.h @@ -17,7 +17,6 @@ #include "api/call/audio_sink.h" #include "api/notifier.h" #include "pc/channel.h" -#include "pc/playout_latency_interface.h" #include "rtc_base/critical_section.h" #include "rtc_base/message_handler.h" @@ -47,8 +46,6 @@ class RemoteAudioSource : public Notifier, // AudioSourceInterface implementation. void SetVolume(double volume) override; - void SetLatency(double latency) override; - double GetLatency() const override; void RegisterAudioObserver(AudioObserver* observer) override; void UnregisterAudioObserver(AudioObserver* observer) override; @@ -72,9 +69,6 @@ class RemoteAudioSource : public Notifier, rtc::CriticalSection sink_lock_; std::list sinks_; SourceState state_; - // Allows to thread safely change playout latency. Handles caching cases if - // |SetLatency| is called before start. - rtc::scoped_refptr latency_; }; } // namespace webrtc diff --git a/pc/rtp_sender_receiver_unittest.cc b/pc/rtp_sender_receiver_unittest.cc index 67cbe2ca4f..90ca03fa8e 100644 --- a/pc/rtp_sender_receiver_unittest.cc +++ b/pc/rtp_sender_receiver_unittest.cc @@ -459,41 +459,6 @@ class RtpSenderReceiverTest RunSetLastLayerAsInactiveTest(video_rtp_sender_.get()); } - void VerifyTrackLatencyBehaviour(cricket::Delayable* media_channel, - MediaStreamTrackInterface* track, - MediaSourceInterface* source, - uint32_t ssrc) { - absl::optional delay_ms; // In milliseconds. - double latency_s = 0.5; // In seconds. - - source->SetLatency(latency_s); - delay_ms = media_channel->GetBaseMinimumPlayoutDelayMs(ssrc); - EXPECT_DOUBLE_EQ(latency_s, delay_ms.value_or(0) / 1000.0); - - // Disabling the track should take no effect on previously set value. - track->set_enabled(false); - delay_ms = media_channel->GetBaseMinimumPlayoutDelayMs(ssrc); - EXPECT_DOUBLE_EQ(latency_s, delay_ms.value_or(0) / 1000.0); - - // When the track is disabled, we still should be able to set latency. - latency_s = 0.3; - source->SetLatency(latency_s); - delay_ms = media_channel->GetBaseMinimumPlayoutDelayMs(ssrc); - EXPECT_DOUBLE_EQ(latency_s, delay_ms.value_or(0) / 1000.0); - - // Enabling the track should take no effect on previously set value. - track->set_enabled(true); - delay_ms = media_channel->GetBaseMinimumPlayoutDelayMs(ssrc); - EXPECT_DOUBLE_EQ(latency_s, delay_ms.value_or(0) / 1000.0); - - // We still should be able to change latency. - latency_s = 0.0; - source->SetLatency(latency_s); - delay_ms = media_channel->GetBaseMinimumPlayoutDelayMs(ssrc); - EXPECT_EQ(0, delay_ms.value_or(-1)); - 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, @@ -687,44 +652,10 @@ TEST_F(RtpSenderReceiverTest, RemoteAudioTrackSetVolume) { DestroyAudioRtpReceiver(); } -TEST_F(RtpSenderReceiverTest, RemoteAudioSourceLatency) { - absl::optional delay_ms; // In milliseconds. - rtc::scoped_refptr source = - new rtc::RefCountedObject(rtc::Thread::Current()); - - // Set it to value different from default zero. - voice_media_channel_->SetBaseMinimumPlayoutDelayMs(kAudioSsrc, 300); - - // Check that calling GetLatency on the source that hasn't been started yet - // won't trigger caching and return default value. - EXPECT_DOUBLE_EQ(source->GetLatency(), 0); - - // Check that cached latency will be applied on start. - source->SetLatency(0.4); - EXPECT_DOUBLE_EQ(source->GetLatency(), 0.4); - source->Start(voice_media_channel_, kAudioSsrc); - delay_ms = voice_media_channel_->GetBaseMinimumPlayoutDelayMs(kAudioSsrc); - EXPECT_EQ(400, delay_ms); -} - -TEST_F(RtpSenderReceiverTest, RemoteAudioTrackLatency) { - CreateAudioRtpReceiver(); - VerifyTrackLatencyBehaviour(voice_media_channel_, audio_track_.get(), - audio_track_->GetSource(), kAudioSsrc); -} - -TEST_F(RtpSenderReceiverTest, RemoteVideoTrackLatency) { - CreateVideoRtpReceiver(); - VerifyTrackLatencyBehaviour(video_media_channel_, video_track_.get(), - 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) { diff --git a/pc/video_rtp_receiver.cc b/pc/video_rtp_receiver.cc index 5a2e77841f..7a9ec984fa 100644 --- a/pc/video_rtp_receiver.cc +++ b/pc/video_rtp_receiver.cc @@ -17,6 +17,8 @@ #include "api/media_stream_proxy.h" #include "api/media_stream_track_proxy.h" #include "api/video_track_source_proxy.h" +#include "pc/jitter_buffer_delay.h" +#include "pc/jitter_buffer_delay_proxy.h" #include "pc/media_stream.h" #include "pc/video_track.h" #include "rtc_base/checks.h" @@ -26,10 +28,6 @@ namespace webrtc { -namespace { -constexpr double kDefaultLatency = 0.0; -} // namespace - VideoRtpReceiver::VideoRtpReceiver(rtc::Thread* worker_thread, std::string receiver_id, std::vector stream_ids) @@ -43,7 +41,7 @@ VideoRtpReceiver::VideoRtpReceiver( const std::vector>& streams) : worker_thread_(worker_thread), id_(receiver_id), - source_(new RefCountedObject(worker_thread_)), + source_(new RefCountedObject()), track_(VideoTrackProxy::Create( rtc::Thread::Current(), worker_thread, @@ -53,7 +51,11 @@ VideoRtpReceiver::VideoRtpReceiver( worker_thread, source_), worker_thread))), - attachment_id_(GenerateUniqueId()) { + attachment_id_(GenerateUniqueId()), + delay_(JitterBufferDelayProxy::Create( + rtc::Thread::Current(), + worker_thread, + new rtc::RefCountedObject(worker_thread))) { RTC_DCHECK(worker_thread_); SetStreams(streams); source_->SetState(MediaSourceInterface::kLive); @@ -127,7 +129,7 @@ void VideoRtpReceiver::Stop() { // media channel has already been deleted. SetSink(nullptr); } - source_->Stop(); + delay_->OnStop(); stopped_ = true; } @@ -148,7 +150,7 @@ void VideoRtpReceiver::SetupMediaChannel(uint32_t ssrc) { MaybeAttachFrameDecryptorToMediaChannel( ssrc_, worker_thread_, frame_decryptor_, media_channel_, stopped_); - source_->Start(media_channel_, ssrc); + delay_->OnStart(media_channel_, ssrc); } void VideoRtpReceiver::set_stream_ids(std::vector stream_ids) { @@ -198,7 +200,7 @@ void VideoRtpReceiver::SetObserver(RtpReceiverObserverInterface* observer) { void VideoRtpReceiver::SetJitterBufferMinimumDelay( absl::optional delay_seconds) { - source_->SetLatency(delay_seconds.value_or(kDefaultLatency)); + delay_->Set(delay_seconds); } void VideoRtpReceiver::SetMediaChannel(cricket::MediaChannel* media_channel) { diff --git a/pc/video_rtp_receiver.h b/pc/video_rtp_receiver.h index 9ad4fe7356..10354b0728 100644 --- a/pc/video_rtp_receiver.h +++ b/pc/video_rtp_receiver.h @@ -27,8 +27,7 @@ #include "api/video/video_source_interface.h" #include "media/base/media_channel.h" #include "media/base/video_broadcaster.h" -#include "pc/playout_latency.h" -#include "pc/playout_latency_proxy.h" +#include "pc/jitter_buffer_delay_interface.h" #include "pc/rtp_receiver.h" #include "pc/video_track_source.h" #include "rtc_base/ref_counted_object.h" @@ -108,42 +107,23 @@ class VideoRtpReceiver : public rtc::RefCountedObject { std::vector GetSources() const override; + private: class VideoRtpTrackSource : public VideoTrackSource { public: - explicit VideoRtpTrackSource(rtc::Thread* worker_thread) - : VideoTrackSource(true /* remote */), - latency_(PlayoutLatencyProxy::Create( - rtc::Thread::Current(), - worker_thread, - new rtc::RefCountedObject(worker_thread))) {} + VideoRtpTrackSource() : VideoTrackSource(true /* remote */) {} rtc::VideoSourceInterface* source() override { return &broadcaster_; } rtc::VideoSinkInterface* sink() { return &broadcaster_; } - void SetLatency(double latency) override { latency_->SetLatency(latency); } - - void Start(cricket::VideoMediaChannel* media_channel, uint32_t ssrc) { - latency_->OnStart(media_channel, ssrc); - } - - void Stop() { latency_->OnStop(); } - - double GetLatency() const override { return latency_->GetLatency(); } - private: - // Allows to thread safely change playout latency. Handles caching cases if - // |SetLatency| is called before start. - rtc::scoped_refptr latency_; - // |broadcaster_| is needed since the decoder can only handle one sink. // It might be better if the decoder can handle multiple sinks and consider // the VideoSinkWants. rtc::VideoBroadcaster broadcaster_; }; - private: bool SetSink(rtc::VideoSinkInterface* sink); rtc::Thread* const worker_thread_; @@ -161,6 +141,9 @@ class VideoRtpReceiver : public rtc::RefCountedObject { int attachment_id_ = 0; rtc::scoped_refptr frame_decryptor_; rtc::scoped_refptr dtls_transport_; + // Allows to thread safely change jitter buffer delay. Handles caching cases + // if |SetJitterBufferMinimumDelay| is called before start. + rtc::scoped_refptr delay_; }; } // namespace webrtc