Undo recent changes to initial frame dropper, fixing a regression.
A recent bugfix[1] introduced having to reconfigure the encoder in
response to restrictions updating for the sake of not getting stuck at
the wrong resolution in some cases.
But the newly added ReconfigureEncoder() calls happened too early/often,
and the initial frame dropper got confused thinking resolution changes
were not in response to adaptation (restrictions) when they in fact were.
The CL wrongly disabled the "reset initial frame dropper" logic, when
the correct solution to this problem should have been to delay the
ReconfigurationEncoder() call to the next frame (using
`pending_encoder_reconfiguration_ = true`).
With this delay the initial frame dropper is not confused and we can
restore the old "reset" logic, fixing the regression.
[1] https://webrtc-review.googlesource.com/c/src/+/360200
Bug: b/364252657
Change-Id: I6b93f4cc44eb12b1bbbda0d8d1e9906c29b615a2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/361740
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Sergey Silkin <ssilkin@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42961}
diff --git a/video/adaptation/video_stream_encoder_resource_manager.cc b/video/adaptation/video_stream_encoder_resource_manager.cc
index 96b7f89..7581c2c 100644
--- a/video/adaptation/video_stream_encoder_resource_manager.cc
+++ b/video/adaptation/video_stream_encoder_resource_manager.cc
@@ -161,21 +161,18 @@
void OnEncoderSettingsUpdated(
const VideoCodec& codec,
- const VideoAdaptationCounters& adaptation_counters,
- bool has_requested_resolution) {
+ const VideoAdaptationCounters& adaptation_counters) {
last_stream_configuration_changed_ = false;
std::vector<bool> active_flags = GetActiveLayersFlags(codec);
// Check if the source resolution has changed for the external reasons,
- // i.e. without any adaptation from WebRTC. This only matters when encoding
- // resolution is relative to input frame, which it isn't if the
- // `requested_resolution` API is used.
+ // i.e. without any adaptation from WebRTC.
const bool source_resolution_changed =
(last_input_width_ != codec.width ||
last_input_height_ != codec.height) &&
adaptation_counters.resolution_adaptations ==
last_adaptation_counters_.resolution_adaptations;
if (!EqualFlags(active_flags, last_active_flags_) ||
- (!has_requested_resolution && source_resolution_changed)) {
+ source_resolution_changed) {
// Streams configuration has changed.
last_stream_configuration_changed_ = true;
// Initial frame drop must be enabled because BWE might be way too low
@@ -417,8 +414,7 @@
encoder_settings_ = std::move(encoder_settings);
bitrate_constraint_->OnEncoderSettingsUpdated(encoder_settings_);
initial_frame_dropper_->OnEncoderSettingsUpdated(
- encoder_settings_->video_codec(), current_adaptation_counters_,
- encoder_settings.encoder_config().HasRequestedResolution());
+ encoder_settings_->video_codec(), current_adaptation_counters_);
MaybeUpdateTargetFrameRate();
}
diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc
index 821a1db..da42b2a 100644
--- a/video/video_stream_encoder.cc
+++ b/video/video_stream_encoder.cc
@@ -2386,8 +2386,8 @@
// automatically on new frame size and we don't need to reconfigure here.
if (encoder_ && max_pixels_updated &&
encoder_config_.HasRequestedResolution()) {
+ // The encoder will be reconfigured on the next frame.
pending_encoder_reconfiguration_ = true;
- ReconfigureEncoder();
}
worker_queue_->PostTask(SafeTask(
