From 5401bad7015d510be212efa1e3d3bc1c149bc77c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Tue, 11 Aug 2020 12:17:42 +0200 Subject: [PATCH] Prepare for deleting VideoCodec::plType Deletes all webrtc usage of this member. Next step is to delete any downstream references, and when that's done, the member can be deleted. Bug: None Change-Id: I3f3a94a063dccf56468a1069653efd3809875b01 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/181201 Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/master@{#31911} --- api/video_codecs/video_codec.h | 2 ++ .../simulcast_encoder_adapter_unittest.cc | 2 -- modules/video_coding/decoder_database.cc | 15 ++++++++--- modules/video_coding/decoder_database.h | 1 + .../utility/simulcast_test_fixture_impl.cc | 2 -- .../video_coding/video_codec_initializer.cc | 3 --- .../video_coding/video_receiver_unittest.cc | 1 - test/video_codec_settings.h | 2 -- video/rtp_video_stream_receiver2.cc | 9 ++++--- video/rtp_video_stream_receiver2.h | 3 ++- video/rtp_video_stream_receiver2_unittest.cc | 25 ++++++++----------- video/rtp_video_stream_receiver_unittest.cc | 8 ------ video/video_receive_stream.cc | 1 - video/video_receive_stream2.cc | 8 +++--- 14 files changed, 36 insertions(+), 46 deletions(-) diff --git a/api/video_codecs/video_codec.h b/api/video_codecs/video_codec.h index dd3d344084..dcdd31fdf1 100644 --- a/api/video_codecs/video_codec.h +++ b/api/video_codecs/video_codec.h @@ -103,6 +103,8 @@ class RTC_EXPORT VideoCodec { // Public variables. TODO(hta): Make them private with accessors. VideoCodecType codecType; + // TODO(nisse): Unused in webrtc, delete as soon as downstream projects are + // updated. unsigned char plType; // TODO(nisse): Change to int, for consistency. diff --git a/media/engine/simulcast_encoder_adapter_unittest.cc b/media/engine/simulcast_encoder_adapter_unittest.cc index 703d283b59..1f1ec7338e 100644 --- a/media/engine/simulcast_encoder_adapter_unittest.cc +++ b/media/engine/simulcast_encoder_adapter_unittest.cc @@ -457,7 +457,6 @@ class TestSimulcastEncoderAdapterFake : public ::testing::Test, const VideoCodec& target = helper_->factory()->encoders()[stream_index]->codec(); EXPECT_EQ(ref.codecType, target.codecType); - EXPECT_EQ(ref.plType, target.plType); EXPECT_EQ(ref.width, target.width); EXPECT_EQ(ref.height, target.height); EXPECT_EQ(ref.startBitrate, target.startBitrate); @@ -753,7 +752,6 @@ TEST_F(TestSimulcastEncoderAdapterFake, ReinitDoesNotReorderEncoderSettings) { // webrtc::VideoCodec does not implement operator==. EXPECT_EQ(codec_before.codecType, codec_after.codecType); - EXPECT_EQ(codec_before.plType, codec_after.plType); EXPECT_EQ(codec_before.width, codec_after.width); EXPECT_EQ(codec_before.height, codec_after.height); EXPECT_EQ(codec_before.startBitrate, codec_after.startBitrate); diff --git a/modules/video_coding/decoder_database.cc b/modules/video_coding/decoder_database.cc index 029ae45600..32be39bcb4 100644 --- a/modules/video_coding/decoder_database.cc +++ b/modules/video_coding/decoder_database.cc @@ -29,7 +29,10 @@ VCMExtDecoderMapItem::VCMExtDecoderMapItem( VCMDecoderMapItem::~VCMDecoderMapItem() {} VCMDecoderDataBase::VCMDecoderDataBase() - : receive_codec_(), dec_map_(), dec_external_map_() {} + : current_payload_type_(0), + receive_codec_(), + dec_map_(), + dec_external_map_() {} VCMDecoderDataBase::~VCMDecoderDataBase() { ptr_decoder_.reset(); @@ -91,9 +94,10 @@ bool VCMDecoderDataBase::DeregisterReceiveCodec(uint8_t payload_type) { } delete it->second; dec_map_.erase(it); - if (receive_codec_.plType == payload_type) { + if (payload_type == current_payload_type_) { // This codec is currently in use. memset(&receive_codec_, 0, sizeof(VideoCodec)); + current_payload_type_ = 0; } return true; } @@ -103,24 +107,27 @@ VCMGenericDecoder* VCMDecoderDataBase::GetDecoder( VCMDecodedFrameCallback* decoded_frame_callback) { RTC_DCHECK(decoded_frame_callback->UserReceiveCallback()); uint8_t payload_type = frame.PayloadType(); - if (payload_type == receive_codec_.plType || payload_type == 0) { + if (payload_type == current_payload_type_ || payload_type == 0) { return ptr_decoder_.get(); } // If decoder exists - delete. if (ptr_decoder_) { ptr_decoder_.reset(); memset(&receive_codec_, 0, sizeof(VideoCodec)); + current_payload_type_ = 0; } ptr_decoder_ = CreateAndInitDecoder(frame, &receive_codec_); if (!ptr_decoder_) { return nullptr; } + current_payload_type_ = frame.PayloadType(); VCMReceiveCallback* callback = decoded_frame_callback->UserReceiveCallback(); - callback->OnIncomingPayloadType(receive_codec_.plType); + callback->OnIncomingPayloadType(current_payload_type_); if (ptr_decoder_->RegisterDecodeCompleteCallback(decoded_frame_callback) < 0) { ptr_decoder_.reset(); memset(&receive_codec_, 0, sizeof(VideoCodec)); + current_payload_type_ = 0; return nullptr; } return ptr_decoder_.get(); diff --git a/modules/video_coding/decoder_database.h b/modules/video_coding/decoder_database.h index d66fea3b9c..abfd81e342 100644 --- a/modules/video_coding/decoder_database.h +++ b/modules/video_coding/decoder_database.h @@ -76,6 +76,7 @@ class VCMDecoderDataBase { const VCMExtDecoderMapItem* FindExternalDecoderItem( uint8_t payload_type) const; + uint8_t current_payload_type_; // Corresponding to receive_codec_. VideoCodec receive_codec_; std::unique_ptr ptr_decoder_; DecoderMap dec_map_; diff --git a/modules/video_coding/utility/simulcast_test_fixture_impl.cc b/modules/video_coding/utility/simulcast_test_fixture_impl.cc index f157734192..f4e029420c 100644 --- a/modules/video_coding/utility/simulcast_test_fixture_impl.cc +++ b/modules/video_coding/utility/simulcast_test_fixture_impl.cc @@ -223,8 +223,6 @@ void SimulcastTestFixtureImpl::DefaultSettings( RTC_CHECK(settings); memset(settings, 0, sizeof(VideoCodec)); settings->codecType = codec_type; - // 96 to 127 dynamic payload types for video codecs - settings->plType = 120; settings->startBitrate = 300; settings->minBitrate = 30; settings->maxBitrate = 0; diff --git a/modules/video_coding/video_codec_initializer.cc b/modules/video_coding/video_codec_initializer.cc index 4023f6ba9e..2859dd022a 100644 --- a/modules/video_coding/video_codec_initializer.cc +++ b/modules/video_coding/video_codec_initializer.cc @@ -72,9 +72,6 @@ VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec( config.content_type == VideoEncoderConfig::ContentType::kScreen && config.legacy_conference_mode; - // TODO(nisse): The plType field should be deleted. Luckily, our - // callers don't need it. - video_codec.plType = 0; video_codec.numberOfSimulcastStreams = static_cast(streams.size()); video_codec.minBitrate = streams[0].min_bitrate_bps / 1000; diff --git a/modules/video_coding/video_receiver_unittest.cc b/modules/video_coding/video_receiver_unittest.cc index cf1e678b19..fcd4f449ca 100644 --- a/modules/video_coding/video_receiver_unittest.cc +++ b/modules/video_coding/video_receiver_unittest.cc @@ -57,7 +57,6 @@ class TestVideoReceiver : public ::testing::Test { // Register decoder. receiver_.RegisterExternalDecoder(&decoder_, kUnusedPayloadType); webrtc::test::CodecSettings(kVideoCodecVP8, &settings_); - settings_.plType = kUnusedPayloadType; EXPECT_EQ( 0, receiver_.RegisterReceiveCodec(kUnusedPayloadType, &settings_, 1)); diff --git a/test/video_codec_settings.h b/test/video_codec_settings.h index b5250486d7..8e565351d3 100644 --- a/test/video_codec_settings.h +++ b/test/video_codec_settings.h @@ -27,8 +27,6 @@ const uint16_t kTestOutlierFrameSizePercent = 250; static void CodecSettings(VideoCodecType codec_type, VideoCodec* settings) { memset(settings, 0, sizeof(VideoCodec)); - settings->plType = kTestPayloadType; - settings->width = kTestWidth; settings->height = kTestHeight; diff --git a/video/rtp_video_stream_receiver2.cc b/video/rtp_video_stream_receiver2.cc index 3f11bb77c4..1a5316517d 100644 --- a/video/rtp_video_stream_receiver2.cc +++ b/video/rtp_video_stream_receiver2.cc @@ -326,15 +326,16 @@ RtpVideoStreamReceiver2::~RtpVideoStreamReceiver2() { } void RtpVideoStreamReceiver2::AddReceiveCodec( + uint8_t payload_type, const VideoCodec& video_codec, const std::map& codec_params, bool raw_payload) { RTC_DCHECK_RUN_ON(&worker_task_checker_); payload_type_map_.emplace( - video_codec.plType, - raw_payload ? std::make_unique() - : CreateVideoRtpDepacketizer(video_codec.codecType)); - pt_codec_params_.emplace(video_codec.plType, codec_params); + payload_type, raw_payload + ? std::make_unique() + : CreateVideoRtpDepacketizer(video_codec.codecType)); + pt_codec_params_.emplace(payload_type, codec_params); } absl::optional RtpVideoStreamReceiver2::GetSyncInfo() const { diff --git a/video/rtp_video_stream_receiver2.h b/video/rtp_video_stream_receiver2.h index d82a7abbfe..2a3befd65b 100644 --- a/video/rtp_video_stream_receiver2.h +++ b/video/rtp_video_stream_receiver2.h @@ -90,7 +90,8 @@ class RtpVideoStreamReceiver2 : public LossNotificationSender, rtc::scoped_refptr frame_transformer); ~RtpVideoStreamReceiver2() override; - void AddReceiveCodec(const VideoCodec& video_codec, + void AddReceiveCodec(uint8_t payload_type, + const VideoCodec& video_codec, const std::map& codec_params, bool raw_payload); diff --git a/video/rtp_video_stream_receiver2_unittest.cc b/video/rtp_video_stream_receiver2_unittest.cc index cda0fe5cfa..acb999a806 100644 --- a/video/rtp_video_stream_receiver2_unittest.cc +++ b/video/rtp_video_stream_receiver2_unittest.cc @@ -180,9 +180,8 @@ class RtpVideoStreamReceiver2Test : public ::testing::Test { &mock_key_frame_request_sender_, &mock_on_complete_frame_callback_, nullptr, nullptr); VideoCodec codec; - codec.plType = kPayloadType; codec.codecType = kVideoCodecGeneric; - rtp_video_stream_receiver_->AddReceiveCodec(codec, {}, + rtp_video_stream_receiver_->AddReceiveCodec(kPayloadType, codec, {}, /*raw_payload=*/false); } @@ -317,10 +316,10 @@ TEST_F(RtpVideoStreamReceiver2Test, CacheColorSpaceFromLastPacketOfKeyframe) { // Prepare the receiver for VP9. VideoCodec codec; - codec.plType = kVp9PayloadType; codec.codecType = kVideoCodecVP9; std::map codec_params; - rtp_video_stream_receiver_->AddReceiveCodec(codec, codec_params, + rtp_video_stream_receiver_->AddReceiveCodec(kVp9PayloadType, codec, + codec_params, /*raw_payload=*/false); // Generate key frame packets. @@ -580,13 +579,12 @@ TEST_P(RtpVideoStreamReceiver2TestH264, MAYBE_InBandSpsPps) { TEST_P(RtpVideoStreamReceiver2TestH264, OutOfBandFmtpSpsPps) { constexpr int kPayloadType = 99; VideoCodec codec; - codec.plType = kPayloadType; std::map codec_params; // Example parameter sets from https://tools.ietf.org/html/rfc3984#section-8.2 // . codec_params.insert( {cricket::kH264FmtpSpropParameterSets, "Z0IACpZTBYmI,aMljiA=="}); - rtp_video_stream_receiver_->AddReceiveCodec(codec, codec_params, + rtp_video_stream_receiver_->AddReceiveCodec(kPayloadType, codec, codec_params, /*raw_payload=*/false); const uint8_t binary_sps[] = {0x67, 0x42, 0x00, 0x0a, 0x96, 0x53, 0x05, 0x89, 0x88}; @@ -877,8 +875,8 @@ TEST_F(RtpVideoStreamReceiver2Test, ParseGenericDescriptorRawPayload) { const int kRawPayloadType = 123; VideoCodec codec; - codec.plType = kRawPayloadType; - rtp_video_stream_receiver_->AddReceiveCodec(codec, {}, /*raw_payload=*/true); + rtp_video_stream_receiver_->AddReceiveCodec(kRawPayloadType, codec, {}, + /*raw_payload=*/true); rtp_video_stream_receiver_->StartReceive(); RtpHeaderExtensionMap extension_map; @@ -909,8 +907,8 @@ TEST_F(RtpVideoStreamReceiver2Test, UnwrapsFrameId) { const int kPayloadType = 123; VideoCodec codec; - codec.plType = kPayloadType; - rtp_video_stream_receiver_->AddReceiveCodec(codec, {}, /*raw_payload=*/true); + rtp_video_stream_receiver_->AddReceiveCodec(kPayloadType, codec, {}, + /*raw_payload=*/true); rtp_video_stream_receiver_->StartReceive(); RtpHeaderExtensionMap extension_map; extension_map.Register(5); @@ -957,8 +955,7 @@ class RtpVideoStreamReceiver2DependencyDescriptorTest public: RtpVideoStreamReceiver2DependencyDescriptorTest() { VideoCodec codec; - codec.plType = payload_type_; - rtp_video_stream_receiver_->AddReceiveCodec(codec, {}, + rtp_video_stream_receiver_->AddReceiveCodec(payload_type_, codec, {}, /*raw_payload=*/true); extension_map_.Register(7); rtp_video_stream_receiver_->StartReceive(); @@ -1141,9 +1138,9 @@ TEST_F(RtpVideoStreamReceiver2Test, TransformFrame) { nullptr, process_thread_.get(), &mock_nack_sender_, nullptr, &mock_on_complete_frame_callback_, nullptr, mock_frame_transformer); VideoCodec video_codec; - video_codec.plType = kPayloadType; video_codec.codecType = kVideoCodecGeneric; - receiver->AddReceiveCodec(video_codec, {}, /*raw_payload=*/false); + receiver->AddReceiveCodec(kPayloadType, video_codec, {}, + /*raw_payload=*/false); RtpPacketReceived rtp_packet; rtp_packet.SetPayloadType(kPayloadType); diff --git a/video/rtp_video_stream_receiver_unittest.cc b/video/rtp_video_stream_receiver_unittest.cc index c0db881316..8821f28848 100644 --- a/video/rtp_video_stream_receiver_unittest.cc +++ b/video/rtp_video_stream_receiver_unittest.cc @@ -178,7 +178,6 @@ class RtpVideoStreamReceiverTest : public ::testing::Test { &mock_nack_sender_, &mock_key_frame_request_sender_, &mock_on_complete_frame_callback_, nullptr, nullptr); VideoCodec codec; - codec.plType = kPayloadType; codec.codecType = kVideoCodecGeneric; rtp_video_stream_receiver_->AddReceiveCodec(kPayloadType, codec, {}, /*raw_payload=*/false); @@ -312,7 +311,6 @@ TEST_F(RtpVideoStreamReceiverTest, CacheColorSpaceFromLastPacketOfKeyframe) { // Prepare the receiver for VP9. VideoCodec codec; - codec.plType = kVp9PayloadType; codec.codecType = kVideoCodecVP9; std::map codec_params; rtp_video_stream_receiver_->AddReceiveCodec(kVp9PayloadType, codec, @@ -569,7 +567,6 @@ TEST_P(RtpVideoStreamReceiverTestH264, InBandSpsPps) { TEST_P(RtpVideoStreamReceiverTestH264, OutOfBandFmtpSpsPps) { constexpr int kPayloadType = 99; VideoCodec codec; - codec.plType = kPayloadType; std::map codec_params; // Example parameter sets from https://tools.ietf.org/html/rfc3984#section-8.2 // . @@ -611,7 +608,6 @@ TEST_P(RtpVideoStreamReceiverTestH264, OutOfBandFmtpSpsPps) { TEST_P(RtpVideoStreamReceiverTestH264, ForceSpsPpsIdrIsKeyframe) { constexpr int kPayloadType = 99; VideoCodec codec; - codec.plType = kPayloadType; std::map codec_params; if (GetParam() == "") { // Forcing can be done either with field trial or codec_params. @@ -937,7 +933,6 @@ TEST_F(RtpVideoStreamReceiverTest, ParseGenericDescriptorRawPayload) { const int kRawPayloadType = 123; VideoCodec codec; - codec.plType = kRawPayloadType; rtp_video_stream_receiver_->AddReceiveCodec(kRawPayloadType, codec, {}, /*raw_payload=*/true); rtp_video_stream_receiver_->StartReceive(); @@ -970,7 +965,6 @@ TEST_F(RtpVideoStreamReceiverTest, UnwrapsFrameId) { const int kPayloadType = 123; VideoCodec codec; - codec.plType = kPayloadType; rtp_video_stream_receiver_->AddReceiveCodec(kPayloadType, codec, {}, /*raw_payload=*/true); rtp_video_stream_receiver_->StartReceive(); @@ -1019,7 +1013,6 @@ class RtpVideoStreamReceiverDependencyDescriptorTest public: RtpVideoStreamReceiverDependencyDescriptorTest() { VideoCodec codec; - codec.plType = payload_type_; rtp_video_stream_receiver_->AddReceiveCodec(payload_type_, codec, {}, /*raw_payload=*/true); extension_map_.Register(7); @@ -1203,7 +1196,6 @@ TEST_F(RtpVideoStreamReceiverTest, TransformFrame) { &mock_nack_sender_, nullptr, &mock_on_complete_frame_callback_, nullptr, mock_frame_transformer); VideoCodec video_codec; - video_codec.plType = kPayloadType; video_codec.codecType = kVideoCodecGeneric; receiver->AddReceiveCodec(kPayloadType, video_codec, {}, /*raw_payload=*/false); diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc index 1072ef215a..125fcb5421 100644 --- a/video/video_receive_stream.cc +++ b/video/video_receive_stream.cc @@ -117,7 +117,6 @@ VideoCodec CreateDecoderVideoCodec(const VideoReceiveStream::Decoder& decoder) { VideoCodec codec; memset(&codec, 0, sizeof(codec)); - codec.plType = decoder.payload_type; codec.codecType = PayloadStringToCodecType(decoder.video_format.name); if (codec.codecType == kVideoCodecVP8) { diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc index e5aad79675..9e4c2ad76b 100644 --- a/video/video_receive_stream2.cc +++ b/video/video_receive_stream2.cc @@ -115,7 +115,6 @@ VideoCodec CreateDecoderVideoCodec(const VideoReceiveStream::Decoder& decoder) { VideoCodec codec; memset(&codec, 0, sizeof(codec)); - codec.plType = decoder.payload_type; codec.codecType = PayloadStringToCodecType(decoder.video_format.name); if (codec.codecType == kVideoCodecVP8) { @@ -348,9 +347,10 @@ void VideoReceiveStream2::Start() { VideoCodec codec = CreateDecoderVideoCodec(decoder); const bool raw_payload = - config_.rtp.raw_payload_types.count(codec.plType) > 0; - rtp_video_stream_receiver_.AddReceiveCodec( - codec, decoder.video_format.parameters, raw_payload); + config_.rtp.raw_payload_types.count(decoder.payload_type) > 0; + rtp_video_stream_receiver_.AddReceiveCodec(decoder.payload_type, codec, + decoder.video_format.parameters, + raw_payload); RTC_CHECK_EQ(VCM_OK, video_receiver_.RegisterReceiveCodec( decoder.payload_type, &codec, num_cpu_cores_)); }