From 5ee6967c4edc667688d736c27db6f2e7be00dd0a Mon Sep 17 00:00:00 2001 From: Sergey Silkin Date: Tue, 2 Jul 2019 14:18:34 +0200 Subject: [PATCH] Don't reset encoder on max/min bitrate change. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Don't reset encoder if max/min bitrate changed. - Removed min/max bitrate DCHECKs from encoder wrappers. - Reset encoder if start_bitrate changed. Only do this if encoding has not yet started. - Updated ReconfigureBitratesSetsEncoderBitratesCorrectly test. - Removed EncoderSetupPropagatesCommonEncoderConfigValues test since it was a subset of ReconfigureBitratesSetsEncoderBitratesCorrectly. Bug: webrtc:10773 Change-Id: Id9cbb2ea229232fd95967819e2a937b26948de9f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144028 Commit-Queue: Sergey Silkin Reviewed-by: Ilya Nikolaevskiy Reviewed-by: Åsa Persson Cr-Commit-Position: refs/heads/master@{#28446} --- media/engine/simulcast_encoder_adapter.cc | 29 ---- .../codecs/h264/h264_encoder_impl.cc | 11 +- .../codecs/vp8/libvpx_vp8_encoder.cc | 8 - modules/video_coding/codecs/vp9/vp9_impl.cc | 10 -- video/video_send_stream_tests.cc | 153 +++++++----------- video/video_stream_encoder.cc | 53 ++++-- video/video_stream_encoder.h | 7 + video/video_stream_encoder_unittest.cc | 88 +++++++++- 8 files changed, 188 insertions(+), 171 deletions(-) diff --git a/media/engine/simulcast_encoder_adapter.cc b/media/engine/simulcast_encoder_adapter.cc index ccbf0d3fae..a128d01a56 100644 --- a/media/engine/simulcast_encoder_adapter.cc +++ b/media/engine/simulcast_encoder_adapter.cc @@ -464,35 +464,6 @@ void SimulcastEncoderAdapter::SetRates( return; } - if (codec_.maxBitrate > 0 && - parameters.bitrate.get_sum_kbps() > codec_.maxBitrate) { - RTC_LOG(LS_WARNING) << "Total bitrate " << parameters.bitrate.get_sum_kbps() - << " exceeds max bitrate: " << codec_.maxBitrate; - return; - } - - if (parameters.bitrate.get_sum_bps() > 0) { - // Make sure the bitrate fits the configured min bitrates. 0 is a special - // value that means paused, though, so leave it alone. - if (parameters.bitrate.get_sum_kbps() < codec_.minBitrate) { - RTC_LOG(LS_WARNING) << "Total bitrate " - << parameters.bitrate.get_sum_kbps() - << " is lower than minimum bitrate: " - << codec_.minBitrate; - return; - } - - if (codec_.numberOfSimulcastStreams > 0 && - parameters.bitrate.get_sum_kbps() < - codec_.simulcastStream[0].minBitrate) { - RTC_LOG(LS_WARNING) << "Total bitrate " - << parameters.bitrate.get_sum_kbps() - << " is lower than minimum bitrate of base layer: " - << codec_.simulcastStream[0].minBitrate; - return; - } - } - codec_.maxFramerate = static_cast(parameters.framerate_fps + 0.5); for (size_t stream_idx = 0; stream_idx < streaminfos_.size(); ++stream_idx) { diff --git a/modules/video_coding/codecs/h264/h264_encoder_impl.cc b/modules/video_coding/codecs/h264/h264_encoder_impl.cc index 4334dc3141..7a1af14cfb 100644 --- a/modules/video_coding/codecs/h264/h264_encoder_impl.cc +++ b/modules/video_coding/codecs/h264/h264_encoder_impl.cc @@ -337,19 +337,12 @@ void H264EncoderImpl::SetRates(const RateControlParameters& parameters) { if (parameters.bitrate.get_sum_bps() == 0) { // Encoder paused, turn off all encoding. - for (size_t i = 0; i < configurations_.size(); ++i) + for (size_t i = 0; i < configurations_.size(); ++i) { configurations_[i].SetStreamState(false); + } return; } - // At this point, bitrate allocation should already match codec settings. - if (codec_.maxBitrate > 0) - RTC_DCHECK_LE(parameters.bitrate.get_sum_kbps(), codec_.maxBitrate); - RTC_DCHECK_GE(parameters.bitrate.get_sum_kbps(), codec_.minBitrate); - if (codec_.numberOfSimulcastStreams > 0) - RTC_DCHECK_GE(parameters.bitrate.get_sum_kbps(), - codec_.simulcastStream[0].minBitrate); - codec_.maxFramerate = static_cast(parameters.framerate_fps); size_t stream_idx = encoders_.size() - 1; diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc index c630e35402..d35abe9dbf 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc @@ -362,14 +362,6 @@ void LibvpxVp8Encoder::SetRates(const RateControlParameters& parameters) { return; } - // At this point, bitrate allocation should already match codec settings. - if (codec_.maxBitrate > 0) - RTC_DCHECK_LE(parameters.bitrate.get_sum_kbps(), codec_.maxBitrate); - RTC_DCHECK_GE(parameters.bitrate.get_sum_kbps(), codec_.minBitrate); - if (codec_.numberOfSimulcastStreams > 0) - RTC_DCHECK_GE(parameters.bitrate.get_sum_kbps(), - codec_.simulcastStream[0].minBitrate); - codec_.maxFramerate = static_cast(parameters.framerate_fps + 0.5); if (encoders_.size() > 1) { diff --git a/modules/video_coding/codecs/vp9/vp9_impl.cc b/modules/video_coding/codecs/vp9/vp9_impl.cc index 58aac2f88b..e8e40ee67b 100644 --- a/modules/video_coding/codecs/vp9/vp9_impl.cc +++ b/modules/video_coding/codecs/vp9/vp9_impl.cc @@ -391,19 +391,9 @@ void VP9EncoderImpl::SetRates(const RateControlParameters& parameters) { << parameters.framerate_fps; return; } - // Update bit rate - if (codec_.maxBitrate > 0 && - parameters.bitrate.get_sum_kbps() > codec_.maxBitrate) { - RTC_LOG(LS_WARNING) << "Target bitrate exceeds maximum: " - << parameters.bitrate.get_sum_kbps() << " vs " - << codec_.maxBitrate; - return; - } codec_.maxFramerate = static_cast(parameters.framerate_fps + 0.5); requested_rate_settings_ = parameters; - - return; } // TODO(eladalon): s/inst/codec_settings/g. diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc index 47053fa155..e327ba0a8b 100644 --- a/video/video_send_stream_tests.cc +++ b/video/video_send_stream_tests.cc @@ -15,6 +15,7 @@ #include "absl/memory/memory.h" #include "api/task_queue/default_task_queue_factory.h" #include "api/test/simulated_network.h" +#include "api/video/builtin_video_bitrate_allocator_factory.h" #include "api/video/encoded_image.h" #include "api/video/video_bitrate_allocation.h" #include "api/video_codecs/video_encoder.h" @@ -2401,72 +2402,6 @@ TEST_F(VideoSendStreamTest, EncoderIsProperlyInitializedAndDestroyed) { EXPECT_EQ(1u, test_encoder.num_releases()); } -TEST_F(VideoSendStreamTest, EncoderSetupPropagatesCommonEncoderConfigValues) { - class VideoCodecConfigObserver : public test::SendTest, - public test::FakeEncoder { - public: - VideoCodecConfigObserver() - : SendTest(kDefaultTimeoutMs), - FakeEncoder(Clock::GetRealTimeClock()), - num_initializations_(0), - stream_(nullptr), - encoder_factory_(this) {} - - private: - void ModifyVideoConfigs( - VideoSendStream::Config* send_config, - std::vector* receive_configs, - VideoEncoderConfig* encoder_config) override { - send_config->encoder_settings.encoder_factory = &encoder_factory_; - encoder_config->max_bitrate_bps = kFirstMaxBitrateBps; - encoder_config_ = encoder_config->Copy(); - } - - void OnVideoStreamsCreated( - VideoSendStream* send_stream, - const std::vector& receive_streams) override { - stream_ = send_stream; - } - - int32_t InitEncode(const VideoCodec* config, - const Settings& settings) override { - if (num_initializations_ == 0) { - // Verify default values. - EXPECT_EQ(kFirstMaxBitrateBps / 1000, config->maxBitrate); - } else { - // Verify that changed values are propagated. - EXPECT_EQ(kSecondMaxBitrateBps / 1000, config->maxBitrate); - } - ++num_initializations_; - init_encode_event_.Set(); - return FakeEncoder::InitEncode(config, settings); - } - - void PerformTest() override { - EXPECT_TRUE(init_encode_event_.Wait(kDefaultTimeoutMs)); - EXPECT_EQ(1u, num_initializations_) << "VideoEncoder not initialized."; - - encoder_config_.max_bitrate_bps = kSecondMaxBitrateBps; - stream_->ReconfigureVideoEncoder(std::move(encoder_config_)); - EXPECT_TRUE(init_encode_event_.Wait(kDefaultTimeoutMs)); - EXPECT_EQ(2u, num_initializations_) - << "ReconfigureVideoEncoder did not reinitialize the encoder with " - "new encoder settings."; - } - - const uint32_t kFirstMaxBitrateBps = 1000000; - const uint32_t kSecondMaxBitrateBps = 2000000; - - rtc::Event init_encode_event_; - size_t num_initializations_; - VideoSendStream* stream_; - test::VideoEncoderProxyFactory encoder_factory_; - VideoEncoderConfig encoder_config_; - } test; - - RunBaseTest(&test); -} - static const size_t kVideoCodecConfigObserverNumberOfTemporalLayers = 3; template class VideoCodecConfigObserver : public test::SendTest, @@ -2803,6 +2738,7 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { "WebRTC-VideoRateControl/bitrate_adjuster:false/"); class EncoderBitrateThresholdObserver : public test::SendTest, + public VideoBitrateAllocatorFactory, public test::FakeEncoder { public: explicit EncoderBitrateThresholdObserver( @@ -2811,39 +2747,57 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { FakeEncoder(Clock::GetRealTimeClock()), task_queue_(task_queue), target_bitrate_(0), - num_initializations_(0), + num_rate_allocator_creations_(0), + num_encoder_initializations_(0), call_(nullptr), send_stream_(nullptr), - encoder_factory_(this) {} + encoder_factory_(this), + bitrate_allocator_factory_( + CreateBuiltinVideoBitrateAllocatorFactory()) {} private: - int32_t InitEncode(const VideoCodec* codecSettings, - const Settings& settings) override { - EXPECT_GE(codecSettings->startBitrate, codecSettings->minBitrate); - EXPECT_LE(codecSettings->startBitrate, codecSettings->maxBitrate); - if (num_initializations_ == 0) { - EXPECT_EQ(static_cast(kMinBitrateKbps), - codecSettings->minBitrate); + std::unique_ptr CreateVideoBitrateAllocator( + const VideoCodec& codec) override { + EXPECT_GE(codec.startBitrate, codec.minBitrate); + EXPECT_LE(codec.startBitrate, codec.maxBitrate); + if (num_rate_allocator_creations_ == 0) { + EXPECT_EQ(static_cast(kMinBitrateKbps), codec.minBitrate); EXPECT_EQ(static_cast(kStartBitrateKbps), - codecSettings->startBitrate); - EXPECT_EQ(static_cast(kMaxBitrateKbps), - codecSettings->maxBitrate); - observation_complete_.Set(); - } else if (num_initializations_ == 1) { + codec.startBitrate); + EXPECT_EQ(static_cast(kMaxBitrateKbps), codec.maxBitrate); + } else if (num_rate_allocator_creations_ == 1) { EXPECT_EQ(static_cast(kLowerMaxBitrateKbps), - codecSettings->maxBitrate); + codec.maxBitrate); // The start bitrate should be kept (-1) and capped to the max bitrate. // Since this is not an end-to-end call no receiver should have been // returning a REMB that could lower this estimate. - EXPECT_EQ(codecSettings->startBitrate, codecSettings->maxBitrate); - } else if (num_initializations_ == 2) { + EXPECT_EQ(codec.startBitrate, codec.maxBitrate); + } else if (num_rate_allocator_creations_ == 2) { EXPECT_EQ(static_cast(kIncreasedMaxBitrateKbps), - codecSettings->maxBitrate); + codec.maxBitrate); // The start bitrate will be whatever the rate BitRateController // has currently configured but in the span of the set max and min // bitrate. } - ++num_initializations_; + ++num_rate_allocator_creations_; + create_rate_allocator_event_.Set(); + + return bitrate_allocator_factory_->CreateVideoBitrateAllocator(codec); + } + + int32_t InitEncode(const VideoCodec* codecSettings, + const Settings& settings) override { + EXPECT_EQ(0, num_encoder_initializations_); + EXPECT_EQ(static_cast(kMinBitrateKbps), + codecSettings->minBitrate); + EXPECT_EQ(static_cast(kStartBitrateKbps), + codecSettings->startBitrate); + EXPECT_EQ(static_cast(kMaxBitrateKbps), + codecSettings->maxBitrate); + + ++num_encoder_initializations_; + + observation_complete_.Set(); init_encode_event_.Set(); return FakeEncoder::InitEncode(codecSettings, settings); @@ -2892,6 +2846,7 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { std::vector* receive_configs, VideoEncoderConfig* encoder_config) override { send_config->encoder_settings.encoder_factory = &encoder_factory_; + send_config->encoder_settings.bitrate_allocator_factory = this; // Set bitrates lower/higher than min/max to make sure they are properly // capped. encoder_config->max_bitrate_bps = kMaxBitrateKbps * 1000; @@ -2912,6 +2867,9 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { } void PerformTest() override { + ASSERT_TRUE(create_rate_allocator_event_.Wait( + VideoSendStreamTest::kDefaultTimeoutMs)) + << "Timed out while waiting for rate allocator to be created."; ASSERT_TRUE( init_encode_event_.Wait(VideoSendStreamTest::kDefaultTimeoutMs)) << "Timed out while waiting for encoder to be configured."; @@ -2927,33 +2885,40 @@ TEST_F(VideoSendStreamTest, ReconfigureBitratesSetsEncoderBitratesCorrectly) { WaitForSetRates(kMaxBitrateKbps); encoder_config_.max_bitrate_bps = kLowerMaxBitrateKbps * 1000; send_stream_->ReconfigureVideoEncoder(encoder_config_.Copy()); - ASSERT_TRUE( - init_encode_event_.Wait(VideoSendStreamTest::kDefaultTimeoutMs)); - EXPECT_EQ(2, num_initializations_) - << "Encoder should have been reconfigured with the new value."; + ASSERT_TRUE(create_rate_allocator_event_.Wait( + VideoSendStreamTest::kDefaultTimeoutMs)); + EXPECT_EQ(2, num_rate_allocator_creations_) + << "Rate allocator should have been recreated."; + WaitForSetRates(kLowerMaxBitrateKbps); + EXPECT_EQ(1, num_encoder_initializations_); encoder_config_.max_bitrate_bps = kIncreasedMaxBitrateKbps * 1000; send_stream_->ReconfigureVideoEncoder(encoder_config_.Copy()); - ASSERT_TRUE( - init_encode_event_.Wait(VideoSendStreamTest::kDefaultTimeoutMs)); - EXPECT_EQ(3, num_initializations_) - << "Encoder should have been reconfigured with the new value."; + ASSERT_TRUE(create_rate_allocator_event_.Wait( + VideoSendStreamTest::kDefaultTimeoutMs)); + EXPECT_EQ(3, num_rate_allocator_creations_) + << "Rate allocator should have been recreated."; + // Expected target bitrate is the start bitrate set in the call to // call_->GetTransportControllerSend()->SetSdpBitrateParameters. WaitForSetRates(kIncreasedStartBitrateKbps); + EXPECT_EQ(1, num_encoder_initializations_); } test::SingleThreadedTaskQueueForTesting* const task_queue_; + rtc::Event create_rate_allocator_event_; rtc::Event init_encode_event_; rtc::Event bitrate_changed_event_; rtc::CriticalSection crit_; uint32_t target_bitrate_ RTC_GUARDED_BY(&crit_); - int num_initializations_; + int num_rate_allocator_creations_; + int num_encoder_initializations_; webrtc::Call* call_; webrtc::VideoSendStream* send_stream_; test::VideoEncoderProxyFactory encoder_factory_; + std::unique_ptr bitrate_allocator_factory_; webrtc::VideoEncoderConfig encoder_config_; } test(&task_queue_); diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 2f03c533f4..9b2f0f291b 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -99,36 +99,42 @@ CpuOveruseOptions GetCpuOveruseOptions( return options; } -bool RequiresEncoderReset(const VideoCodec& previous_send_codec, - const VideoCodec& new_send_codec) { - // Does not check startBitrate or maxFramerate. - if (new_send_codec.codecType != previous_send_codec.codecType || - new_send_codec.width != previous_send_codec.width || - new_send_codec.height != previous_send_codec.height || - new_send_codec.maxBitrate != previous_send_codec.maxBitrate || - new_send_codec.minBitrate != previous_send_codec.minBitrate || - new_send_codec.qpMax != previous_send_codec.qpMax || +bool RequiresEncoderReset(const VideoCodec& prev_send_codec, + const VideoCodec& new_send_codec, + bool was_encode_called_since_last_initialization) { + // Does not check max/minBitrate or maxFramerate. + if (new_send_codec.codecType != prev_send_codec.codecType || + new_send_codec.width != prev_send_codec.width || + new_send_codec.height != prev_send_codec.height || + new_send_codec.qpMax != prev_send_codec.qpMax || new_send_codec.numberOfSimulcastStreams != - previous_send_codec.numberOfSimulcastStreams || - new_send_codec.mode != previous_send_codec.mode) { + prev_send_codec.numberOfSimulcastStreams || + new_send_codec.mode != prev_send_codec.mode) { + return true; + } + + if (!was_encode_called_since_last_initialization && + (new_send_codec.startBitrate != prev_send_codec.startBitrate)) { + // If start bitrate has changed reconfigure encoder only if encoding had not + // yet started. return true; } switch (new_send_codec.codecType) { case kVideoCodecVP8: - if (new_send_codec.VP8() != previous_send_codec.VP8()) { + if (new_send_codec.VP8() != prev_send_codec.VP8()) { return true; } break; case kVideoCodecVP9: - if (new_send_codec.VP9() != previous_send_codec.VP9()) { + if (new_send_codec.VP9() != prev_send_codec.VP9()) { return true; } break; case kVideoCodecH264: - if (new_send_codec.H264() != previous_send_codec.H264()) { + if (new_send_codec.H264() != prev_send_codec.H264()) { return true; } break; @@ -138,9 +144,18 @@ bool RequiresEncoderReset(const VideoCodec& previous_send_codec, } for (unsigned char i = 0; i < new_send_codec.numberOfSimulcastStreams; ++i) { - if (new_send_codec.simulcastStream[i] != - previous_send_codec.simulcastStream[i]) + if (new_send_codec.simulcastStream[i].width != + prev_send_codec.simulcastStream[i].width || + new_send_codec.simulcastStream[i].height != + prev_send_codec.simulcastStream[i].height || + new_send_codec.simulcastStream[i].numberOfTemporalLayers != + prev_send_codec.simulcastStream[i].numberOfTemporalLayers || + new_send_codec.simulcastStream[i].qpMax != + prev_send_codec.simulcastStream[i].qpMax || + new_send_codec.simulcastStream[i].active != + prev_send_codec.simulcastStream[i].active) { return true; + } } return false; } @@ -475,6 +490,7 @@ VideoStreamEncoder::VideoStreamEncoder( encoder_start_bitrate_bps_(0), max_data_payload_length_(0), encoder_paused_and_dropped_frame_(false), + was_encode_called_since_last_initialization_(false), clock_(clock), degradation_preference_(DegradationPreference::DISABLED), posted_frames_waiting_for_encode_(0), @@ -726,7 +742,8 @@ void VideoStreamEncoder::ReconfigureEncoder() { // Reset (release existing encoder) if one exists and anything except // start bitrate or max framerate has changed. - const bool reset_required = RequiresEncoderReset(codec, send_codec_); + const bool reset_required = RequiresEncoderReset( + codec, send_codec_, was_encode_called_since_last_initialization_); send_codec_ = codec; // Keep the same encoder, as long as the video_format is unchanged. @@ -771,6 +788,7 @@ void VideoStreamEncoder::ReconfigureEncoder() { frame_encode_metadata_writer_.Reset(); last_encode_info_ms_ = absl::nullopt; + was_encode_called_since_last_initialization_ = false; } if (success) { @@ -1361,6 +1379,7 @@ void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame, frame_encode_metadata_writer_.OnEncodeStarted(out_frame); const int32_t encode_status = encoder_->Encode(out_frame, &next_frame_types_); + was_encode_called_since_last_initialization_ = true; if (encode_status < 0) { RTC_LOG(LS_ERROR) << "Failed to encode frame. Error code: " diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index 3802b02332..87c8547bf4 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -265,6 +265,7 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, // Set when configuration must create a new encoder object, e.g., // because of a codec change. bool pending_encoder_creation_ RTC_GUARDED_BY(&encoder_queue_); + absl::optional last_frame_info_ RTC_GUARDED_BY(&encoder_queue_); int crop_width_ RTC_GUARDED_BY(&encoder_queue_); @@ -274,6 +275,12 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, absl::optional last_encoder_rate_settings_ RTC_GUARDED_BY(&encoder_queue_); bool encoder_paused_and_dropped_frame_ RTC_GUARDED_BY(&encoder_queue_); + + // Set to true if at least one frame was sent to encoder since last encoder + // initialization. + bool was_encode_called_since_last_initialization_ + RTC_GUARDED_BY(&encoder_queue_); + Clock* const clock_; // Counters used for deciding if the video resolution or framerate is // currently restricted, and if so, why, on a per degradation preference diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index 982ba2d497..c973e87571 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -57,6 +57,7 @@ const int kMinBalancedFramerateFps = 7; const int64_t kFrameTimeoutMs = 100; const size_t kMaxPayloadLength = 1440; const uint32_t kTargetBitrateBps = 1000000; +const uint32_t kStartBitrateBps = 600000; const uint32_t kSimulcastTargetBitrateBps = 3150000; const uint32_t kLowTargetBitrateBps = kTargetBitrateBps / 10; const int kMaxInitialFramedrop = 4; @@ -355,7 +356,6 @@ class VideoStreamEncoderTest : public ::testing::Test { task_queue_factory_(CreateDefaultTaskQueueFactory()), fake_encoder_(), encoder_factory_(&fake_encoder_), - bitrate_allocator_factory_(CreateBuiltinVideoBitrateAllocatorFactory()), stats_proxy_(new MockableSendStatisticsProxy( Clock::GetRealTimeClock(), video_send_config_, @@ -367,7 +367,7 @@ class VideoStreamEncoderTest : public ::testing::Test { video_send_config_ = VideoSendStream::Config(nullptr); video_send_config_.encoder_settings.encoder_factory = &encoder_factory_; video_send_config_.encoder_settings.bitrate_allocator_factory = - bitrate_allocator_factory_.get(); + &bitrate_allocator_factory_; video_send_config_.rtp.payload_name = "FAKE"; video_send_config_.rtp.payload_type = 125; @@ -761,6 +761,11 @@ class VideoStreamEncoderTest : public ::testing::Test { return allocation; } + int GetNumEncoderInitializations() const { + rtc::CritScope lock(&local_crit_sect_); + return num_encoder_initializations_; + } + private: int32_t Encode(const VideoFrame& input_image, const std::vector* frame_types) override { @@ -796,8 +801,12 @@ class VideoStreamEncoderTest : public ::testing::Test { int32_t InitEncode(const VideoCodec* config, const Settings& settings) override { int res = FakeEncoder::InitEncode(config, settings); + rtc::CritScope lock(&local_crit_sect_); EXPECT_EQ(initialized_, EncoderState::kUninitialized); + + ++num_encoder_initializations_; + if (config->codecType == kVideoCodecVP8) { // Simulate setting up temporal layers, in order to validate the life // cycle of these objects. @@ -872,6 +881,7 @@ class VideoStreamEncoderTest : public ::testing::Test { EncodedImageCallback* encoded_image_callback_ RTC_GUARDED_BY(local_crit_sect_) = nullptr; MockFecControllerOverride fec_controller_override_; + int num_encoder_initializations_ RTC_GUARDED_BY(local_crit_sect_) = 0; }; class TestSink : public VideoStreamEncoder::EncoderSink { @@ -999,7 +1009,7 @@ class VideoStreamEncoderTest : public ::testing::Test { std::vector streams, VideoEncoderConfig::ContentType content_type, int min_transmit_bitrate_bps) override { - rtc::CriticalSection crit_; + rtc::CritScope lock(&crit_); ++number_of_reconfigurations_; min_transmit_bitrate_bps_ = min_transmit_bitrate_bps; } @@ -1021,6 +1031,32 @@ class VideoStreamEncoderTest : public ::testing::Test { int min_transmit_bitrate_bps_ = 0; }; + class VideoBitrateAllocatorProxyFactory + : public VideoBitrateAllocatorFactory { + public: + VideoBitrateAllocatorProxyFactory() + : bitrate_allocator_factory_( + CreateBuiltinVideoBitrateAllocatorFactory()) {} + + std::unique_ptr CreateVideoBitrateAllocator( + const VideoCodec& codec) override { + rtc::CritScope lock(&crit_); + codec_config_ = codec; + return bitrate_allocator_factory_->CreateVideoBitrateAllocator(codec); + } + + VideoCodec codec_config() const { + rtc::CritScope lock(&crit_); + return codec_config_; + } + + private: + std::unique_ptr bitrate_allocator_factory_; + + rtc::CriticalSection crit_; + VideoCodec codec_config_ RTC_GUARDED_BY(crit_); + }; + VideoSendStream::Config video_send_config_; VideoEncoderConfig video_encoder_config_; int codec_width_; @@ -1029,7 +1065,7 @@ class VideoStreamEncoderTest : public ::testing::Test { const std::unique_ptr task_queue_factory_; TestEncoder fake_encoder_; test::VideoEncoderProxyFactory encoder_factory_; - std::unique_ptr bitrate_allocator_factory_; + VideoBitrateAllocatorProxyFactory bitrate_allocator_factory_; std::unique_ptr stats_proxy_; TestSink sink_; AdaptingFrameForwarder video_source_; @@ -1228,6 +1264,50 @@ TEST_F(VideoStreamEncoderTest, FrameResolutionChangeReconfigureEncoder) { video_stream_encoder_->Stop(); } +TEST_F(VideoStreamEncoderTest, BitrateLimitsChangeReconfigureRateAllocator) { + video_stream_encoder_->OnBitrateUpdated( + DataRate::bps(kTargetBitrateBps), DataRate::bps(kTargetBitrateBps), 0, 0); + + VideoEncoderConfig video_encoder_config; + test::FillEncoderConfiguration(kVideoCodecVP8, 1, &video_encoder_config); + video_encoder_config.max_bitrate_bps = kTargetBitrateBps; + video_stream_encoder_->SetStartBitrate(kStartBitrateBps); + video_stream_encoder_->ConfigureEncoder(video_encoder_config.Copy(), + kMaxPayloadLength); + + // Capture a frame and wait for it to synchronize with the encoder thread. + video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr)); + WaitForEncodedFrame(1); + // The encoder will have been configured once when the first frame is + // received. + EXPECT_EQ(1, sink_.number_of_reconfigurations()); + EXPECT_EQ(kTargetBitrateBps, + bitrate_allocator_factory_.codec_config().maxBitrate * 1000); + EXPECT_EQ(kStartBitrateBps, + bitrate_allocator_factory_.codec_config().startBitrate * 1000); + + test::FillEncoderConfiguration(kVideoCodecVP8, 1, &video_encoder_config); + + video_encoder_config.max_bitrate_bps = kTargetBitrateBps * 2; + video_stream_encoder_->SetStartBitrate(kStartBitrateBps * 2); + video_stream_encoder_->ConfigureEncoder(std::move(video_encoder_config), + kMaxPayloadLength); + + // Capture a frame and wait for it to synchronize with the encoder thread. + video_source_.IncomingCapturedFrame(CreateFrame(2, nullptr)); + WaitForEncodedFrame(2); + EXPECT_EQ(2, sink_.number_of_reconfigurations()); + // Bitrate limits have changed - rate allocator should be reconfigured, + // encoder should not be reconfigured. + EXPECT_EQ(kTargetBitrateBps * 2, + bitrate_allocator_factory_.codec_config().maxBitrate * 1000); + EXPECT_EQ(kStartBitrateBps * 2, + bitrate_allocator_factory_.codec_config().startBitrate * 1000); + EXPECT_EQ(1, fake_encoder_.GetNumEncoderInitializations()); + + video_stream_encoder_->Stop(); +} + TEST_F(VideoStreamEncoderTest, SwitchSourceDeregisterEncoderAsSink) { EXPECT_TRUE(video_source_.has_sinks()); test::FrameForwarder new_video_source;