A few simplifications to CodecDatabase and VCMGenericDecoder.
* Remove the ReleaseDecoder and Release methods that were used in combination with deleting the decoder object. Now simply deleting the object does the right thing.
* Remove 'friend' relationship between the two classes since they don't need to touch each other's state directly anymore.
* Use std::unique_ptr for holding pointers and transferring ownership.
These changes were previously reviewed here:
https://codereview.webrtc.org/2764573002/
BUG=webrtc:7361, 695438
Review-Url: https://codereview.webrtc.org/2966823002
Cr-Commit-Position: refs/heads/master@{#18908}
diff --git a/webrtc/modules/video_coding/codec_database.cc b/webrtc/modules/video_coding/codec_database.cc
index 92ae5d2..2f5e7fb 100644
--- a/webrtc/modules/video_coding/codec_database.cc
+++ b/webrtc/modules/video_coding/codec_database.cc
@@ -71,6 +71,31 @@
return h264_settings;
}
+// Create an internal Decoder given a codec type
+static std::unique_ptr<VCMGenericDecoder> CreateDecoder(VideoCodecType type) {
+ switch (type) {
+ case kVideoCodecVP8:
+ return std::unique_ptr<VCMGenericDecoder>(
+ new VCMGenericDecoder(VP8Decoder::Create()));
+ case kVideoCodecVP9:
+ return std::unique_ptr<VCMGenericDecoder>(
+ new VCMGenericDecoder(VP9Decoder::Create()));
+ case kVideoCodecI420:
+ return std::unique_ptr<VCMGenericDecoder>(
+ new VCMGenericDecoder(new I420Decoder()));
+ case kVideoCodecH264:
+ if (H264Decoder::IsSupported()) {
+ return std::unique_ptr<VCMGenericDecoder>(
+ new VCMGenericDecoder(H264Decoder::Create()));
+ }
+ break;
+ default:
+ break;
+ }
+ LOG(LS_WARNING) << "No internal decoder of this type exists.";
+ return std::unique_ptr<VCMGenericDecoder>();
+}
+
VCMDecoderMapItem::VCMDecoderMapItem(VideoCodec* settings,
int number_of_cores,
bool require_key_frame)
@@ -98,13 +123,12 @@
external_encoder_(nullptr),
internal_source_(false),
encoded_frame_callback_(encoded_frame_callback),
- ptr_decoder_(nullptr),
dec_map_(),
dec_external_map_() {}
VCMCodecDataBase::~VCMCodecDataBase() {
DeleteEncoder();
- ReleaseDecoder(ptr_decoder_);
+ ptr_decoder_.reset();
for (auto& kv : dec_map_)
delete kv.second;
for (auto& kv : dec_external_map_)
@@ -400,11 +424,10 @@
// 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_ != nullptr &&
- ptr_decoder_->_decoder == (*it).second->external_decoder_instance) {
+ if (ptr_decoder_ &&
+ ptr_decoder_->IsSameDecoder((*it).second->external_decoder_instance)) {
// Release it if it was registered and in use.
- ReleaseDecoder(ptr_decoder_);
- ptr_decoder_ = nullptr;
+ ptr_decoder_.reset();
}
DeregisterReceiveCodec(payload_type);
delete it->second;
@@ -464,12 +487,11 @@
RTC_DCHECK(decoded_frame_callback->UserReceiveCallback());
uint8_t payload_type = frame.PayloadType();
if (payload_type == receive_codec_.plType || payload_type == 0) {
- return ptr_decoder_;
+ return ptr_decoder_.get();
}
// Check for exisitng decoder, if exists - delete.
if (ptr_decoder_) {
- ReleaseDecoder(ptr_decoder_);
- ptr_decoder_ = nullptr;
+ ptr_decoder_.reset();
memset(&receive_codec_, 0, sizeof(VideoCodec));
}
ptr_decoder_ = CreateAndInitDecoder(frame, &receive_codec_);
@@ -480,36 +502,26 @@
callback->OnIncomingPayloadType(receive_codec_.plType);
if (ptr_decoder_->RegisterDecodeCompleteCallback(decoded_frame_callback) <
0) {
- ReleaseDecoder(ptr_decoder_);
- ptr_decoder_ = nullptr;
+ ptr_decoder_.reset();
memset(&receive_codec_, 0, sizeof(VideoCodec));
return nullptr;
}
- return ptr_decoder_;
+ return ptr_decoder_.get();
}
-void VCMCodecDataBase::ReleaseDecoder(VCMGenericDecoder* decoder) const {
- if (decoder) {
- RTC_DCHECK(decoder->_decoder);
- decoder->Release();
- if (!decoder->External()) {
- delete decoder->_decoder;
- }
- delete decoder;
- }
+VCMGenericDecoder* VCMCodecDataBase::GetCurrentDecoder() {
+ return ptr_decoder_.get();
}
bool VCMCodecDataBase::PrefersLateDecoding() const {
- if (!ptr_decoder_)
- return true;
- return ptr_decoder_->PrefersLateDecoding();
+ return ptr_decoder_ ? ptr_decoder_->PrefersLateDecoding() : true;
}
bool VCMCodecDataBase::MatchesCurrentResolution(int width, int height) const {
return send_codec_.width == width && send_codec_.height == height;
}
-VCMGenericDecoder* VCMCodecDataBase::CreateAndInitDecoder(
+std::unique_ptr<VCMGenericDecoder> VCMCodecDataBase::CreateAndInitDecoder(
const VCMEncodedFrame& frame,
VideoCodec* new_codec) const {
uint8_t payload_type = frame.PayloadType();
@@ -522,13 +534,13 @@
<< static_cast<int>(payload_type);
return nullptr;
}
- VCMGenericDecoder* ptr_decoder = nullptr;
+ std::unique_ptr<VCMGenericDecoder> ptr_decoder;
const VCMExtDecoderMapItem* external_dec_item =
FindExternalDecoderItem(payload_type);
if (external_dec_item) {
// External codec.
- ptr_decoder = new VCMGenericDecoder(
- external_dec_item->external_decoder_instance, true);
+ ptr_decoder.reset(new VCMGenericDecoder(
+ external_dec_item->external_decoder_instance, true));
} else {
// Create decoder.
ptr_decoder = CreateDecoder(decoder_item->settings->codecType);
@@ -547,7 +559,6 @@
}
if (ptr_decoder->InitDecode(decoder_item->settings.get(),
decoder_item->number_of_cores) < 0) {
- ReleaseDecoder(ptr_decoder);
return nullptr;
}
memcpy(new_codec, decoder_item->settings.get(), sizeof(VideoCodec));
@@ -561,26 +572,6 @@
ptr_encoder_.reset();
}
-VCMGenericDecoder* VCMCodecDataBase::CreateDecoder(VideoCodecType type) const {
- switch (type) {
- case kVideoCodecVP8:
- return new VCMGenericDecoder(VP8Decoder::Create());
- case kVideoCodecVP9:
- return new VCMGenericDecoder(VP9Decoder::Create());
- case kVideoCodecI420:
- return new VCMGenericDecoder(new I420Decoder());
- case kVideoCodecH264:
- if (H264Decoder::IsSupported()) {
- return new VCMGenericDecoder(H264Decoder::Create());
- }
- break;
- default:
- break;
- }
- LOG(LS_WARNING) << "No internal decoder of this type exists.";
- return nullptr;
-}
-
const VCMDecoderMapItem* VCMCodecDataBase::FindDecoderItem(
uint8_t payload_type) const {
DecoderMap::const_iterator it = dec_map_.find(payload_type);
diff --git a/webrtc/modules/video_coding/codec_database.h b/webrtc/modules/video_coding/codec_database.h
index 1317e22..5f8656d 100644
--- a/webrtc/modules/video_coding/codec_database.h
+++ b/webrtc/modules/video_coding/codec_database.h
@@ -107,9 +107,9 @@
const VCMEncodedFrame& frame,
VCMDecodedFrameCallback* decoded_frame_callback);
- // Deletes the memory of the decoder instance |decoder|. Used to delete
- // deep copies returned by CreateDecoderCopy().
- void ReleaseDecoder(VCMGenericDecoder* decoder) const;
+ // Returns the current decoder (i.e. the same value as was last returned from
+ // GetDecoder();
+ VCMGenericDecoder* GetCurrentDecoder();
// Returns true if the currently active decoder prefer to decode frames late.
// That means that frames must be decoded near the render times stamp.
@@ -121,8 +121,9 @@
typedef std::map<uint8_t, VCMDecoderMapItem*> DecoderMap;
typedef std::map<uint8_t, VCMExtDecoderMapItem*> ExternalDecoderMap;
- VCMGenericDecoder* CreateAndInitDecoder(const VCMEncodedFrame& frame,
- VideoCodec* new_codec) const;
+ std::unique_ptr<VCMGenericDecoder> CreateAndInitDecoder(
+ const VCMEncodedFrame& frame,
+ VideoCodec* new_codec) const;
// Determines whether a new codec has to be created or not.
// Checks every setting apart from maxFramerate and startBitrate.
@@ -130,9 +131,6 @@
void DeleteEncoder();
- // Create an internal Decoder given a codec type
- VCMGenericDecoder* CreateDecoder(VideoCodecType type) const;
-
const VCMDecoderMapItem* FindDecoderItem(uint8_t payload_type) const;
const VCMExtDecoderMapItem* FindExternalDecoderItem(
@@ -149,7 +147,7 @@
bool internal_source_;
VCMEncodedFrameCallback* const encoded_frame_callback_;
std::unique_ptr<VCMGenericEncoder> ptr_encoder_;
- VCMGenericDecoder* ptr_decoder_;
+ std::unique_ptr<VCMGenericDecoder> ptr_decoder_;
DecoderMap dec_map_;
ExternalDecoderMap dec_external_map_;
}; // VCMCodecDataBase
diff --git a/webrtc/modules/video_coding/generic_decoder.cc b/webrtc/modules/video_coding/generic_decoder.cc
index 722b0b4..42ee8b6 100644
--- a/webrtc/modules/video_coding/generic_decoder.cc
+++ b/webrtc/modules/video_coding/generic_decoder.cc
@@ -8,12 +8,13 @@
* be found in the AUTHORS file in the root of the source tree.
*/
+#include "webrtc/modules/video_coding/generic_decoder.h"
+
#include "webrtc/base/checks.h"
#include "webrtc/base/logging.h"
#include "webrtc/base/timeutils.h"
#include "webrtc/base/trace_event.h"
#include "webrtc/modules/video_coding/include/video_coding.h"
-#include "webrtc/modules/video_coding/generic_decoder.h"
#include "webrtc/modules/video_coding/internal_defines.h"
#include "webrtc/system_wrappers/include/clock.h"
@@ -156,20 +157,26 @@
: _callback(NULL),
_frameInfos(),
_nextFrameInfoIdx(0),
- _decoder(decoder),
+ decoder_(decoder),
_codecType(kVideoCodecUnknown),
_isExternal(isExternal),
- _keyFrameDecoded(false),
- _last_keyframe_content_type(VideoContentType::UNSPECIFIED) {}
+ _last_keyframe_content_type(VideoContentType::UNSPECIFIED) {
+ RTC_DCHECK(decoder_);
+}
-VCMGenericDecoder::~VCMGenericDecoder() {}
+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;
- return _decoder->InitDecode(settings, numberOfCores);
+ return decoder_->InitDecode(settings, numberOfCores);
}
int32_t VCMGenericDecoder::Decode(const VCMEncodedFrame& frame, int64_t nowMs) {
@@ -192,11 +199,11 @@
_nextFrameInfoIdx = (_nextFrameInfoIdx + 1) % kDecoderFrameMemoryLength;
const RTPFragmentationHeader dummy_header;
- int32_t ret = _decoder->Decode(frame.EncodedImage(), frame.MissingFrame(),
+ int32_t ret = decoder_->Decode(frame.EncodedImage(), frame.MissingFrame(),
&dummy_header,
frame.CodecSpecific(), frame.RenderTimeMs());
- _callback->OnDecoderImplementationName(_decoder->ImplementationName());
+ _callback->OnDecoderImplementationName(decoder_->ImplementationName());
if (ret < WEBRTC_VIDEO_CODEC_OK) {
LOG(LS_WARNING) << "Failed to decode frame with timestamp "
<< frame.TimeStamp() << ", error code: " << ret;
@@ -210,22 +217,14 @@
return ret;
}
-int32_t VCMGenericDecoder::Release() {
- return _decoder->Release();
-}
-
int32_t VCMGenericDecoder::RegisterDecodeCompleteCallback(
VCMDecodedFrameCallback* callback) {
_callback = callback;
- return _decoder->RegisterDecodeCompleteCallback(callback);
-}
-
-bool VCMGenericDecoder::External() const {
- return _isExternal;
+ return decoder_->RegisterDecodeCompleteCallback(callback);
}
bool VCMGenericDecoder::PrefersLateDecoding() const {
- return _decoder->PrefersLateDecoding();
+ return decoder_->PrefersLateDecoding();
}
} // namespace webrtc
diff --git a/webrtc/modules/video_coding/generic_decoder.h b/webrtc/modules/video_coding/generic_decoder.h
index 8e26a8b..f7196f0 100644
--- a/webrtc/modules/video_coding/generic_decoder.h
+++ b/webrtc/modules/video_coding/generic_decoder.h
@@ -11,6 +11,8 @@
#ifndef WEBRTC_MODULES_VIDEO_CODING_GENERIC_DECODER_H_
#define WEBRTC_MODULES_VIDEO_CODING_GENERIC_DECODER_H_
+#include <memory>
+
#include "webrtc/base/criticalsection.h"
#include "webrtc/base/thread_checker.h"
#include "webrtc/modules/include/module_common_types.h"
@@ -73,8 +75,6 @@
};
class VCMGenericDecoder {
- friend class VCMCodecDataBase;
-
public:
explicit VCMGenericDecoder(VideoDecoder* decoder, bool isExternal = false);
~VCMGenericDecoder();
@@ -92,26 +92,23 @@
int32_t Decode(const VCMEncodedFrame& inputFrame, int64_t nowMs);
/**
- * Free the decoder memory
- */
- int32_t Release();
-
- /**
* Set decode callback. Deregistering while decoding is illegal.
*/
int32_t RegisterDecodeCompleteCallback(VCMDecodedFrameCallback* callback);
bool External() const;
bool PrefersLateDecoding() const;
+ bool IsSameDecoder(VideoDecoder* decoder) const {
+ return decoder_.get() == decoder;
+ }
private:
VCMDecodedFrameCallback* _callback;
VCMFrameInformation _frameInfos[kDecoderFrameMemoryLength];
uint32_t _nextFrameInfoIdx;
- VideoDecoder* const _decoder;
+ std::unique_ptr<VideoDecoder> decoder_;
VideoCodecType _codecType;
- bool _isExternal;
- bool _keyFrameDecoded;
+ const bool _isExternal;
VideoContentType _last_keyframe_content_type;
};
