Clean up FrameDecodeScheduler

* Migrate callback to one-time absl::AnyInvocable.
* Clean tests to use MockFunction.
* Use main thread instead of helper function in unittests.
* Fix some spelling mistakes.

Bug: None
Change-Id: I6145f5f5e2748dfa5278898cfdfd762c1840ff8d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/274170
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38016}

video/BUILD.gn

@@ -278,7 +278,10 @@ rtc_source_set("frame_decode_scheduler") {
     ":frame_decode_timing",
     "../api/units:timestamp",
   ]
-  absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ]
+  absl_deps = [
+    "//third_party/abseil-cpp/absl/functional:any_invocable",
+    "//third_party/abseil-cpp/absl/types:optional",
+  ]
 }
 
 rtc_library("task_queue_frame_decode_scheduler") {

video/decode_synchronizer.cc

@@ -38,7 +38,7 @@ void DecodeSynchronizer::ScheduledFrame::RunFrameReleaseCallback() && {
   // before execution to ensure internal state is cleared after callback
   // execution.
   auto sf = std::move(*this);
-  sf.callback_(sf.rtp_timestamp_, sf.schedule_.render_time);
+  std::move(sf.callback_)(sf.rtp_timestamp_, sf.schedule_.render_time);
 }
 
 Timestamp DecodeSynchronizer::ScheduledFrame::LatestDecodeTime() const {

video/decode_synchronizer.h

@@ -45,7 +45,7 @@ namespace webrtc {
 // DecodeSynchronizer, it will instead be executed on the metronome during the
 // tick interval where `max_decode_time` occurs. For example, if a frame is
 // scheduled for decode in 50ms and the tick interval is 20ms, then the frame
-// will be released for decoding in 2 ticks. See below for illustation,
+// will be released for decoding in 2 ticks. See below for illustration,
 //
 // In the case where the decode time is in the past, or must occur before the
 // next metronome tick then the frame will be released right away, allowing a

video/frame_decode_scheduler.h

@@ -13,8 +13,7 @@
 
 #include <stdint.h>
 
-#include <functional>
+#include "absl/functional/any_invocable.h"
-
 #include "absl/types/optional.h"
 #include "api/units/timestamp.h"
 #include "video/frame_decode_timing.h"
@@ -25,7 +24,8 @@ class FrameDecodeScheduler {
  public:
   // Invoked when a frame with `rtp_timestamp` is ready for decoding.
   using FrameReleaseCallback =
-      std::function<void(uint32_t rtp_timestamp, Timestamp render_time)>;
+      absl::AnyInvocable<void(uint32_t rtp_timestamp,
+                              Timestamp render_time) &&>;
 
   virtual ~FrameDecodeScheduler() = default;
 
@@ -33,8 +33,9 @@ class FrameDecodeScheduler {
   // `nullopt` if no frame is currently scheduled.
   virtual absl::optional<uint32_t> ScheduledRtpTimestamp() = 0;
 
-  // Shedules a frame for release based on `schedule`. When released, `callback`
+  // Schedules a frame for release based on `schedule`. When released,
-  // will be invoked with the `rtp` timestamp of the frame and the `render_time`
+  // `callback` will be invoked with the `rtp` timestamp of the frame and the
+  // `render_time`
   virtual void ScheduleFrame(uint32_t rtp,
                              FrameDecodeTiming::FrameSchedule schedule,
                              FrameReleaseCallback callback) = 0;

video/task_queue_frame_decode_scheduler.cc

@@ -14,6 +14,7 @@
 #include <utility>
 
 #include "api/sequence_checker.h"
+#include "api/task_queue/task_queue_base.h"
 #include "rtc_base/checks.h"
 
 namespace webrtc {
@@ -46,14 +47,14 @@ void TaskQueueFrameDecodeScheduler::ScheduleFrame(
       TimeDelta::Zero(), schedule.latest_decode_time - clock_->CurrentTime());
   bookkeeping_queue_->PostDelayedTask(
       SafeTask(task_safety_.flag(),
-               [this, rtp, schedule, cb = std::move(cb)] {
+               [this, rtp, schedule, cb = std::move(cb)]() mutable {
                  RTC_DCHECK_RUN_ON(bookkeeping_queue_);
                  // If the next frame rtp  has changed since this task was
                  // this scheduled  release should be skipped.
                  if (scheduled_rtp_ != rtp)
                    return;
                  scheduled_rtp_ = absl::nullopt;
-                 cb(rtp, schedule.render_time);
+                 std::move(cb)(rtp, schedule.render_time);
                }),
       wait);
 }

video/task_queue_frame_decode_scheduler_unittest.cc

@@ -15,121 +15,88 @@
 #include <memory>
 #include <utility>
 
-#include "absl/functional/any_invocable.h"
-#include "absl/functional/bind_front.h"
 #include "absl/types/optional.h"
 #include "api/units/time_delta.h"
 #include "api/units/timestamp.h"
-#include "rtc_base/task_queue.h"
 #include "test/gmock.h"
 #include "test/gtest.h"
 #include "test/time_controller/simulated_time_controller.h"
 
 namespace webrtc {
 
+using ::testing::_;
 using ::testing::Eq;
+using ::testing::MockFunction;
 using ::testing::Optional;
 
-class TaskQueueFrameDecodeSchedulerTest : public ::testing::Test {
+TEST(TaskQueueFrameDecodeSchedulerTest, FrameYieldedAfterSpecifiedPeriod) {
- public:
+  GlobalSimulatedTimeController time_controller_(Timestamp::Seconds(2000));
-  TaskQueueFrameDecodeSchedulerTest()
+  TaskQueueFrameDecodeScheduler scheduler(time_controller_.GetClock(),
-      : time_controller_(Timestamp::Millis(2000)),
+                                          time_controller_.GetMainThread());
-        task_queue_(time_controller_.GetTaskQueueFactory()->CreateTaskQueue(
-            "scheduler",
-            TaskQueueFactory::Priority::NORMAL)),
-        scheduler_(std::make_unique<TaskQueueFrameDecodeScheduler>(
-            time_controller_.GetClock(),
-            task_queue_.Get())) {}
-
-  ~TaskQueueFrameDecodeSchedulerTest() override {
-    if (scheduler_) {
-      OnQueue([&] {
-        scheduler_->Stop();
-        scheduler_ = nullptr;
-      });
-    }
-  }
-
-  void FrameReadyForDecode(uint32_t timestamp, Timestamp render_time) {
-    last_rtp_ = timestamp;
-    last_render_time_ = render_time;
-  }
-
- protected:
-  void OnQueue(absl::AnyInvocable<void() &&> t) {
-    task_queue_.PostTask(std::move(t));
-    time_controller_.AdvanceTime(TimeDelta::Zero());
-  }
-
-  GlobalSimulatedTimeController time_controller_;
-  rtc::TaskQueue task_queue_;
-  std::unique_ptr<FrameDecodeScheduler> scheduler_;
-  absl::optional<uint32_t> last_rtp_;
-  absl::optional<Timestamp> last_render_time_;
-};
-
-TEST_F(TaskQueueFrameDecodeSchedulerTest, FrameYieldedAfterSpecifiedPeriod) {
   constexpr TimeDelta decode_delay = TimeDelta::Millis(5);
+
   const Timestamp now = time_controller_.GetClock()->CurrentTime();
   const uint32_t rtp = 90000;
   const Timestamp render_time = now + TimeDelta::Millis(15);
   FrameDecodeTiming::FrameSchedule schedule = {
       .latest_decode_time = now + decode_delay, .render_time = render_time};
-  OnQueue([&] {
-    scheduler_->ScheduleFrame(
-        rtp, schedule,
-        absl::bind_front(
-            &TaskQueueFrameDecodeSchedulerTest::FrameReadyForDecode, this));
-    EXPECT_THAT(scheduler_->ScheduledRtpTimestamp(), Optional(rtp));
-  });
-  EXPECT_THAT(last_rtp_, Eq(absl::nullopt));
 
+  MockFunction<void(uint32_t, Timestamp)> ready_cb;
+  scheduler.ScheduleFrame(rtp, schedule, ready_cb.AsStdFunction());
+  EXPECT_CALL(ready_cb, Call(_, _)).Times(0);
+  EXPECT_THAT(scheduler.ScheduledRtpTimestamp(), Optional(rtp));
+  time_controller_.AdvanceTime(TimeDelta::Zero());
+  // Check that `ready_cb` has not been invoked yet.
+  ::testing::Mock::VerifyAndClearExpectations(&ready_cb);
+
+  EXPECT_CALL(ready_cb, Call(rtp, render_time)).Times(1);
   time_controller_.AdvanceTime(decode_delay);
-  EXPECT_THAT(last_rtp_, Optional(rtp));
+
-  EXPECT_THAT(last_render_time_, Optional(render_time));
+  scheduler.Stop();
 }
 
-TEST_F(TaskQueueFrameDecodeSchedulerTest, NegativeDecodeDelayIsRoundedToZero) {
+TEST(TaskQueueFrameDecodeSchedulerTest, NegativeDecodeDelayIsRoundedToZero) {
+  GlobalSimulatedTimeController time_controller_(Timestamp::Seconds(2000));
+  TaskQueueFrameDecodeScheduler scheduler(time_controller_.GetClock(),
+                                          time_controller_.GetMainThread());
   constexpr TimeDelta decode_delay = TimeDelta::Millis(-5);
   const Timestamp now = time_controller_.GetClock()->CurrentTime();
   const uint32_t rtp = 90000;
   const Timestamp render_time = now + TimeDelta::Millis(15);
   FrameDecodeTiming::FrameSchedule schedule = {
       .latest_decode_time = now + decode_delay, .render_time = render_time};
-  OnQueue([&] {
+
-    scheduler_->ScheduleFrame(
+  MockFunction<void(uint32_t, Timestamp)> ready_cb;
-        rtp, schedule,
+  EXPECT_CALL(ready_cb, Call(rtp, render_time)).Times(1);
-        absl::bind_front(
+  scheduler.ScheduleFrame(rtp, schedule, ready_cb.AsStdFunction());
-            &TaskQueueFrameDecodeSchedulerTest::FrameReadyForDecode, this));
+  EXPECT_THAT(scheduler.ScheduledRtpTimestamp(), Optional(rtp));
-    EXPECT_THAT(scheduler_->ScheduledRtpTimestamp(), Optional(rtp));
+  time_controller_.AdvanceTime(TimeDelta::Zero());
-  });
+
-  EXPECT_THAT(last_rtp_, Optional(rtp));
+  scheduler.Stop();
-  EXPECT_THAT(last_render_time_, Optional(render_time));
 }
 
-TEST_F(TaskQueueFrameDecodeSchedulerTest, CancelOutstanding) {
+TEST(TaskQueueFrameDecodeSchedulerTest, CancelOutstanding) {
+  GlobalSimulatedTimeController time_controller_(Timestamp::Seconds(2000));
+  TaskQueueFrameDecodeScheduler scheduler(time_controller_.GetClock(),
+                                          time_controller_.GetMainThread());
   constexpr TimeDelta decode_delay = TimeDelta::Millis(50);
   const Timestamp now = time_controller_.GetClock()->CurrentTime();
   const uint32_t rtp = 90000;
   FrameDecodeTiming::FrameSchedule schedule = {
       .latest_decode_time = now + decode_delay,
       .render_time = now + TimeDelta::Millis(75)};
-  OnQueue([&] {
+
-    scheduler_->ScheduleFrame(
+  MockFunction<void(uint32_t, Timestamp)> ready_cb;
-        rtp, schedule,
+  EXPECT_CALL(ready_cb, Call).Times(0);
-        absl::bind_front(
+  scheduler.ScheduleFrame(rtp, schedule, ready_cb.AsStdFunction());
-            &TaskQueueFrameDecodeSchedulerTest::FrameReadyForDecode, this));
+  EXPECT_THAT(scheduler.ScheduledRtpTimestamp(), Optional(rtp));
-    EXPECT_THAT(scheduler_->ScheduledRtpTimestamp(), Optional(rtp));
-  });
   time_controller_.AdvanceTime(decode_delay / 2);
-  OnQueue([&] {
+  EXPECT_THAT(scheduler.ScheduledRtpTimestamp(), Optional(rtp));
-    EXPECT_THAT(scheduler_->ScheduledRtpTimestamp(), Optional(rtp));
+  scheduler.CancelOutstanding();
-    scheduler_->CancelOutstanding();
+  EXPECT_THAT(scheduler.ScheduledRtpTimestamp(), Eq(absl::nullopt));
-    EXPECT_THAT(scheduler_->ScheduledRtpTimestamp(), Eq(absl::nullopt));
-  });
   time_controller_.AdvanceTime(decode_delay / 2);
-  EXPECT_THAT(last_rtp_, Eq(absl::nullopt));
+
-  EXPECT_THAT(last_render_time_, Eq(absl::nullopt));
+  scheduler.Stop();
 }
 
 }  // namespace webrtc