On receive stream shutdown, deregister decoders on decoder thread.
Now Configure(), Decode() and Release() calls to the decoders should
all happen on the decoder thread. Added thread checkers to verify.
Bug: None
Change-Id: I2a1cf2cf7f3c3c7c50e382d82a3638e916ed9c34
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/272368
Reviewed-by: Evan Shrubsole <eshr@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37840}
diff --git a/modules/video_coding/decoder_database.cc b/modules/video_coding/decoder_database.cc
index 01120dc..882de27 100644
--- a/modules/video_coding/decoder_database.cc
+++ b/modules/video_coding/decoder_database.cc
@@ -15,7 +15,12 @@
namespace webrtc {
+VCMDecoderDataBase::VCMDecoderDataBase() {
+ decoder_sequence_checker_.Detach();
+}
+
bool 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.
@@ -37,6 +42,7 @@
void VCMDecoderDataBase::RegisterExternalDecoder(
uint8_t payload_type,
VideoDecoder* external_decoder) {
+ RTC_DCHECK_RUN_ON(&decoder_sequence_checker_);
// If payload value already exists, erase old and insert new.
DeregisterExternalDecoder(payload_type);
decoders_[payload_type] = external_decoder;
@@ -44,6 +50,7 @@
bool VCMDecoderDataBase::IsExternalDecoderRegistered(
uint8_t payload_type) const {
+ RTC_DCHECK_RUN_ON(&decoder_sequence_checker_);
return payload_type == current_payload_type_ ||
decoders_.find(payload_type) != decoders_.end();
}
@@ -72,6 +79,7 @@
VCMGenericDecoder* VCMDecoderDataBase::GetDecoder(
const VCMEncodedFrame& frame,
VCMDecodedFrameCallback* decoded_frame_callback) {
+ RTC_DCHECK_RUN_ON(&decoder_sequence_checker_);
RTC_DCHECK(decoded_frame_callback->UserReceiveCallback());
uint8_t payload_type = frame.PayloadType();
if (payload_type == current_payload_type_ || payload_type == 0) {
diff --git a/modules/video_coding/decoder_database.h b/modules/video_coding/decoder_database.h
index 98deb18..98bbf85 100644
--- a/modules/video_coding/decoder_database.h
+++ b/modules/video_coding/decoder_database.h
@@ -16,6 +16,7 @@
#include <map>
#include "absl/types/optional.h"
+#include "api/sequence_checker.h"
#include "api/video_codecs/video_decoder.h"
#include "modules/video_coding/encoded_frame.h"
#include "modules/video_coding/generic_decoder.h"
@@ -24,7 +25,7 @@
class VCMDecoderDataBase {
public:
- VCMDecoderDataBase() = default;
+ VCMDecoderDataBase();
VCMDecoderDataBase(const VCMDecoderDataBase&) = delete;
VCMDecoderDataBase& operator=(const VCMDecoderDataBase&) = delete;
~VCMDecoderDataBase() = default;
@@ -48,14 +49,19 @@
VCMDecodedFrameCallback* decoded_frame_callback);
private:
- void CreateAndInitDecoder(const VCMEncodedFrame& frame);
+ void CreateAndInitDecoder(const VCMEncodedFrame& frame)
+ RTC_RUN_ON(decoder_sequence_checker_);
+
+ SequenceChecker decoder_sequence_checker_;
absl::optional<uint8_t> current_payload_type_;
- absl::optional<VCMGenericDecoder> current_decoder_;
+ absl::optional<VCMGenericDecoder> current_decoder_
+ RTC_GUARDED_BY(decoder_sequence_checker_);
// 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, VideoDecoder*> decoders_
+ RTC_GUARDED_BY(decoder_sequence_checker_);
};
} // namespace webrtc
diff --git a/modules/video_coding/video_receiver2.cc b/modules/video_coding/video_receiver2.cc
index 2e10020..fe35420 100644
--- a/modules/video_coding/video_receiver2.cc
+++ b/modules/video_coding/video_receiver2.cc
@@ -47,33 +47,22 @@
int32_t VideoReceiver2::RegisterReceiveCallback(
VCMReceiveCallback* receiveCallback) {
RTC_DCHECK_RUN_ON(&construction_sequence_checker_);
- RTC_DCHECK(!IsDecoderThreadRunning());
// This value is set before the decoder thread starts and unset after
// the decoder thread has been stopped.
decodedFrameCallback_.SetUserReceiveCallback(receiveCallback);
return VCM_OK;
}
-// Register an externally defined decoder object. This may be called on either
-// the construction sequence or the decoder sequence to allow for lazy creation
-// of video decoders. If called on the decoder sequence `externalDecoder` cannot
-// be a nullptr. It's the responsibility of the caller to make sure that the
-// access from the two sequences are mutually exclusive.
void VideoReceiver2::RegisterExternalDecoder(VideoDecoder* externalDecoder,
uint8_t payloadType) {
- if (IsDecoderThreadRunning()) {
- RTC_DCHECK_RUN_ON(&decoder_sequence_checker_);
- // Don't allow deregistering decoders on the decoder thread.
- RTC_DCHECK(externalDecoder != nullptr);
- } else {
- RTC_DCHECK_RUN_ON(&construction_sequence_checker_);
- }
+ RTC_DCHECK_RUN_ON(&decoder_sequence_checker_);
+ RTC_DCHECK(decodedFrameCallback_.UserReceiveCallback());
if (externalDecoder == nullptr) {
codecDataBase_.DeregisterExternalDecoder(payloadType);
- return;
+ } else {
+ codecDataBase_.RegisterExternalDecoder(payloadType, externalDecoder);
}
- codecDataBase_.RegisterExternalDecoder(payloadType, externalDecoder);
}
bool VideoReceiver2::IsExternalDecoderRegistered(uint8_t payloadType) const {
@@ -81,28 +70,11 @@
return codecDataBase_.IsExternalDecoderRegistered(payloadType);
}
-void VideoReceiver2::DecoderThreadStarting() {
- RTC_DCHECK_RUN_ON(&construction_sequence_checker_);
- RTC_DCHECK(!IsDecoderThreadRunning());
-#if RTC_DCHECK_IS_ON
- decoder_thread_is_running_ = true;
-#endif
-}
-
-void VideoReceiver2::DecoderThreadStopped() {
- RTC_DCHECK_RUN_ON(&construction_sequence_checker_);
- RTC_DCHECK(IsDecoderThreadRunning());
-#if RTC_DCHECK_IS_ON
- decoder_thread_is_running_ = false;
- decoder_sequence_checker_.Detach();
-#endif
-}
-
// Must be called from inside the receive side critical section.
int32_t VideoReceiver2::Decode(const VCMEncodedFrame* frame) {
RTC_DCHECK_RUN_ON(&decoder_sequence_checker_);
TRACE_EVENT0("webrtc", "VideoReceiver2::Decode");
- // Change decoder if payload type has changed
+ // Change decoder if payload type has changed.
VCMGenericDecoder* decoder =
codecDataBase_.GetDecoder(*frame, &decodedFrameCallback_);
if (decoder == nullptr) {
@@ -111,21 +83,13 @@
return decoder->Decode(*frame, clock_->CurrentTime());
}
-// Register possible receive codecs, can be called multiple times
+// Register possible receive codecs, can be called multiple times.
+// Called before decoder thread is started.
void VideoReceiver2::RegisterReceiveCodec(
uint8_t payload_type,
const VideoDecoder::Settings& settings) {
RTC_DCHECK_RUN_ON(&construction_sequence_checker_);
- RTC_DCHECK(!IsDecoderThreadRunning());
codecDataBase_.RegisterReceiveCodec(payload_type, settings);
}
-bool VideoReceiver2::IsDecoderThreadRunning() {
-#if RTC_DCHECK_IS_ON
- return decoder_thread_is_running_;
-#else
- return true;
-#endif
-}
-
} // namespace webrtc
diff --git a/modules/video_coding/video_receiver2.h b/modules/video_coding/video_receiver2.h
index c7db2fe..86f7f55 100644
--- a/modules/video_coding/video_receiver2.h
+++ b/modules/video_coding/video_receiver2.h
@@ -44,20 +44,7 @@
int32_t Decode(const webrtc::VCMEncodedFrame* frame);
- // Notification methods that are used to check our internal state and validate
- // threading assumptions. These are called by VideoReceiveStreamInterface.
- // See `IsDecoderThreadRunning()` for more details.
- void DecoderThreadStarting();
- void DecoderThreadStopped();
-
private:
- // Used for DCHECKing thread correctness.
- // In build where DCHECKs are enabled, will return false before
- // DecoderThreadStarting is called, then true until DecoderThreadStopped
- // is called.
- // In builds where DCHECKs aren't enabled, it will return true.
- bool IsDecoderThreadRunning();
-
SequenceChecker construction_sequence_checker_;
SequenceChecker decoder_sequence_checker_;
Clock* const clock_;
@@ -68,10 +55,6 @@
// Once the decoder thread has been started, usage of `_codecDataBase` moves
// over to the decoder thread.
VCMDecoderDataBase codecDataBase_;
-
-#if RTC_DCHECK_IS_ON
- bool decoder_thread_is_running_ = false;
-#endif
};
} // namespace webrtc