FrameCadenceAdapter: use distinct signal for queue overload.
The FrameCadenceAdapterInterface::Callback::OnFrame method in
VideoStreamEncoder only changed frame handling on
frames_scheduled_for_processing being 1. This CL changes
the parameter to more explicitly signal queue overload via
boolean parameter.
Bug: None
Change-Id: I1eb46b34fc4d748b7e2f1921642497c939adf197
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/327761
Commit-Queue: Markus Handell <handellm@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41226}
diff --git a/video/frame_cadence_adapter.cc b/video/frame_cadence_adapter.cc
index 721b401..99d4001 100644
--- a/video/frame_cadence_adapter.cc
+++ b/video/frame_cadence_adapter.cc
@@ -50,7 +50,7 @@
// Called on the worker thread for every frame that enters.
virtual void OnFrame(Timestamp post_time,
- int frames_scheduled_for_processing,
+ bool queue_overload,
const VideoFrame& frame) = 0;
// Returns the currently estimated input framerate.
@@ -71,10 +71,10 @@
// Adapter overrides.
void OnFrame(Timestamp post_time,
- int frames_scheduled_for_processing,
+ bool queue_overload,
const VideoFrame& frame) override {
RTC_DCHECK_RUN_ON(&sequence_checker_);
- callback_->OnFrame(post_time, frames_scheduled_for_processing, frame);
+ callback_->OnFrame(post_time, queue_overload, frame);
}
absl::optional<uint32_t> GetInputFrameRateFps() override {
@@ -119,7 +119,7 @@
// Adapter overrides.
void OnFrame(Timestamp post_time,
- int frames_scheduled_for_processing,
+ bool queue_overload,
const VideoFrame& frame) override;
absl::optional<uint32_t> GetInputFrameRateFps() override;
void UpdateFrameRate() override {}
@@ -356,7 +356,7 @@
}
void ZeroHertzAdapterMode::OnFrame(Timestamp post_time,
- int frames_scheduled_for_processing,
+ bool queue_overload,
const VideoFrame& frame) {
RTC_DCHECK_RUN_ON(&sequence_checker_);
TRACE_EVENT0("webrtc", "ZeroHertzAdapterMode::OnFrame");
@@ -566,8 +566,10 @@
TimeDelta delay = (now - post_time);
RTC_HISTOGRAM_COUNTS_10000("WebRTC.Screenshare.ZeroHz.DelayMs", delay.ms());
}
- callback_->OnFrame(/*post_time=*/now, /*frames_scheduled_for_processing*/ 1,
- frame);
+ // TODO(crbug.com/1255737): ensure queue_overload is computed from current
+ // conditions on the encoder queue.
+ callback_->OnFrame(/*post_time=*/now,
+ /*queue_overload=*/false, frame);
}
TimeDelta ZeroHertzAdapterMode::RepeatDuration(bool idle_repeat) const {
@@ -689,7 +691,7 @@
const int frames_scheduled_for_processing =
frames_scheduled_for_processing_.fetch_sub(1,
std::memory_order_relaxed);
- OnFrameOnMainQueue(post_time, frames_scheduled_for_processing,
+ OnFrameOnMainQueue(post_time, frames_scheduled_for_processing > 1,
std::move(frame));
}));
}
diff --git a/video/frame_cadence_adapter.h b/video/frame_cadence_adapter.h
index d0eab7e..37c4a43 100644
--- a/video/frame_cadence_adapter.h
+++ b/video/frame_cadence_adapter.h
@@ -60,14 +60,11 @@
// The |post_time| parameter indicates the current time sampled when
// FrameCadenceAdapterInterface::OnFrame was called.
//
- // |frames_scheduled_for_processing| indicates how many frames that have
- // been scheduled for processing. During sequential conditions where
- // FrameCadenceAdapterInterface::OnFrame is invoked and subsequently ending
- // up in this callback, this value will read 1. Otherwise if the
- // |queue| gets stalled for some reason, the value will increase
- // beyond 1.
+ // |queue_overload| is true if the frame cadence adapter notices it's
+ // not able to deliver the incoming |frame| to the |queue| in the expected
+ // time.
virtual void OnFrame(Timestamp post_time,
- int frames_scheduled_for_processing,
+ bool queue_overload,
const VideoFrame& frame) = 0;
// Called when the source has discarded a frame.
diff --git a/video/frame_cadence_adapter_unittest.cc b/video/frame_cadence_adapter_unittest.cc
index 11be450..2fd40fe 100644
--- a/video/frame_cadence_adapter_unittest.cc
+++ b/video/frame_cadence_adapter_unittest.cc
@@ -70,7 +70,7 @@
class MockCallback : public FrameCadenceAdapterInterface::Callback {
public:
- MOCK_METHOD(void, OnFrame, (Timestamp, int, const VideoFrame&), (override));
+ MOCK_METHOD(void, OnFrame, (Timestamp, bool, const VideoFrame&), (override));
MOCK_METHOD(void, OnDiscardedFrame, (), (override));
MOCK_METHOD(void, RequestRefreshFrame, (), (override));
};
@@ -115,13 +115,13 @@
MockCallback callback;
auto adapter = CreateAdapter(no_field_trials, time_controller.GetClock());
adapter->Initialize(&callback);
- EXPECT_CALL(callback, OnFrame(_, 2, _)).Times(1);
- EXPECT_CALL(callback, OnFrame(_, 1, _)).Times(1);
+ EXPECT_CALL(callback, OnFrame(_, true, _)).Times(1);
+ EXPECT_CALL(callback, OnFrame(_, false, _)).Times(1);
auto frame = CreateFrame();
adapter->OnFrame(frame);
adapter->OnFrame(frame);
time_controller.AdvanceTime(TimeDelta::Zero());
- EXPECT_CALL(callback, OnFrame(_, 1, _)).Times(1);
+ EXPECT_CALL(callback, OnFrame(_, false, _)).Times(1);
adapter->OnFrame(frame);
time_controller.AdvanceTime(TimeDelta::Zero());
}
@@ -253,7 +253,7 @@
EXPECT_CALL(callback, OnFrame).Times(0);
adapter->OnFrame(frame);
EXPECT_CALL(callback, OnFrame)
- .WillOnce(Invoke([&](Timestamp post_time, int,
+ .WillOnce(Invoke([&](Timestamp post_time, bool,
const VideoFrame& frame) {
EXPECT_EQ(post_time, time_controller.GetClock()->CurrentTime());
EXPECT_EQ(frame.timestamp_us(),
@@ -332,7 +332,7 @@
adapter->OnFrame(frame);
EXPECT_CALL(callback, OnFrame)
- .WillOnce(Invoke([&](Timestamp post_time, int, const VideoFrame& frame) {
+ .WillOnce(Invoke([&](Timestamp post_time, bool, const VideoFrame& frame) {
EXPECT_EQ(post_time, time_controller.GetClock()->CurrentTime());
EXPECT_EQ(frame.timestamp_us(), original_timestamp_us);
EXPECT_EQ(frame.ntp_time_ms(), original_ntp_time.ToMs());
@@ -341,7 +341,7 @@
Mock::VerifyAndClearExpectations(&callback);
EXPECT_CALL(callback, OnFrame)
- .WillOnce(Invoke([&](Timestamp post_time, int, const VideoFrame& frame) {
+ .WillOnce(Invoke([&](Timestamp post_time, bool, const VideoFrame& frame) {
EXPECT_EQ(post_time, time_controller.GetClock()->CurrentTime());
EXPECT_EQ(frame.timestamp_us(),
original_timestamp_us + rtc::kNumMicrosecsPerSec);
@@ -352,7 +352,7 @@
Mock::VerifyAndClearExpectations(&callback);
EXPECT_CALL(callback, OnFrame)
- .WillOnce(Invoke([&](Timestamp post_time, int, const VideoFrame& frame) {
+ .WillOnce(Invoke([&](Timestamp post_time, bool, const VideoFrame& frame) {
EXPECT_EQ(post_time, time_controller.GetClock()->CurrentTime());
EXPECT_EQ(frame.timestamp_us(),
original_timestamp_us + 2 * rtc::kNumMicrosecsPerSec);
@@ -382,7 +382,7 @@
// Send one frame, expect a repeat.
adapter->OnFrame(CreateFrame());
EXPECT_CALL(callback, OnFrame)
- .WillOnce(Invoke([&](Timestamp post_time, int, const VideoFrame& frame) {
+ .WillOnce(Invoke([&](Timestamp post_time, bool, const VideoFrame& frame) {
EXPECT_EQ(post_time, time_controller.GetClock()->CurrentTime());
EXPECT_EQ(frame.timestamp_us(), 0);
EXPECT_EQ(frame.ntp_time_ms(), 0);
@@ -390,7 +390,7 @@
time_controller.AdvanceTime(TimeDelta::Seconds(1));
Mock::VerifyAndClearExpectations(&callback);
EXPECT_CALL(callback, OnFrame)
- .WillOnce(Invoke([&](Timestamp post_time, int, const VideoFrame& frame) {
+ .WillOnce(Invoke([&](Timestamp post_time, bool, const VideoFrame& frame) {
EXPECT_EQ(post_time, time_controller.GetClock()->CurrentTime());
EXPECT_EQ(frame.timestamp_us(), 0);
EXPECT_EQ(frame.ntp_time_ms(), 0);
@@ -422,7 +422,7 @@
// Send the new frame at 2.5s, which should appear after 3.5s.
adapter->OnFrame(CreateFrameWithTimestamps(&time_controller));
EXPECT_CALL(callback, OnFrame)
- .WillOnce(Invoke([&](Timestamp, int, const VideoFrame& frame) {
+ .WillOnce(Invoke([&](Timestamp, bool, const VideoFrame& frame) {
EXPECT_EQ(frame.timestamp_us(), 5 * rtc::kNumMicrosecsPerSec / 2);
EXPECT_EQ(frame.ntp_time_ms(),
original_ntp_time.ToMs() + 5u * rtc::kNumMillisecsPerSec / 2);
@@ -748,7 +748,7 @@
std::initializer_list<TimeDelta> list) {
Timestamp origin = time_controller_.GetClock()->CurrentTime();
for (auto delay : list) {
- EXPECT_CALL(callback_, OnFrame(origin + delay, _, _));
+ EXPECT_CALL(callback_, OnFrame(origin + delay, false, _));
time_controller_.AdvanceTime(origin + delay -
time_controller_.GetClock()->CurrentTime());
}
@@ -892,7 +892,7 @@
constexpr int kSleepMs = rtc::kNumMillisecsPerSec / 2;
EXPECT_CALL(callback, OnFrame)
.WillRepeatedly(
- Invoke([&](Timestamp, int, const VideoFrame& incoming_frame) {
+ Invoke([&](Timestamp, bool, const VideoFrame& incoming_frame) {
++frame_counter;
// Avoid the first OnFrame and sleep on the second.
if (frame_counter == 2) {
diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc
index eeabe04..7144b57 100644
--- a/video/video_stream_encoder.cc
+++ b/video/video_stream_encoder.cc
@@ -1482,7 +1482,7 @@
}
void VideoStreamEncoder::OnFrame(Timestamp post_time,
- int frames_scheduled_for_processing,
+ bool queue_overload,
const VideoFrame& video_frame) {
RTC_DCHECK_RUN_ON(&encoder_queue_);
VideoFrame incoming_frame = video_frame;
@@ -1541,7 +1541,7 @@
bool cwnd_frame_drop =
cwnd_frame_drop_interval_ &&
(cwnd_frame_counter_++ % cwnd_frame_drop_interval_.value() == 0);
- if (frames_scheduled_for_processing == 1 && !cwnd_frame_drop) {
+ if (!queue_overload && !cwnd_frame_drop) {
MaybeEncodeVideoFrame(incoming_frame, post_time.us());
} else {
if (cwnd_frame_drop) {
diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h
index c884c73..2276d79 100644
--- a/video/video_stream_encoder.h
+++ b/video/video_stream_encoder.h
@@ -160,10 +160,9 @@
: video_stream_encoder_(video_stream_encoder) {}
// FrameCadenceAdapterInterface::Callback overrides.
void OnFrame(Timestamp post_time,
- int frames_scheduled_for_processing,
+ bool queue_overload,
const VideoFrame& frame) override {
- video_stream_encoder_.OnFrame(post_time, frames_scheduled_for_processing,
- frame);
+ video_stream_encoder_.OnFrame(post_time, queue_overload, frame);
}
void OnDiscardedFrame() override {
video_stream_encoder_.OnDiscardedFrame();
@@ -212,7 +211,7 @@
void ReconfigureEncoder() RTC_RUN_ON(&encoder_queue_);
void OnEncoderSettingsChanged() RTC_RUN_ON(&encoder_queue_);
void OnFrame(Timestamp post_time,
- int frames_scheduled_for_processing,
+ bool queue_overload,
const VideoFrame& video_frame);
void OnDiscardedFrame();
void RequestRefreshFrame();
diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc
index fa28368..06e772d 100644
--- a/video/video_stream_encoder_unittest.cc
+++ b/video/video_stream_encoder_unittest.cc
@@ -136,8 +136,8 @@
FrameCadenceAdapterInterface::Callback* video_stream_encoder_callback,
int64_t ntp_time_ms) {
encoder_queue->PostTask([video_stream_encoder_callback, ntp_time_ms] {
- video_stream_encoder_callback->OnFrame(Timestamp::Millis(ntp_time_ms), 1,
- CreateSimpleNV12Frame());
+ video_stream_encoder_callback->OnFrame(Timestamp::Millis(ntp_time_ms),
+ false, CreateSimpleNV12Frame());
});
}