From 5cbc528c03eab8d8bb0e573be39c3fb03f9456cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sami=20Kalliom=C3=A4ki?= Date: Mon, 25 Feb 2019 15:56:27 +0000 Subject: [PATCH] Revert "Remove VCMEncoderDataBase and put remaining code into VideoStreamEncoder" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 715c4765b1ac20017e6e3b8b925d02536c6610c3. Reason for revert: Breaks WebRTC roll to Chromium. https://chromium-review.googlesource.com/c/chromium/src/+/1484629 # Fatal error in: ../../third_party/webrtc/modules/rtp_rtcp/source/rtp_sender.cc, line 796 # last system error: 0 # Check failed: diff_ms >= static_cast(0) (-307 vs. 0) # Original change's description: > Remove VCMEncoderDataBase and put remaining code into VideoStreamEncoder > > 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} TBR=brandtr@webrtc.org,nisse@webrtc.org,sprang@webrtc.org Change-Id: I5432878c4c2e497cd848c4ce1b190e0307df03ca Bug: webrtc:10164 Reviewed-on: https://webrtc-review.googlesource.com/c/124402 Commit-Queue: Sami Kalliomäki Reviewed-by: Sami Kalliomäki Cr-Commit-Position: refs/heads/master@{#26841} --- 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, 319 insertions(+), 119 deletions(-) create mode 100644 modules/video_coding/encoder_database.cc create mode 100644 modules/video_coding/encoder_database.h diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn index f3807fa3bd..8c56078c70 100644 --- a/modules/video_coding/BUILD.gn +++ b/modules/video_coding/BUILD.gn @@ -102,6 +102,8 @@ 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 new file mode 100644 index 0000000000..0497e3a72c --- /dev/null +++ b/modules/video_coding/encoder_database.cc @@ -0,0 +1,186 @@ +/* + * 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 new file mode 100644 index 0000000000..09baff01e5 --- /dev/null +++ b/modules/video_coding/encoder_database.h @@ -0,0 +1,68 @@ +/* + * 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 0681eec464..87fc5f6c42 100644 --- a/modules/video_coding/video_coding_impl.h +++ b/modules/video_coding/video_coding_impl.h @@ -19,6 +19,7 @@ #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" @@ -56,6 +57,47 @@ 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 64202cad5d..8dcb8d92cd 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -61,8 +61,6 @@ 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) { @@ -120,51 +118,6 @@ 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 @@ -441,6 +394,7 @@ 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); @@ -460,11 +414,9 @@ void VideoStreamEncoder::Stop() { encoder_queue_.PostTask([this] { RTC_DCHECK_RUN_ON(&encoder_queue_); overuse_detector_->StopCheckForOveruse(); - rate_allocator_ = nullptr; + rate_allocator_.reset(); bitrate_observer_ = nullptr; - if (encoder_ != nullptr && generic_encoder_ != nullptr) { - encoder_->Release(); - } + codec_database_.DeregisterExternalEncoder(); generic_encoder_ = nullptr; quality_scaler_ = nullptr; shutdown_event_.Set(); @@ -641,37 +593,13 @@ 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_) { - encoder_->Release(); + codec_database_.DeregisterExternalEncoder(); generic_encoder_ = nullptr; } @@ -683,25 +611,15 @@ void VideoStreamEncoder::ReconfigureEncoder() { codec_info_ = settings_.encoder_factory->QueryVideoEncoder( encoder_config_.video_format); - } else if (reset_required) { - RTC_DCHECK(encoder_); - encoder_->Release(); + + codec_database_.RegisterExternalEncoder(encoder_.get(), + HasInternalSource()); } - 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; - } - } + // 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(); if (success) { RTC_DCHECK(generic_encoder_); @@ -715,7 +633,7 @@ void VideoStreamEncoder::ReconfigureEncoder() { << " max payload size " << max_data_payload_length_; } else { RTC_LOG(LS_ERROR) << "Failed to configure encoder."; - rate_allocator_ = nullptr; + rate_allocator_.reset(); } if (pending_encoder_creation_) { @@ -1199,8 +1117,8 @@ void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame, encoder_info_ = info; RTC_DCHECK(generic_encoder_); - RTC_DCHECK_EQ(send_codec_.width, out_frame.width()); - RTC_DCHECK_EQ(send_codec_.height, out_frame.height()); + RTC_DCHECK(codec_database_.MatchesCurrentResolution(out_frame.width(), + 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 122c9fb66d..11aa57029e 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -304,12 +304,11 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, RTC_GUARDED_BY(&encoder_queue_); // TODO(webrtc:10164): Refactor/remove these VCM classes. - std::unique_ptr generic_encoder_ - RTC_GUARDED_BY(&encoder_queue_); + // |generic_encoder_| instance is owned by |codec_database_|. + VCMGenericEncoder* generic_encoder_ RTC_GUARDED_BY(&encoder_queue_); VCMEncodedFrameCallback generic_encoder_callback_ RTC_GUARDED_BY(&encoder_queue_); - VideoCodec send_codec_ RTC_GUARDED_BY(&encoder_queue_); - + VCMEncoderDataBase codec_database_ 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 4a26772d73..9bf9cfd697 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_ == EncoderState::kInitialized) { + if (initialized_) { if (quality_scaling_) { info.scaling_settings = VideoEncoder::ScalingSettings(1, 2, kMinPixelsPerFrame); @@ -662,7 +662,6 @@ 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. @@ -673,22 +672,13 @@ class VideoStreamEncoderTest : public ::testing::Test { config->VP8().numberOfTemporalLayers)); } } - if (force_init_encode_failed_) { - initialized_ = EncoderState::kInitializationFailed; + if (force_init_encode_failed_) return -1; - } - initialized_ = EncoderState::kInitialized; + initialized_ = true; 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_); @@ -710,12 +700,7 @@ class VideoStreamEncoderTest : public ::testing::Test { } rtc::CriticalSection local_crit_sect_; - enum class EncoderState { - kUninitialized, - kInitializationFailed, - kInitialized - } initialized_ RTC_GUARDED_BY(local_crit_sect_) = - EncoderState::kUninitialized; + bool initialized_ RTC_GUARDED_BY(local_crit_sect_) = false; 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;