diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h index 6d19cea88..64980051c 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h +++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h @@ -51,9 +51,39 @@ struct VideoPayload { H264::Profile h264_profile; }; -union PayloadUnion { - AudioPayload Audio; - VideoPayload Video; +class PayloadUnion { + public: + explicit PayloadUnion(const AudioPayload& payload) + : Audio(payload), is_audio_(true) {} + explicit PayloadUnion(const VideoPayload& payload) + : Video(payload), is_audio_(false) {} + + bool is_audio() const { return is_audio_; } + bool is_video() const { return !is_audio_; } + const AudioPayload& audio_payload() const { + RTC_DCHECK(is_audio_); + return Audio; + } + const VideoPayload& video_payload() const { + RTC_DCHECK(!is_audio_); + return Video; + } + AudioPayload& audio_payload() { + RTC_DCHECK(is_audio_); + return Audio; + } + VideoPayload& video_payload() { + RTC_DCHECK(!is_audio_); + return Video; + } + + public: + // These two are public for backwards compatibilty. They'll go private soon. + AudioPayload Audio; + VideoPayload Video; + + private: + bool is_audio_; }; enum RTPAliveType { kRtpDead = 0, kRtpNoRtp = 1, kRtpAlive = 2 }; diff --git a/modules/rtp_rtcp/source/rtp_payload_registry.cc b/modules/rtp_rtcp/source/rtp_payload_registry.cc index fcd0276c0..9effcc291 100644 --- a/modules/rtp_rtcp/source/rtp_payload_registry.cc +++ b/modules/rtp_rtcp/source/rtp_payload_registry.cc @@ -46,15 +46,11 @@ bool PayloadIsCompatible(const RtpUtility::Payload& payload, } RtpUtility::Payload CreatePayloadType(const CodecInst& audio_codec) { - RtpUtility::Payload payload; - payload.name[RTP_PAYLOAD_NAME_SIZE - 1] = 0; - strncpy(payload.name, audio_codec.plname, RTP_PAYLOAD_NAME_SIZE - 1); RTC_DCHECK_GE(audio_codec.plfreq, 1000); - payload.typeSpecific.Audio.frequency = audio_codec.plfreq; - payload.typeSpecific.Audio.channels = audio_codec.channels; - payload.typeSpecific.Audio.rate = 0; - payload.audio = true; - return payload; + return {audio_codec.plname, + PayloadUnion( + AudioPayload{rtc::dchecked_cast(audio_codec.plfreq), + audio_codec.channels, 0})}; } RtpVideoCodecTypes ConvertToRtpVideoCodecType(VideoCodecType type) { @@ -74,15 +70,11 @@ RtpVideoCodecTypes ConvertToRtpVideoCodecType(VideoCodecType type) { } RtpUtility::Payload CreatePayloadType(const VideoCodec& video_codec) { - RtpUtility::Payload payload; - payload.name[RTP_PAYLOAD_NAME_SIZE - 1] = 0; - strncpy(payload.name, video_codec.plName, RTP_PAYLOAD_NAME_SIZE - 1); - payload.typeSpecific.Video.videoCodecType = - ConvertToRtpVideoCodecType(video_codec.codecType); + VideoPayload p; + p.videoCodecType = ConvertToRtpVideoCodecType(video_codec.codecType); if (video_codec.codecType == kVideoCodecH264) - payload.typeSpecific.Video.h264_profile = video_codec.H264().profile; - payload.audio = false; - return payload; + p.h264_profile = video_codec.H264().profile; + return {video_codec.plName, PayloadUnion(p)}; } bool IsPayloadTypeValid(int8_t payload_type) { @@ -172,7 +164,9 @@ int32_t RTPPayloadRegistry::RegisterReceivePayload(const CodecInst& audio_codec, // Audio codecs must be unique. DeregisterAudioCodecOrRedTypeRegardlessOfPayloadType(audio_codec); - payload_type_map_[audio_codec.pltype] = CreatePayloadType(audio_codec); + const auto insert_status = payload_type_map_.emplace( + audio_codec.pltype, CreatePayloadType(audio_codec)); + RTC_DCHECK(insert_status.second); // Insertion succeeded. *created_new_payload = true; // Successful set of payload type, clear the value of last received payload @@ -205,7 +199,9 @@ int32_t RTPPayloadRegistry::RegisterReceivePayload( return -1; } - payload_type_map_[video_codec.plType] = CreatePayloadType(video_codec); + const auto insert_status = payload_type_map_.emplace( + video_codec.plType, CreatePayloadType(video_codec)); + RTC_DCHECK(insert_status.second); // Insertion succeeded. // Successful set of payload type, clear the value of last received payload // type since it might mean something else. diff --git a/modules/rtp_rtcp/source/rtp_receiver_audio.cc b/modules/rtp_rtcp/source/rtp_receiver_audio.cc index d399ad53a..9ccc3ec44 100644 --- a/modules/rtp_rtcp/source/rtp_receiver_audio.cc +++ b/modules/rtp_rtcp/source/rtp_receiver_audio.cc @@ -35,7 +35,7 @@ RTPReceiverAudio::RTPReceiverAudio(RtpData* data_callback) cng_fb_payload_type_(-1), num_energy_(0), current_remote_energy_() { - last_payload_.Audio.channels = 1; + last_payload_.emplace(AudioPayload{0, 1, 0}); memset(current_remote_energy_, 0, sizeof(current_remote_energy_)); } diff --git a/modules/rtp_rtcp/source/rtp_receiver_strategy.cc b/modules/rtp_rtcp/source/rtp_receiver_strategy.cc index de58b615e..6db24c9a0 100644 --- a/modules/rtp_rtcp/source/rtp_receiver_strategy.cc +++ b/modules/rtp_rtcp/source/rtp_receiver_strategy.cc @@ -15,20 +15,20 @@ namespace webrtc { RTPReceiverStrategy::RTPReceiverStrategy(RtpData* data_callback) - : data_callback_(data_callback) { - memset(&last_payload_, 0, sizeof(last_payload_)); -} + : data_callback_(data_callback) {} void RTPReceiverStrategy::GetLastMediaSpecificPayload( PayloadUnion* payload) const { rtc::CritScope cs(&crit_sect_); - memcpy(payload, &last_payload_, sizeof(*payload)); + if (last_payload_) { + *payload = *last_payload_; + } } void RTPReceiverStrategy::SetLastMediaSpecificPayload( const PayloadUnion& payload) { rtc::CritScope cs(&crit_sect_); - memcpy(&last_payload_, &payload, sizeof(last_payload_)); + last_payload_.emplace(payload); } void RTPReceiverStrategy::CheckPayloadChanged(int8_t payload_type, diff --git a/modules/rtp_rtcp/source/rtp_receiver_strategy.h b/modules/rtp_rtcp/source/rtp_receiver_strategy.h index af1868e9f..cc1d1e6e1 100644 --- a/modules/rtp_rtcp/source/rtp_receiver_strategy.h +++ b/modules/rtp_rtcp/source/rtp_receiver_strategy.h @@ -89,7 +89,7 @@ class RTPReceiverStrategy { explicit RTPReceiverStrategy(RtpData* data_callback); rtc::CriticalSection crit_sect_; - PayloadUnion last_payload_; + rtc::Optional last_payload_; RtpData* data_callback_; }; } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtp_receiver_unittest.cc index c1cde0f59..f1d5233e2 100644 --- a/modules/rtp_rtcp/source/rtp_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_receiver_unittest.cc @@ -90,7 +90,7 @@ TEST_F(RtpReceiverTest, GetSources) { header.numCSRCs = 2; header.arrOfCSRCs[0] = kCsrc1; header.arrOfCSRCs[1] = kCsrc2; - PayloadUnion payload_specific = {AudioPayload()}; + const PayloadUnion payload_specific{AudioPayload()}; EXPECT_TRUE(rtp_receiver_->IncomingRtpPacket( header, kTestPayload, sizeof(kTestPayload), payload_specific, !kInOrder)); @@ -140,7 +140,7 @@ TEST_F(RtpReceiverTest, GetSourcesChangeSSRC) { header.payloadType = kPcmuPayloadType; header.ssrc = kSsrc1; header.timestamp = rtp_timestamp(now_ms); - PayloadUnion payload_specific = {AudioPayload()}; + const PayloadUnion payload_specific{AudioPayload()}; EXPECT_TRUE(rtp_receiver_->IncomingRtpPacket( header, kTestPayload, sizeof(kTestPayload), payload_specific, !kInOrder)); @@ -191,7 +191,7 @@ TEST_F(RtpReceiverTest, GetSourcesRemoveOutdatedSource) { RTPHeader header; header.payloadType = kPcmuPayloadType; header.timestamp = rtp_timestamp(now_ms); - PayloadUnion payload_specific = {AudioPayload()}; + const PayloadUnion payload_specific{AudioPayload()}; header.numCSRCs = 1; size_t kSourceListSize = 20; @@ -265,7 +265,7 @@ TEST_F(RtpReceiverTest, GetSourcesContainsAudioLevelExtension) { header.timestamp = rtp_timestamp(time1_ms); header.extension.hasAudioLevel = true; header.extension.audioLevel = 10; - PayloadUnion payload_specific = {AudioPayload()}; + const PayloadUnion payload_specific{AudioPayload()}; EXPECT_TRUE(rtp_receiver_->IncomingRtpPacket( header, kTestPayload, sizeof(kTestPayload), payload_specific, !kInOrder)); @@ -317,7 +317,7 @@ TEST_F(RtpReceiverTest, header.timestamp = rtp_timestamp(time1_ms); header.extension.hasAudioLevel = true; header.extension.audioLevel = 10; - PayloadUnion payload_specific = {AudioPayload()}; + const PayloadUnion payload_specific{AudioPayload()}; EXPECT_TRUE(rtp_receiver_->IncomingRtpPacket( header, kTestPayload, sizeof(kTestPayload), payload_specific, !kInOrder)); diff --git a/modules/rtp_rtcp/source/rtp_sender_audio.cc b/modules/rtp_rtcp/source/rtp_sender_audio.cc index f75452fde..dadf30b53 100644 --- a/modules/rtp_rtcp/source/rtp_sender_audio.cc +++ b/modules/rtp_rtcp/source/rtp_sender_audio.cc @@ -65,13 +65,8 @@ int32_t RTPSenderAudio::RegisterAudioPayload( dtmf_payload_freq_ = frequency; return 0; } - *payload = new RtpUtility::Payload; - (*payload)->typeSpecific.Audio.frequency = frequency; - (*payload)->typeSpecific.Audio.channels = channels; - (*payload)->typeSpecific.Audio.rate = rate; - (*payload)->audio = true; - (*payload)->name[RTP_PAYLOAD_NAME_SIZE - 1] = '\0'; - strncpy((*payload)->name, payloadName, RTP_PAYLOAD_NAME_SIZE - 1); + *payload = new RtpUtility::Payload( + payloadName, PayloadUnion(AudioPayload{frequency, channels, rate})); return 0; } diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index 888dc7277..60636d1e2 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -92,12 +92,9 @@ RtpUtility::Payload* RTPSenderVideo::CreateVideoPayload( } else { video_type = kRtpVideoGeneric; } - RtpUtility::Payload* payload = new RtpUtility::Payload(); - payload->name[RTP_PAYLOAD_NAME_SIZE - 1] = 0; - strncpy(payload->name, payload_name, RTP_PAYLOAD_NAME_SIZE - 1); - payload->typeSpecific.Video.videoCodecType = video_type; - payload->audio = false; - return payload; + VideoPayload vp; + vp.videoCodecType = video_type; + return new RtpUtility::Payload(payload_name, PayloadUnion(vp)); } void RTPSenderVideo::SendVideoPacket(std::unique_ptr packet, diff --git a/modules/rtp_rtcp/source/rtp_utility.h b/modules/rtp_rtcp/source/rtp_utility.h index cd4968eff..04eb4383d 100644 --- a/modules/rtp_rtcp/source/rtp_utility.h +++ b/modules/rtp_rtcp/source/rtp_utility.h @@ -11,6 +11,7 @@ #ifndef MODULES_RTP_RTCP_SOURCE_RTP_UTILITY_H_ #define MODULES_RTP_RTCP_SOURCE_RTP_UTILITY_H_ +#include #include #include "modules/rtp_rtcp/include/receive_statistics.h" @@ -29,6 +30,11 @@ RtpFeedback* NullObjectRtpFeedback(); namespace RtpUtility { struct Payload { + Payload(const char* name, const PayloadUnion& pu) + : audio(pu.is_audio()), typeSpecific(pu) { + std::strncpy(this->name, name, sizeof(this->name) - 1); + this->name[sizeof(this->name) - 1] = '\0'; + } char name[RTP_PAYLOAD_NAME_SIZE]; bool audio; PayloadUnion typeSpecific;