Invalidate encoder rates on VideoStreamEncoder::ReconfigureEncoder
Without invalidation the codec configuration may not get updated. This is prone to happen when the bandwidth is below the min or above the max in which case the last_encoder_rate_settings_ may have not changed. This led to a regression in b6a45dd, where last_encoder_rate_settings_ would not change the allocation or frame rate, but the frame rate would have been set by the video adapter. Thus the frame rates were set incorrectly, leading to lower values in the regression tests. I have re-run this scenario against some of the metric drops and the regression appears to be fixed. Bug: webrtc:10126 Change-Id: I0fa6c9b71e7aff5dd80e53119db109d97eed98b6 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/154343 Commit-Queue: Evan Shrubsole <eshr@google.com> Reviewed-by: Erik Språng <sprang@webrtc.org> Cr-Commit-Position: refs/heads/master@{#29301}
This commit is contained in:
committed by
Commit Bot
parent
0e3b1ff8c4
commit
e32ae4f8fb
@ -934,9 +934,13 @@ void VideoStreamEncoder::ReconfigureEncoder() {
|
||||
if (rate_allocator_ && last_encoder_rate_settings_) {
|
||||
// We have a new rate allocator instance and already configured target
|
||||
// bitrate. Update the rate allocation and notify observers.
|
||||
last_encoder_rate_settings_->framerate_fps = GetInputFramerateFps();
|
||||
SetEncoderRates(
|
||||
UpdateBitrateAllocationAndNotifyObserver(*last_encoder_rate_settings_));
|
||||
// We must invalidate the last_encoder_rate_settings_ to ensure
|
||||
// the changes get propagated to all listeners.
|
||||
EncoderRateSettings rate_settings = *last_encoder_rate_settings_;
|
||||
last_encoder_rate_settings_.reset();
|
||||
rate_settings.framerate_fps = GetInputFramerateFps();
|
||||
|
||||
SetEncoderRates(UpdateBitrateAllocationAndNotifyObserver(rate_settings));
|
||||
}
|
||||
|
||||
encoder_stats_observer_->OnEncoderReconfigured(encoder_config_, streams);
|
||||
|
||||
@ -4900,6 +4900,51 @@ TEST_F(VideoStreamEncoderTest, BandwidthAllocationLowerBound) {
|
||||
video_stream_encoder_->Stop();
|
||||
}
|
||||
|
||||
TEST_F(VideoStreamEncoderTest, EncoderRatesPropegatedOnReconfigure) {
|
||||
video_stream_encoder_->OnBitrateUpdated(
|
||||
DataRate::bps(kTargetBitrateBps), DataRate::bps(kTargetBitrateBps),
|
||||
DataRate::bps(kTargetBitrateBps), 0, 0);
|
||||
// Capture a frame and wait for it to synchronize with the encoder thread.
|
||||
int64_t timestamp_ms = fake_clock_.TimeNanos() / rtc::kNumNanosecsPerMillisec;
|
||||
video_source_.IncomingCapturedFrame(CreateFrame(timestamp_ms, nullptr));
|
||||
WaitForEncodedFrame(1);
|
||||
|
||||
auto prev_rate_settings = fake_encoder_.GetAndResetLastRateControlSettings();
|
||||
ASSERT_TRUE(prev_rate_settings.has_value());
|
||||
EXPECT_EQ(static_cast<int>(prev_rate_settings->framerate_fps),
|
||||
kDefaultFramerate);
|
||||
|
||||
// Send 1s of video to ensure the framerate is stable at kDefaultFramerate.
|
||||
for (int i = 0; i < 2 * kDefaultFramerate; i++) {
|
||||
timestamp_ms += 1000 / kDefaultFramerate;
|
||||
video_source_.IncomingCapturedFrame(CreateFrame(timestamp_ms, nullptr));
|
||||
WaitForEncodedFrame(timestamp_ms);
|
||||
}
|
||||
EXPECT_EQ(static_cast<int>(fake_encoder_.GetLastFramerate()),
|
||||
kDefaultFramerate);
|
||||
// Capture larger frame to trigger a reconfigure.
|
||||
codec_height_ *= 2;
|
||||
codec_width_ *= 2;
|
||||
timestamp_ms += 1000 / kDefaultFramerate;
|
||||
video_source_.IncomingCapturedFrame(CreateFrame(timestamp_ms, nullptr));
|
||||
WaitForEncodedFrame(timestamp_ms);
|
||||
|
||||
EXPECT_EQ(2, sink_.number_of_reconfigurations());
|
||||
auto current_rate_settings =
|
||||
fake_encoder_.GetAndResetLastRateControlSettings();
|
||||
// Ensure we have actually reconfigured twice
|
||||
// The rate settings should have been set again even though
|
||||
// they haven't changed.
|
||||
ASSERT_TRUE(current_rate_settings.has_value());
|
||||
EXPECT_EQ(prev_rate_settings->bitrate, current_rate_settings->bitrate);
|
||||
EXPECT_EQ(prev_rate_settings->framerate_fps,
|
||||
current_rate_settings->framerate_fps);
|
||||
EXPECT_EQ(prev_rate_settings->bandwidth_allocation,
|
||||
current_rate_settings->bandwidth_allocation);
|
||||
|
||||
video_stream_encoder_->Stop();
|
||||
}
|
||||
|
||||
struct MockEncoderSwitchRequestCallback : public EncoderSwitchRequestCallback {
|
||||
MOCK_METHOD0(RequestEncoderFallback, void());
|
||||
MOCK_METHOD1(RequestEncoderSwitch, void(const Config& conf));
|
||||
|
||||
Reference in New Issue
Block a user