commit aee5b17f669341d9af03ab4dc15b38382f423628
author Markus Handell <handellm@webrtc.org> Fri Apr 28 09:16:30 2023
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Apr 28 10:50:57 2023
tree 25c35703e19e0475bcd7b20db84f4564b5f4a907
parent 272b464e92d08d5abe36af5542f53bd6a82252c5

DecodeSynchronizer: avoid duplicate tick callback registration.

With repeated CreateSynchronizedFrameScheduler/Stop calls, the
DecodeSynchronizer can register & keep multiple callbacks in
the metronome. Fix this to only keep at most one callback
installed.

Fixed: chromium:1434747
Change-Id: I61f67a871339dbcc7560e9d545a5217f361a9b87
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/303840
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Markus Handell <handellm@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39964}

video/decode_synchronizer.cc

diff --git a/video/decode_synchronizer.cc b/video/decode_synchronizer.cc
index 2b9a658..32702c2 100644
--- a/video/decode_synchronizer.cc
+++ b/video/decode_synchronizer.cc
@@ -175,6 +175,10 @@
 
 void DecodeSynchronizer::ScheduleNextTick() {
   RTC_DCHECK_RUN_ON(worker_queue_);
+  if (tick_scheduled_) {
+    return;
+  }
+  tick_scheduled_ = true;
   metronome_->RequestCallOnNextTick(
       SafeTask(safety_.flag(), [this] { OnTick(); }));
 }
@@ -182,6 +186,7 @@
 void DecodeSynchronizer::OnTick() {
   TRACE_EVENT0("webrtc", __func__);
   RTC_DCHECK_RUN_ON(worker_queue_);
+  tick_scheduled_ = false;
   expected_next_tick_ = clock_->CurrentTime() + metronome_->TickPeriod();
 
   for (auto* scheduler : schedulers_) {

video/decode_synchronizer.h

diff --git a/video/decode_synchronizer.h b/video/decode_synchronizer.h
index c6f8efd..1718186 100644
--- a/video/decode_synchronizer.h
+++ b/video/decode_synchronizer.h
@@ -129,6 +129,7 @@
   Timestamp expected_next_tick_ = Timestamp::PlusInfinity();
   std::set<SynchronizedFrameDecodeScheduler*> schedulers_
       RTC_GUARDED_BY(worker_queue_);
+  bool tick_scheduled_ = false;
   ScopedTaskSafetyDetached safety_;
 };
 

video/decode_synchronizer_unittest.cc

diff --git a/video/decode_synchronizer_unittest.cc b/video/decode_synchronizer_unittest.cc
index 7a0d833..fb48a7e 100644
--- a/video/decode_synchronizer_unittest.cc
+++ b/video/decode_synchronizer_unittest.cc
@@ -27,6 +27,8 @@
 using ::testing::_;
 using ::testing::Eq;
 using ::testing::Invoke;
+using ::testing::Mock;
+using ::testing::MockFunction;
 using ::testing::Return;
 
 namespace webrtc {
@@ -60,10 +62,10 @@
 };
 
 TEST_F(DecodeSynchronizerTest, AllFramesReadyBeforeNextTickDecoded) {
-  ::testing::MockFunction<void(uint32_t, Timestamp)> mock_callback1;
+  MockFunction<void(uint32_t, Timestamp)> mock_callback1;
   auto scheduler1 = decode_synchronizer_.CreateSynchronizedFrameScheduler();
 
-  testing::MockFunction<void(unsigned int, Timestamp)> mock_callback2;
+  MockFunction<void(unsigned int, Timestamp)> mock_callback2;
   auto scheduler2 = decode_synchronizer_.CreateSynchronizedFrameScheduler();
 
   {
@@ -97,7 +99,7 @@
 }
 
 TEST_F(DecodeSynchronizerTest, FramesNotDecodedIfDecodeTimeIsInNextInterval) {
-  ::testing::MockFunction<void(unsigned int, Timestamp)> mock_callback;
+  MockFunction<void(unsigned int, Timestamp)> mock_callback;
   auto scheduler = decode_synchronizer_.CreateSynchronizedFrameScheduler();
 
   uint32_t frame_rtp = 90000;
@@ -125,7 +127,7 @@
 }
 
 TEST_F(DecodeSynchronizerTest, FrameDecodedOnce) {
-  ::testing::MockFunction<void(unsigned int, Timestamp)> mock_callback;
+  MockFunction<void(unsigned int, Timestamp)> mock_callback;
   auto scheduler = decode_synchronizer_.CreateSynchronizedFrameScheduler();
 
   uint32_t frame_rtp = 90000;
@@ -149,7 +151,7 @@
 }
 
 TEST_F(DecodeSynchronizerTest, FrameWithDecodeTimeInPastDecodedImmediately) {
-  ::testing::MockFunction<void(unsigned int, Timestamp)> mock_callback;
+  MockFunction<void(unsigned int, Timestamp)> mock_callback;
   auto scheduler = decode_synchronizer_.CreateSynchronizedFrameScheduler();
 
   uint32_t frame_rtp = 90000;
@@ -171,7 +173,7 @@
 
 TEST_F(DecodeSynchronizerTest,
        FrameWithDecodeTimeFarBeforeNextTickDecodedImmediately) {
-  ::testing::MockFunction<void(unsigned int, Timestamp)> mock_callback;
+  MockFunction<void(unsigned int, Timestamp)> mock_callback;
   auto scheduler = decode_synchronizer_.CreateSynchronizedFrameScheduler();
 
   // Frame which would be behind by more than kMaxAllowedFrameDelay after
@@ -210,7 +212,7 @@
 }
 
 TEST_F(DecodeSynchronizerTest, FramesNotReleasedAfterStop) {
-  ::testing::MockFunction<void(unsigned int, Timestamp)> mock_callback;
+  MockFunction<void(unsigned int, Timestamp)> mock_callback;
   auto scheduler = decode_synchronizer_.CreateSynchronizedFrameScheduler();
 
   uint32_t frame_rtp = 90000;
@@ -249,4 +251,21 @@
   (std::move)(callback)();
 }
 
+TEST(DecodeSynchronizerStandaloneTest,
+     RegistersCallbackOnceDuringRepeatedRegistrations) {
+  GlobalSimulatedTimeController time_controller(Timestamp::Millis(4711));
+  Clock* clock(time_controller.GetClock());
+  MockMetronome metronome;
+  ON_CALL(metronome, TickPeriod).WillByDefault(Return(TimeDelta::Seconds(1)));
+  DecodeSynchronizer decode_synchronizer_(clock, &metronome,
+                                          time_controller.GetMainThread());
+  // Expect at most 1 call to register a callback.
+  EXPECT_CALL(metronome, RequestCallOnNextTick);
+  auto scheduler1 = decode_synchronizer_.CreateSynchronizedFrameScheduler();
+  scheduler1->Stop();
+  auto scheduler2 = decode_synchronizer_.CreateSynchronizedFrameScheduler();
+  Mock::VerifyAndClearExpectations(&metronome);
+  scheduler2->Stop();
+}
+
 }  // namespace webrtc