commit cc52f07087f121536382469b9ef3dd8bd29089a0
author Evan Shrubsole <eshr@webrtc.org> Tue May 24 13:00:00 2022
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue May 24 15:09:32 2022
tree 41fcf83661915f5950b9807ca0296b13df7ac787
parent 5ca01b5a5bbfd231dd111b7140f872d211fee4f9

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 <kron@google.com>
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36988}

modules/video_coding/timing.cc

diff --git a/modules/video_coding/timing.cc b/modules/video_coding/timing.cc
index b7103c4..7a559ba 100644
--- a/modules/video_coding/timing.cc
+++ b/modules/video_coding/timing.cc
@@ -25,6 +25,21 @@
 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 @@
 
 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;
   }
 }