Cleanup VCMDecoderDataBase and neigbour VCMGenericDecoder classes
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 <sprang@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34665}
diff --git a/modules/video_coding/decoder_database.cc b/modules/video_coding/decoder_database.cc
index 6aa332e..d4841cd 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 @@
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<VCMGenericDecoder> 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<int>(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<int>(payload_type);
- return nullptr;
+ << int{payload_type};
+ return;
}
- std::unique_ptr<VCMGenericDecoder> 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 @@
// 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 81c68e4..406b53e 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 <map>
-#include <memory>
+#include <stdint.h>
+#include <map>
+
+#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<VideoCodec> 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 @@
VCMDecodedFrameCallback* decoded_frame_callback);
private:
- typedef std::map<uint8_t, VCMDecoderMapItem*> DecoderMap;
- typedef std::map<uint8_t, VCMExtDecoderMapItem*> ExternalDecoderMap;
+ struct DecoderSettings {
+ VideoCodec settings;
+ int number_of_cores;
+ };
- std::unique_ptr<VCMGenericDecoder> 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<VCMGenericDecoder> ptr_decoder_;
- DecoderMap dec_map_;
- ExternalDecoderMap dec_external_map_;
+ absl::optional<uint8_t> current_payload_type_;
+ absl::optional<VCMGenericDecoder> current_decoder_;
+ // Initialization paramaters for decoders keyed by payload type.
+ std::map<uint8_t, DecoderSettings> decoder_settings_;
+ // Decoders keyed by payload type.
+ std::map<uint8_t, VideoDecoder*> decoders_;
};
} // namespace webrtc
diff --git a/modules/video_coding/generic_decoder.cc b/modules/video_coding/generic_decoder.cc
index acb4307..58184bb 100644
--- a/modules/video_coding/generic_decoder.cc
+++ b/modules/video_coding/generic_decoder.cc
@@ -234,29 +234,20 @@
}
}
-VCMGenericDecoder::VCMGenericDecoder(std::unique_ptr<VideoDecoder> 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 8e79cb4..4beae60 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 <memory>
#include <string>
#include "api/sequence_checker.h"
@@ -77,8 +76,7 @@
class VCMGenericDecoder {
public:
- explicit VCMGenericDecoder(std::unique_ptr<VideoDecoder> decoder);
- explicit VCMGenericDecoder(VideoDecoder* decoder, bool isExternal = false);
+ explicit VCMGenericDecoder(VideoDecoder* decoder);
~VCMGenericDecoder();
/**
@@ -99,14 +97,12 @@
int32_t RegisterDecodeCompleteCallback(VCMDecodedFrameCallback* callback);
bool IsSameDecoder(VideoDecoder* decoder) const {
- return decoder_.get() == decoder;
+ return decoder_ == decoder;
}
private:
- VCMDecodedFrameCallback* _callback;
- std::unique_ptr<VideoDecoder> 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 a4cc5b0..6237c96 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 <memory>
#include <vector>
#include "absl/types/optional.h"
@@ -68,7 +69,7 @@
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 43dbc9f..f06a9ab 100644
--- a/modules/video_coding/video_receiver.cc
+++ b/modules/video_coding/video_receiver.cc
@@ -141,7 +141,7 @@
RTC_CHECK(_codecDataBase.DeregisterExternalDecoder(payloadType));
return;
}
- _codecDataBase.RegisterExternalDecoder(externalDecoder, payloadType);
+ _codecDataBase.RegisterExternalDecoder(payloadType, externalDecoder);
}
// Register a frame type request callback.
@@ -253,7 +253,7 @@
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 b893b95..380f4ca 100644
--- a/modules/video_coding/video_receiver2.cc
+++ b/modules/video_coding/video_receiver2.cc
@@ -71,7 +71,7 @@
codecDataBase_.DeregisterExternalDecoder(payloadType);
return;
}
- codecDataBase_.RegisterExternalDecoder(externalDecoder, payloadType);
+ codecDataBase_.RegisterExternalDecoder(payloadType, externalDecoder);
}
bool VideoReceiver2::IsExternalDecoderRegistered(uint8_t payloadType) const {
@@ -118,7 +118,7 @@
if (receiveCodec == nullptr) {
return VCM_PARAMETER_ERROR;
}
- if (!codecDataBase_.RegisterReceiveCodec(payload_type, receiveCodec,
+ if (!codecDataBase_.RegisterReceiveCodec(payload_type, *receiveCodec,
numberOfCores)) {
return -1;
}