From 2e1d78495674dd5f6e959e29fd77c5977853c976 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Fri, 23 Feb 2018 15:41:13 +0100 Subject: [PATCH] Delete the VideoCodec::plName string. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It holds the same information as codecType, but in different format. Bug: webrtc:8830 Change-Id: Ia83e2dff4fd9a5ddb489501b7a1fe80759fa4218 Reviewed-on: https://webrtc-review.googlesource.com/56100 Reviewed-by: Erik Språng Reviewed-by: Karl Wiberg Reviewed-by: Danil Chapovalov Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/master@{#22307} --- common_types.cc | 1 - common_types.h | 3 +++ media/engine/simulcast_encoder_adapter_unittest.cc | 1 - modules/rtp_rtcp/include/rtp_rtcp.h | 3 --- modules/rtp_rtcp/source/rtp_payload_registry.cc | 5 +++-- modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc | 1 - modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 6 ------ modules/rtp_rtcp/source/rtp_rtcp_impl.h | 2 -- modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc | 3 +-- modules/rtp_rtcp/test/testAPI/test_api_video.cc | 5 ++--- modules/video_coding/codecs/vp8/simulcast_test_utility.h | 1 - modules/video_coding/encoder_database.cc | 1 - modules/video_coding/generic_encoder.cc | 5 +++-- modules/video_coding/video_codec_initializer.cc | 4 ---- modules/video_coding/video_sender.cc | 4 ++-- test/video_codec_settings.h | 4 ---- video/rtp_video_stream_receiver.cc | 2 -- video/rtp_video_stream_receiver_unittest.cc | 1 - video/video_receive_stream.cc | 3 --- 19 files changed, 14 insertions(+), 41 deletions(-) diff --git a/common_types.cc b/common_types.cc index 7f3b906f7b..35987a0d65 100644 --- a/common_types.cc +++ b/common_types.cc @@ -22,7 +22,6 @@ namespace webrtc { VideoCodec::VideoCodec() : codecType(kVideoCodecUnknown), - plName(), plType(0), width(0), height(0), diff --git a/common_types.h b/common_types.h index f41903ab8d..682a202104 100644 --- a/common_types.h +++ b/common_types.h @@ -397,6 +397,7 @@ enum class VideoType { }; // Video codec +// TODO(nisse): Delete together with VideoCodec::plName, below. enum { kPayloadNameSize = 32 }; enum { kMaxSimulcastStreams = 4 }; enum { kMaxSpatialLayers = 5 }; @@ -520,6 +521,8 @@ class VideoCodec { // Public variables. TODO(hta): Make them private with accessors. VideoCodecType codecType; + // TODO(nisse): Unused in webrtc, delete as soon as downstream + // applications are updated to not use it. char plName[kPayloadNameSize]; unsigned char plType; diff --git a/media/engine/simulcast_encoder_adapter_unittest.cc b/media/engine/simulcast_encoder_adapter_unittest.cc index 7454d681df..bd8f044573 100644 --- a/media/engine/simulcast_encoder_adapter_unittest.cc +++ b/media/engine/simulcast_encoder_adapter_unittest.cc @@ -324,7 +324,6 @@ class TestSimulcastEncoderAdapterFake : public ::testing::Test, const VideoCodec& target = helper_->factory()->encoders()[stream_index]->codec(); EXPECT_EQ(ref.codecType, target.codecType); - EXPECT_EQ(0, strcmp(ref.plName, target.plName)); EXPECT_EQ(ref.plType, target.plType); EXPECT_EQ(ref.width, target.width); EXPECT_EQ(ref.height, target.height); diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h index 3d64db0f3f..dd25d5677f 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/modules/rtp_rtcp/include/rtp_rtcp.h @@ -130,9 +130,6 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface { // Sets codec name and payload type. Returns -1 on failure else 0. virtual int32_t RegisterSendPayload(const CodecInst& voice_codec) = 0; - // Sets codec name and payload type. Return -1 on failure else 0. - virtual int32_t RegisterSendPayload(const VideoCodec& video_codec) = 0; - virtual void RegisterVideoSendPayload(int payload_type, const char* payload_name) = 0; diff --git a/modules/rtp_rtcp/source/rtp_payload_registry.cc b/modules/rtp_rtcp/source/rtp_payload_registry.cc index 6cb9c868cb..7f7b564fbf 100644 --- a/modules/rtp_rtcp/source/rtp_payload_registry.cc +++ b/modules/rtp_rtcp/source/rtp_payload_registry.cc @@ -31,7 +31,8 @@ bool PayloadIsCompatible(const RtpUtility::Payload& payload, bool PayloadIsCompatible(const RtpUtility::Payload& payload, const VideoCodec& video_codec) { if (!payload.typeSpecific.is_video() || - _stricmp(payload.name, video_codec.plName) != 0) + _stricmp(payload.name, + CodecTypeToPayloadString(video_codec.codecType)) != 0) return false; // For H264, profiles must match as well. if (video_codec.codecType == kVideoCodecH264) { @@ -73,7 +74,7 @@ RtpUtility::Payload CreatePayloadType(const VideoCodec& video_codec) { p.videoCodecType = ConvertToRtpVideoCodecType(video_codec.codecType); if (video_codec.codecType == kVideoCodecH264) p.h264_profile = video_codec.H264().profile; - return {video_codec.plName, PayloadUnion(p)}; + return {CodecTypeToPayloadString(video_codec.codecType), PayloadUnion(p)}; } bool IsPayloadTypeValid(int8_t payload_type) { diff --git a/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc b/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc index ee2c016d26..1b22d388e9 100644 --- a/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc @@ -30,7 +30,6 @@ TEST(RtpPayloadRegistryTest, const uint8_t payload_type = 97; VideoCodec video_codec; video_codec.codecType = kVideoCodecVP8; - strncpy(video_codec.plName, "VP8", RTP_PAYLOAD_NAME_SIZE); video_codec.plType = payload_type; EXPECT_EQ(0, rtp_payload_registry.RegisterReceivePayload(video_codec)); diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index c4373f4b41..7bdfa968fa 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -17,7 +17,6 @@ #include #include "api/rtpparameters.h" -#include "common_types.h" // NOLINT(build/include) #include "rtc_base/checks.h" #include "rtc_base/logging.h" @@ -283,11 +282,6 @@ int32_t ModuleRtpRtcpImpl::RegisterSendPayload( voice_codec.channels, (voice_codec.rate < 0) ? 0 : voice_codec.rate); } -int32_t ModuleRtpRtcpImpl::RegisterSendPayload(const VideoCodec& video_codec) { - return rtp_sender_->RegisterPayload(video_codec.plName, video_codec.plType, - 90000, 0, 0); -} - void ModuleRtpRtcpImpl::RegisterVideoSendPayload(int payload_type, const char* payload_name) { RTC_CHECK_EQ( diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h index fffce815d9..b08fdf8b12 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -52,8 +52,6 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { int32_t RegisterSendPayload(const CodecInst& voice_codec) override; - int32_t RegisterSendPayload(const VideoCodec& video_codec) override; - void RegisterVideoSendPayload(int payload_type, const char* payload_name) override; diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc index 022e1ccd3c..403dacaab6 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc @@ -214,10 +214,9 @@ class RtpRtcpImplTest : public ::testing::Test { memset(&codec_, 0, sizeof(VideoCodec)); codec_.plType = 100; - strncpy(codec_.plName, "VP8", 3); codec_.width = 320; codec_.height = 180; - EXPECT_EQ(0, sender_.impl_->RegisterSendPayload(codec_)); + sender_.impl_->RegisterVideoSendPayload(codec_.plType, "VP8"); // Receive module. EXPECT_EQ(0, receiver_.impl_->SetSendingStatus(false)); diff --git a/modules/rtp_rtcp/test/testAPI/test_api_video.cc b/modules/rtp_rtcp/test/testAPI/test_api_video.cc index 63b78514dc..a77a92a9b7 100644 --- a/modules/rtp_rtcp/test/testAPI/test_api_video.cc +++ b/modules/rtp_rtcp/test/testAPI/test_api_video.cc @@ -67,9 +67,9 @@ class RtpRtcpVideoTest : public ::testing::Test { VideoCodec video_codec; memset(&video_codec, 0, sizeof(video_codec)); video_codec.plType = 123; - memcpy(video_codec.plName, "I420", 5); + video_codec.codecType = kVideoCodecI420; - EXPECT_EQ(0, video_module_->RegisterSendPayload(video_codec)); + video_module_->RegisterVideoSendPayload(123, "I420"); EXPECT_EQ(0, rtp_payload_registry_.RegisterReceivePayload(video_codec)); payload_data_length_ = sizeof(video_frame_); @@ -155,7 +155,6 @@ TEST_F(RtpRtcpVideoTest, PaddingOnlyFrames) { VideoCodec codec; codec.codecType = kVideoCodecVP8; codec.plType = kPayloadType; - strncpy(codec.plName, "VP8", 4); EXPECT_EQ(0, rtp_payload_registry_.RegisterReceivePayload(codec)); for (int frame_idx = 0; frame_idx < 10; ++frame_idx) { for (int packet_idx = 0; packet_idx < 5; ++packet_idx) { diff --git a/modules/video_coding/codecs/vp8/simulcast_test_utility.h b/modules/video_coding/codecs/vp8/simulcast_test_utility.h index 5162f22642..b5e46fce5e 100644 --- a/modules/video_coding/codecs/vp8/simulcast_test_utility.h +++ b/modules/video_coding/codecs/vp8/simulcast_test_utility.h @@ -192,7 +192,6 @@ class TestVp8Simulcast : public ::testing::Test { const int* temporal_layer_profile) { RTC_CHECK(settings); memset(settings, 0, sizeof(VideoCodec)); - strncpy(settings->plName, "VP8", 4); settings->codecType = kVideoCodecVP8; // 96 to 127 dynamic payload types for video codecs settings->plType = 120; diff --git a/modules/video_coding/encoder_database.cc b/modules/video_coding/encoder_database.cc index 1bf7d4d6db..c37d47b685 100644 --- a/modules/video_coding/encoder_database.cc +++ b/modules/video_coding/encoder_database.cc @@ -142,7 +142,6 @@ bool VCMEncoderDataBase::RequiresEncoderReset( // Does not check startBitrate or maxFramerate if (new_send_codec.codecType != send_codec_.codecType || - strcmp(new_send_codec.plName, send_codec_.plName) != 0 || new_send_codec.plType != send_codec_.plType || new_send_codec.width != send_codec_.width || new_send_codec.height != send_codec_.height || diff --git a/modules/video_coding/generic_encoder.cc b/modules/video_coding/generic_encoder.cc index 034fe0330b..678862fb33 100644 --- a/modules/video_coding/generic_encoder.cc +++ b/modules/video_coding/generic_encoder.cc @@ -68,8 +68,9 @@ int32_t VCMGenericEncoder::InitEncode(const VideoCodec* settings, if (encoder_->InitEncode(settings, number_of_cores, max_payload_size) != 0) { RTC_LOG(LS_ERROR) << "Failed to initialize the encoder associated with " - "payload name: " - << settings->plName; + "codec type: " + << CodecTypeToPayloadString(settings->codecType) + << " (" << settings->codecType <<")"; return -1; } vcm_encoded_frame_callback_->Reset(); diff --git a/modules/video_coding/video_codec_initializer.cc b/modules/video_coding/video_codec_initializer.cc index 37588236ac..f7e12c8668 100644 --- a/modules/video_coding/video_codec_initializer.cc +++ b/modules/video_coding/video_codec_initializer.cc @@ -42,8 +42,6 @@ bool VideoCodecInitializer::SetupCodec( return false; } codec->codecType = kVideoCodecMultiplex; - strncpy(codec->plName, settings.payload_name.c_str(), - sizeof(codec->plName)); return true; } @@ -185,8 +183,6 @@ VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec( break; } - strncpy(video_codec.plName, payload_name.c_str(), kPayloadNameSize - 1); - video_codec.plName[kPayloadNameSize - 1] = '\0'; video_codec.plType = payload_type; video_codec.numberOfSimulcastStreams = static_cast(streams.size()); diff --git a/modules/video_coding/video_sender.cc b/modules/video_coding/video_sender.cc index 37bcbae022..78189fc360 100644 --- a/modules/video_coding/video_sender.cc +++ b/modules/video_coding/video_sender.cc @@ -68,8 +68,8 @@ int32_t VideoSender::RegisterSendCodec(const VideoCodec* sendCodec, current_codec_ = *sendCodec; if (!ret) { - RTC_LOG(LS_ERROR) << "Failed to initialize set encoder with payload name '" - << sendCodec->plName << "'."; + RTC_LOG(LS_ERROR) << "Failed to initialize set encoder with codec type '" + << sendCodec->codecType << "'."; return VCM_CODEC_ERROR; } diff --git a/test/video_codec_settings.h b/test/video_codec_settings.h index 8465013261..c02bdcc887 100644 --- a/test/video_codec_settings.h +++ b/test/video_codec_settings.h @@ -50,23 +50,19 @@ static void CodecSettings(VideoCodecType codec_type, VideoCodec* settings) { switch (codec_type) { case kVideoCodecVP8: - strncpy(settings->plName, "VP8", 4); settings->codecType = kVideoCodecVP8; *(settings->VP8()) = VideoEncoder::GetDefaultVp8Settings(); return; case kVideoCodecVP9: - strncpy(settings->plName, "VP9", 4); settings->codecType = kVideoCodecVP9; *(settings->VP9()) = VideoEncoder::GetDefaultVp9Settings(); return; case kVideoCodecH264: - strncpy(settings->plName, "H264", 5); settings->codecType = kVideoCodecH264; // TODO(brandtr): Set |qpMax| here, when the OpenH264 wrapper supports it. *(settings->H264()) = VideoEncoder::GetDefaultH264Settings(); return; case kVideoCodecI420: - strncpy(settings->plName, "I420", 5); settings->codecType = kVideoCodecI420; // Bitrate needed for this size and framerate. settings->startBitrate = diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index 6055174372..6d69a6fe09 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -146,7 +146,6 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver( if (IsUlpfecEnabled()) { VideoCodec ulpfec_codec = {}; ulpfec_codec.codecType = kVideoCodecULPFEC; - strncpy(ulpfec_codec.plName, "ulpfec", sizeof(ulpfec_codec.plName)); ulpfec_codec.plType = config_.rtp.ulpfec_payload_type; RTC_CHECK(AddReceiveCodec(ulpfec_codec)); } @@ -154,7 +153,6 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver( if (IsRedEnabled()) { VideoCodec red_codec = {}; red_codec.codecType = kVideoCodecRED; - strncpy(red_codec.plName, "red", sizeof(red_codec.plName)); red_codec.plType = config_.rtp.red_payload_type; RTC_CHECK(AddReceiveCodec(red_codec)); } diff --git a/video/rtp_video_stream_receiver_unittest.cc b/video/rtp_video_stream_receiver_unittest.cc index 93be6551e2..687e3ecf90 100644 --- a/video/rtp_video_stream_receiver_unittest.cc +++ b/video/rtp_video_stream_receiver_unittest.cc @@ -218,7 +218,6 @@ TEST_F(RtpVideoStreamReceiverTest, NoInfiniteRecursionOnEncapsulatedRedPacket) { const uint8_t kRedPayloadType = 125; VideoCodec codec; codec.plType = kRedPayloadType; - memcpy(codec.plName, "red", sizeof("red")); rtp_video_stream_receiver_->AddReceiveCodec(codec, {}); const std::vector data({0x80, // RTP version. kRedPayloadType, // Payload type. diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc index 0c13216e9d..73ca7befce 100644 --- a/video/video_receive_stream.cc +++ b/video/video_receive_stream.cc @@ -49,7 +49,6 @@ VideoCodec CreateDecoderVideoCodec(const VideoReceiveStream::Decoder& decoder) { memset(&codec, 0, sizeof(codec)); codec.plType = decoder.payload_type; - strncpy(codec.plName, decoder.payload_name.c_str(), sizeof(codec.plName)); codec.codecType = PayloadStringToCodecType(decoder.payload_name); if (codec.codecType == kVideoCodecVP8) { @@ -65,8 +64,6 @@ VideoCodec CreateDecoderVideoCodec(const VideoReceiveStream::Decoder& decoder) { associated_decoder.payload_name = CodecTypeToPayloadString(kVideoCodecVP9); VideoCodec associated_codec = CreateDecoderVideoCodec(associated_decoder); associated_codec.codecType = kVideoCodecMultiplex; - strncpy(associated_codec.plName, decoder.payload_name.c_str(), - sizeof(associated_codec.plName)); return associated_codec; }