Move decoder instance ownership to VideoReceiver2
This moves the ownership away from VideoReceiveStream2 and closer to
VCMDecoderDataBase. That facilitates unregistration (upcoming change)
without recreating receive streams.
Bug: none
Change-Id: I812175134730a0ffbf7077fd149c8489481c73d8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/272481
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37866}
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index 4005de8..dc404dc 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -3012,7 +3012,6 @@
}
if (codec.ulpfec.red_rtx_payload_type != -1) {
- // TODO(tommi): Look into if/when this happens in practice.
rtx_associated_payload_types[codec.ulpfec.red_rtx_payload_type] =
codec.ulpfec.red_payload_type;
}
diff --git a/modules/video_coding/decoder_database.cc b/modules/video_coding/decoder_database.cc
index 882de27..15d8dcc 100644
--- a/modules/video_coding/decoder_database.cc
+++ b/modules/video_coding/decoder_database.cc
@@ -19,13 +19,14 @@
decoder_sequence_checker_.Detach();
}
-bool VCMDecoderDataBase::DeregisterExternalDecoder(uint8_t payload_type) {
+VideoDecoder* VCMDecoderDataBase::DeregisterExternalDecoder(
+ uint8_t payload_type) {
RTC_DCHECK_RUN_ON(&decoder_sequence_checker_);
auto it = decoders_.find(payload_type);
if (it == decoders_.end()) {
- // Not found.
- return false;
+ return nullptr;
}
+
// 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).
@@ -33,8 +34,9 @@
// Release it if it was registered and in use.
current_decoder_ = absl::nullopt;
}
+ VideoDecoder* ret = it->second;
decoders_.erase(it);
- return true;
+ return ret;
}
// Add the external decoder object to the list of external decoders.
diff --git a/modules/video_coding/decoder_database.h b/modules/video_coding/decoder_database.h
index 98bbf85..59b683b 100644
--- a/modules/video_coding/decoder_database.h
+++ b/modules/video_coding/decoder_database.h
@@ -30,7 +30,9 @@
VCMDecoderDataBase& operator=(const VCMDecoderDataBase&) = delete;
~VCMDecoderDataBase() = default;
- bool DeregisterExternalDecoder(uint8_t payload_type);
+ // 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 RegisterExternalDecoder(uint8_t payload_type,
VideoDecoder* external_decoder);
bool IsExternalDecoderRegistered(uint8_t payload_type) const;
diff --git a/modules/video_coding/video_receiver2.cc b/modules/video_coding/video_receiver2.cc
index fe35420..36f53a7 100644
--- a/modules/video_coding/video_receiver2.cc
+++ b/modules/video_coding/video_receiver2.cc
@@ -13,8 +13,10 @@
#include <stddef.h>
#include <cstdint>
+#include <utility>
#include <vector>
+#include "absl/algorithm/container.h"
#include "api/video_codecs/video_codec.h"
#include "api/video_codecs/video_decoder.h"
#include "modules/video_coding/decoder_database.h"
@@ -32,9 +34,8 @@
VCMTiming* timing,
const FieldTrialsView& field_trials)
: clock_(clock),
- timing_(timing),
- decodedFrameCallback_(timing_, clock_, field_trials),
- codecDataBase_() {
+ decoded_frame_callback_(timing, clock_, field_trials),
+ codec_database_() {
decoder_sequence_checker_.Detach();
}
@@ -45,29 +46,38 @@
// Register a receive callback. Will be called whenever there is a new frame
// ready for rendering.
int32_t VideoReceiver2::RegisterReceiveCallback(
- VCMReceiveCallback* receiveCallback) {
+ VCMReceiveCallback* receive_callback) {
RTC_DCHECK_RUN_ON(&construction_sequence_checker_);
// This value is set before the decoder thread starts and unset after
// the decoder thread has been stopped.
- decodedFrameCallback_.SetUserReceiveCallback(receiveCallback);
+ decoded_frame_callback_.SetUserReceiveCallback(receive_callback);
return VCM_OK;
}
-void VideoReceiver2::RegisterExternalDecoder(VideoDecoder* externalDecoder,
- uint8_t payloadType) {
+void VideoReceiver2::RegisterExternalDecoder(
+ std::unique_ptr<VideoDecoder> decoder,
+ uint8_t payload_type) {
RTC_DCHECK_RUN_ON(&decoder_sequence_checker_);
- RTC_DCHECK(decodedFrameCallback_.UserReceiveCallback());
+ RTC_DCHECK(decoded_frame_callback_.UserReceiveCallback());
- if (externalDecoder == nullptr) {
- codecDataBase_.DeregisterExternalDecoder(payloadType);
+ if (decoder) {
+ RTC_DCHECK(!codec_database_.IsExternalDecoderRegistered(payload_type));
+ codec_database_.RegisterExternalDecoder(payload_type, decoder.get());
+ video_decoders_.push_back(std::move(decoder));
} else {
- codecDataBase_.RegisterExternalDecoder(payloadType, externalDecoder);
+ 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; }));
+ }
}
}
-bool VideoReceiver2::IsExternalDecoderRegistered(uint8_t payloadType) const {
+bool VideoReceiver2::IsExternalDecoderRegistered(uint8_t payload_type) const {
RTC_DCHECK_RUN_ON(&decoder_sequence_checker_);
- return codecDataBase_.IsExternalDecoderRegistered(payloadType);
+ return codec_database_.IsExternalDecoderRegistered(payload_type);
}
// Must be called from inside the receive side critical section.
@@ -76,7 +86,7 @@
TRACE_EVENT0("webrtc", "VideoReceiver2::Decode");
// Change decoder if payload type has changed.
VCMGenericDecoder* decoder =
- codecDataBase_.GetDecoder(*frame, &decodedFrameCallback_);
+ codec_database_.GetDecoder(*frame, &decoded_frame_callback_);
if (decoder == nullptr) {
return VCM_NO_CODEC_REGISTERED;
}
@@ -89,7 +99,7 @@
uint8_t payload_type,
const VideoDecoder::Settings& settings) {
RTC_DCHECK_RUN_ON(&construction_sequence_checker_);
- codecDataBase_.RegisterReceiveCodec(payload_type, settings);
+ codec_database_.RegisterReceiveCodec(payload_type, settings);
}
} // namespace webrtc
diff --git a/modules/video_coding/video_receiver2.h b/modules/video_coding/video_receiver2.h
index 86f7f55..cbc7885 100644
--- a/modules/video_coding/video_receiver2.h
+++ b/modules/video_coding/video_receiver2.h
@@ -11,6 +11,9 @@
#ifndef MODULES_VIDEO_CODING_VIDEO_RECEIVER2_H_
#define MODULES_VIDEO_CODING_VIDEO_RECEIVER2_H_
+#include <memory>
+#include <vector>
+
#include "api/field_trials_view.h"
#include "api/sequence_checker.h"
#include "api/video_codecs/video_decoder.h"
@@ -37,24 +40,27 @@
void RegisterReceiveCodec(uint8_t payload_type,
const VideoDecoder::Settings& decoder_settings);
- void RegisterExternalDecoder(VideoDecoder* externalDecoder,
- uint8_t payloadType);
- bool IsExternalDecoderRegistered(uint8_t payloadType) const;
- int32_t RegisterReceiveCallback(VCMReceiveCallback* receiveCallback);
+ void RegisterExternalDecoder(std::unique_ptr<VideoDecoder> decoder,
+ uint8_t payload_type);
- int32_t Decode(const webrtc::VCMEncodedFrame* frame);
+ bool IsExternalDecoderRegistered(uint8_t payload_type) const;
+ int32_t RegisterReceiveCallback(VCMReceiveCallback* receive_callback);
+
+ int32_t Decode(const VCMEncodedFrame* frame);
private:
SequenceChecker construction_sequence_checker_;
SequenceChecker decoder_sequence_checker_;
Clock* const clock_;
- VCMTiming* timing_;
- VCMDecodedFrameCallback decodedFrameCallback_;
+ 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.
- VCMDecoderDataBase codecDataBase_;
+ VCMDecoderDataBase codec_database_;
};
} // namespace webrtc
diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc
index 94dedaa..0c2ab0f 100644
--- a/video/video_receive_stream2.cc
+++ b/video/video_receive_stream2.cc
@@ -602,8 +602,7 @@
std::move(video_decoder), FileWrapper::OpenWriteOnly(ssb.str()));
}
- video_decoders_.push_back(std::move(video_decoder));
- video_receiver_.RegisterExternalDecoder(video_decoders_.back().get(),
+ video_receiver_.RegisterExternalDecoder(std::move(video_decoder),
decoder.payload_type);
}
diff --git a/video/video_receive_stream2.h b/video/video_receive_stream2.h
index 753d5bb..9d4cac7 100644
--- a/video/video_receive_stream2.h
+++ b/video/video_receive_stream2.h
@@ -276,10 +276,6 @@
std::unique_ptr<VideoStreamDecoder> video_stream_decoder_;
RtpStreamsSynchronizer rtp_stream_sync_;
- // TODO(nisse, philipel): Creation and ownership of video encoders should be
- // moved to the new VideoStreamDecoder.
- std::vector<std::unique_ptr<VideoDecoder>> video_decoders_;
-
std::unique_ptr<VideoStreamBufferController> buffer_;
std::unique_ptr<RtpStreamReceiverInterface> media_receiver_
