admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Undo recent changes to initial frame dropper, fixing a regression.

Browse Source 
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}
This commit is contained in:
Henrik Boström 2024-09-05 13:02:43 +02:00 committed by WebRTC LUCI CQ
parent 9463095df8
commit d4c5843bae
2 changed files with 5 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
video/adaptation/video_stream_encoder_resource_manager.cc
View File

@ -161,21 +161,18 @@ class VideoStreamEncoderResourceManager::InitialFrameDropper {
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 @@ void VideoStreamEncoderResourceManager::SetEncoderSettings(
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();
}

2
video/video_stream_encoder.cc
View File

@ -2386,8 +2386,8 @@ void VideoStreamEncoder::OnVideoSourceRestrictionsUpdated(
// 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(