FrameCadenceAdapter: account for encode sequence contention.
The synthetic delay added in ZeroHzAdapterMode::OnFrame does not
account for delay with respect to the initial frame post from
FrameCadenceAdapter::OnFrame. Fix this to account for time spent
in contention on the encode sequence.
Bug: webrtc:15456
Change-Id: I63446e8dfe8f62b09d972434a705e912f8a73d69
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/318420
Commit-Queue: Markus Handell <handellm@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40675}
diff --git a/test/time_controller/simulated_time_controller.cc b/test/time_controller/simulated_time_controller.cc
index 1ed2b30..dbb36fd 100644
--- a/test/time_controller/simulated_time_controller.cc
+++ b/test/time_controller/simulated_time_controller.cc
@@ -211,6 +211,18 @@
impl_.RunReadyRunners();
}
+void GlobalSimulatedTimeController::SkipForwardBy(TimeDelta duration) {
+ rtc::ScopedYieldPolicy yield_policy(&impl_);
+ Timestamp current_time = impl_.CurrentTime();
+ Timestamp target_time = current_time + duration;
+ impl_.AdvanceTime(target_time);
+ sim_clock_.AdvanceTimeMicroseconds(duration.us());
+ global_clock_.AdvanceTime(duration);
+
+ // Run tasks that were pending during the skip.
+ impl_.RunReadyRunners();
+}
+
void GlobalSimulatedTimeController::Register(
sim_time_impl::SimulatedSequenceRunner* runner) {
impl_.Register(runner);
diff --git a/test/time_controller/simulated_time_controller.h b/test/time_controller/simulated_time_controller.h
index 98b816c..f3f0da9 100644
--- a/test/time_controller/simulated_time_controller.h
+++ b/test/time_controller/simulated_time_controller.h
@@ -138,6 +138,11 @@
void AdvanceTime(TimeDelta duration) override;
+ // Advances time by `duration`and do not run delayed tasks in the meantime.
+ // Runs any pending tasks at the end.
+ // Useful for simulating contention on destination queues.
+ void SkipForwardBy(TimeDelta duration);
+
// Makes the simulated time controller aware of a custom
// SimulatedSequenceRunner.
// TODO(bugs.webrtc.org/11581): remove method once the ModuleRtpRtcpImpl2 unit
diff --git a/test/time_controller/simulated_time_controller_unittest.cc b/test/time_controller/simulated_time_controller_unittest.cc
index 1ee592c..f223ffe 100644
--- a/test/time_controller/simulated_time_controller_unittest.cc
+++ b/test/time_controller/simulated_time_controller_unittest.cc
@@ -13,6 +13,7 @@
#include <atomic>
#include <memory>
+#include "api/units/time_delta.h"
#include "rtc_base/event.h"
#include "rtc_base/task_queue.h"
#include "rtc_base/task_queue_for_test.h"
@@ -146,4 +147,18 @@
ASSERT_TRUE(task_has_run);
}
+TEST(SimulatedTimeControllerTest, SkipsDelayedTaskForward) {
+ GlobalSimulatedTimeController sim(kStartTime);
+ auto main_thread = sim.GetMainThread();
+ constexpr auto duration_during_which_nothing_runs = TimeDelta::Seconds(2);
+ constexpr auto shorter_duration = TimeDelta::Seconds(1);
+ MockFunction<void()> fun;
+ EXPECT_CALL(fun, Call).WillOnce(Invoke([&] {
+ ASSERT_EQ(sim.GetClock()->CurrentTime(),
+ kStartTime + duration_during_which_nothing_runs);
+ }));
+ main_thread->PostDelayedTask(fun.AsStdFunction(), shorter_duration);
+ sim.SkipForwardBy(duration_during_which_nothing_runs);
+}
+
} // namespace webrtc
diff --git a/video/frame_cadence_adapter.cc b/video/frame_cadence_adapter.cc
index 9390c05..250a82b 100644
--- a/video/frame_cadence_adapter.cc
+++ b/video/frame_cadence_adapter.cc
@@ -10,6 +10,7 @@
#include "video/frame_cadence_adapter.h"
+#include <algorithm>
#include <atomic>
#include <deque>
#include <memory>
@@ -377,13 +378,14 @@
queued_frames_.push_back(frame);
current_frame_id_++;
scheduled_repeat_ = absl::nullopt;
+ TimeDelta time_spent_since_post = clock_->CurrentTime() - post_time;
queue_->PostDelayedHighPrecisionTask(
SafeTask(safety_.flag(),
[this] {
RTC_DCHECK_RUN_ON(&sequence_checker_);
ProcessOnDelayedCadence();
}),
- frame_delay_);
+ std::max(frame_delay_ - time_spent_since_post, TimeDelta::Zero()));
}
void ZeroHertzAdapterMode::OnDiscardedFrame() {
diff --git a/video/frame_cadence_adapter_unittest.cc b/video/frame_cadence_adapter_unittest.cc
index f9745c8..b826672 100644
--- a/video/frame_cadence_adapter_unittest.cc
+++ b/video/frame_cadence_adapter_unittest.cc
@@ -38,6 +38,7 @@
using ::testing::_;
using ::testing::ElementsAre;
using ::testing::Invoke;
+using ::testing::InvokeWithoutArgs;
using ::testing::Mock;
using ::testing::Pair;
using ::testing::Values;
@@ -241,6 +242,50 @@
}
}
+TEST(FrameCadenceAdapterTest, DelayedProcessingUnderSlightContention) {
+ ZeroHertzFieldTrialEnabler enabler;
+ GlobalSimulatedTimeController time_controller(Timestamp::Zero());
+ auto adapter = CreateAdapter(enabler, time_controller.GetClock());
+ MockCallback callback;
+ adapter->Initialize(&callback);
+ adapter->SetZeroHertzModeEnabled(
+ FrameCadenceAdapterInterface::ZeroHertzModeParams{});
+ adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, 1});
+
+ // Expect frame delivery at 1 sec despite target sequence not running
+ // callbacks for the time skipped.
+ constexpr TimeDelta time_skipped = TimeDelta::Millis(999);
+ EXPECT_CALL(callback, OnFrame).WillOnce(InvokeWithoutArgs([&] {
+ EXPECT_EQ(time_controller.GetClock()->CurrentTime(),
+ Timestamp::Zero() + TimeDelta::Seconds(1));
+ }));
+ adapter->OnFrame(CreateFrame());
+ time_controller.SkipForwardBy(time_skipped);
+ time_controller.AdvanceTime(TimeDelta::Seconds(1) - time_skipped);
+}
+
+TEST(FrameCadenceAdapterTest, DelayedProcessingUnderHeavyContention) {
+ ZeroHertzFieldTrialEnabler enabler;
+ GlobalSimulatedTimeController time_controller(Timestamp::Zero());
+ auto adapter = CreateAdapter(enabler, time_controller.GetClock());
+ MockCallback callback;
+ adapter->Initialize(&callback);
+ adapter->SetZeroHertzModeEnabled(
+ FrameCadenceAdapterInterface::ZeroHertzModeParams{});
+ adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, 1});
+
+ // Expect frame delivery at origin + `time_skipped` when the target sequence
+ // is not running callbacks for the initial 1+ sec.
+ constexpr TimeDelta time_skipped =
+ TimeDelta::Seconds(1) + TimeDelta::Micros(1);
+ EXPECT_CALL(callback, OnFrame).WillOnce(InvokeWithoutArgs([&] {
+ EXPECT_EQ(time_controller.GetClock()->CurrentTime(),
+ Timestamp::Zero() + time_skipped);
+ }));
+ adapter->OnFrame(CreateFrame());
+ time_controller.SkipForwardBy(time_skipped);
+}
+
TEST(FrameCadenceAdapterTest, RepeatsFramesDelayed) {
// Logic in the frame cadence adapter avoids modifying frame NTP and render
// timestamps if these timestamps looks unset, which is the case when the
