From 7b78a3142d88cdc09d49562f420fc11ce25fbf71 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Fri, 6 Aug 2021 12:30:02 +0200 Subject: [PATCH] Cleanup VCMDecoderDataBase and neigbour VCMGenericDecoder classes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove private members that are no longer used or always have same value Use less allocations Bug: None Change-Id: I5430c2356f0039212baf8b248b92775e8852ce1b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/227765 Reviewed-by: Erik Språng Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#34665} --- modules/video_coding/decoder_database.cc | 157 ++++++------------ modules/video_coding/decoder_database.h | 64 +++---- modules/video_coding/generic_decoder.cc | 11 +- modules/video_coding/generic_decoder.h | 12 +- .../video_coding/generic_decoder_unittest.cc | 3 +- modules/video_coding/video_receiver.cc | 4 +- modules/video_coding/video_receiver2.cc | 4 +- 7 files changed, 83 insertions(+), 172 deletions(-) diff --git a/modules/video_coding/decoder_database.cc b/modules/video_coding/decoder_database.cc index 6aa332eb88..d4841cd0a6 100644 --- a/modules/video_coding/decoder_database.cc +++ b/modules/video_coding/decoder_database.cc @@ -15,94 +15,62 @@ namespace webrtc { -VCMDecoderMapItem::VCMDecoderMapItem(VideoCodec* settings, int number_of_cores) - : settings(settings), number_of_cores(number_of_cores) { - RTC_DCHECK_GE(number_of_cores, 0); -} - -VCMExtDecoderMapItem::VCMExtDecoderMapItem( - VideoDecoder* external_decoder_instance, - uint8_t payload_type) - : payload_type(payload_type), - external_decoder_instance(external_decoder_instance) {} - -VCMDecoderMapItem::~VCMDecoderMapItem() {} - -VCMDecoderDataBase::VCMDecoderDataBase() - : current_payload_type_(0), - receive_codec_(), - dec_map_(), - dec_external_map_() {} - -VCMDecoderDataBase::~VCMDecoderDataBase() { - ptr_decoder_.reset(); - for (auto& kv : dec_map_) - delete kv.second; - for (auto& kv : dec_external_map_) - delete kv.second; -} - bool VCMDecoderDataBase::DeregisterExternalDecoder(uint8_t payload_type) { - ExternalDecoderMap::iterator it = dec_external_map_.find(payload_type); - if (it == dec_external_map_.end()) { + auto it = decoders_.find(payload_type); + if (it == decoders_.end()) { // Not found. return false; } // We can't use payload_type to check if the decoder is currently in use, // because payload type may be out of date (e.g. before we decode the first // frame after RegisterReceiveCodec). - if (ptr_decoder_ && - ptr_decoder_->IsSameDecoder((*it).second->external_decoder_instance)) { + if (current_decoder_ && current_decoder_->IsSameDecoder(it->second)) { // Release it if it was registered and in use. - ptr_decoder_.reset(); + current_decoder_ = absl::nullopt; } - delete it->second; - dec_external_map_.erase(it); + decoders_.erase(it); return true; } // Add the external decoder object to the list of external decoders. // Won't be registered as a receive codec until RegisterReceiveCodec is called. -void VCMDecoderDataBase::RegisterExternalDecoder(VideoDecoder* external_decoder, - uint8_t payload_type) { +void VCMDecoderDataBase::RegisterExternalDecoder( + uint8_t payload_type, + VideoDecoder* external_decoder) { // If payload value already exists, erase old and insert new. - VCMExtDecoderMapItem* ext_decoder = - new VCMExtDecoderMapItem(external_decoder, payload_type); DeregisterExternalDecoder(payload_type); - dec_external_map_[payload_type] = ext_decoder; + decoders_[payload_type] = external_decoder; } bool VCMDecoderDataBase::IsExternalDecoderRegistered( uint8_t payload_type) const { return payload_type == current_payload_type_ || - FindExternalDecoderItem(payload_type); + decoders_.find(payload_type) != decoders_.end(); } bool VCMDecoderDataBase::RegisterReceiveCodec(uint8_t payload_type, - const VideoCodec* receive_codec, + const VideoCodec& receive_codec, int number_of_cores) { if (number_of_cores < 0) { return false; } // If payload value already exists, erase old and insert new. - DeregisterReceiveCodec(payload_type); - VideoCodec* new_receive_codec = new VideoCodec(*receive_codec); - dec_map_[payload_type] = - new VCMDecoderMapItem(new_receive_codec, number_of_cores); + if (payload_type == current_payload_type_) { + current_payload_type_ = absl::nullopt; + } + auto& entry = decoder_settings_[payload_type]; + entry.settings = receive_codec; + entry.number_of_cores = number_of_cores; return true; } bool VCMDecoderDataBase::DeregisterReceiveCodec(uint8_t payload_type) { - DecoderMap::iterator it = dec_map_.find(payload_type); - if (it == dec_map_.end()) { + if (decoder_settings_.erase(payload_type) == 0) { return false; } - delete it->second; - dec_map_.erase(it); if (payload_type == current_payload_type_) { // This codec is currently in use. - receive_codec_ = {}; - current_payload_type_ = 0; + current_payload_type_ = absl::nullopt; } return true; } @@ -113,56 +81,47 @@ VCMGenericDecoder* VCMDecoderDataBase::GetDecoder( RTC_DCHECK(decoded_frame_callback->UserReceiveCallback()); uint8_t payload_type = frame.PayloadType(); if (payload_type == current_payload_type_ || payload_type == 0) { - return ptr_decoder_.get(); + return current_decoder_.has_value() ? &*current_decoder_ : nullptr; } // If decoder exists - delete. - if (ptr_decoder_) { - ptr_decoder_.reset(); - receive_codec_ = {}; - current_payload_type_ = 0; + if (current_decoder_.has_value()) { + current_decoder_ = absl::nullopt; + current_payload_type_ = absl::nullopt; } - ptr_decoder_ = CreateAndInitDecoder(frame, &receive_codec_); - if (!ptr_decoder_) { + + CreateAndInitDecoder(frame); + if (current_decoder_ == absl::nullopt) { return nullptr; } - current_payload_type_ = frame.PayloadType(); + VCMReceiveCallback* callback = decoded_frame_callback->UserReceiveCallback(); - callback->OnIncomingPayloadType(current_payload_type_); - if (ptr_decoder_->RegisterDecodeCompleteCallback(decoded_frame_callback) < + callback->OnIncomingPayloadType(payload_type); + if (current_decoder_->RegisterDecodeCompleteCallback(decoded_frame_callback) < 0) { - ptr_decoder_.reset(); - receive_codec_ = {}; - current_payload_type_ = 0; + current_decoder_ = absl::nullopt; return nullptr; } - return ptr_decoder_.get(); + + current_payload_type_ = payload_type; + return &*current_decoder_; } -std::unique_ptr VCMDecoderDataBase::CreateAndInitDecoder( - const VCMEncodedFrame& frame, - VideoCodec* new_codec) const { +void VCMDecoderDataBase::CreateAndInitDecoder(const VCMEncodedFrame& frame) { uint8_t payload_type = frame.PayloadType(); RTC_LOG(LS_INFO) << "Initializing decoder with payload type '" - << static_cast(payload_type) << "'."; - RTC_DCHECK(new_codec); - const VCMDecoderMapItem* decoder_item = FindDecoderItem(payload_type); - if (!decoder_item) { + << int{payload_type} << "'."; + auto decoder_item = decoder_settings_.find(payload_type); + if (decoder_item == decoder_settings_.end()) { RTC_LOG(LS_ERROR) << "Can't find a decoder associated with payload type: " - << static_cast(payload_type); - return nullptr; + << int{payload_type}; + return; } - std::unique_ptr ptr_decoder; - const VCMExtDecoderMapItem* external_dec_item = - FindExternalDecoderItem(payload_type); - if (external_dec_item) { - // External codec. - ptr_decoder.reset(new VCMGenericDecoder( - external_dec_item->external_decoder_instance, true)); - } else { + auto external_dec_item = decoders_.find(payload_type); + if (external_dec_item == decoders_.end()) { RTC_LOG(LS_ERROR) << "No decoder of this type exists."; + return; } - if (!ptr_decoder) - return nullptr; + current_decoder_.emplace(external_dec_item->second); // Copy over input resolutions to prevent codec reinitialization due to // the first frame being of a different resolution than the database values. @@ -170,35 +129,15 @@ std::unique_ptr VCMDecoderDataBase::CreateAndInitDecoder( // parsed yet (and may be zero). if (frame.EncodedImage()._encodedWidth > 0 && frame.EncodedImage()._encodedHeight > 0) { - decoder_item->settings->width = frame.EncodedImage()._encodedWidth; - decoder_item->settings->height = frame.EncodedImage()._encodedHeight; + decoder_item->second.settings.width = frame.EncodedImage()._encodedWidth; + decoder_item->second.settings.height = frame.EncodedImage()._encodedHeight; } - int err = ptr_decoder->InitDecode(decoder_item->settings.get(), - decoder_item->number_of_cores); + int err = current_decoder_->InitDecode(&decoder_item->second.settings, + decoder_item->second.number_of_cores); if (err < 0) { + current_decoder_ = absl::nullopt; RTC_LOG(LS_ERROR) << "Failed to initialize decoder. Error code: " << err; - return nullptr; } - *new_codec = *decoder_item->settings.get(); - return ptr_decoder; -} - -const VCMDecoderMapItem* VCMDecoderDataBase::FindDecoderItem( - uint8_t payload_type) const { - DecoderMap::const_iterator it = dec_map_.find(payload_type); - if (it != dec_map_.end()) { - return (*it).second; - } - return nullptr; -} - -const VCMExtDecoderMapItem* VCMDecoderDataBase::FindExternalDecoderItem( - uint8_t payload_type) const { - ExternalDecoderMap::const_iterator it = dec_external_map_.find(payload_type); - if (it != dec_external_map_.end()) { - return (*it).second; - } - return nullptr; } } // namespace webrtc diff --git a/modules/video_coding/decoder_database.h b/modules/video_coding/decoder_database.h index 81c68e4138..406b53e5ac 100644 --- a/modules/video_coding/decoder_database.h +++ b/modules/video_coding/decoder_database.h @@ -11,43 +11,31 @@ #ifndef MODULES_VIDEO_CODING_DECODER_DATABASE_H_ #define MODULES_VIDEO_CODING_DECODER_DATABASE_H_ -#include -#include +#include +#include + +#include "absl/types/optional.h" +#include "api/video_codecs/video_decoder.h" +#include "modules/video_coding/encoded_frame.h" #include "modules/video_coding/generic_decoder.h" namespace webrtc { -struct VCMDecoderMapItem { - public: - VCMDecoderMapItem(VideoCodec* settings, int number_of_cores); - ~VCMDecoderMapItem(); - - std::unique_ptr settings; - int number_of_cores; -}; - -struct VCMExtDecoderMapItem { - public: - VCMExtDecoderMapItem(VideoDecoder* external_decoder_instance, - uint8_t payload_type); - - uint8_t payload_type; - VideoDecoder* external_decoder_instance; -}; - class VCMDecoderDataBase { public: - VCMDecoderDataBase(); - ~VCMDecoderDataBase(); + VCMDecoderDataBase() = default; + VCMDecoderDataBase(const VCMDecoderDataBase&) = delete; + VCMDecoderDataBase& operator=(const VCMDecoderDataBase&) = delete; + ~VCMDecoderDataBase() = default; bool DeregisterExternalDecoder(uint8_t payload_type); - void RegisterExternalDecoder(VideoDecoder* external_decoder, - uint8_t payload_type); + void RegisterExternalDecoder(uint8_t payload_type, + VideoDecoder* external_decoder); bool IsExternalDecoderRegistered(uint8_t payload_type) const; bool RegisterReceiveCodec(uint8_t payload_type, - const VideoCodec* receive_codec, + const VideoCodec& receive_codec, int number_of_cores); bool DeregisterReceiveCodec(uint8_t payload_type); @@ -61,23 +49,19 @@ class VCMDecoderDataBase { VCMDecodedFrameCallback* decoded_frame_callback); private: - typedef std::map DecoderMap; - typedef std::map ExternalDecoderMap; + struct DecoderSettings { + VideoCodec settings; + int number_of_cores; + }; - std::unique_ptr CreateAndInitDecoder( - const VCMEncodedFrame& frame, - VideoCodec* new_codec) const; + void CreateAndInitDecoder(const VCMEncodedFrame& frame); - const VCMDecoderMapItem* FindDecoderItem(uint8_t payload_type) const; - - const VCMExtDecoderMapItem* FindExternalDecoderItem( - uint8_t payload_type) const; - - uint8_t current_payload_type_; // Corresponding to receive_codec_. - VideoCodec receive_codec_; - std::unique_ptr ptr_decoder_; - DecoderMap dec_map_; - ExternalDecoderMap dec_external_map_; + absl::optional current_payload_type_; + absl::optional current_decoder_; + // Initialization paramaters for decoders keyed by payload type. + std::map decoder_settings_; + // Decoders keyed by payload type. + std::map decoders_; }; } // namespace webrtc diff --git a/modules/video_coding/generic_decoder.cc b/modules/video_coding/generic_decoder.cc index acb4307f3f..58184bbf6c 100644 --- a/modules/video_coding/generic_decoder.cc +++ b/modules/video_coding/generic_decoder.cc @@ -234,29 +234,20 @@ void VCMDecodedFrameCallback::ClearTimestampMap() { } } -VCMGenericDecoder::VCMGenericDecoder(std::unique_ptr decoder) - : VCMGenericDecoder(decoder.release(), false /* isExternal */) {} - -VCMGenericDecoder::VCMGenericDecoder(VideoDecoder* decoder, bool isExternal) +VCMGenericDecoder::VCMGenericDecoder(VideoDecoder* decoder) : _callback(NULL), decoder_(decoder), - _codecType(kVideoCodecGeneric), - _isExternal(isExternal), _last_keyframe_content_type(VideoContentType::UNSPECIFIED) { RTC_DCHECK(decoder_); } VCMGenericDecoder::~VCMGenericDecoder() { decoder_->Release(); - if (_isExternal) - decoder_.release(); - RTC_DCHECK(_isExternal || decoder_); } int32_t VCMGenericDecoder::InitDecode(const VideoCodec* settings, int32_t numberOfCores) { TRACE_EVENT0("webrtc", "VCMGenericDecoder::InitDecode"); - _codecType = settings->codecType; int err = decoder_->InitDecode(settings, numberOfCores); decoder_info_ = decoder_->GetDecoderInfo(); diff --git a/modules/video_coding/generic_decoder.h b/modules/video_coding/generic_decoder.h index 8e79cb4e19..4beae604f4 100644 --- a/modules/video_coding/generic_decoder.h +++ b/modules/video_coding/generic_decoder.h @@ -11,7 +11,6 @@ #ifndef MODULES_VIDEO_CODING_GENERIC_DECODER_H_ #define MODULES_VIDEO_CODING_GENERIC_DECODER_H_ -#include #include #include "api/sequence_checker.h" @@ -77,8 +76,7 @@ class VCMDecodedFrameCallback : public DecodedImageCallback { class VCMGenericDecoder { public: - explicit VCMGenericDecoder(std::unique_ptr decoder); - explicit VCMGenericDecoder(VideoDecoder* decoder, bool isExternal = false); + explicit VCMGenericDecoder(VideoDecoder* decoder); ~VCMGenericDecoder(); /** @@ -99,14 +97,12 @@ class VCMGenericDecoder { int32_t RegisterDecodeCompleteCallback(VCMDecodedFrameCallback* callback); bool IsSameDecoder(VideoDecoder* decoder) const { - return decoder_.get() == decoder; + return decoder_ == decoder; } private: - VCMDecodedFrameCallback* _callback; - std::unique_ptr decoder_; - VideoCodecType _codecType; - const bool _isExternal; + VCMDecodedFrameCallback* _callback = nullptr; + VideoDecoder* const decoder_; VideoContentType _last_keyframe_content_type; VideoDecoder::DecoderInfo decoder_info_; }; diff --git a/modules/video_coding/generic_decoder_unittest.cc b/modules/video_coding/generic_decoder_unittest.cc index a4cc5b0ded..6237c96b8a 100644 --- a/modules/video_coding/generic_decoder_unittest.cc +++ b/modules/video_coding/generic_decoder_unittest.cc @@ -10,6 +10,7 @@ #include "modules/video_coding/generic_decoder.h" +#include #include #include "absl/types/optional.h" @@ -68,7 +69,7 @@ class GenericDecoderTest : public ::testing::Test { task_queue_factory_(CreateDefaultTaskQueueFactory()), decoder_(task_queue_factory_.get()), vcm_callback_(&timing_, &clock_), - generic_decoder_(&decoder_, /*isExternal=*/true) {} + generic_decoder_(&decoder_) {} void SetUp() override { generic_decoder_.RegisterDecodeCompleteCallback(&vcm_callback_); diff --git a/modules/video_coding/video_receiver.cc b/modules/video_coding/video_receiver.cc index 43dbc9f0b2..f06a9abc01 100644 --- a/modules/video_coding/video_receiver.cc +++ b/modules/video_coding/video_receiver.cc @@ -141,7 +141,7 @@ void VideoReceiver::RegisterExternalDecoder(VideoDecoder* externalDecoder, RTC_CHECK(_codecDataBase.DeregisterExternalDecoder(payloadType)); return; } - _codecDataBase.RegisterExternalDecoder(externalDecoder, payloadType); + _codecDataBase.RegisterExternalDecoder(payloadType, externalDecoder); } // Register a frame type request callback. @@ -253,7 +253,7 @@ int32_t VideoReceiver::RegisterReceiveCodec(uint8_t payload_type, if (receiveCodec == nullptr) { return VCM_PARAMETER_ERROR; } - if (!_codecDataBase.RegisterReceiveCodec(payload_type, receiveCodec, + if (!_codecDataBase.RegisterReceiveCodec(payload_type, *receiveCodec, numberOfCores)) { return -1; } diff --git a/modules/video_coding/video_receiver2.cc b/modules/video_coding/video_receiver2.cc index b893b954bc..380f4ca775 100644 --- a/modules/video_coding/video_receiver2.cc +++ b/modules/video_coding/video_receiver2.cc @@ -71,7 +71,7 @@ void VideoReceiver2::RegisterExternalDecoder(VideoDecoder* externalDecoder, codecDataBase_.DeregisterExternalDecoder(payloadType); return; } - codecDataBase_.RegisterExternalDecoder(externalDecoder, payloadType); + codecDataBase_.RegisterExternalDecoder(payloadType, externalDecoder); } bool VideoReceiver2::IsExternalDecoderRegistered(uint8_t payloadType) const { @@ -118,7 +118,7 @@ int32_t VideoReceiver2::RegisterReceiveCodec(uint8_t payload_type, if (receiveCodec == nullptr) { return VCM_PARAMETER_ERROR; } - if (!codecDataBase_.RegisterReceiveCodec(payload_type, receiveCodec, + if (!codecDataBase_.RegisterReceiveCodec(payload_type, *receiveCodec, numberOfCores)) { return -1; }