Move ownership of decoders to VCMDecoderDatabase
Bug: webrtc:14497
Change-Id: Idf719a1d1605f19fcf46eff7990c61144f2b9e3b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/277401
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38251}
diff --git a/modules/video_coding/decoder_database.cc b/modules/video_coding/decoder_database.cc
index 6d78e70..3410edc 100644
--- a/modules/video_coding/decoder_database.cc
+++ b/modules/video_coding/decoder_database.cc
@@ -10,6 +10,9 @@
#include "modules/video_coding/decoder_database.h"
+#include <memory>
+#include <utility>
+
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
@@ -19,36 +22,34 @@
decoder_sequence_checker_.Detach();
}
-VideoDecoder* VCMDecoderDatabase::DeregisterExternalDecoder(
- uint8_t payload_type) {
+void VCMDecoderDatabase::DeregisterExternalDecoder(uint8_t payload_type) {
RTC_DCHECK_RUN_ON(&decoder_sequence_checker_);
auto it = decoders_.find(payload_type);
if (it == decoders_.end()) {
- return nullptr;
+ return;
}
// 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 (current_decoder_ && current_decoder_->IsSameDecoder(it->second)) {
+ if (current_decoder_ && current_decoder_->IsSameDecoder(it->second.get())) {
// Release it if it was registered and in use.
current_decoder_ = absl::nullopt;
}
- VideoDecoder* ret = it->second;
decoders_.erase(it);
- return ret;
}
// 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(
uint8_t payload_type,
- VideoDecoder* external_decoder) {
+ std::unique_ptr<VideoDecoder> external_decoder) {
RTC_DCHECK_RUN_ON(&decoder_sequence_checker_);
// If payload value already exists, erase old and insert new.
DeregisterExternalDecoder(payload_type);
if (external_decoder) {
- decoders_[payload_type] = external_decoder;
+ decoders_.emplace(
+ std::make_pair(payload_type, std::move(external_decoder)));
}
}
@@ -131,7 +132,7 @@
RTC_LOG(LS_ERROR) << "No decoder of this type exists.";
return;
}
- current_decoder_.emplace(external_dec_item->second);
+ current_decoder_.emplace(external_dec_item->second.get());
// Copy over input resolutions to prevent codec reinitialization due to
// the first frame being of a different resolution than the database values.
diff --git a/modules/video_coding/decoder_database.h b/modules/video_coding/decoder_database.h
index 8969fea..98f4335 100644
--- a/modules/video_coding/decoder_database.h
+++ b/modules/video_coding/decoder_database.h
@@ -14,6 +14,7 @@
#include <stdint.h>
#include <map>
+#include <memory>
#include "absl/types/optional.h"
#include "api/sequence_checker.h"
@@ -32,9 +33,9 @@
// Returns a pointer to the previously registered decoder or nullptr if none
// was registered for the `payload_type`.
- VideoDecoder* DeregisterExternalDecoder(uint8_t payload_type);
+ void DeregisterExternalDecoder(uint8_t payload_type);
void RegisterExternalDecoder(uint8_t payload_type,
- VideoDecoder* external_decoder);
+ std::unique_ptr<VideoDecoder> external_decoder);
bool IsExternalDecoderRegistered(uint8_t payload_type) const;
void RegisterReceiveCodec(uint8_t payload_type,
@@ -63,7 +64,7 @@
// Initialization paramaters for decoders keyed by payload type.
std::map<uint8_t, VideoDecoder::Settings> decoder_settings_;
// Decoders keyed by payload type.
- std::map<uint8_t, VideoDecoder*> decoders_
+ std::map<uint8_t, std::unique_ptr<VideoDecoder>> decoders_
RTC_GUARDED_BY(decoder_sequence_checker_);
};
diff --git a/modules/video_coding/decoder_database_unittest.cc b/modules/video_coding/decoder_database_unittest.cc
index f2f250c..2e9c91b 100644
--- a/modules/video_coding/decoder_database_unittest.cc
+++ b/modules/video_coding/decoder_database_unittest.cc
@@ -10,6 +10,9 @@
#include "modules/video_coding/decoder_database.h"
+#include <memory>
+#include <utility>
+
#include "api/test/mock_video_decoder.h"
#include "test/gmock.h"
#include "test/gtest.h"
@@ -25,10 +28,16 @@
constexpr int kPayloadType = 1;
ASSERT_FALSE(db.IsExternalDecoderRegistered(kPayloadType));
- NiceMock<MockVideoDecoder> decoder;
- db.RegisterExternalDecoder(kPayloadType, &decoder);
+ auto decoder = std::make_unique<NiceMock<MockVideoDecoder>>();
+ bool decoder_deleted = false;
+ EXPECT_CALL(*decoder, Destruct).WillOnce([&decoder_deleted] {
+ decoder_deleted = true;
+ });
+
+ db.RegisterExternalDecoder(kPayloadType, std::move(decoder));
EXPECT_TRUE(db.IsExternalDecoderRegistered(kPayloadType));
- EXPECT_EQ(db.DeregisterExternalDecoder(kPayloadType), &decoder);
+ db.DeregisterExternalDecoder(kPayloadType);
+ EXPECT_TRUE(decoder_deleted);
EXPECT_FALSE(db.IsExternalDecoderRegistered(kPayloadType));
}
diff --git a/modules/video_coding/video_receiver2.cc b/modules/video_coding/video_receiver2.cc
index a79055e..0751869 100644
--- a/modules/video_coding/video_receiver2.cc
+++ b/modules/video_coding/video_receiver2.cc
@@ -62,16 +62,9 @@
if (decoder) {
RTC_DCHECK(!codec_database_.IsExternalDecoderRegistered(payload_type));
- codec_database_.RegisterExternalDecoder(payload_type, decoder.get());
- video_decoders_.push_back(std::move(decoder));
+ codec_database_.RegisterExternalDecoder(payload_type, std::move(decoder));
} else {
- VideoDecoder* registered =
- codec_database_.DeregisterExternalDecoder(payload_type);
- if (registered) {
- video_decoders_.erase(absl::c_find_if(
- video_decoders_,
- [registered](const auto& d) { return d.get() == registered; }));
- }
+ codec_database_.DeregisterExternalDecoder(payload_type);
}
}
diff --git a/modules/video_coding/video_receiver2.h b/modules/video_coding/video_receiver2.h
index 67cb9af..4457a5b 100644
--- a/modules/video_coding/video_receiver2.h
+++ b/modules/video_coding/video_receiver2.h
@@ -56,10 +56,6 @@
RTC_NO_UNIQUE_ADDRESS SequenceChecker decoder_sequence_checker_;
Clock* const clock_;
VCMDecodedFrameCallback decoded_frame_callback_;
- // Holds/owns the decoder instances that are registered via
- // `RegisterExternalDecoder` and referenced by `codec_database_`.
- std::vector<std::unique_ptr<VideoDecoder>> video_decoders_;
-
// Callbacks are set before the decoder thread starts.
// Once the decoder thread has been started, usage of `_codecDataBase` moves
// over to the decoder thread.