Remove statistics tracking from FrameBuffer2
This was only set to nullptr in non-test environments and was thusly
unused. With this change, the stats callbacks are gaurenteed to only
come from the VideoStreamBufferController and so the thread checks can
be removed.
Bug: webrtc:14003
Change-Id: Iaf0e77aa7c45a317e38ae27739edcefd3123d832
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/272021
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37816}
diff --git a/modules/video_coding/frame_buffer2.cc b/modules/video_coding/frame_buffer2.cc
index fbd3c53..9ecbf40 100644
--- a/modules/video_coding/frame_buffer2.cc
+++ b/modules/video_coding/frame_buffer2.cc
@@ -57,7 +57,6 @@
FrameBuffer::FrameBuffer(Clock* clock,
VCMTiming* timing,
- VCMReceiveStatisticsCallback* stats_callback,
const FieldTrialsView& field_trials)
: decoded_frames_history_(kMaxFramesHistory),
clock_(clock),
@@ -66,7 +65,6 @@
timing_(timing),
stopped_(false),
protection_mode_(kProtectionNack),
- stats_callback_(stats_callback),
last_log_non_decoded_ms_(-kLogNonDecodedIntervalMs),
rtt_mult_settings_(RttMultExperiment::GetRttMultValue()),
zero_playout_delay_max_decode_queue_size_(
@@ -276,18 +274,6 @@
PropagateDecodability(frame_it->second);
decoded_frames_history_.InsertDecoded(frame_it->first, frame->Timestamp());
- // Remove decoded frame and all undecoded frames before it.
- if (stats_callback_) {
- unsigned int dropped_frames =
- std::count_if(frames_.begin(), frame_it,
- [](const std::pair<const int64_t, FrameInfo>& frame) {
- return frame.second.frame != nullptr;
- });
- if (dropped_frames > 0) {
- stats_callback_->OnDroppedFrames(dropped_frames);
- }
- }
-
frames_.erase(frames_.begin(), ++frame_it);
frames_out.emplace_back(std::move(frame));
@@ -316,9 +302,6 @@
jitter_estimator_.FrameNacked();
}
- UpdateJitterDelay();
- UpdateTimingFrameInfo();
-
if (frames_out.size() == 1) {
return std::move(frames_out[0]);
} else {
@@ -456,11 +439,6 @@
// It can happen that a frame will be reported as fully received even if a
// lower spatial layer frame is missing.
- if (stats_callback_ && frame->is_last_spatial_layer) {
- stats_callback_->OnCompleteFrame(frame->is_keyframe(), frame->size(),
- frame->contentType());
- }
-
info->second.frame = std::move(frame);
if (info->second.num_missing_continuous == 0) {
@@ -591,39 +569,8 @@
return true;
}
-void FrameBuffer::UpdateJitterDelay() {
- TRACE_EVENT0("webrtc", "FrameBuffer::UpdateJitterDelay");
- if (!stats_callback_)
- return;
-
- auto timings = timing_->GetTimings();
- if (timings.num_decoded_frames > 0) {
- stats_callback_->OnFrameBufferTimingsUpdated(
- timings.max_decode_duration.ms(), timings.current_delay.ms(),
- timings.target_delay.ms(), timings.jitter_buffer_delay.ms(),
- timings.min_playout_delay.ms(), timings.render_delay.ms());
- }
-}
-
-void FrameBuffer::UpdateTimingFrameInfo() {
- TRACE_EVENT0("webrtc", "FrameBuffer::UpdateTimingFrameInfo");
- absl::optional<TimingFrameInfo> info = timing_->GetTimingFrameInfo();
- if (info && stats_callback_)
- stats_callback_->OnTimingFrameInfoUpdated(*info);
-}
-
void FrameBuffer::ClearFramesAndHistory() {
TRACE_EVENT0("webrtc", "FrameBuffer::ClearFramesAndHistory");
- if (stats_callback_) {
- unsigned int dropped_frames =
- std::count_if(frames_.begin(), frames_.end(),
- [](const std::pair<const int64_t, FrameInfo>& frame) {
- return frame.second.frame != nullptr;
- });
- if (dropped_frames > 0) {
- stats_callback_->OnDroppedFrames(dropped_frames);
- }
- }
frames_.clear();
last_continuous_frame_.reset();
frames_to_decode_.clear();
diff --git a/modules/video_coding/frame_buffer2.h b/modules/video_coding/frame_buffer2.h
index 13918b0..1383c40 100644
--- a/modules/video_coding/frame_buffer2.h
+++ b/modules/video_coding/frame_buffer2.h
@@ -48,7 +48,6 @@
public:
FrameBuffer(Clock* clock,
VCMTiming* timing,
- VCMReceiveStatisticsCallback* stats_callback,
const FieldTrialsView& field_trials);
FrameBuffer() = delete;
@@ -143,10 +142,6 @@
FrameMap::iterator info)
RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
- void UpdateJitterDelay() RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
-
- void UpdateTimingFrameInfo() RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
-
void ClearFramesAndHistory() RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
// The cleaner solution would be to have the NextFrame function return a
@@ -179,7 +174,6 @@
std::vector<FrameMap::iterator> frames_to_decode_ RTC_GUARDED_BY(mutex_);
bool stopped_ RTC_GUARDED_BY(mutex_);
VCMVideoProtection protection_mode_ RTC_GUARDED_BY(mutex_);
- VCMReceiveStatisticsCallback* const stats_callback_;
int64_t last_log_non_decoded_ms_ RTC_GUARDED_BY(mutex_);
// rtt_mult experiment settings.
diff --git a/modules/video_coding/frame_buffer2_unittest.cc b/modules/video_coding/frame_buffer2_unittest.cc
index 13ad66c..0fabd9b 100644
--- a/modules/video_coding/frame_buffer2_unittest.cc
+++ b/modules/video_coding/frame_buffer2_unittest.cc
@@ -137,7 +137,6 @@
timing_(time_controller_.GetClock(), field_trials_),
buffer_(new FrameBuffer(time_controller_.GetClock(),
&timing_,
- &stats_callback_,
field_trials_)),
rand_(0x34678213) {}
@@ -225,7 +224,6 @@
std::unique_ptr<FrameBuffer> buffer_;
std::vector<std::unique_ptr<EncodedFrame>> frames_;
Random rand_;
- ::testing::NiceMock<VCMReceiveStatisticsCallbackMock> stats_callback_;
};
// From https://en.cppreference.com/w/cpp/language/static: "If ... a constexpr
@@ -284,8 +282,8 @@
TEST_F(TestFrameBuffer2, ZeroPlayoutDelay) {
test::ScopedKeyValueConfig field_trials;
VCMTiming timing(time_controller_.GetClock(), field_trials);
- buffer_.reset(new FrameBuffer(time_controller_.GetClock(), &timing,
- &stats_callback_, field_trials));
+ buffer_ = std::make_unique<FrameBuffer>(time_controller_.GetClock(), &timing,
+ field_trials);
const VideoPlayoutDelay kPlayoutDelayMs = {0, 0};
std::unique_ptr<FrameObjectFake> test_frame(new FrameObjectFake());
test_frame->SetId(0);
@@ -379,8 +377,6 @@
pid + i - 1);
}
- EXPECT_CALL(stats_callback_, OnDroppedFrames(1)).Times(3);
-
for (int i = 0; i < 10; ++i) {
ExtractFrame();
time_controller_.AdvanceTime(TimeDelta::Millis(70));
@@ -411,7 +407,6 @@
// Jump forward in time, simulating the system being stalled for some reason.
time_controller_.AdvanceTime(TimeDelta::Millis(3) * kFps10);
// Extract one more frame, expect second and third frame to be dropped.
- EXPECT_CALL(stats_callback_, OnDroppedFrames(2)).Times(1);
ExtractFrame();
CheckFrame(0, pid + 0, 0);
@@ -428,7 +423,6 @@
}
// All frames should be dropped when Clear is called.
- EXPECT_CALL(stats_callback_, OnDroppedFrames(5)).Times(1);
buffer_->Clear();
}
@@ -520,31 +514,6 @@
CheckNoFrame(2);
}
-TEST_F(TestFrameBuffer2, StatsCallback) {
- uint16_t pid = Rand();
- uint32_t ts = Rand();
- const int kFrameSize = 5000;
-
- EXPECT_CALL(stats_callback_,
- OnCompleteFrame(true, kFrameSize, VideoContentType::UNSPECIFIED));
- EXPECT_CALL(stats_callback_, OnFrameBufferTimingsUpdated(_, _, _, _, _, _));
- // Stats callback requires a previously decoded frame.
- timing_.StopDecodeTimer(TimeDelta::Millis(1), Timestamp::Zero());
-
- {
- std::unique_ptr<FrameObjectFake> frame(new FrameObjectFake());
- frame->SetEncodedData(EncodedImageBuffer::Create(kFrameSize));
- frame->SetId(pid);
- frame->SetTimestamp(ts);
- frame->num_references = 0;
-
- EXPECT_EQ(buffer_->InsertFrame(std::move(frame)), pid);
- }
-
- ExtractFrame();
- CheckFrame(0, pid, 0);
-}
-
TEST_F(TestFrameBuffer2, ForwardJumps) {
EXPECT_EQ(5453, InsertFrame(5453, 0, 1, true, kFrameSize));
ExtractFrame();
diff --git a/test/fuzzers/frame_buffer2_fuzzer.cc b/test/fuzzers/frame_buffer2_fuzzer.cc
index 944f069..ec1bbbb 100644
--- a/test/fuzzers/frame_buffer2_fuzzer.cc
+++ b/test/fuzzers/frame_buffer2_fuzzer.cc
@@ -75,7 +75,7 @@
test::ScopedKeyValueConfig field_trials;
VCMTiming timing(time_controller.GetClock(), field_trials);
video_coding::FrameBuffer frame_buffer(time_controller.GetClock(), &timing,
- nullptr, field_trials);
+ field_trials);
bool next_frame_task_running = false;
diff --git a/video/receive_statistics_proxy2.cc b/video/receive_statistics_proxy2.cc
index 3448ab5..01e4f09 100644
--- a/video/receive_statistics_proxy2.cc
+++ b/video/receive_statistics_proxy2.cc
@@ -664,20 +664,6 @@
int jitter_buffer_ms,
int min_playout_delay_ms,
int render_delay_ms) {
- // Only called on main_thread_ with FrameBuffer3
- if (!worker_thread_->IsCurrent()) {
- RTC_DCHECK_RUN_ON(&decode_queue_);
- worker_thread_->PostTask(SafeTask(
- task_safety_.flag(),
- [max_decode_ms, current_delay_ms, target_delay_ms, jitter_buffer_ms,
- min_playout_delay_ms, render_delay_ms, this]() {
- OnFrameBufferTimingsUpdated(max_decode_ms, current_delay_ms,
- target_delay_ms, jitter_buffer_ms,
- min_playout_delay_ms, render_delay_ms);
- }));
- return;
- }
-
RTC_DCHECK_RUN_ON(&main_thread_);
stats_.max_decode_ms = max_decode_ms;
stats_.current_delay_ms = current_delay_ms;
@@ -700,14 +686,6 @@
void ReceiveStatisticsProxy::OnTimingFrameInfoUpdated(
const TimingFrameInfo& info) {
- // Only called on main_thread_ with FrameBuffer3
- if (!worker_thread_->IsCurrent()) {
- RTC_DCHECK_RUN_ON(&decode_queue_);
- worker_thread_->PostTask(SafeTask(task_safety_.flag(), [info, this]() {
- OnTimingFrameInfoUpdated(info);
- }));
- return;
- }
RTC_DCHECK_RUN_ON(&main_thread_);
if (info.flags != VideoSendTiming::kInvalid) {
int64_t now_ms = clock_->TimeInMilliseconds();
diff --git a/video/video_stream_decoder_impl.cc b/video/video_stream_decoder_impl.cc
index fdc79eb..516aceb 100644
--- a/video/video_stream_decoder_impl.cc
+++ b/video/video_stream_decoder_impl.cc
@@ -33,10 +33,7 @@
decoder_factory_(decoder_factory),
decoder_settings_(std::move(decoder_settings)),
shut_down_(false),
- frame_buffer_(Clock::GetRealTimeClock(),
- &timing_,
- nullptr,
- *field_trials_),
+ frame_buffer_(Clock::GetRealTimeClock(), &timing_, *field_trials_),
bookkeeping_queue_(task_queue_factory->CreateTaskQueue(
"video_stream_decoder_bookkeeping_queue",
TaskQueueFactory::Priority::NORMAL)),
