diff --git a/webrtc/modules/video_coding/video_sender.cc b/webrtc/modules/video_coding/video_sender.cc index 33d8439973..0b54d13b29 100644 --- a/webrtc/modules/video_coding/video_sender.cc +++ b/webrtc/modules/video_coding/video_sender.cc @@ -63,12 +63,6 @@ void VideoSender::Process() { send_stats_callback_->SendStatistics(bitRate, frameRate); } } - { - rtc::CritScope cs(¶ms_crit_); - // Force an encoder parameters update, so that incoming frame rate is - // updated even if bandwidth hasn't changed. - encoder_params_.input_frame_rate = _mediaOpt.InputFrameRate(); - } } int64_t VideoSender::TimeUntilNextProcess() { diff --git a/webrtc/modules/video_coding/video_sender_unittest.cc b/webrtc/modules/video_coding/video_sender_unittest.cc index f0246ca673..50e17bfe70 100644 --- a/webrtc/modules/video_coding/video_sender_unittest.cc +++ b/webrtc/modules/video_coding/video_sender_unittest.cc @@ -317,22 +317,6 @@ TEST_F(TestVideoSenderWithMockEncoder, TestSetRate) { AddFrame(); clock_.AdvanceTimeMilliseconds(kFrameIntervalMs); - // Add enough frames so that input frame rate will be updated. - const int kFramesToSend = - (VCMProcessTimer::kDefaultProcessIntervalMs / kFrameIntervalMs) + 1; - for (int i = 0; i < kFramesToSend; ++i) { - AddFrame(); - clock_.AdvanceTimeMilliseconds(kFrameIntervalMs); - } - - EXPECT_CALL(encoder_, - SetRateAllocation(new_rate_allocation, kActualFrameRate)) - .Times(1) - .WillOnce(Return(0)); - - sender_->Process(); - AddFrame(); - // Expect no call to encoder_.SetRates if the new bitrate is zero. EXPECT_CALL(encoder_, SetRateAllocation(_, _)).Times(0); sender_->SetChannelParameters(0, 0, 200, rate_allocator_.get(), nullptr); @@ -376,23 +360,6 @@ TEST_F(TestVideoSenderWithMockEncoder, TestEncoderParametersForInternalSource) { rate_allocator_.get(), nullptr); } -TEST_F(TestVideoSenderWithMockEncoder, EncoderFramerateUpdatedViaProcess) { - sender_->SetChannelParameters(settings_.startBitrate * 1000, 0, 200, - rate_allocator_.get(), nullptr); - const int64_t kRateStatsWindowMs = 2000; - const uint32_t kInputFps = 20; - int64_t start_time = clock_.TimeInMilliseconds(); - while (clock_.TimeInMilliseconds() < start_time + kRateStatsWindowMs) { - AddFrame(); - clock_.AdvanceTimeMilliseconds(1000 / kInputFps); - } - EXPECT_CALL(encoder_, SetRateAllocation(_, kInputFps)) - .Times(1) - .WillOnce(Return(0)); - sender_->Process(); - AddFrame(); -} - TEST_F(TestVideoSenderWithMockEncoder, NoRedundantSetChannelParameterOrSetRatesCalls) { const uint8_t kLossRate = 4; @@ -412,13 +379,7 @@ TEST_F(TestVideoSenderWithMockEncoder, AddFrame(); clock_.AdvanceTimeMilliseconds(1000 / kInputFps); } - // After process, input framerate should be updated but not ChannelParameters - // as they are the same as before. - EXPECT_CALL(encoder_, SetRateAllocation(_, kInputFps)) - .Times(1) - .WillOnce(Return(0)); - sender_->Process(); - AddFrame(); + // Call to SetChannelParameters with changed bitrate should call encoder // SetRates but not encoder SetChannelParameters (that are unchanged). uint32_t new_bitrate_bps = 2 * settings_.startBitrate * 1000; diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc index bead39d765..eaf693af16 100644 --- a/webrtc/video/vie_encoder.cc +++ b/webrtc/video/vie_encoder.cc @@ -545,9 +545,16 @@ void ViEEncoder::EncodeVideoFrame(const VideoFrame& video_frame, << ", texture=" << last_frame_info_->is_texture; } + int64_t now_ms = clock_->TimeInMilliseconds(); if (pending_encoder_reconfiguration_) { ReconfigureEncoder(); + } else if (!last_parameters_update_ms_ || + now_ms - *last_parameters_update_ms_ >= + vcm::VCMProcessTimer::kDefaultProcessIntervalMs) { + video_sender_.UpdateChannelParemeters(rate_allocator_.get(), + bitrate_observer_); } + last_parameters_update_ms_.emplace(now_ms); if (EncoderPaused()) { TraceFrameDropStart(); diff --git a/webrtc/video/vie_encoder.h b/webrtc/video/vie_encoder.h index ec8ee381d5..4a2ca1f301 100644 --- a/webrtc/video/vie_encoder.h +++ b/webrtc/video/vie_encoder.h @@ -235,6 +235,7 @@ class ViEEncoder : public rtc::VideoSinkInterface, int dropped_frame_count_ ACCESS_ON(&encoder_queue_); VideoBitrateAllocationObserver* bitrate_observer_ ACCESS_ON(&encoder_queue_); + rtc::Optional last_parameters_update_ms_ ACCESS_ON(&encoder_queue_); // All public methods are proxied to |encoder_queue_|. It must must be // destroyed first to make sure no tasks are run that use other members. diff --git a/webrtc/video/vie_encoder_unittest.cc b/webrtc/video/vie_encoder_unittest.cc index a50f7aaf98..dae172dea3 100644 --- a/webrtc/video/vie_encoder_unittest.cc +++ b/webrtc/video/vie_encoder_unittest.cc @@ -13,10 +13,13 @@ #include "webrtc/api/video/i420_buffer.h" #include "webrtc/base/logging.h" +#include "webrtc/modules/video_coding/utility/default_video_bitrate_allocator.h" #include "webrtc/system_wrappers/include/metrics_default.h" +#include "webrtc/system_wrappers/include/sleep.h" #include "webrtc/test/encoder_settings.h" #include "webrtc/test/fake_encoder.h" #include "webrtc/test/frame_generator.h" +#include "webrtc/test/gmock.h" #include "webrtc/test/gtest.h" #include "webrtc/video/send_statistics_proxy.h" #include "webrtc/video/vie_encoder.h" @@ -35,6 +38,8 @@ namespace webrtc { using DegredationPreference = VideoSendStream::DegradationPreference; using ScaleReason = ScalingObserverInterface::ScaleReason; +using ::testing::_; +using ::testing::Return; namespace { const size_t kMaxPayloadLength = 1440; @@ -163,19 +168,20 @@ class ViEEncoderTest : public ::testing::Test { ConfigureEncoder(std::move(video_encoder_config), nack_enabled); } - VideoFrame CreateFrame(int64_t ntp_ts, rtc::Event* destruction_event) const { + VideoFrame CreateFrame(int64_t ntp_time_ms, + rtc::Event* destruction_event) const { VideoFrame frame(new rtc::RefCountedObject( destruction_event, codec_width_, codec_height_), 99, 99, kVideoRotation_0); - frame.set_ntp_time_ms(ntp_ts); + frame.set_ntp_time_ms(ntp_time_ms); return frame; } - VideoFrame CreateFrame(int64_t ntp_ts, int width, int height) const { + VideoFrame CreateFrame(int64_t ntp_time_ms, int width, int height) const { VideoFrame frame( new rtc::RefCountedObject(nullptr, width, height), 99, 99, kVideoRotation_0); - frame.set_ntp_time_ms(ntp_ts); + frame.set_ntp_time_ms(ntp_time_ms); return frame; } @@ -979,4 +985,48 @@ TEST_F(ViEEncoderTest, UMACpuLimitedResolutionInPercent) { 1, metrics::NumEvents("WebRTC.Video.CpuLimitedResolutionInPercent", 50)); } +TEST_F(ViEEncoderTest, CallsBitrateObserver) { + class MockBitrateObserver : public VideoBitrateAllocationObserver { + public: + MOCK_METHOD1(OnBitrateAllocationUpdated, void(const BitrateAllocation&)); + } bitrate_observer; + vie_encoder_->SetBitrateObserver(&bitrate_observer); + + const int kDefaultFps = 30; + const BitrateAllocation expected_bitrate = + DefaultVideoBitrateAllocator(fake_encoder_.codec_config()) + .GetAllocation(kTargetBitrateBps, kDefaultFps); + + // First called on bitrate updated, then again on first frame. + EXPECT_CALL(bitrate_observer, OnBitrateAllocationUpdated(expected_bitrate)) + .Times(2); + vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); + + const int64_t kStartTimeMs = 1; + video_source_.IncomingCapturedFrame( + CreateFrame(kStartTimeMs, codec_width_, codec_height_)); + sink_.WaitForEncodedFrame(kStartTimeMs); + + // Not called on second frame. + EXPECT_CALL(bitrate_observer, OnBitrateAllocationUpdated(expected_bitrate)) + .Times(0); + video_source_.IncomingCapturedFrame( + CreateFrame(kStartTimeMs + 1, codec_width_, codec_height_)); + sink_.WaitForEncodedFrame(kStartTimeMs + 1); + + // Called after a process interval. + const int64_t kProcessIntervalMs = + vcm::VCMProcessTimer::kDefaultProcessIntervalMs; + // TODO(sprang): ViEEncoder should die and/or get injectable clock. + // Sleep for one processing interval plus one frame to avoid flakiness. + SleepMs(kProcessIntervalMs + 1000 / kDefaultFps); + EXPECT_CALL(bitrate_observer, OnBitrateAllocationUpdated(expected_bitrate)) + .Times(1); + video_source_.IncomingCapturedFrame(CreateFrame( + kStartTimeMs + kProcessIntervalMs, codec_width_, codec_height_)); + sink_.WaitForEncodedFrame(kStartTimeMs + kProcessIntervalMs); + + vie_encoder_->Stop(); +} + } // namespace webrtc