mollyim/webrtc
1
0
Fork 0
mirror of https://github.com/mollyim/webrtc.git synced 2025-05-13 05:40:42 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

DecodeSynchronizer: avoid duplicate tick callback registration.

Browse source 
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}
This commit is contained in:
Markus Handell 2023-04-28 11:16:30 +02:00 committed by WebRTC LUCI CQ
parent 272b464e92
commit aee5b17f66
3 changed files with 32 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
video/decode_synchronizer.cc
View file

@ -175,6 +175,10 @@ void DecodeSynchronizer::RemoveFrameScheduler(
void DecodeSynchronizer::ScheduleNextTick() { void DecodeSynchronizer::ScheduleNextTick() {
RTC_DCHECK_RUN_ON(worker_queue_); RTC_DCHECK_RUN_ON(worker_queue_);
if (tick_scheduled_) {
return;
}
tick_scheduled_ = true;
metronome_->RequestCallOnNextTick( metronome_->RequestCallOnNextTick(
SafeTask(safety_.flag(), [this] { OnTick(); })); SafeTask(safety_.flag(), [this] { OnTick(); }));
} }
@ -182,6 +186,7 @@ void DecodeSynchronizer::ScheduleNextTick() {
void DecodeSynchronizer::OnTick() { void DecodeSynchronizer::OnTick() {
TRACE_EVENT0("webrtc", __func__); TRACE_EVENT0("webrtc", __func__);
RTC_DCHECK_RUN_ON(worker_queue_); RTC_DCHECK_RUN_ON(worker_queue_);
tick_scheduled_ = false;
expected_next_tick_ = clock_->CurrentTime() + metronome_->TickPeriod(); expected_next_tick_ = clock_->CurrentTime() + metronome_->TickPeriod();
for (auto* scheduler : schedulers_) { for (auto* scheduler : schedulers_) {

1
video/decode_synchronizer.h
View file

@ -129,6 +129,7 @@ class DecodeSynchronizer {
Timestamp expected_next_tick_ = Timestamp::PlusInfinity(); Timestamp expected_next_tick_ = Timestamp::PlusInfinity();
std::set<SynchronizedFrameDecodeScheduler*> schedulers_ std::set<SynchronizedFrameDecodeScheduler*> schedulers_
RTC_GUARDED_BY(worker_queue_); RTC_GUARDED_BY(worker_queue_);
bool tick_scheduled_ = false;
ScopedTaskSafetyDetached safety_; ScopedTaskSafetyDetached safety_;
}; };

33
video/decode_synchronizer_unittest.cc
View file

@ -27,6 +27,8 @@
using ::testing::_; using ::testing::_;
using ::testing::Eq; using ::testing::Eq;
using ::testing::Invoke; using ::testing::Invoke;
using ::testing::Mock;
using ::testing::MockFunction;
using ::testing::Return; using ::testing::Return;
namespace webrtc { namespace webrtc {
@ -60,10 +62,10 @@ class DecodeSynchronizerTest : public ::testing::Test {
}; };
TEST_F(DecodeSynchronizerTest, AllFramesReadyBeforeNextTickDecoded) { TEST_F(DecodeSynchronizerTest, AllFramesReadyBeforeNextTickDecoded) {
::testing::MockFunction<void(uint32_t, Timestamp)> mock_callback1; MockFunction<void(uint32_t, Timestamp)> mock_callback1;
auto scheduler1 = decode_synchronizer_.CreateSynchronizedFrameScheduler(); 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(); auto scheduler2 = decode_synchronizer_.CreateSynchronizedFrameScheduler();
{ {
@ -97,7 +99,7 @@ TEST_F(DecodeSynchronizerTest, AllFramesReadyBeforeNextTickDecoded) {
} }
TEST_F(DecodeSynchronizerTest, FramesNotDecodedIfDecodeTimeIsInNextInterval) { TEST_F(DecodeSynchronizerTest, FramesNotDecodedIfDecodeTimeIsInNextInterval) {
::testing::MockFunction<void(unsigned int, Timestamp)> mock_callback; MockFunction<void(unsigned int, Timestamp)> mock_callback;
auto scheduler = decode_synchronizer_.CreateSynchronizedFrameScheduler(); auto scheduler = decode_synchronizer_.CreateSynchronizedFrameScheduler();
uint32_t frame_rtp = 90000; uint32_t frame_rtp = 90000;
@ -125,7 +127,7 @@ TEST_F(DecodeSynchronizerTest, FramesNotDecodedIfDecodeTimeIsInNextInterval) {
} }
TEST_F(DecodeSynchronizerTest, FrameDecodedOnce) { TEST_F(DecodeSynchronizerTest, FrameDecodedOnce) {
::testing::MockFunction<void(unsigned int, Timestamp)> mock_callback; MockFunction<void(unsigned int, Timestamp)> mock_callback;
auto scheduler = decode_synchronizer_.CreateSynchronizedFrameScheduler(); auto scheduler = decode_synchronizer_.CreateSynchronizedFrameScheduler();
uint32_t frame_rtp = 90000; uint32_t frame_rtp = 90000;
@ -149,7 +151,7 @@ TEST_F(DecodeSynchronizerTest, FrameDecodedOnce) {
} }
TEST_F(DecodeSynchronizerTest, FrameWithDecodeTimeInPastDecodedImmediately) { TEST_F(DecodeSynchronizerTest, FrameWithDecodeTimeInPastDecodedImmediately) {
::testing::MockFunction<void(unsigned int, Timestamp)> mock_callback; MockFunction<void(unsigned int, Timestamp)> mock_callback;
auto scheduler = decode_synchronizer_.CreateSynchronizedFrameScheduler(); auto scheduler = decode_synchronizer_.CreateSynchronizedFrameScheduler();
uint32_t frame_rtp = 90000; uint32_t frame_rtp = 90000;
@ -171,7 +173,7 @@ TEST_F(DecodeSynchronizerTest, FrameWithDecodeTimeInPastDecodedImmediately) {
TEST_F(DecodeSynchronizerTest, TEST_F(DecodeSynchronizerTest,
FrameWithDecodeTimeFarBeforeNextTickDecodedImmediately) { FrameWithDecodeTimeFarBeforeNextTickDecodedImmediately) {
::testing::MockFunction<void(unsigned int, Timestamp)> mock_callback; MockFunction<void(unsigned int, Timestamp)> mock_callback;
auto scheduler = decode_synchronizer_.CreateSynchronizedFrameScheduler(); auto scheduler = decode_synchronizer_.CreateSynchronizedFrameScheduler();
// Frame which would be behind by more than kMaxAllowedFrameDelay after // Frame which would be behind by more than kMaxAllowedFrameDelay after
@ -210,7 +212,7 @@ TEST_F(DecodeSynchronizerTest,
} }
TEST_F(DecodeSynchronizerTest, FramesNotReleasedAfterStop) { TEST_F(DecodeSynchronizerTest, FramesNotReleasedAfterStop) {
::testing::MockFunction<void(unsigned int, Timestamp)> mock_callback; MockFunction<void(unsigned int, Timestamp)> mock_callback;
auto scheduler = decode_synchronizer_.CreateSynchronizedFrameScheduler(); auto scheduler = decode_synchronizer_.CreateSynchronizedFrameScheduler();
uint32_t frame_rtp = 90000; uint32_t frame_rtp = 90000;
@ -249,4 +251,21 @@ TEST(DecodeSynchronizerStandaloneTest,
(std::move)(callback)(); (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 } // namespace webrtc