Clean up decoders when stopping video receive stream.
This updates VideoReceiveStream2::Stop() to symmetrically tear down
state that's built up in VideoReceiveStream2::Start().
Bug: webrtc:11993, webrtc:14486
Change-Id: I41f4feea5584e5baaeed2143432136f8b9761321
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/272537
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38244}
diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn
index bae0213..5613e86 100644
--- a/modules/video_coding/BUILD.gn
+++ b/modules/video_coding/BUILD.gn
@@ -1126,6 +1126,7 @@
"codecs/vp8/libvpx_vp8_simulcast_test.cc",
"codecs/vp8/screenshare_layers_unittest.cc",
"codecs/vp9/svc_config_unittest.cc",
+ "decoder_database_unittest.cc",
"decoding_state_unittest.cc",
"fec_controller_unittest.cc",
"frame_buffer2_unittest.cc",
@@ -1157,6 +1158,7 @@
"utility/simulcast_rate_allocator_unittest.cc",
"utility/vp9_uncompressed_header_parser_unittest.cc",
"video_codec_initializer_unittest.cc",
+ "video_receiver2_unittest.cc",
"video_receiver_unittest.cc",
]
if (rtc_use_h264) {
diff --git a/modules/video_coding/decoder_database.cc b/modules/video_coding/decoder_database.cc
index 15d8dcc..7998302 100644
--- a/modules/video_coding/decoder_database.cc
+++ b/modules/video_coding/decoder_database.cc
@@ -47,14 +47,15 @@
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;
+ if (external_decoder) {
+ decoders_[payload_type] = external_decoder;
+ }
}
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();
+ return decoders_.find(payload_type) != decoders_.end();
}
void VCMDecoderDataBase::RegisterReceiveCodec(
@@ -78,6 +79,11 @@
return true;
}
+void VCMDecoderDataBase::DeregisterReceiveCodecs() {
+ current_payload_type_ = absl::nullopt;
+ decoder_settings_.clear();
+}
+
VCMGenericDecoder* VCMDecoderDataBase::GetDecoder(
const VCMEncodedFrame& frame,
VCMDecodedFrameCallback* decoded_frame_callback) {
@@ -112,8 +118,8 @@
void VCMDecoderDataBase::CreateAndInitDecoder(const VCMEncodedFrame& frame) {
uint8_t payload_type = frame.PayloadType();
- RTC_LOG(LS_INFO) << "Initializing decoder with payload type '"
- << int{payload_type} << "'.";
+ RTC_DLOG(LS_INFO) << "Initializing decoder with payload type '"
+ << 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: "
diff --git a/modules/video_coding/decoder_database.h b/modules/video_coding/decoder_database.h
index 59b683b..4229786 100644
--- a/modules/video_coding/decoder_database.h
+++ b/modules/video_coding/decoder_database.h
@@ -40,6 +40,7 @@
void RegisterReceiveCodec(uint8_t payload_type,
const VideoDecoder::Settings& settings);
bool DeregisterReceiveCodec(uint8_t payload_type);
+ void DeregisterReceiveCodecs();
// Returns a decoder specified by frame.PayloadType. The decoded frame
// callback of the decoder is set to `decoded_frame_callback`. If no such
diff --git a/modules/video_coding/decoder_database_unittest.cc b/modules/video_coding/decoder_database_unittest.cc
new file mode 100644
index 0000000..c7453d8
--- /dev/null
+++ b/modules/video_coding/decoder_database_unittest.cc
@@ -0,0 +1,75 @@
+/*
+ * Copyright (c) 2022 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "modules/video_coding/decoder_database.h"
+
+#include "api/test/mock_video_decoder.h"
+#include "test/gmock.h"
+#include "test/gtest.h"
+
+namespace webrtc {
+namespace {
+
+using ::testing::NiceMock;
+
+// Test registering and unregistering an external decoder instance.
+TEST(VCMDecoderDataBaseTest, RegisterExternalDecoder) {
+ VCMDecoderDataBase db;
+ constexpr int kPayloadType = 1;
+ ASSERT_FALSE(db.IsExternalDecoderRegistered(kPayloadType));
+
+ NiceMock<MockVideoDecoder> decoder;
+ db.RegisterExternalDecoder(kPayloadType, &decoder);
+ EXPECT_TRUE(db.IsExternalDecoderRegistered(kPayloadType));
+ EXPECT_EQ(db.DeregisterExternalDecoder(kPayloadType), &decoder);
+ EXPECT_FALSE(db.IsExternalDecoderRegistered(kPayloadType));
+}
+
+TEST(VCMDecoderDataBaseTest, RegisterReceiveCodec) {
+ VCMDecoderDataBase db;
+ constexpr int kPayloadType = 1;
+ ASSERT_FALSE(db.DeregisterReceiveCodec(kPayloadType));
+
+ VideoDecoder::Settings settings;
+ settings.set_codec_type(kVideoCodecVP8);
+ settings.set_max_render_resolution({10, 10});
+ settings.set_number_of_cores(4);
+ db.RegisterReceiveCodec(kPayloadType, settings);
+
+ EXPECT_TRUE(db.DeregisterReceiveCodec(kPayloadType));
+}
+
+TEST(VCMDecoderDataBaseTest, DeregisterReceiveCodecs) {
+ VCMDecoderDataBase db;
+ constexpr int kPayloadType1 = 1;
+ constexpr int kPayloadType2 = 2;
+ ASSERT_FALSE(db.DeregisterReceiveCodec(kPayloadType1));
+ ASSERT_FALSE(db.DeregisterReceiveCodec(kPayloadType2));
+
+ VideoDecoder::Settings settings1;
+ settings1.set_codec_type(kVideoCodecVP8);
+ settings1.set_max_render_resolution({10, 10});
+ settings1.set_number_of_cores(4);
+
+ VideoDecoder::Settings settings2 = settings1;
+ settings2.set_codec_type(kVideoCodecVP9);
+
+ db.RegisterReceiveCodec(kPayloadType1, settings1);
+ db.RegisterReceiveCodec(kPayloadType2, settings2);
+
+ db.DeregisterReceiveCodecs();
+
+ // All receive codecs must have been removed.
+ EXPECT_FALSE(db.DeregisterReceiveCodec(kPayloadType1));
+ EXPECT_FALSE(db.DeregisterReceiveCodec(kPayloadType2));
+}
+
+} // namespace
+} // namespace webrtc
diff --git a/modules/video_coding/packet_buffer.cc b/modules/video_coding/packet_buffer.cc
index 477ba10..3dcfc48 100644
--- a/modules/video_coding/packet_buffer.cc
+++ b/modules/video_coding/packet_buffer.cc
@@ -167,6 +167,10 @@
sps_pps_idr_is_h264_keyframe_ = true;
}
+void PacketBuffer::ResetSpsPpsIdrIsH264Keyframe() {
+ sps_pps_idr_is_h264_keyframe_ = false;
+}
+
void PacketBuffer::ClearInternal() {
for (auto& entry : buffer_) {
entry = nullptr;
diff --git a/modules/video_coding/packet_buffer.h b/modules/video_coding/packet_buffer.h
index 680955a..53e08c9 100644
--- a/modules/video_coding/packet_buffer.h
+++ b/modules/video_coding/packet_buffer.h
@@ -82,6 +82,7 @@
void Clear();
void ForceSpsPpsIdrIsH264Keyframe();
+ void ResetSpsPpsIdrIsH264Keyframe();
private:
void ClearInternal();
diff --git a/modules/video_coding/video_receiver2.cc b/modules/video_coding/video_receiver2.cc
index 36f53a7..a79055e 100644
--- a/modules/video_coding/video_receiver2.cc
+++ b/modules/video_coding/video_receiver2.cc
@@ -102,4 +102,14 @@
codec_database_.RegisterReceiveCodec(payload_type, settings);
}
+void VideoReceiver2::DeregisterReceiveCodec(uint8_t payload_type) {
+ RTC_DCHECK_RUN_ON(&construction_sequence_checker_);
+ codec_database_.DeregisterReceiveCodec(payload_type);
+}
+
+void VideoReceiver2::DeregisterReceiveCodecs() {
+ RTC_DCHECK_RUN_ON(&construction_sequence_checker_);
+ codec_database_.DeregisterReceiveCodecs();
+}
+
} // namespace webrtc
diff --git a/modules/video_coding/video_receiver2.h b/modules/video_coding/video_receiver2.h
index cbc7885..f9b5993 100644
--- a/modules/video_coding/video_receiver2.h
+++ b/modules/video_coding/video_receiver2.h
@@ -21,6 +21,7 @@
#include "modules/video_coding/encoded_frame.h"
#include "modules/video_coding/generic_decoder.h"
#include "modules/video_coding/timing/timing.h"
+#include "rtc_base/system/no_unique_address.h"
#include "system_wrappers/include/clock.h"
namespace webrtc {
@@ -39,6 +40,8 @@
void RegisterReceiveCodec(uint8_t payload_type,
const VideoDecoder::Settings& decoder_settings);
+ void DeregisterReceiveCodec(uint8_t payload_type);
+ void DeregisterReceiveCodecs();
void RegisterExternalDecoder(std::unique_ptr<VideoDecoder> decoder,
uint8_t payload_type);
@@ -49,8 +52,8 @@
int32_t Decode(const VCMEncodedFrame* frame);
private:
- SequenceChecker construction_sequence_checker_;
- SequenceChecker decoder_sequence_checker_;
+ RTC_NO_UNIQUE_ADDRESS SequenceChecker construction_sequence_checker_;
+ RTC_NO_UNIQUE_ADDRESS SequenceChecker decoder_sequence_checker_;
Clock* const clock_;
VCMDecodedFrameCallback decoded_frame_callback_;
// Holds/owns the decoder instances that are registered via
diff --git a/modules/video_coding/video_receiver2_unittest.cc b/modules/video_coding/video_receiver2_unittest.cc
new file mode 100644
index 0000000..703c6c3
--- /dev/null
+++ b/modules/video_coding/video_receiver2_unittest.cc
@@ -0,0 +1,142 @@
+/*
+ * Copyright (c) 2022 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "modules/video_coding/video_receiver2.h"
+
+#include <memory>
+#include <utility>
+
+#include "api/test/mock_video_decoder.h"
+#include "api/units/timestamp.h"
+#include "api/video/encoded_frame.h"
+#include "common_video/test/utilities.h"
+#include "modules/video_coding/decoder_database.h"
+#include "modules/video_coding/timing/timing.h"
+#include "system_wrappers/include/clock.h"
+#include "test/gmock.h"
+#include "test/gtest.h"
+#include "test/scoped_key_value_config.h"
+
+namespace webrtc {
+namespace {
+
+using ::testing::_;
+using ::testing::NiceMock;
+using ::testing::Return;
+
+class MockVCMReceiveCallback : public VCMReceiveCallback {
+ public:
+ MockVCMReceiveCallback() = default;
+
+ MOCK_METHOD(
+ int32_t,
+ FrameToRender,
+ (VideoFrame&, absl::optional<uint8_t>, TimeDelta, VideoContentType),
+ (override));
+ MOCK_METHOD(void, OnIncomingPayloadType, (int), (override));
+ MOCK_METHOD(void, OnDecoderImplementationName, (const char*), (override));
+};
+
+class TestEncodedFrame : public EncodedFrame {
+ public:
+ explicit TestEncodedFrame(int payload_type) {
+ _payloadType = payload_type;
+ SetPacketInfos(CreatePacketInfos(3));
+ }
+
+ void SetReceivedTime(webrtc::Timestamp received_time) {
+ received_time_ = received_time;
+ }
+
+ int64_t ReceivedTime() const override { return received_time_.ms(); }
+
+ int64_t RenderTime() const override { return _renderTimeMs; }
+
+ private:
+ webrtc::Timestamp received_time_ = webrtc::Timestamp::Millis(0);
+};
+
+class VideoReceiver2Test : public ::testing::Test {
+ protected:
+ VideoReceiver2Test() {
+ receiver_.RegisterReceiveCallback(&receive_callback_);
+ }
+
+ void RegisterReceiveCodecSettings(
+ int payload_type,
+ VideoCodecType codec_type = kVideoCodecVP8) {
+ VideoDecoder::Settings settings;
+ settings.set_codec_type(codec_type);
+ settings.set_max_render_resolution({10, 10});
+ settings.set_number_of_cores(4);
+ receiver_.RegisterReceiveCodec(payload_type, settings);
+ }
+
+ test::ScopedKeyValueConfig field_trials_;
+ SimulatedClock clock_{Timestamp::Millis(1337)};
+ VCMTiming timing_{&clock_, field_trials_};
+ NiceMock<MockVCMReceiveCallback> receive_callback_;
+ VideoReceiver2 receiver_{&clock_, &timing_, field_trials_};
+};
+
+TEST_F(VideoReceiver2Test, RegisterExternalDecoder) {
+ constexpr int kPayloadType = 1;
+ ASSERT_FALSE(receiver_.IsExternalDecoderRegistered(kPayloadType));
+
+ // Register a decoder, check for correctness, then unregister and check again.
+ auto decoder = std::make_unique<NiceMock<MockVideoDecoder>>();
+ bool decoder_deleted = false;
+ EXPECT_CALL(*decoder, Destruct).WillOnce([&decoder_deleted] {
+ decoder_deleted = true;
+ });
+ receiver_.RegisterExternalDecoder(std::move(decoder), kPayloadType);
+ EXPECT_TRUE(receiver_.IsExternalDecoderRegistered(kPayloadType));
+ receiver_.RegisterExternalDecoder(nullptr, kPayloadType);
+ EXPECT_TRUE(decoder_deleted);
+ EXPECT_FALSE(receiver_.IsExternalDecoderRegistered(kPayloadType));
+}
+
+TEST_F(VideoReceiver2Test, RegisterReceiveCodecs) {
+ constexpr int kPayloadType = 1;
+
+ RegisterReceiveCodecSettings(kPayloadType);
+
+ TestEncodedFrame frame(kPayloadType);
+
+ // A decoder has not been registered yet, so an attempt to decode should fail.
+ EXPECT_EQ(receiver_.Decode(&frame), VCM_NO_CODEC_REGISTERED);
+
+ // Register a decoder that will accept the Decode operation.
+ auto decoder = std::make_unique<NiceMock<MockVideoDecoder>>();
+ EXPECT_CALL(*decoder, RegisterDecodeCompleteCallback)
+ .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK));
+ EXPECT_CALL(*decoder, Decode).WillOnce(Return(WEBRTC_VIDEO_CODEC_OK));
+ EXPECT_CALL(*decoder, Release).WillOnce(Return(WEBRTC_VIDEO_CODEC_OK));
+
+ // Register the decoder. Note that this moves ownership of the mock object
+ // to the `receiver_`.
+ receiver_.RegisterExternalDecoder(std::move(decoder), kPayloadType);
+ EXPECT_TRUE(receiver_.IsExternalDecoderRegistered(kPayloadType));
+
+ EXPECT_CALL(receive_callback_, OnIncomingPayloadType(kPayloadType));
+ EXPECT_CALL(receive_callback_, OnDecoderImplementationName);
+
+ // Call `Decode`. This triggers the above call expectations.
+ EXPECT_EQ(receiver_.Decode(&frame), VCM_OK);
+
+ // Unregister the decoder and verify.
+ receiver_.RegisterExternalDecoder(nullptr, kPayloadType);
+ EXPECT_FALSE(receiver_.IsExternalDecoderRegistered(kPayloadType));
+
+ receiver_.DeregisterReceiveCodec(kPayloadType);
+}
+
+} // namespace
+} // namespace webrtc