From bef7b058f5af9176f9bb551defb989d821b6696c Mon Sep 17 00:00:00 2001 From: Ivo Creusen Date: Tue, 8 Sep 2020 16:30:25 +0200 Subject: [PATCH] Make AV sync robust to failures to set a desired minimum delay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Setting a minimum delay can fail in some cases. It is important that the AV sync code is aware of failures and can act accordingly to recover and prevent sync delays that keep increasing indefinitely. Bug: webrtc:11805 Change-Id: I0deed951dc6c6d0905536a949af875e0a6d9f7fa Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/183360 Commit-Queue: Ivo Creusen Reviewed-by: Åsa Persson Reviewed-by: Björn Terelius Reviewed-by: Sam Zackrisson Cr-Commit-Position: refs/heads/master@{#32062} --- audio/audio_receive_stream.cc | 2 +- audio/audio_receive_stream.h | 2 +- audio/channel_receive.cc | 6 ++++-- audio/channel_receive.h | 2 +- audio/mock_voe_channel_proxy.h | 2 +- call/syncable.h | 2 +- video/rtp_streams_synchronizer2.cc | 8 ++++++-- video/stream_synchronization.cc | 8 ++++++++ video/stream_synchronization.h | 6 ++++++ video/video_receive_stream.cc | 3 ++- video/video_receive_stream.h | 2 +- video/video_receive_stream2.cc | 3 ++- video/video_receive_stream2.h | 2 +- 13 files changed, 35 insertions(+), 13 deletions(-) diff --git a/audio/audio_receive_stream.cc b/audio/audio_receive_stream.cc index 883116495d..0bd6ce69fa 100644 --- a/audio/audio_receive_stream.cc +++ b/audio/audio_receive_stream.cc @@ -329,7 +329,7 @@ void AudioReceiveStream::SetEstimatedPlayoutNtpTimestampMs( time_ms); } -void AudioReceiveStream::SetMinimumPlayoutDelay(int delay_ms) { +bool AudioReceiveStream::SetMinimumPlayoutDelay(int delay_ms) { RTC_DCHECK_RUN_ON(&module_process_thread_checker_); return channel_receive_->SetMinimumPlayoutDelay(delay_ms); } diff --git a/audio/audio_receive_stream.h b/audio/audio_receive_stream.h index 0b91efebd3..b58f60077a 100644 --- a/audio/audio_receive_stream.h +++ b/audio/audio_receive_stream.h @@ -87,7 +87,7 @@ class AudioReceiveStream final : public webrtc::AudioReceiveStream, int64_t* time_ms) const override; void SetEstimatedPlayoutNtpTimestampMs(int64_t ntp_timestamp_ms, int64_t time_ms) override; - void SetMinimumPlayoutDelay(int delay_ms) override; + bool SetMinimumPlayoutDelay(int delay_ms) override; void AssociateSendStream(AudioSendStream* send_stream); void DeliverRtcp(const uint8_t* packet, size_t length); diff --git a/audio/channel_receive.cc b/audio/channel_receive.cc index 9cbaabbbb0..14184be263 100644 --- a/audio/channel_receive.cc +++ b/audio/channel_receive.cc @@ -133,7 +133,7 @@ class ChannelReceive : public ChannelReceiveInterface { // Audio+Video Sync. uint32_t GetDelayEstimate() const override; - void SetMinimumPlayoutDelay(int delayMs) override; + bool SetMinimumPlayoutDelay(int delayMs) override; bool GetPlayoutRtpTimestamp(uint32_t* rtp_timestamp, int64_t* time_ms) const override; void SetEstimatedPlayoutNtpTimestampMs(int64_t ntp_timestamp_ms, @@ -822,7 +822,7 @@ uint32_t ChannelReceive::GetDelayEstimate() const { return acm_receiver_.FilteredCurrentDelayMs() + playout_delay_ms_; } -void ChannelReceive::SetMinimumPlayoutDelay(int delay_ms) { +bool ChannelReceive::SetMinimumPlayoutDelay(int delay_ms) { RTC_DCHECK(module_process_thread_checker_.IsCurrent()); // Limit to range accepted by both VoE and ACM, so we're at least getting as // close as possible, instead of failing. @@ -831,7 +831,9 @@ void ChannelReceive::SetMinimumPlayoutDelay(int delay_ms) { if (acm_receiver_.SetMinimumDelay(delay_ms) != 0) { RTC_DLOG(LS_ERROR) << "SetMinimumPlayoutDelay() failed to set min playout delay"; + return false; } + return true; } bool ChannelReceive::GetPlayoutRtpTimestamp(uint32_t* rtp_timestamp, diff --git a/audio/channel_receive.h b/audio/channel_receive.h index bc02ff3023..8a65861525 100644 --- a/audio/channel_receive.h +++ b/audio/channel_receive.h @@ -104,7 +104,7 @@ class ChannelReceiveInterface : public RtpPacketSinkInterface { // Audio+Video Sync. virtual uint32_t GetDelayEstimate() const = 0; - virtual void SetMinimumPlayoutDelay(int delay_ms) = 0; + virtual bool SetMinimumPlayoutDelay(int delay_ms) = 0; virtual bool GetPlayoutRtpTimestamp(uint32_t* rtp_timestamp, int64_t* time_ms) const = 0; virtual void SetEstimatedPlayoutNtpTimestampMs(int64_t ntp_timestamp_ms, diff --git a/audio/mock_voe_channel_proxy.h b/audio/mock_voe_channel_proxy.h index 542358f687..97d1069422 100644 --- a/audio/mock_voe_channel_proxy.h +++ b/audio/mock_voe_channel_proxy.h @@ -76,7 +76,7 @@ class MockChannelReceive : public voe::ChannelReceiveInterface { GetSyncInfo, (), (const, override)); - MOCK_METHOD(void, SetMinimumPlayoutDelay, (int delay_ms), (override)); + MOCK_METHOD(bool, SetMinimumPlayoutDelay, (int delay_ms), (override)); MOCK_METHOD(bool, SetBaseMinimumPlayoutDelayMs, (int delay_ms), (override)); MOCK_METHOD(int, GetBaseMinimumPlayoutDelayMs, (), (const, override)); MOCK_METHOD((absl::optional>), diff --git a/call/syncable.h b/call/syncable.h index 3bbe50c8d1..43b16a0720 100644 --- a/call/syncable.h +++ b/call/syncable.h @@ -37,7 +37,7 @@ class Syncable { virtual absl::optional GetInfo() const = 0; virtual bool GetPlayoutRtpTimestamp(uint32_t* rtp_timestamp, int64_t* time_ms) const = 0; - virtual void SetMinimumPlayoutDelay(int delay_ms) = 0; + virtual bool SetMinimumPlayoutDelay(int delay_ms) = 0; virtual void SetEstimatedPlayoutNtpTimestampMs(int64_t ntp_timestamp_ms, int64_t time_ms) = 0; }; diff --git a/video/rtp_streams_synchronizer2.cc b/video/rtp_streams_synchronizer2.cc index 49be355a38..cc084bc0aa 100644 --- a/video/rtp_streams_synchronizer2.cc +++ b/video/rtp_streams_synchronizer2.cc @@ -147,8 +147,12 @@ void RtpStreamsSynchronizer::UpdateDelay() { << "target_delay_ms: " << target_video_delay_ms << "} "; } - syncable_audio_->SetMinimumPlayoutDelay(target_audio_delay_ms); - syncable_video_->SetMinimumPlayoutDelay(target_video_delay_ms); + if (!syncable_audio_->SetMinimumPlayoutDelay(target_audio_delay_ms)) { + sync_->ReduceAudioDelay(); + } + if (!syncable_video_->SetMinimumPlayoutDelay(target_video_delay_ms)) { + sync_->ReduceVideoDelay(); + } } // TODO(https://bugs.webrtc.org/7065): Move RtpToNtpEstimator out of diff --git a/video/stream_synchronization.cc b/video/stream_synchronization.cc index 8c808f13c6..d5c77c1eca 100644 --- a/video/stream_synchronization.cc +++ b/video/stream_synchronization.cc @@ -184,4 +184,12 @@ void StreamSynchronization::SetTargetBufferingDelay(int target_delay_ms) { base_target_delay_ms_ = target_delay_ms; } +void StreamSynchronization::ReduceAudioDelay() { + audio_delay_.extra_ms *= 0.9f; +} + +void StreamSynchronization::ReduceVideoDelay() { + video_delay_.extra_ms *= 0.9f; +} + } // namespace webrtc diff --git a/video/stream_synchronization.h b/video/stream_synchronization.h index 1aba62d1e7..2da6a49a14 100644 --- a/video/stream_synchronization.h +++ b/video/stream_synchronization.h @@ -44,6 +44,12 @@ class StreamSynchronization { // |target_delay_ms|. void SetTargetBufferingDelay(int target_delay_ms); + // Lowers the audio delay by 10%. Can be used to recover from errors. + void ReduceAudioDelay(); + + // Lowers the video delay by 10%. Can be used to recover from errors. + void ReduceVideoDelay(); + uint32_t audio_stream_id() const { return audio_stream_id_; } uint32_t video_stream_id() const { return video_stream_id_; } diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc index 1158c41623..02eb9034fc 100644 --- a/video/video_receive_stream.cc +++ b/video/video_receive_stream.cc @@ -615,11 +615,12 @@ void VideoReceiveStream::SetEstimatedPlayoutNtpTimestampMs( RTC_NOTREACHED(); } -void VideoReceiveStream::SetMinimumPlayoutDelay(int delay_ms) { +bool VideoReceiveStream::SetMinimumPlayoutDelay(int delay_ms) { RTC_DCHECK_RUN_ON(&module_process_sequence_checker_); MutexLock lock(&playout_delay_lock_); syncable_minimum_playout_delay_ms_ = delay_ms; UpdatePlayoutDelays(); + return true; } int64_t VideoReceiveStream::GetWaitMs() const { diff --git a/video/video_receive_stream.h b/video/video_receive_stream.h index 72ddf61083..5fb9cf72da 100644 --- a/video/video_receive_stream.h +++ b/video/video_receive_stream.h @@ -126,7 +126,7 @@ class VideoReceiveStream : public webrtc::VideoReceiveStream, int64_t time_ms) override; // SetMinimumPlayoutDelay is only called by A/V sync. - void SetMinimumPlayoutDelay(int delay_ms) override; + bool SetMinimumPlayoutDelay(int delay_ms) override; std::vector GetSources() const override; diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc index 7fcb42b17b..21c188fe7c 100644 --- a/video/video_receive_stream2.cc +++ b/video/video_receive_stream2.cc @@ -607,10 +607,11 @@ void VideoReceiveStream2::SetEstimatedPlayoutNtpTimestampMs( RTC_NOTREACHED(); } -void VideoReceiveStream2::SetMinimumPlayoutDelay(int delay_ms) { +bool VideoReceiveStream2::SetMinimumPlayoutDelay(int delay_ms) { RTC_DCHECK_RUN_ON(&worker_sequence_checker_); syncable_minimum_playout_delay_ms_ = delay_ms; UpdatePlayoutDelays(); + return true; } int64_t VideoReceiveStream2::GetMaxWaitMs() const { diff --git a/video/video_receive_stream2.h b/video/video_receive_stream2.h index 594a538aab..9b152f420a 100644 --- a/video/video_receive_stream2.h +++ b/video/video_receive_stream2.h @@ -148,7 +148,7 @@ class VideoReceiveStream2 : public webrtc::VideoReceiveStream, int64_t time_ms) override; // SetMinimumPlayoutDelay is only called by A/V sync. - void SetMinimumPlayoutDelay(int delay_ms) override; + bool SetMinimumPlayoutDelay(int delay_ms) override; std::vector GetSources() const override;