From 6019fdb0f181a9994e196384bb6d1582853a3a1d Mon Sep 17 00:00:00 2001 From: Michael Froman Date: Sun, 20 Nov 2022 17:35:53 -0600 Subject: [PATCH] Bug 1800920 - Vendor libwebrtc from 1cdcbe12e8 Upstream commit: https://webrtc.googlesource.com/src/+/1cdcbe12e8091d0f067503837131c86b2c3214fc [106] FrameCadenceAdapter: survive layer updates for unconfigured layers. The frame cadence adapter would previously crash on encountering unconfigured layer updates. Fix this to silently ignore such updates. (cherry picked from commit 5a77e51c17762fdc938e04541578ab06f50dbdc2) Fixed: webrtc:14417 Bug: chromium:1360936 Change-Id: I524aa196f6aed10ffb8cd4ddcb4b053c71dfabba Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/273325 Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Markus Handell Cr-Original-Commit-Position: refs/heads/main@{#37980} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275201 Cr-Commit-Position: refs/branch-heads/5249@{#4} Cr-Branched-From: 7aaeb5a270ba23f5844f7301a50aaff9b6ca6126-refs/heads/main@{#37825} --- third_party/libwebrtc/README.moz-ff-commit | 3 +++ third_party/libwebrtc/README.mozilla | 2 ++ .../libwebrtc/video/frame_cadence_adapter.cc | 24 +++++++++++-------- .../libwebrtc/video/frame_cadence_adapter.h | 6 ++--- .../video/frame_cadence_adapter_unittest.cc | 20 +++++++++++++++- .../video/video_stream_encoder_unittest.cc | 8 +++---- 6 files changed, 45 insertions(+), 18 deletions(-) diff --git a/third_party/libwebrtc/README.moz-ff-commit b/third_party/libwebrtc/README.moz-ff-commit index f4221a2d0786e..18b10c7902480 100644 --- a/third_party/libwebrtc/README.moz-ff-commit +++ b/third_party/libwebrtc/README.moz-ff-commit @@ -17028,3 +17028,6 @@ cefe4777a0 # MOZ_LIBWEBRTC_SRC=/Users/mfroman/git-checkouts/moz-libwebrtc MOZ_LIBWEBRTC_COMMIT=mozpatches bash dom/media/webrtc/third_party_build/fast-forward-libwebrtc.sh # base of lastest vendoring 860ad5321f +# MOZ_LIBWEBRTC_SRC=/Users/mfroman/git-checkouts/moz-libwebrtc MOZ_LIBWEBRTC_COMMIT=mozpatches bash dom/media/webrtc/third_party_build/fast-forward-libwebrtc.sh +# base of lastest vendoring +1cdcbe12e8 diff --git a/third_party/libwebrtc/README.mozilla b/third_party/libwebrtc/README.mozilla index 19c52a49851cf..b704b981e2d4d 100644 --- a/third_party/libwebrtc/README.mozilla +++ b/third_party/libwebrtc/README.mozilla @@ -11374,3 +11374,5 @@ libwebrtc updated from /Users/mfroman/git-checkouts/moz-libwebrtc commit mozpatc libwebrtc updated from /Users/mfroman/git-checkouts/moz-libwebrtc commit mozpatches on 2022-11-20T23:31:44.072500. # python3 vendor-libwebrtc.py --from-local /Users/mfroman/git-checkouts/moz-libwebrtc --commit mozpatches libwebrtc libwebrtc updated from /Users/mfroman/git-checkouts/moz-libwebrtc commit mozpatches on 2022-11-20T23:34:53.214517. +# python3 vendor-libwebrtc.py --from-local /Users/mfroman/git-checkouts/moz-libwebrtc --commit mozpatches libwebrtc +libwebrtc updated from /Users/mfroman/git-checkouts/moz-libwebrtc commit mozpatches on 2022-11-20T23:35:47.565456. diff --git a/third_party/libwebrtc/video/frame_cadence_adapter.cc b/third_party/libwebrtc/video/frame_cadence_adapter.cc index c24a82cbe4cb3..efffa9672a638 100644 --- a/third_party/libwebrtc/video/frame_cadence_adapter.cc +++ b/third_party/libwebrtc/video/frame_cadence_adapter.cc @@ -107,10 +107,11 @@ class ZeroHertzAdapterMode : public AdapterMode { const FrameCadenceAdapterInterface::ZeroHertzModeParams& params); // Updates spatial layer quality convergence status. - void UpdateLayerQualityConvergence(int spatial_index, bool quality_converged); + void UpdateLayerQualityConvergence(size_t spatial_index, + bool quality_converged); // Updates spatial layer enabled status. - void UpdateLayerStatus(int spatial_index, bool enabled); + void UpdateLayerStatus(size_t spatial_index, bool enabled); // Adapter overrides. void OnFrame(Timestamp post_time, @@ -232,9 +233,9 @@ class FrameCadenceAdapterImpl : public FrameCadenceAdapterInterface { absl::optional params) override; absl::optional GetInputFrameRateFps() override; void UpdateFrameRate() override; - void UpdateLayerQualityConvergence(int spatial_index, + void UpdateLayerQualityConvergence(size_t spatial_index, bool quality_converged) override; - void UpdateLayerStatus(int spatial_index, bool enabled) override; + void UpdateLayerStatus(size_t spatial_index, bool enabled) override; void ProcessKeyFrameRequest() override; // VideoFrameSink overrides. @@ -323,19 +324,22 @@ void ZeroHertzAdapterMode::ReconfigureParameters( } void ZeroHertzAdapterMode::UpdateLayerQualityConvergence( - int spatial_index, + size_t spatial_index, bool quality_converged) { RTC_DCHECK_RUN_ON(&sequence_checker_); - RTC_DCHECK_LT(spatial_index, layer_trackers_.size()); RTC_LOG(LS_INFO) << __func__ << " this " << this << " layer " << spatial_index << " quality has converged: " << quality_converged; + if (spatial_index >= layer_trackers_.size()) + return; if (layer_trackers_[spatial_index].quality_converged.has_value()) layer_trackers_[spatial_index].quality_converged = quality_converged; } -void ZeroHertzAdapterMode::UpdateLayerStatus(int spatial_index, bool enabled) { +void ZeroHertzAdapterMode::UpdateLayerStatus(size_t spatial_index, + bool enabled) { RTC_DCHECK_RUN_ON(&sequence_checker_); - RTC_DCHECK_LT(spatial_index, layer_trackers_.size()); + if (spatial_index >= layer_trackers_.size()) + return; if (enabled) { if (!layer_trackers_[spatial_index].quality_converged.has_value()) { // Assume quality has not converged until hearing otherwise. @@ -624,14 +628,14 @@ void FrameCadenceAdapterImpl::UpdateFrameRate() { } void FrameCadenceAdapterImpl::UpdateLayerQualityConvergence( - int spatial_index, + size_t spatial_index, bool quality_converged) { if (zero_hertz_adapter_.has_value()) zero_hertz_adapter_->UpdateLayerQualityConvergence(spatial_index, quality_converged); } -void FrameCadenceAdapterImpl::UpdateLayerStatus(int spatial_index, +void FrameCadenceAdapterImpl::UpdateLayerStatus(size_t spatial_index, bool enabled) { if (zero_hertz_adapter_.has_value()) zero_hertz_adapter_->UpdateLayerStatus(spatial_index, enabled); diff --git a/third_party/libwebrtc/video/frame_cadence_adapter.h b/third_party/libwebrtc/video/frame_cadence_adapter.h index 149a9156242d8..d0eab7e770a2f 100644 --- a/third_party/libwebrtc/video/frame_cadence_adapter.h +++ b/third_party/libwebrtc/video/frame_cadence_adapter.h @@ -47,7 +47,7 @@ class FrameCadenceAdapterInterface struct ZeroHertzModeParams { // The number of simulcast layers used in this configuration. - int num_simulcast_layers = 0; + size_t num_simulcast_layers = 0; }; // Callback interface used to inform instance owners. @@ -106,11 +106,11 @@ class FrameCadenceAdapterInterface // Updates quality convergence status for an enabled spatial layer. // Convergence means QP has dropped to a low-enough level to warrant ceasing // to send identical frames at high frequency. - virtual void UpdateLayerQualityConvergence(int spatial_index, + virtual void UpdateLayerQualityConvergence(size_t spatial_index, bool converged) = 0; // Updates spatial layer enabled status. - virtual void UpdateLayerStatus(int spatial_index, bool enabled) = 0; + virtual void UpdateLayerStatus(size_t spatial_index, bool enabled) = 0; // Conditionally requests a refresh frame via // Callback::RequestRefreshFrame. diff --git a/third_party/libwebrtc/video/frame_cadence_adapter_unittest.cc b/third_party/libwebrtc/video/frame_cadence_adapter_unittest.cc index 0824ac309b737..afc675ffdeb91 100644 --- a/third_party/libwebrtc/video/frame_cadence_adapter_unittest.cc +++ b/third_party/libwebrtc/video/frame_cadence_adapter_unittest.cc @@ -499,6 +499,24 @@ TEST(FrameCadenceAdapterTest, OmitsRefreshAfterFrameDropWithTimelyFrameEntry) { Mock::VerifyAndClearExpectations(&callback); } +TEST(FrameCadenceAdapterTest, AcceptsUnconfiguredLayerFeedback) { + // This is a regression test for bugs.webrtc.org/14417. + ZeroHertzFieldTrialEnabler enabler; + MockCallback callback; + GlobalSimulatedTimeController time_controller(Timestamp::Zero()); + auto adapter = CreateAdapter(enabler, time_controller.GetClock()); + adapter->Initialize(&callback); + adapter->SetZeroHertzModeEnabled( + FrameCadenceAdapterInterface::ZeroHertzModeParams{.num_simulcast_layers = + 1}); + constexpr int kMaxFps = 10; + adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, kMaxFps}); + time_controller.AdvanceTime(TimeDelta::Zero()); + + adapter->UpdateLayerQualityConvergence(2, false); + adapter->UpdateLayerStatus(2, false); +} + class FrameCadenceAdapterSimulcastLayersParamTest : public ::testing::TestWithParam { public: @@ -514,7 +532,7 @@ class FrameCadenceAdapterSimulcastLayersParamTest time_controller_.AdvanceTime(TimeDelta::Zero()); adapter_->SetZeroHertzModeEnabled( FrameCadenceAdapterInterface::ZeroHertzModeParams{}); - const int num_spatial_layers = GetParam(); + const size_t num_spatial_layers = GetParam(); adapter_->SetZeroHertzModeEnabled( FrameCadenceAdapterInterface::ZeroHertzModeParams{num_spatial_layers}); } diff --git a/third_party/libwebrtc/video/video_stream_encoder_unittest.cc b/third_party/libwebrtc/video/video_stream_encoder_unittest.cc index cf3b2073c975a..0c00417b50546 100644 --- a/third_party/libwebrtc/video/video_stream_encoder_unittest.cc +++ b/third_party/libwebrtc/video/video_stream_encoder_unittest.cc @@ -776,11 +776,11 @@ class MockFrameCadenceAdapter : public FrameCadenceAdapterInterface { MOCK_METHOD(void, UpdateFrameRate, (), (override)); MOCK_METHOD(void, UpdateLayerQualityConvergence, - (int spatial_index, bool converged), + (size_t spatial_index, bool converged), (override)); MOCK_METHOD(void, UpdateLayerStatus, - (int spatial_index, bool enabled), + (size_t spatial_index, bool enabled), (override)); MOCK_METHOD(void, ProcessKeyFrameRequest, (), (override)); }; @@ -9170,7 +9170,7 @@ TEST(VideoStreamEncoderFrameCadenceTest, ActivatesFrameCadenceOnContentType) { EXPECT_CALL(*adapter_ptr, SetZeroHertzModeEnabled(Optional(Field( &FrameCadenceAdapterInterface:: ZeroHertzModeParams::num_simulcast_layers, - Eq(0))))); + Eq(0u))))); VideoEncoderConfig config; test::FillEncoderConfiguration(kVideoCodecVP8, 1, &config); config.content_type = VideoEncoderConfig::ContentType::kScreen; @@ -9182,7 +9182,7 @@ TEST(VideoStreamEncoderFrameCadenceTest, ActivatesFrameCadenceOnContentType) { EXPECT_CALL(*adapter_ptr, SetZeroHertzModeEnabled(Optional(Field( &FrameCadenceAdapterInterface:: ZeroHertzModeParams::num_simulcast_layers, - Gt(0))))); + Gt(0u))))); PassAFrame(encoder_queue, video_stream_encoder_callback, /*ntp_time_ms=*/1); factory.DepleteTaskQueues(); Mock::VerifyAndClearExpectations(adapter_ptr);