From 715c4765b1ac20017e6e3b8b925d02536c6610c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Mon, 25 Feb 2019 11:18:24 +0100 Subject: [PATCH] Remove VCMEncoderDataBase and put remaining code into VideoStreamEncoder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since this "data base" only holds a single encoder instance it just serves to confuse object ownership. Removing it and giving ownership of generic encoder instance to VideoStreamEncoder. This CL also removes VideoSender interface from video_coding_impl.h, which is mostly a leftover from https://webrtc-review.googlesource.com/c/src/+/123540 Bug: webrtc:10164 Change-Id: I9b7fec940dbcbccf3aa1278c2555da3bd5169ae1 Reviewed-on: https://webrtc-review.googlesource.com/c/123920 Commit-Queue: Erik Språng Reviewed-by: Rasmus Brandt Reviewed-by: Niels Moller Cr-Commit-Position: refs/heads/master@{#26835} --- modules/video_coding/BUILD.gn | 2 - modules/video_coding/encoder_database.cc | 186 ----------------------- modules/video_coding/encoder_database.h | 68 --------- modules/video_coding/video_coding_impl.h | 42 ----- video/video_stream_encoder.cc | 110 ++++++++++++-- video/video_stream_encoder.h | 7 +- video/video_stream_encoder_unittest.cc | 23 ++- 7 files changed, 119 insertions(+), 319 deletions(-) delete mode 100644 modules/video_coding/encoder_database.cc delete mode 100644 modules/video_coding/encoder_database.h diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn index 8c56078c70..f3807fa3bd 100644 --- a/modules/video_coding/BUILD.gn +++ b/modules/video_coding/BUILD.gn @@ -102,8 +102,6 @@ rtc_static_library("video_coding") { "decoder_database.h", "decoding_state.cc", "decoding_state.h", - "encoder_database.cc", - "encoder_database.h", "fec_controller_default.cc", "fec_controller_default.h", "fec_rate_table.h", diff --git a/modules/video_coding/encoder_database.cc b/modules/video_coding/encoder_database.cc deleted file mode 100644 index 0497e3a72c..0000000000 --- a/modules/video_coding/encoder_database.cc +++ /dev/null @@ -1,186 +0,0 @@ -/* - * Copyright (c) 2018 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "modules/video_coding/encoder_database.h" - -#include - -#include "common_types.h" // NOLINT(build/include) -#include "rtc_base/checks.h" -#include "rtc_base/logging.h" - -namespace webrtc { - -namespace { -const size_t kDefaultPayloadSize = 1440; -} - -VCMEncoderDataBase::VCMEncoderDataBase( - VCMEncodedFrameCallback* encoded_frame_callback) - : number_of_cores_(0), - max_payload_size_(kDefaultPayloadSize), - pending_encoder_reset_(true), - send_codec_(), - external_encoder_(nullptr), - internal_source_(false), - encoded_frame_callback_(encoded_frame_callback) {} - -VCMEncoderDataBase::~VCMEncoderDataBase() { - DeleteEncoder(); -} - -// Assuming only one registered encoder - since only one used, no need for more. -bool VCMEncoderDataBase::SetSendCodec(const VideoCodec* send_codec, - int number_of_cores, - size_t max_payload_size) { - RTC_DCHECK(send_codec); - if (max_payload_size == 0) { - max_payload_size = kDefaultPayloadSize; - } - RTC_DCHECK_GE(number_of_cores, 1); - // Make sure the start bit rate is sane... - RTC_DCHECK_LE(send_codec->startBitrate, 1000000); - bool reset_required = pending_encoder_reset_; - if (number_of_cores_ != number_of_cores) { - number_of_cores_ = number_of_cores; - reset_required = true; - } - if (max_payload_size_ != max_payload_size) { - max_payload_size_ = max_payload_size; - reset_required = true; - } - - VideoCodec new_send_codec; - memcpy(&new_send_codec, send_codec, sizeof(new_send_codec)); - - if (new_send_codec.maxBitrate == 0) { - // max is one bit per pixel - new_send_codec.maxBitrate = (static_cast(send_codec->height) * - static_cast(send_codec->width) * - static_cast(send_codec->maxFramerate)) / - 1000; - if (send_codec->startBitrate > new_send_codec.maxBitrate) { - // But if the user tries to set a higher start bit rate we will - // increase the max accordingly. - new_send_codec.maxBitrate = send_codec->startBitrate; - } - } - - if (new_send_codec.startBitrate > new_send_codec.maxBitrate) - new_send_codec.startBitrate = new_send_codec.maxBitrate; - - if (!reset_required) { - reset_required = RequiresEncoderReset(new_send_codec); - } - - memcpy(&send_codec_, &new_send_codec, sizeof(send_codec_)); - - if (!reset_required) { - return true; - } - - // If encoder exists, will destroy it and create new one. - DeleteEncoder(); - ptr_encoder_.reset(new VCMGenericEncoder( - external_encoder_, encoded_frame_callback_, internal_source_)); - encoded_frame_callback_->SetInternalSource(internal_source_); - if (ptr_encoder_->InitEncode(&send_codec_, number_of_cores_, - max_payload_size_) < 0) { - RTC_LOG(LS_ERROR) << "Failed to initialize video encoder."; - DeleteEncoder(); - return false; - } - - pending_encoder_reset_ = false; - - return true; -} - -void VCMEncoderDataBase::DeregisterExternalEncoder() { - DeleteEncoder(); - memset(&send_codec_, 0, sizeof(VideoCodec)); - external_encoder_ = nullptr; - internal_source_ = false; -} - -void VCMEncoderDataBase::RegisterExternalEncoder(VideoEncoder* external_encoder, - bool internal_source) { - // Since only one encoder can be used at a given time, only one external - // encoder can be registered/used. - RTC_CHECK(external_encoder_ == nullptr); - external_encoder_ = external_encoder; - internal_source_ = internal_source; - pending_encoder_reset_ = true; -} - -bool VCMEncoderDataBase::RequiresEncoderReset( - const VideoCodec& new_send_codec) { - if (!ptr_encoder_) - return true; - - // Does not check startBitrate, maxFramerate or plType - if (new_send_codec.codecType != send_codec_.codecType || - new_send_codec.width != send_codec_.width || - new_send_codec.height != send_codec_.height || - new_send_codec.maxBitrate != send_codec_.maxBitrate || - new_send_codec.minBitrate != send_codec_.minBitrate || - new_send_codec.qpMax != send_codec_.qpMax || - new_send_codec.numberOfSimulcastStreams != - send_codec_.numberOfSimulcastStreams || - new_send_codec.mode != send_codec_.mode) { - return true; - } - - switch (new_send_codec.codecType) { - case kVideoCodecVP8: - if (new_send_codec.VP8() != *send_codec_.VP8()) { - return true; - } - break; - - case kVideoCodecVP9: - if (new_send_codec.VP9() != *send_codec_.VP9()) { - return true; - } - break; - - case kVideoCodecH264: - if (new_send_codec.H264() != *send_codec_.H264()) { - return true; - } - break; - - default: - break; - } - - for (unsigned char i = 0; i < new_send_codec.numberOfSimulcastStreams; ++i) { - if (new_send_codec.simulcastStream[i] != send_codec_.simulcastStream[i]) - return true; - } - return false; -} - -VCMGenericEncoder* VCMEncoderDataBase::GetEncoder() { - return ptr_encoder_.get(); -} - -void VCMEncoderDataBase::DeleteEncoder() { - if (!ptr_encoder_) - return; - ptr_encoder_->Release(); - ptr_encoder_.reset(); -} - -bool VCMEncoderDataBase::MatchesCurrentResolution(int width, int height) const { - return send_codec_.width == width && send_codec_.height == height; -} - -} // namespace webrtc diff --git a/modules/video_coding/encoder_database.h b/modules/video_coding/encoder_database.h deleted file mode 100644 index 09baff01e5..0000000000 --- a/modules/video_coding/encoder_database.h +++ /dev/null @@ -1,68 +0,0 @@ -/* - * Copyright (c) 2018 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#ifndef MODULES_VIDEO_CODING_ENCODER_DATABASE_H_ -#define MODULES_VIDEO_CODING_ENCODER_DATABASE_H_ - -#include -#include - -#include "api/video_codecs/video_codec.h" -#include "api/video_codecs/video_encoder.h" -#include "modules/video_coding/generic_encoder.h" - -namespace webrtc { - -class VCMEncoderDataBase { - public: - explicit VCMEncoderDataBase(VCMEncodedFrameCallback* encoded_frame_callback); - ~VCMEncoderDataBase(); - - // Sets the sender side codec and initiates the desired codec given the - // VideoCodec struct. - // Returns true if the codec was successfully registered, false otherwise. - bool SetSendCodec(const VideoCodec* send_codec, - int number_of_cores, - size_t max_payload_size); - - // Registers and initializes an external encoder object. - // |internal_source| should be set to true if the codec has an internal - // video source and doesn't need the user to provide it with frames via - // the Encode() method. - void RegisterExternalEncoder(VideoEncoder* external_encoder, - bool internal_source); - - // Deregisters any external encoder. - void DeregisterExternalEncoder(); - - VCMGenericEncoder* GetEncoder(); - - bool MatchesCurrentResolution(int width, int height) const; - - private: - // Determines whether a new codec has to be created or not. - // Checks every setting apart from maxFramerate and startBitrate. - bool RequiresEncoderReset(const VideoCodec& send_codec); - - void DeleteEncoder(); - - int number_of_cores_; - size_t max_payload_size_; - bool pending_encoder_reset_; - VideoCodec send_codec_; - VideoEncoder* external_encoder_; - bool internal_source_; - VCMEncodedFrameCallback* const encoded_frame_callback_; - std::unique_ptr ptr_encoder_; -}; - -} // namespace webrtc - -#endif // MODULES_VIDEO_CODING_ENCODER_DATABASE_H_ diff --git a/modules/video_coding/video_coding_impl.h b/modules/video_coding/video_coding_impl.h index 87fc5f6c42..0681eec464 100644 --- a/modules/video_coding/video_coding_impl.h +++ b/modules/video_coding/video_coding_impl.h @@ -19,7 +19,6 @@ #include "absl/types/optional.h" #include "modules/video_coding/decoder_database.h" -#include "modules/video_coding/encoder_database.h" #include "modules/video_coding/frame_buffer.h" #include "modules/video_coding/generic_decoder.h" #include "modules/video_coding/generic_encoder.h" @@ -57,47 +56,6 @@ class VCMProcessTimer { int64_t _latestMs; }; -class VideoSender { - public: - typedef VideoCodingModule::SenderNackMode SenderNackMode; - - VideoSender(Clock* clock, EncodedImageCallback* post_encode_callback); - ~VideoSender(); - - // Register the send codec to be used. - // This method must be called on the construction thread. - int32_t RegisterSendCodec(const VideoCodec* sendCodec, - uint32_t numberOfCores, - uint32_t maxPayloadSize); - - void RegisterExternalEncoder(VideoEncoder* externalEncoder, - bool internalSource); - - // Update the the encoder with new bitrate allocation and framerate. - int32_t SetChannelParameters(const VideoBitrateAllocation& bitrate_allocation, - uint32_t framerate_fps); - - int32_t AddVideoFrame(const VideoFrame& videoFrame, - const CodecSpecificInfo* codecSpecificInfo, - absl::optional encoder_info); - - int32_t IntraFrameRequest(size_t stream_index); - - private: - rtc::CriticalSection encoder_crit_; - VCMGenericEncoder* _encoder; - VCMEncodedFrameCallback _encodedFrameCallback RTC_GUARDED_BY(encoder_crit_); - VCMEncoderDataBase _codecDataBase RTC_GUARDED_BY(encoder_crit_); - - // Must be accessed on the construction thread of VideoSender. - VideoCodec current_codec_; - rtc::SequencedTaskChecker sequenced_checker_; - - rtc::CriticalSection params_crit_; - bool encoder_has_internal_source_ RTC_GUARDED_BY(params_crit_); - std::vector next_frame_types_ RTC_GUARDED_BY(params_crit_); -}; - class VideoReceiver : public Module { public: VideoReceiver(Clock* clock, diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 8dcb8d92cd..64202cad5d 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -61,6 +61,8 @@ const float kFramedropThreshold = 0.3; // optimization module defaults. const int64_t kFrameRateAvergingWindowSizeMs = (1000 / 30) * 90; +const size_t kDefaultPayloadSize = 1440; + // Initial limits for BALANCED degradation preference. int MinFps(int pixels) { if (pixels <= 320 * 240) { @@ -118,6 +120,51 @@ 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 || + new_send_codec.numberOfSimulcastStreams != + previous_send_codec.numberOfSimulcastStreams || + new_send_codec.mode != previous_send_codec.mode) { + return true; + } + + switch (new_send_codec.codecType) { + case kVideoCodecVP8: + if (new_send_codec.VP8() != previous_send_codec.VP8()) { + return true; + } + break; + + case kVideoCodecVP9: + if (new_send_codec.VP9() != previous_send_codec.VP9()) { + return true; + } + break; + + case kVideoCodecH264: + if (new_send_codec.H264() != previous_send_codec.H264()) { + return true; + } + break; + + default: + break; + } + + for (unsigned char i = 0; i < new_send_codec.numberOfSimulcastStreams; ++i) { + if (new_send_codec.simulcastStream[i] != + previous_send_codec.simulcastStream[i]) + return true; + } + return false; +} } // namespace // VideoSourceProxy is responsible ensuring thread safety between calls to @@ -394,7 +441,6 @@ VideoStreamEncoder::VideoStreamEncoder( pending_frame_drops_(0), generic_encoder_(nullptr), generic_encoder_callback_(this), - codec_database_(&generic_encoder_callback_), next_frame_types_(1, kVideoFrameDelta), encoder_queue_("EncoderQueue") { RTC_DCHECK(encoder_stats_observer); @@ -414,9 +460,11 @@ void VideoStreamEncoder::Stop() { encoder_queue_.PostTask([this] { RTC_DCHECK_RUN_ON(&encoder_queue_); overuse_detector_->StopCheckForOveruse(); - rate_allocator_.reset(); + rate_allocator_ = nullptr; bitrate_observer_ = nullptr; - codec_database_.DeregisterExternalEncoder(); + if (encoder_ != nullptr && generic_encoder_ != nullptr) { + encoder_->Release(); + } generic_encoder_ = nullptr; quality_scaler_ = nullptr; shutdown_event_.Set(); @@ -593,13 +641,37 @@ void VideoStreamEncoder::ReconfigureEncoder() { } source_proxy_->SetMaxFramerate(max_framerate); + if (codec.maxBitrate == 0) { + // max is one bit per pixel + codec.maxBitrate = + (static_cast(codec.height) * static_cast(codec.width) * + static_cast(codec.maxFramerate)) / + 1000; + if (codec.startBitrate > codec.maxBitrate) { + // But if the user tries to set a higher start bit rate we will + // increase the max accordingly. + codec.maxBitrate = codec.startBitrate; + } + } + + if (codec.startBitrate > codec.maxBitrate) { + codec.startBitrate = codec.maxBitrate; + } + + // Reset (release existing encoder) if one exists and anything except + // start bitrate or max framerate has changed. Don't call Release() if + // |pending_encoder_creation_| as that means this is a new encoder + // that has not yet been initialized. + const bool reset_required = RequiresEncoderReset(codec, send_codec_); + send_codec_ = codec; + // Keep the same encoder, as long as the video_format is unchanged. // Encoder creation block is split in two since EncoderInfo needed to start // CPU adaptation with the correct settings should be polled after // encoder_->InitEncode(). if (pending_encoder_creation_) { if (encoder_) { - codec_database_.DeregisterExternalEncoder(); + encoder_->Release(); generic_encoder_ = nullptr; } @@ -611,15 +683,25 @@ void VideoStreamEncoder::ReconfigureEncoder() { codec_info_ = settings_.encoder_factory->QueryVideoEncoder( encoder_config_.video_format); - - codec_database_.RegisterExternalEncoder(encoder_.get(), - HasInternalSource()); + } else if (reset_required) { + RTC_DCHECK(encoder_); + encoder_->Release(); } - // SetSendCodec implies an unconditional call to encoder_->InitEncode(). - bool success = codec_database_.SetSendCodec(&codec, number_of_cores_, - max_data_payload_length_); - generic_encoder_ = codec_database_.GetEncoder(); + bool success = true; + if (pending_encoder_creation_ || reset_required || !generic_encoder_) { + RTC_DCHECK(encoder_); + generic_encoder_ = absl::make_unique( + encoder_.get(), &generic_encoder_callback_, HasInternalSource()); + if (generic_encoder_->InitEncode(&send_codec_, number_of_cores_, + max_data_payload_length_ > 0 + ? max_data_payload_length_ + : kDefaultPayloadSize) < 0) { + encoder_->Release(); + generic_encoder_ = nullptr; + success = false; + } + } if (success) { RTC_DCHECK(generic_encoder_); @@ -633,7 +715,7 @@ void VideoStreamEncoder::ReconfigureEncoder() { << " max payload size " << max_data_payload_length_; } else { RTC_LOG(LS_ERROR) << "Failed to configure encoder."; - rate_allocator_.reset(); + rate_allocator_ = nullptr; } if (pending_encoder_creation_) { @@ -1117,8 +1199,8 @@ void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame, encoder_info_ = info; RTC_DCHECK(generic_encoder_); - RTC_DCHECK(codec_database_.MatchesCurrentResolution(out_frame.width(), - out_frame.height())); + RTC_DCHECK_EQ(send_codec_.width, out_frame.width()); + RTC_DCHECK_EQ(send_codec_.height, out_frame.height()); const VideoFrameBuffer::Type buffer_type = out_frame.video_frame_buffer()->type(); const bool is_buffer_type_supported = diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index 11aa57029e..122c9fb66d 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -304,11 +304,12 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, RTC_GUARDED_BY(&encoder_queue_); // TODO(webrtc:10164): Refactor/remove these VCM classes. - // |generic_encoder_| instance is owned by |codec_database_|. - VCMGenericEncoder* generic_encoder_ RTC_GUARDED_BY(&encoder_queue_); + std::unique_ptr generic_encoder_ + RTC_GUARDED_BY(&encoder_queue_); VCMEncodedFrameCallback generic_encoder_callback_ RTC_GUARDED_BY(&encoder_queue_); - VCMEncoderDataBase codec_database_ RTC_GUARDED_BY(&encoder_queue_); + VideoCodec send_codec_ RTC_GUARDED_BY(&encoder_queue_); + // TODO(sprang): Change actually support keyframe per simulcast stream, or // turn this into a simple bool |pending_keyframe_request_|. std::vector next_frame_types_ RTC_GUARDED_BY(&encoder_queue_); diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index 9bf9cfd697..4a26772d73 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -547,7 +547,7 @@ class VideoStreamEncoderTest : public ::testing::Test { VideoEncoder::EncoderInfo GetEncoderInfo() const override { rtc::CritScope lock(&local_crit_sect_); EncoderInfo info; - if (initialized_) { + if (initialized_ == EncoderState::kInitialized) { if (quality_scaling_) { info.scaling_settings = VideoEncoder::ScalingSettings(1, 2, kMinPixelsPerFrame); @@ -662,6 +662,7 @@ class VideoStreamEncoderTest : public ::testing::Test { int res = FakeEncoder::InitEncode(config, number_of_cores, max_payload_size); rtc::CritScope lock(&local_crit_sect_); + EXPECT_EQ(initialized_, EncoderState::kUninitialized); if (config->codecType == kVideoCodecVP8) { // Simulate setting up temporal layers, in order to validate the life // cycle of these objects. @@ -672,13 +673,22 @@ class VideoStreamEncoderTest : public ::testing::Test { config->VP8().numberOfTemporalLayers)); } } - if (force_init_encode_failed_) + if (force_init_encode_failed_) { + initialized_ = EncoderState::kInitializationFailed; return -1; + } - initialized_ = true; + initialized_ = EncoderState::kInitialized; return res; } + int32_t Release() override { + rtc::CritScope lock(&local_crit_sect_); + EXPECT_NE(initialized_, EncoderState::kUninitialized); + initialized_ = EncoderState::kUninitialized; + return FakeEncoder::Release(); + } + int32_t SetRateAllocation(const VideoBitrateAllocation& rate_allocation, uint32_t framerate) { rtc::CritScope lock(&local_crit_sect_); @@ -700,7 +710,12 @@ class VideoStreamEncoderTest : public ::testing::Test { } rtc::CriticalSection local_crit_sect_; - bool initialized_ RTC_GUARDED_BY(local_crit_sect_) = false; + enum class EncoderState { + kUninitialized, + kInitializationFailed, + kInitialized + } initialized_ RTC_GUARDED_BY(local_crit_sect_) = + EncoderState::kUninitialized; bool block_next_encode_ RTC_GUARDED_BY(local_crit_sect_) = false; rtc::Event continue_encode_event_; uint32_t timestamp_ RTC_GUARDED_BY(local_crit_sect_) = 0;