From cc52f07087f121536382469b9ef3dd8bd29089a0 Mon Sep 17 00:00:00 2001 From: Evan Shrubsole Date: Tue, 24 May 2022 15:00:00 +0200 Subject: [PATCH] Log conflicting video playout delays when min delay set This is a continuation of https://webrtc-review.googlesource.com/c/src/+/263202 which added logging for max delay. However, if the max delay was already set and a new min delay was set this logging could have been missed. Bug: None Change-Id: I2e7e5bdf920fa68aa723ec8480d564b322813712 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/263480 Reviewed-by: Johannes Kron Commit-Queue: Evan Shrubsole Reviewed-by: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/main@{#36988} --- modules/video_coding/timing.cc | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/modules/video_coding/timing.cc b/modules/video_coding/timing.cc index b7103c4c15..7a559ba1f2 100644 --- a/modules/video_coding/timing.cc +++ b/modules/video_coding/timing.cc @@ -25,6 +25,21 @@ namespace { constexpr TimeDelta kZeroPlayoutDelayDefaultMinPacing = TimeDelta::Millis(8); constexpr TimeDelta kLowLatencyRendererMaxPlayoutDelay = TimeDelta::Millis(500); +void CheckDelaysValid(TimeDelta min_delay, TimeDelta max_delay) { + if (min_delay > max_delay) { + RTC_LOG(LS_ERROR) + << "Playout delays set incorrectly: min playout delay (" << min_delay + << ") > max playout delay (" << max_delay + << "). This is undefined behaviour. Application writers should " + "ensure that the min delay is always less than or equals max " + "delay. If trying to use the playout delay header extensions " + "described in " + "https://webrtc.googlesource.com/src/+/refs/heads/main/docs/" + "native-code/rtp-hdrext/playout-delay/, be careful that a playout " + "delay hint or A/V sync settings may have caused this conflict."; + } +} + } // namespace VCMTiming::VCMTiming(Clock* clock, const FieldTrialsView& field_trials) @@ -69,21 +84,16 @@ TimeDelta VCMTiming::min_playout_delay() const { void VCMTiming::set_min_playout_delay(TimeDelta min_playout_delay) { MutexLock lock(&mutex_); - min_playout_delay_ = min_playout_delay; + if (min_playout_delay_ != min_playout_delay) { + CheckDelaysValid(min_playout_delay, max_playout_delay_); + min_playout_delay_ = min_playout_delay; + } } void VCMTiming::set_max_playout_delay(TimeDelta max_playout_delay) { MutexLock lock(&mutex_); if (max_playout_delay_ != max_playout_delay) { - if (min_playout_delay_ > max_playout_delay) { - RTC_LOG(LS_ERROR) - << "Playout delays set incorrectly: min playout delay (" - << min_playout_delay_ << ") > max playout delay (" - << max_playout_delay - << "). This is undefined behaviour. Application writers should " - "ensure that the min delay is always less than or equals max " - "delay."; - } + CheckDelaysValid(min_playout_delay_, max_playout_delay); max_playout_delay_ = max_playout_delay; } }