From d4badbcb6d46fb268f2c9256701f207c6534012d Mon Sep 17 00:00:00 2001 From: noahric Date: Wed, 13 Apr 2016 14:59:48 -0700 Subject: [PATCH] Fix SetRates for encoders with internal sources. An earlier change moved SetRates to happen on every input frame, but encoders with internal sources don't receive input frames, so they weren't getting updated bitrate (and framerate) signals. BUG= Review URL: https://codereview.webrtc.org/1682253005 Cr-Commit-Position: refs/heads/master@{#12354} --- webrtc/modules/video_coding/video_sender.cc | 20 +++++++++++++++++-- .../video_coding/video_sender_unittest.cc | 13 ++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/webrtc/modules/video_coding/video_sender.cc b/webrtc/modules/video_coding/video_sender.cc index 4969069788..122bbfda37 100644 --- a/webrtc/modules/video_coding/video_sender.cc +++ b/webrtc/modules/video_coding/video_sender.cc @@ -202,8 +202,24 @@ int32_t VideoSender::SetChannelParameters(uint32_t target_bitrate, uint32_t input_frame_rate = _mediaOpt.InputFrameRate(); - rtc::CritScope cs(¶ms_crit_); - encoder_params_ = {target_rate, lossRate, rtt, input_frame_rate}; + EncoderParameters encoder_params = {target_rate, lossRate, rtt, + input_frame_rate}; + bool encoder_has_internal_source; + { + rtc::CritScope cs(¶ms_crit_); + encoder_params_ = encoder_params; + encoder_has_internal_source = encoder_has_internal_source_; + } + + // For encoders with internal sources, we need to tell the encoder directly, + // instead of waiting for an AddVideoFrame that will never come (internal + // source encoders don't get input frames). + if (encoder_has_internal_source) { + rtc::CritScope cs(&encoder_crit_); + if (_encoder) { + SetEncoderParameters(encoder_params); + } + } return VCM_OK; } diff --git a/webrtc/modules/video_coding/video_sender_unittest.cc b/webrtc/modules/video_coding/video_sender_unittest.cc index 3f9ba4eadd..e15e87285a 100644 --- a/webrtc/modules/video_coding/video_sender_unittest.cc +++ b/webrtc/modules/video_coding/video_sender_unittest.cc @@ -317,6 +317,19 @@ TEST_F(TestVideoSenderWithMockEncoder, TestIntraRequestsInternalCapture) { EXPECT_EQ(-1, sender_->IntraFrameRequest(-1)); } +TEST_F(TestVideoSenderWithMockEncoder, TestEncoderParametersForInternalSource) { + // De-register current external encoder. + sender_->RegisterExternalEncoder(nullptr, kUnusedPayloadType, false); + // Register encoder with internal capture. + sender_->RegisterExternalEncoder(&encoder_, kUnusedPayloadType, true); + EXPECT_EQ(0, sender_->RegisterSendCodec(&settings_, 1, 1200)); + // Update encoder bitrate parameters. We expect that to immediately call + // SetRates on the encoder without waiting for AddFrame processing. + const uint32_t new_bitrate = settings_.startBitrate + 300; + EXPECT_CALL(encoder_, SetRates(new_bitrate, _)).Times(1).WillOnce(Return(0)); + sender_->SetChannelParameters(new_bitrate * 1000, 0, 200); +} + TEST_F(TestVideoSenderWithMockEncoder, EncoderFramerateUpdatedViaProcess) { sender_->SetChannelParameters(settings_.startBitrate * 1000, 0, 200); const int64_t kRateStatsWindowMs = 2000;