From 82fad3d5134e626906bb55fc6f2060da4e2160ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Wed, 21 Mar 2018 09:57:23 +0100 Subject: [PATCH] Remove TemporalLayersFactory and associated classes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As the rate allocation has been moved into entirely into SimulcastRateAllocator, and the listeners are thus no longer needed, this class doesn't fill any other purpose than to determine if ScreenshareLayers or TemporalLayers should be created for a given simulcast stream. This can however be done just from looking at the VideoCodec instance, so changing this into a static factory method. Due to dependencies from upstream projects, keep the class name and field in VideoCodec around for now. Bug: webrtc:9012 Change-Id: I028fe6b2a19e0d16b35956cc2df01dcf5bfa7979 Reviewed-on: https://webrtc-review.googlesource.com/63264 Commit-Queue: Erik Språng Reviewed-by: Ilya Nikolaevskiy Reviewed-by: Stefan Holmer Cr-Commit-Position: refs/heads/master@{#22529} --- media/engine/simulcast_encoder_adapter.cc | 32 +--------- .../simulcast_encoder_adapter_unittest.cc | 21 ++----- ...encodersoftwarefallbackwrapper_unittest.cc | 18 +----- .../vp8_encoder_simulcast_proxy_unittest.cc | 2 - .../codecs/test/videoprocessor.cc | 15 +---- .../codecs/vp8/default_temporal_layers.cc | 36 ----------- .../vp8/default_temporal_layers_unittest.cc | 2 +- .../codecs/vp8/libvpx_vp8_encoder.cc | 20 ++---- .../codecs/vp8/screenshare_layers.cc | 34 ---------- .../codecs/vp8/simulcast_rate_allocator.cc | 23 +------ .../codecs/vp8/simulcast_rate_allocator.h | 12 +--- .../codecs/vp8/simulcast_test_utility.h | 5 +- .../codecs/vp8/temporal_layers.cc | 52 ++++++++++++++++ .../video_coding/codecs/vp8/temporal_layers.h | 62 +++++-------------- .../codecs/vp8/test/vp8_impl_unittest.cc | 3 - .../include/video_codec_initializer.h | 4 +- .../simulcast_rate_allocator_unittest.cc | 31 +--------- .../video_coding/video_codec_initializer.cc | 34 +--------- .../video_codec_initializer_unittest.cc | 25 +------- modules/video_coding/video_coding_impl.cc | 7 +-- modules/video_coding/video_sender_unittest.cc | 5 +- video/video_send_stream_tests.cc | 4 +- video/video_stream_encoder_unittest.cc | 6 +- 23 files changed, 103 insertions(+), 350 deletions(-) diff --git a/media/engine/simulcast_encoder_adapter.cc b/media/engine/simulcast_encoder_adapter.cc index ce6a5b1c31..e33666f152 100644 --- a/media/engine/simulcast_encoder_adapter.cc +++ b/media/engine/simulcast_encoder_adapter.cc @@ -103,33 +103,6 @@ class AdapterEncodedImageCallback : public webrtc::EncodedImageCallback { webrtc::SimulcastEncoderAdapter* const adapter_; const size_t stream_idx_; }; - -// Utility class used to adapt the simulcast id as reported by the temporal -// layers factory, since each sub-encoder will report stream 0. -class TemporalLayersFactoryAdapter : public webrtc::TemporalLayersFactory { - public: - TemporalLayersFactoryAdapter(int adapted_simulcast_id, - const TemporalLayersFactory& tl_factory) - : adapted_simulcast_id_(adapted_simulcast_id), tl_factory_(tl_factory) {} - ~TemporalLayersFactoryAdapter() override {} - webrtc::TemporalLayers* Create(int simulcast_id, - int temporal_layers, - uint8_t initial_tl0_pic_idx) const override { - return tl_factory_.Create(adapted_simulcast_id_, temporal_layers, - initial_tl0_pic_idx); - } - std::unique_ptr CreateChecker( - int simulcast_id, - int temporal_layers, - uint8_t initial_tl0_pic_idx) const override { - return tl_factory_.CreateChecker(adapted_simulcast_id_, temporal_layers, - initial_tl0_pic_idx); - } - - const int adapted_simulcast_id_; - const TemporalLayersFactory& tl_factory_; -}; - } // namespace namespace webrtc { @@ -204,7 +177,7 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst, } codec_ = *inst; - SimulcastRateAllocator rate_allocator(codec_, nullptr); + SimulcastRateAllocator rate_allocator(codec_); BitrateAllocation allocation = rate_allocator.GetAllocation( codec_.startBitrate * 1000, codec_.maxFramerate); std::vector start_bitrates; @@ -230,9 +203,6 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst, PopulateStreamCodec(codec_, i, start_bitrate_kbps, highest_resolution_stream, &stream_codec); } - TemporalLayersFactoryAdapter tl_factory_adapter(i, - *codec_.VP8()->tl_factory); - stream_codec.VP8()->tl_factory = &tl_factory_adapter; // TODO(ronghuawu): Remove once this is handled in LibvpxVp8Encoder. if (stream_codec.qpMax < kDefaultMinQp) { diff --git a/media/engine/simulcast_encoder_adapter_unittest.cc b/media/engine/simulcast_encoder_adapter_unittest.cc index bd8f044573..d2acd94858 100644 --- a/media/engine/simulcast_encoder_adapter_unittest.cc +++ b/media/engine/simulcast_encoder_adapter_unittest.cc @@ -123,7 +123,7 @@ class MockVideoEncoderFactory : public VideoEncoderFactory { class MockVideoEncoder : public VideoEncoder { public: explicit MockVideoEncoder(MockVideoEncoderFactory* factory) - : factory_(factory) {} + : factory_(factory), callback_(nullptr) {} // TODO(nisse): Valid overrides commented out, because the gmock // methods don't use any override declarations, and we want to avoid @@ -313,9 +313,7 @@ class TestSimulcastEncoderAdapterFake : public ::testing::Test, void SetupCodec() { TestVp8Simulcast::DefaultSettings( &codec_, static_cast(kTestTemporalLayerProfile)); - rate_allocator_.reset(new SimulcastRateAllocator(codec_, nullptr)); - tl_factory_.SetListener(rate_allocator_.get()); - codec_.VP8()->tl_factory = &tl_factory_; + rate_allocator_.reset(new SimulcastRateAllocator(codec_)); EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200)); adapter_->RegisterEncodeCompleteCallback(this); } @@ -351,7 +349,6 @@ class TestSimulcastEncoderAdapterFake : public ::testing::Test, *ref_codec = codec_; ref_codec->VP8()->numberOfTemporalLayers = kTestTemporalLayerProfile[stream_index]; - ref_codec->VP8()->tl_factory = &tl_factory_; ref_codec->width = codec_.simulcastStream[stream_index].width; ref_codec->height = codec_.simulcastStream[stream_index].height; ref_codec->maxBitrate = codec_.simulcastStream[stream_index].maxBitrate; @@ -395,7 +392,6 @@ class TestSimulcastEncoderAdapterFake : public ::testing::Test, int last_encoded_image_width_; int last_encoded_image_height_; int last_encoded_image_simulcast_index_; - TemporalLayersFactory tl_factory_; std::unique_ptr rate_allocator_; }; @@ -466,9 +462,7 @@ TEST_F(TestSimulcastEncoderAdapterFake, ReusesEncodersInOrder) { // Set up common settings for three streams. TestVp8Simulcast::DefaultSettings( &codec_, static_cast(kTestTemporalLayerProfile)); - rate_allocator_.reset(new SimulcastRateAllocator(codec_, nullptr)); - tl_factory_.SetListener(rate_allocator_.get()); - codec_.VP8()->tl_factory = &tl_factory_; + rate_allocator_.reset(new SimulcastRateAllocator(codec_)); adapter_->RegisterEncodeCompleteCallback(this); // Input data. @@ -666,7 +660,6 @@ TEST_F(TestSimulcastEncoderAdapterFake, ReinitDoesNotReorderFrameSimulcastIdx) { TEST_F(TestSimulcastEncoderAdapterFake, SupportsNativeHandleForSingleStreams) { TestVp8Simulcast::DefaultSettings( &codec_, static_cast(kTestTemporalLayerProfile)); - codec_.VP8()->tl_factory = &tl_factory_; codec_.numberOfSimulcastStreams = 1; EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200)); adapter_->RegisterEncodeCompleteCallback(this); @@ -680,11 +673,10 @@ TEST_F(TestSimulcastEncoderAdapterFake, SupportsNativeHandleForSingleStreams) { TEST_F(TestSimulcastEncoderAdapterFake, SetRatesUnderMinBitrate) { TestVp8Simulcast::DefaultSettings( &codec_, static_cast(kTestTemporalLayerProfile)); - codec_.VP8()->tl_factory = &tl_factory_; codec_.minBitrate = 50; codec_.numberOfSimulcastStreams = 1; EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200)); - rate_allocator_.reset(new SimulcastRateAllocator(codec_, nullptr)); + rate_allocator_.reset(new SimulcastRateAllocator(codec_)); // Above min should be respected. BitrateAllocation target_bitrate = @@ -710,7 +702,6 @@ TEST_F(TestSimulcastEncoderAdapterFake, SupportsImplementationName) { EXPECT_STREQ("SimulcastEncoderAdapter", adapter_->ImplementationName()); TestVp8Simulcast::DefaultSettings( &codec_, static_cast(kTestTemporalLayerProfile)); - codec_.VP8()->tl_factory = &tl_factory_; std::vector encoder_names; encoder_names.push_back("codec1"); encoder_names.push_back("codec2"); @@ -733,7 +724,6 @@ TEST_F(TestSimulcastEncoderAdapterFake, SupportsNativeHandleForMultipleStreams) { TestVp8Simulcast::DefaultSettings( &codec_, static_cast(kTestTemporalLayerProfile)); - codec_.VP8()->tl_factory = &tl_factory_; codec_.numberOfSimulcastStreams = 3; EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200)); adapter_->RegisterEncodeCompleteCallback(this); @@ -771,7 +761,6 @@ TEST_F(TestSimulcastEncoderAdapterFake, NativeHandleForwardingForMultipleStreams) { TestVp8Simulcast::DefaultSettings( &codec_, static_cast(kTestTemporalLayerProfile)); - codec_.VP8()->tl_factory = &tl_factory_; codec_.numberOfSimulcastStreams = 3; // High start bitrate, so all streams are enabled. codec_.startBitrate = 3000; @@ -796,7 +785,6 @@ TEST_F(TestSimulcastEncoderAdapterFake, TEST_F(TestSimulcastEncoderAdapterFake, TestFailureReturnCodesFromEncodeCalls) { TestVp8Simulcast::DefaultSettings( &codec_, static_cast(kTestTemporalLayerProfile)); - codec_.VP8()->tl_factory = &tl_factory_; codec_.numberOfSimulcastStreams = 3; EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200)); adapter_->RegisterEncodeCompleteCallback(this); @@ -818,7 +806,6 @@ TEST_F(TestSimulcastEncoderAdapterFake, TestFailureReturnCodesFromEncodeCalls) { TEST_F(TestSimulcastEncoderAdapterFake, TestInitFailureCleansUpEncoders) { TestVp8Simulcast::DefaultSettings( &codec_, static_cast(kTestTemporalLayerProfile)); - codec_.VP8()->tl_factory = &tl_factory_; codec_.numberOfSimulcastStreams = 3; helper_->factory()->set_init_encode_return_value( WEBRTC_VIDEO_CODEC_FALLBACK_SOFTWARE); diff --git a/media/engine/videoencodersoftwarefallbackwrapper_unittest.cc b/media/engine/videoencodersoftwarefallbackwrapper_unittest.cc index f2a7ccf211..df04d8082b 100644 --- a/media/engine/videoencodersoftwarefallbackwrapper_unittest.cc +++ b/media/engine/videoencodersoftwarefallbackwrapper_unittest.cc @@ -171,11 +171,7 @@ void VideoEncoderSoftwareFallbackWrapperTest::UtilizeFallbackEncoder() { codec_.width = kWidth; codec_.height = kHeight; codec_.VP8()->numberOfTemporalLayers = 1; - std::unique_ptr tl_factory( - new TemporalLayersFactory()); - codec_.VP8()->tl_factory = tl_factory.get(); - rate_allocator_.reset( - new SimulcastRateAllocator(codec_, std::move(tl_factory))); + rate_allocator_.reset(new SimulcastRateAllocator(codec_)); fake_encoder_->init_encode_return_code_ = WEBRTC_VIDEO_CODEC_ERROR; EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, @@ -199,11 +195,7 @@ void VideoEncoderSoftwareFallbackWrapperTest::FallbackFromEncodeRequest() { codec_.width = kWidth; codec_.height = kHeight; codec_.VP8()->numberOfTemporalLayers = 1; - std::unique_ptr tl_factory( - new TemporalLayersFactory()); - codec_.VP8()->tl_factory = tl_factory.get(); - rate_allocator_.reset( - new SimulcastRateAllocator(codec_, std::move(tl_factory))); + rate_allocator_.reset(new SimulcastRateAllocator(codec_)); fallback_wrapper_.InitEncode(&codec_, 2, kMaxPayloadSize); EXPECT_EQ( WEBRTC_VIDEO_CODEC_OK, @@ -352,8 +344,6 @@ class ForcedFallbackTest : public VideoEncoderSoftwareFallbackWrapperTest { void ConfigureVp8Codec() { fallback_wrapper_.RegisterEncodeCompleteCallback(&callback_); - std::unique_ptr tl_factory( - new TemporalLayersFactory()); codec_.codecType = kVideoCodecVP8; codec_.maxFramerate = kFramerate; codec_.width = kWidth; @@ -361,9 +351,7 @@ class ForcedFallbackTest : public VideoEncoderSoftwareFallbackWrapperTest { codec_.VP8()->numberOfTemporalLayers = 1; codec_.VP8()->automaticResizeOn = true; codec_.VP8()->frameDroppingOn = true; - codec_.VP8()->tl_factory = tl_factory.get(); - rate_allocator_.reset( - new SimulcastRateAllocator(codec_, std::move(tl_factory))); + rate_allocator_.reset(new SimulcastRateAllocator(codec_)); } void InitEncode(int width, int height) { diff --git a/media/engine/vp8_encoder_simulcast_proxy_unittest.cc b/media/engine/vp8_encoder_simulcast_proxy_unittest.cc index 3e2761cfb6..6b61b18856 100644 --- a/media/engine/vp8_encoder_simulcast_proxy_unittest.cc +++ b/media/engine/vp8_encoder_simulcast_proxy_unittest.cc @@ -63,8 +63,6 @@ TEST(VP8EncoderSimulcastProxy, ChoosesCorrectImplementation) { "SimulcastEncoderAdapter (Fake, Fake, Fake)"; VideoCodec codec_settings; webrtc::test::CodecSettings(kVideoCodecVP8, &codec_settings); - TemporalLayersFactory tl_factory; - codec_settings.VP8()->tl_factory = &tl_factory; codec_settings.simulcastStream[0] = { test::kTestWidth, test::kTestHeight, 2, 2000, 1000, 1000, 56}; codec_settings.simulcastStream[1] = { diff --git a/modules/video_coding/codecs/test/videoprocessor.cc b/modules/video_coding/codecs/test/videoprocessor.cc index abec34500f..7c4df053f9 100644 --- a/modules/video_coding/codecs/test/videoprocessor.cc +++ b/modules/video_coding/codecs/test/videoprocessor.cc @@ -37,18 +37,6 @@ namespace { const int kMsToRtpTimestamp = kVideoPayloadTypeFrequency / 1000; const int kMaxBufferedInputFrames = 10; -std::unique_ptr CreateBitrateAllocator( - TestConfig* config) { - std::unique_ptr tl_factory; - if (config->codec_settings.codecType == VideoCodecType::kVideoCodecVP8) { - tl_factory.reset(new TemporalLayersFactory()); - config->codec_settings.VP8()->tl_factory = tl_factory.get(); - } - return std::unique_ptr( - VideoCodecInitializer::CreateBitrateAllocator(config->codec_settings, - std::move(tl_factory))); -} - size_t GetMaxNaluSizeBytes(const EncodedImage& encoded_frame, const TestConfig& config) { if (config.codec_settings.codecType != kVideoCodecH264) @@ -179,7 +167,8 @@ VideoProcessor::VideoProcessor(webrtc::VideoEncoder* encoder, stats_(stats), encoder_(encoder), decoders_(decoders), - bitrate_allocator_(CreateBitrateAllocator(&config_)), + bitrate_allocator_(VideoCodecInitializer::CreateBitrateAllocator( + config_.codec_settings)), framerate_fps_(0), encode_callback_(this), decode_callback_(this), diff --git a/modules/video_coding/codecs/vp8/default_temporal_layers.cc b/modules/video_coding/codecs/vp8/default_temporal_layers.cc index 02b3a3708a..bcead35d05 100644 --- a/modules/video_coding/codecs/vp8/default_temporal_layers.cc +++ b/modules/video_coding/codecs/vp8/default_temporal_layers.cc @@ -284,18 +284,6 @@ std::vector GetTemporalPattern(size_t num_layers) { return {TemporalLayers::FrameConfig( TemporalLayers::kNone, TemporalLayers::kNone, TemporalLayers::kNone)}; } - -// Temporary fix for forced SW fallback. -// For VP8 SW codec, |TemporalLayers| is created and reported to -// SimulcastRateAllocator::OnTemporalLayersCreated but not for VP8 HW. -// Causes an issue when going from forced SW -> HW as |TemporalLayers| is not -// deregistred when deleted by SW codec (tl factory might not exist, owned by -// SimulcastRateAllocator). -bool ExcludeOnTemporalLayersCreated(int num_temporal_layers) { - return webrtc::field_trial::IsEnabled( - "WebRTC-VP8-Forced-Fallback-Encoder-v2") && - num_temporal_layers == 1; -} } // namespace DefaultTemporalLayers::DefaultTemporalLayers(int number_of_temporal_layers, @@ -399,30 +387,6 @@ void DefaultTemporalLayers::PopulateCodecSpecific( } } -TemporalLayers* TemporalLayersFactory::Create( - int simulcast_id, - int temporal_layers, - uint8_t initial_tl0_pic_idx) const { - TemporalLayers* tl = - new DefaultTemporalLayers(temporal_layers, initial_tl0_pic_idx); - if (listener_ && !ExcludeOnTemporalLayersCreated(temporal_layers)) - listener_->OnTemporalLayersCreated(simulcast_id, tl); - return tl; -} - -std::unique_ptr TemporalLayersFactory::CreateChecker( - int /*simulcast_id*/, - int temporal_layers, - uint8_t initial_tl0_pic_idx) const { - TemporalLayersChecker* tlc = - new DefaultTemporalLayersChecker(temporal_layers, initial_tl0_pic_idx); - return std::unique_ptr(tlc); -} - -void TemporalLayersFactory::SetListener(TemporalLayersListener* listener) { - listener_ = listener; -} - // Returns list of temporal dependencies for each frame in the temporal pattern. // Values are lists of indecies in the pattern. std::vector> GetTemporalDependencies( diff --git a/modules/video_coding/codecs/vp8/default_temporal_layers_unittest.cc b/modules/video_coding/codecs/vp8/default_temporal_layers_unittest.cc index 8314e2e47b..c11a7c933f 100644 --- a/modules/video_coding/codecs/vp8/default_temporal_layers_unittest.cc +++ b/modules/video_coding/codecs/vp8/default_temporal_layers_unittest.cc @@ -63,7 +63,7 @@ std::vector GetTemporalLayerRates(int target_bitrate_kbps, codec.simulcastStream[0].maxBitrate = target_bitrate_kbps; codec.simulcastStream[0].numberOfTemporalLayers = num_temporal_layers; codec.simulcastStream[0].active = true; - SimulcastRateAllocator allocator(codec, nullptr); + SimulcastRateAllocator allocator(codec); return allocator.GetAllocation(target_bitrate_kbps, framerate_fps) .GetTemporalLayerAllocation(0); } diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc index f1effc7624..1f442a8017 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc @@ -344,24 +344,12 @@ void LibvpxVp8Encoder::SetStreamState(bool send_stream, int stream_idx) { void LibvpxVp8Encoder::SetupTemporalLayers(int num_streams, int num_temporal_layers, const VideoCodec& codec) { - RTC_DCHECK(codec.VP8().tl_factory != nullptr); - const TemporalLayersFactory* tl_factory = codec.VP8().tl_factory; RTC_DCHECK(temporal_layers_.empty()); - if (num_streams == 1) { + for (int i = 0; i < num_streams; ++i) { temporal_layers_.emplace_back( - tl_factory->Create(0, num_temporal_layers, tl0_pic_idx_[0])); + TemporalLayers::CreateTemporalLayers(codec, i, tl0_pic_idx_[i])); temporal_layers_checkers_.emplace_back( - tl_factory->CreateChecker(0, num_temporal_layers, tl0_pic_idx_[0])); - } else { - for (int i = 0; i < num_streams; ++i) { - RTC_CHECK_GT(num_temporal_layers, 0); - int layers = std::max(static_cast(1), - codec.simulcastStream[i].numberOfTemporalLayers); - temporal_layers_.emplace_back( - tl_factory->Create(i, layers, tl0_pic_idx_[i])); - temporal_layers_checkers_.emplace_back( - tl_factory->CreateChecker(i, layers, tl0_pic_idx_[i])); - } + TemporalLayers::CreateTemporalLayersChecker(codec, i, tl0_pic_idx_[i])); } } @@ -541,7 +529,7 @@ int LibvpxVp8Encoder::InitEncode(const VideoCodec* inst, // Note the order we use is different from webm, we have lowest resolution // at position 0 and they have highest resolution at position 0. int stream_idx = encoders_.size() - 1; - SimulcastRateAllocator init_allocator(codec_, nullptr); + SimulcastRateAllocator init_allocator(codec_); BitrateAllocation allocation = init_allocator.GetAllocation( inst->startBitrate * 1000, inst->maxFramerate); std::vector stream_bitrates; diff --git a/modules/video_coding/codecs/vp8/screenshare_layers.cc b/modules/video_coding/codecs/vp8/screenshare_layers.cc index 1e8427b7e1..f3d8359da0 100644 --- a/modules/video_coding/codecs/vp8/screenshare_layers.cc +++ b/modules/video_coding/codecs/vp8/screenshare_layers.cc @@ -37,40 +37,6 @@ constexpr int ScreenshareLayers::kMaxNumTemporalLayers; // been exceeded. This prevents needless keyframe requests. const int ScreenshareLayers::kMaxFrameIntervalMs = 2750; -webrtc::TemporalLayers* ScreenshareTemporalLayersFactory::Create( - int simulcast_id, - int num_temporal_layers, - uint8_t initial_tl0_pic_idx) const { - webrtc::TemporalLayers* tl; - if (simulcast_id == 0) { - tl = new webrtc::ScreenshareLayers(num_temporal_layers, rand(), - webrtc::Clock::GetRealTimeClock()); - } else { - TemporalLayersFactory rt_tl_factory; - tl = rt_tl_factory.Create(simulcast_id, num_temporal_layers, rand()); - } - if (listener_) - listener_->OnTemporalLayersCreated(simulcast_id, tl); - return tl; -} - -std::unique_ptr -ScreenshareTemporalLayersFactory::CreateChecker( - int simulcast_id, - int temporal_layers, - uint8_t initial_tl0_pic_idx) const { - webrtc::TemporalLayersChecker* tlc; - if (simulcast_id == 0) { - tlc = - new webrtc::TemporalLayersChecker(temporal_layers, initial_tl0_pic_idx); - } else { - TemporalLayersFactory rt_tl_factory; - return rt_tl_factory.CreateChecker(simulcast_id, temporal_layers, - initial_tl0_pic_idx); - } - return std::unique_ptr(tlc); -} - ScreenshareLayers::ScreenshareLayers(int num_temporal_layers, uint8_t initial_tl0_pic_idx, Clock* clock) diff --git a/modules/video_coding/codecs/vp8/simulcast_rate_allocator.cc b/modules/video_coding/codecs/vp8/simulcast_rate_allocator.cc index 483a36e92e..b819caa80e 100644 --- a/modules/video_coding/codecs/vp8/simulcast_rate_allocator.cc +++ b/modules/video_coding/codecs/vp8/simulcast_rate_allocator.cc @@ -20,20 +20,8 @@ namespace webrtc { -SimulcastRateAllocator::SimulcastRateAllocator( - const VideoCodec& codec, - std::unique_ptr tl_factory) - : codec_(codec), tl_factory_(std::move(tl_factory)) { - if (tl_factory_.get()) - tl_factory_->SetListener(this); -} - -void SimulcastRateAllocator::OnTemporalLayersCreated(int simulcast_id, - TemporalLayers* layers) { - RTC_DCHECK(temporal_layers_.find(simulcast_id) == temporal_layers_.end()); - RTC_DCHECK(layers); - temporal_layers_[simulcast_id] = layers; -} +SimulcastRateAllocator::SimulcastRateAllocator(const VideoCodec& codec) + : codec_(codec) {} BitrateAllocation SimulcastRateAllocator::GetAllocation( uint32_t total_bitrate_bps, @@ -241,12 +229,7 @@ SimulcastRateAllocator::ScreenshareTemporalLayerAllocation( } uint32_t SimulcastRateAllocator::GetPreferredBitrateBps(uint32_t framerate) { - // Create a temporary instance without temporal layers, as they may be - // stateful, and updating the bitrate to max here can cause side effects. - SimulcastRateAllocator temp_allocator(codec_, nullptr); - BitrateAllocation allocation = - temp_allocator.GetAllocation(codec_.maxBitrate * 1000, framerate); - return allocation.get_sum_bps(); + return GetAllocation(codec_.maxBitrate * 1000, framerate).get_sum_bps(); } const VideoCodec& webrtc::SimulcastRateAllocator::GetCodec() const { diff --git a/modules/video_coding/codecs/vp8/simulcast_rate_allocator.h b/modules/video_coding/codecs/vp8/simulcast_rate_allocator.h index 958fdbc293..23d158b91b 100644 --- a/modules/video_coding/codecs/vp8/simulcast_rate_allocator.h +++ b/modules/video_coding/codecs/vp8/simulcast_rate_allocator.h @@ -24,15 +24,9 @@ namespace webrtc { -class SimulcastRateAllocator : public VideoBitrateAllocator, - public TemporalLayersListener { +class SimulcastRateAllocator : public VideoBitrateAllocator { public: - explicit SimulcastRateAllocator( - const VideoCodec& codec, - std::unique_ptr tl_factory); - - void OnTemporalLayersCreated(int simulcast_id, - TemporalLayers* layers) override; + explicit SimulcastRateAllocator(const VideoCodec& codec); BitrateAllocation GetAllocation(uint32_t total_bitrate_bps, uint32_t framerate) override; @@ -58,8 +52,6 @@ class SimulcastRateAllocator : public VideoBitrateAllocator, int NumTemporalStreams(size_t simulcast_id) const; const VideoCodec codec_; - std::map temporal_layers_; - std::unique_ptr tl_factory_; RTC_DISALLOW_COPY_AND_ASSIGN(SimulcastRateAllocator); }; diff --git a/modules/video_coding/codecs/vp8/simulcast_test_utility.h b/modules/video_coding/codecs/vp8/simulcast_test_utility.h index 231d36e1b5..ba2bdcb1c3 100644 --- a/modules/video_coding/codecs/vp8/simulcast_test_utility.h +++ b/modules/video_coding/codecs/vp8/simulcast_test_utility.h @@ -271,10 +271,7 @@ class TestVp8Simulcast : public ::testing::Test { } void SetUpRateAllocator() { - TemporalLayersFactory* tl_factory = new TemporalLayersFactory(); - rate_allocator_.reset(new SimulcastRateAllocator( - settings_, std::unique_ptr(tl_factory))); - settings_.VP8()->tl_factory = tl_factory; + rate_allocator_.reset(new SimulcastRateAllocator(settings_)); } void SetRates(uint32_t bitrate_kbps, uint32_t fps) { diff --git a/modules/video_coding/codecs/vp8/temporal_layers.cc b/modules/video_coding/codecs/vp8/temporal_layers.cc index 0a9b993129..6339516952 100644 --- a/modules/video_coding/codecs/vp8/temporal_layers.cc +++ b/modules/video_coding/codecs/vp8/temporal_layers.cc @@ -15,13 +15,65 @@ #include #include "modules/include/module_common_types.h" +#include "modules/video_coding/codecs/vp8/default_temporal_layers.h" #include "modules/video_coding/codecs/vp8/include/vp8_common_types.h" +#include "modules/video_coding/codecs/vp8/screenshare_layers.h" #include "modules/video_coding/include/video_codec_interface.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" +#include "rtc_base/ptr_util.h" +#include "system_wrappers/include/clock.h" #include "system_wrappers/include/field_trial.h" namespace webrtc { +namespace { +uint8_t NumTemporalLayers(const VideoCodec& codec, int spatial_id) { + uint8_t num_temporal_layers = + std::max(1, codec.VP8().numberOfTemporalLayers); + if (codec.numberOfSimulcastStreams > 0) { + RTC_DCHECK_LT(spatial_id, codec.numberOfSimulcastStreams); + num_temporal_layers = + std::max(num_temporal_layers, + codec.simulcastStream[spatial_id].numberOfTemporalLayers); + } + return num_temporal_layers; +} + +bool IsConferenceModeScreenshare(const VideoCodec& codec) { + if (codec.mode != kScreensharing) { + return false; + } + return NumTemporalLayers(codec, 0) == 2; +} +} // namespace + +std::unique_ptr TemporalLayers::CreateTemporalLayers( + const VideoCodec& codec, + size_t spatial_id, + uint8_t initial_tl0_pic_idx) { + if (IsConferenceModeScreenshare(codec) && spatial_id == 0) { + // Conference mode temporal layering for screen content in base stream. + return rtc::MakeUnique(2, initial_tl0_pic_idx, + Clock::GetRealTimeClock()); + } + + return rtc::MakeUnique( + NumTemporalLayers(codec, spatial_id), initial_tl0_pic_idx); +} + +std::unique_ptr +TemporalLayers::CreateTemporalLayersChecker(const VideoCodec& codec, + size_t spatial_id, + uint8_t initial_tl0_pic_idx) { + if (IsConferenceModeScreenshare(codec) && spatial_id == 0) { + // Conference mode temporal layering for screen content in base stream, + // use generic checker. + return rtc::MakeUnique(2, initial_tl0_pic_idx); + } + + return rtc::MakeUnique( + NumTemporalLayers(codec, spatial_id), initial_tl0_pic_idx); +} TemporalLayersChecker::TemporalLayersChecker(int num_temporal_layers, uint8_t /*initial_tl0_pic_idx*/) diff --git a/modules/video_coding/codecs/vp8/temporal_layers.h b/modules/video_coding/codecs/vp8/temporal_layers.h index e9af0124d2..3d0912c082 100644 --- a/modules/video_coding/codecs/vp8/temporal_layers.h +++ b/modules/video_coding/codecs/vp8/temporal_layers.h @@ -15,6 +15,7 @@ #include #include +#include "common_types.h" // NOLINT(build/include) #include "typedefs.h" // NOLINT(build/include) #define VP8_TS_MAX_PERIODICITY 16 @@ -35,6 +36,7 @@ struct Vp8EncoderConfig { unsigned int rc_max_quantizer; }; +class TemporalLayersChecker; class TemporalLayers { public: enum BufferFlags { @@ -95,6 +97,15 @@ class TemporalLayers { bool freeze_entropy); }; + static std::unique_ptr CreateTemporalLayers( + const VideoCodec& codec, + size_t spatial_id, + uint8_t initial_tl0_pic_idx); + static std::unique_ptr CreateTemporalLayersChecker( + const VideoCodec& codec, + size_t spatial_id, + uint8_t initial_tl0_pic_idx); + // Factory for TemporalLayer strategy. Default behavior is a fixed pattern // of temporal layers. See default_temporal_layers.cc virtual ~TemporalLayers() {} @@ -124,57 +135,14 @@ class TemporalLayers { virtual uint8_t Tl0PicIdx() const = 0; }; -class TemporalLayersListener; -class TemporalLayersChecker; - +// TODO(webrtc:9012): Remove TemporalLayersFactory type and field once all +// upstream usage is gone. class TemporalLayersFactory { public: - TemporalLayersFactory() : listener_(nullptr) {} - virtual ~TemporalLayersFactory() {} - virtual TemporalLayers* Create(int simulcast_id, - int temporal_layers, - uint8_t initial_tl0_pic_idx) const; - - // Creates helper class which performs online checks of a correctness of - // temporal layers dependencies returned by TemporalLayers class created in - // the same factory. - virtual std::unique_ptr CreateChecker( - int simulcast_id, - int temporal_layers, - uint8_t initial_tl0_pic_idx) const; - - void SetListener(TemporalLayersListener* listener); - - protected: - TemporalLayersListener* listener_; + TemporalLayersFactory() = default; + virtual ~TemporalLayersFactory() = default; }; -class ScreenshareTemporalLayersFactory : public webrtc::TemporalLayersFactory { - public: - ScreenshareTemporalLayersFactory() {} - virtual ~ScreenshareTemporalLayersFactory() {} - - webrtc::TemporalLayers* Create(int simulcast_id, - int num_temporal_layers, - uint8_t initial_tl0_pic_idx) const override; - - // Creates helper class which performs online checks of a correctness of - // temporal layers dependencies returned by TemporalLayers class created in - // the same factory. - std::unique_ptr CreateChecker( - int simulcast_id, - int temporal_layers, - uint8_t initial_tl0_pic_idx) const override; -}; - -class TemporalLayersListener { - public: - TemporalLayersListener() {} - virtual ~TemporalLayersListener() {} - - virtual void OnTemporalLayersCreated(int simulcast_id, - TemporalLayers* layers) = 0; -}; // Used only inside RTC_DCHECK(). It checks correctness of temporal layers // dependencies and sync bits. The only method of this class is called after diff --git a/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc b/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc index c99498be6b..504cc3b986 100644 --- a/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc +++ b/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc @@ -60,7 +60,6 @@ class TestVp8Impl : public VideoCodecUnitTest { codec_settings.VP8()->frameDroppingOn = false; codec_settings.VP8()->automaticResizeOn = false; codec_settings.VP8()->complexity = kComplexityNormal; - codec_settings.VP8()->tl_factory = &tl_factory_; return codec_settings; } @@ -120,8 +119,6 @@ class TestVp8Impl : public VideoCodecUnitTest { ASSERT_TRUE(vp8::GetQp(encoded_frame._buffer, encoded_frame._length, &qp)); EXPECT_EQ(encoded_frame.qp_, qp) << "Encoder QP != parsed bitstream QP."; } - - TemporalLayersFactory tl_factory_; }; TEST_F(TestVp8Impl, SetRateAllocation) { diff --git a/modules/video_coding/include/video_codec_initializer.h b/modules/video_coding/include/video_codec_initializer.h index 078d7ab327..aded1f0ff7 100644 --- a/modules/video_coding/include/video_codec_initializer.h +++ b/modules/video_coding/include/video_codec_initializer.h @@ -19,7 +19,6 @@ namespace webrtc { -class TemporalLayersFactory; class VideoBitrateAllocator; class VideoCodec; class VideoEncoderConfig; @@ -44,8 +43,7 @@ class VideoCodecInitializer { // optional, if it is populated, ownership of that instance will be // transferred to the VideoBitrateAllocator instance. static std::unique_ptr CreateBitrateAllocator( - const VideoCodec& codec, - std::unique_ptr tl_factory); + const VideoCodec& codec); private: static VideoCodec VideoEncoderConfigToVideoCodec( diff --git a/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc b/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc index 880ed79047..f986d3b636 100644 --- a/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc +++ b/modules/video_coding/utility/simulcast_rate_allocator_unittest.cc @@ -81,23 +81,7 @@ class SimulcastRateAllocatorTest : public ::testing::TestWithParam { } void CreateAllocator() { - std::unique_ptr tl_factory(GetTlFactory()); - codec_.VP8()->tl_factory = tl_factory.get(); - allocator_.reset(new SimulcastRateAllocator(codec_, std::move(tl_factory))); - - // Simulate InitEncode(). - tl_factories_.clear(); - if (codec_.numberOfSimulcastStreams == 0) { - tl_factories_.push_back( - std::unique_ptr(codec_.VP8()->tl_factory->Create( - 0, codec_.VP8()->numberOfTemporalLayers, 0))); - } else { - for (uint32_t i = 0; i < codec_.numberOfSimulcastStreams; ++i) { - tl_factories_.push_back( - std::unique_ptr(codec_.VP8()->tl_factory->Create( - i, codec_.simulcastStream[i].numberOfTemporalLayers, 0))); - } - } + allocator_.reset(new SimulcastRateAllocator(codec_)); } void SetupCodecThreeSimulcastStreams( @@ -128,10 +112,6 @@ class SimulcastRateAllocatorTest : public ::testing::TestWithParam { } } - virtual std::unique_ptr GetTlFactory() { - return std::unique_ptr(new TemporalLayersFactory()); - } - BitrateAllocation GetAllocation(uint32_t target_bitrate) { return allocator_->GetAllocation(target_bitrate * 1000U, kDefaultFrameRate); } @@ -140,7 +120,6 @@ class SimulcastRateAllocatorTest : public ::testing::TestWithParam { static const int kDefaultFrameRate = 30; VideoCodec codec_; std::unique_ptr allocator_; - std::vector> tl_factories_; }; TEST_F(SimulcastRateAllocatorTest, NoSimulcastBelowMin) { @@ -485,8 +464,7 @@ TEST_F(SimulcastRateAllocatorTest, ThreeStreamsMiddleInactive) { TEST_F(SimulcastRateAllocatorTest, GetPreferredBitrateBps) { MockTemporalLayers mock_layers; - allocator_.reset(new SimulcastRateAllocator(codec_, nullptr)); - allocator_->OnTemporalLayersCreated(0, &mock_layers); + allocator_.reset(new SimulcastRateAllocator(codec_)); EXPECT_CALL(mock_layers, OnRatesUpdated(_, _)).Times(0); EXPECT_EQ(codec_.maxBitrate * 1000, allocator_->GetPreferredBitrateBps(codec_.maxFramerate)); @@ -540,11 +518,6 @@ class ScreenshareRateAllocationTest : public SimulcastRateAllocatorTest { codec_.active = active; } } - - std::unique_ptr GetTlFactory() override { - return std::unique_ptr( - new ScreenshareTemporalLayersFactory()); - } }; INSTANTIATE_TEST_CASE_P(ScreenshareTest, diff --git a/modules/video_coding/video_codec_initializer.cc b/modules/video_coding/video_codec_initializer.cc index f7e12c8668..e473a16581 100644 --- a/modules/video_coding/video_codec_initializer.cc +++ b/modules/video_coding/video_codec_initializer.cc @@ -48,47 +48,19 @@ bool VideoCodecInitializer::SetupCodec( *codec = VideoEncoderConfigToVideoCodec(config, streams, settings.payload_name, settings.payload_type, nack_enabled); - - std::unique_ptr tl_factory; - switch (codec->codecType) { - case kVideoCodecVP8: { - if (!codec->VP8()->tl_factory) { - if (codec->mode == kScreensharing && - (codec->numberOfSimulcastStreams > 1 || - (codec->numberOfSimulcastStreams == 1 && - codec->VP8()->numberOfTemporalLayers == 2))) { - // Conference mode temporal layering for screen content. - tl_factory.reset(new ScreenshareTemporalLayersFactory()); - } else { - // Standard video temporal layers. - tl_factory.reset(new TemporalLayersFactory()); - } - codec->VP8()->tl_factory = tl_factory.get(); - } - break; - } - default: { - // TODO(sprang): Warn, once we have specific allocators for all supported - // codec types. - break; - } - } - *bitrate_allocator = CreateBitrateAllocator(*codec, std::move(tl_factory)); + *bitrate_allocator = CreateBitrateAllocator(*codec); return true; } std::unique_ptr -VideoCodecInitializer::CreateBitrateAllocator( - const VideoCodec& codec, - std::unique_ptr tl_factory) { +VideoCodecInitializer::CreateBitrateAllocator(const VideoCodec& codec) { std::unique_ptr rate_allocator; switch (codec.codecType) { case kVideoCodecVP8: { // Set up default VP8 temporal layer factory, if not provided. - rate_allocator.reset( - new SimulcastRateAllocator(codec, std::move(tl_factory))); + rate_allocator.reset(new SimulcastRateAllocator(codec)); } break; default: rate_allocator.reset(new DefaultVideoBitrateAllocator(codec)); diff --git a/modules/video_coding/video_codec_initializer_unittest.cc b/modules/video_coding/video_codec_initializer_unittest.cc index 46e26470ab..4e99fbbded 100644 --- a/modules/video_coding/video_codec_initializer_unittest.cc +++ b/modules/video_coding/video_codec_initializer_unittest.cc @@ -37,23 +37,6 @@ static const uint32_t kHighScreenshareTl0Bps = 800000; static const uint32_t kHighScreenshareTl1Bps = 1200000; } // namespace -/* - * static bool SetupCodec( - const VideoEncoderConfig& config, - const VideoSendStream::Config::EncoderSettings settings, - const std::vector& streams, - bool nack_enabled, - VideoCodec* codec, - std::unique_ptr* bitrate_allocator); - - // Create a bitrate allocator for the specified codec. |tl_factory| is - // optional, if it is populated, ownership of that instance will be - // transferred to the VideoBitrateAllocator instance. - static std::unique_ptr CreateBitrateAllocator( - const VideoCodec& codec, - std::unique_ptr tl_factory); - */ - // TODO(sprang): Extend coverage to handle the rest of the codec initializer. class VideoCodecInitializerTest : public ::testing::Test { public: @@ -96,14 +79,12 @@ class VideoCodecInitializerTest : public ::testing::Test { } if (codec_out_.codecType == VideoCodecType::kVideoCodecMultiplex) return true; + // Make sure temporal layers instances have been created. if (codec_out_.codecType == VideoCodecType::kVideoCodecVP8) { - if (!codec_out_.VP8()->tl_factory) - return false; - for (int i = 0; i < codec_out_.numberOfSimulcastStreams; ++i) { - temporal_layers_.emplace_back(codec_out_.VP8()->tl_factory->Create( - i, *streams_[i].num_temporal_layers, 0)); + temporal_layers_.emplace_back( + TemporalLayers::CreateTemporalLayers(codec_out_, i, 0)); } } return true; diff --git a/modules/video_coding/video_coding_impl.cc b/modules/video_coding/video_coding_impl.cc index c45f41e3ff..1b098ede22 100644 --- a/modules/video_coding/video_coding_impl.cc +++ b/modules/video_coding/video_coding_impl.cc @@ -114,11 +114,8 @@ class VideoCodingModuleImpl : public VideoCodingModule { // asynchronously keep the instance alive until destruction or until a // new send codec is registered. VideoCodec vp8_codec = *sendCodec; - std::unique_ptr tl_factory( - new TemporalLayersFactory()); - vp8_codec.VP8()->tl_factory = tl_factory.get(); - rate_allocator_ = VideoCodecInitializer::CreateBitrateAllocator( - vp8_codec, std::move(tl_factory)); + rate_allocator_ = + VideoCodecInitializer::CreateBitrateAllocator(vp8_codec); return sender_.RegisterSendCodec(&vp8_codec, numberOfCores, maxPayloadSize); } diff --git a/modules/video_coding/video_sender_unittest.cc b/modules/video_coding/video_sender_unittest.cc index bce3f626cf..8e1330af68 100644 --- a/modules/video_coding/video_sender_unittest.cc +++ b/modules/video_coding/video_sender_unittest.cc @@ -413,10 +413,7 @@ class TestVideoSenderWithVp8 : public TestVideoSender { codec_.startBitrate = codec_bitrate_kbps_; codec_.maxBitrate = codec_bitrate_kbps_; - TemporalLayersFactory* tl_factory = new TemporalLayersFactory(); - rate_allocator_.reset(new SimulcastRateAllocator( - codec_, std::unique_ptr(tl_factory))); - codec_.VP8()->tl_factory = tl_factory; + rate_allocator_.reset(new SimulcastRateAllocator(codec_)); encoder_ = VP8Encoder::Create(); sender_->RegisterExternalEncoder(encoder_.get(), false); diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc index e6f9244fcf..b038f9c3e9 100644 --- a/video/video_send_stream_tests.cc +++ b/video/video_send_stream_tests.cc @@ -2578,12 +2578,10 @@ void VideoCodecConfigObserver::VerifyCodecSpecifics( } // Set expected temporal layers as they should have been set when - // reconfiguring the encoder and not match the set config. Also copy the - // TemporalLayersFactory pointer that has been injected by VideoStreamEncoder. + // reconfiguring the encoder and not match the set config. VideoCodecVP8 encoder_settings = encoder_settings_; encoder_settings.numberOfTemporalLayers = kVideoCodecConfigObserverNumberOfTemporalLayers; - encoder_settings.tl_factory = config.VP8().tl_factory; EXPECT_EQ( 0, memcmp(&config.VP8(), &encoder_settings, sizeof(encoder_settings_))); } diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index 8e2eae376e..8c12d8de71 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -556,15 +556,13 @@ class VideoStreamEncoderTest : public ::testing::Test { int res = FakeEncoder::InitEncode(config, number_of_cores, max_payload_size); rtc::CritScope lock(&local_crit_sect_); - if (config->codecType == kVideoCodecVP8 && config->VP8().tl_factory) { + if (config->codecType == kVideoCodecVP8) { // Simulate setting up temporal layers, in order to validate the life // cycle of these objects. int num_streams = std::max(1, config->numberOfSimulcastStreams); - int num_temporal_layers = - std::max(1, config->VP8().numberOfTemporalLayers); for (int i = 0; i < num_streams; ++i) { allocated_temporal_layers_.emplace_back( - config->VP8().tl_factory->Create(i, num_temporal_layers, 42)); + TemporalLayers::CreateTemporalLayers(*config, i, 42)); } } if (force_init_encode_failed_)