Convert PayloadUnion from a union to a class, step 1

I need to replace the audio part of PayloadUnion with SdpAudioFormat,
but that's a non-trivially-deletable class and those just don't work
well in unions, especially unions that don't have a discriminator that
says which member is currently active.

This CL converts the union to a class, adds a discriminator, and
provides accessor functions. CL #2 in the series will change all
outsiders to use the accessors instead of the public member variables
directly, and CL #3 will remove the public member variables. (It needs
to be done in separate steps like this because PayloadUnion is
unfortunately part of the API, and just changing it all in one go
would break users.)

BUG=webrtc:8159

Change-Id: I38c44bbb21a2d38600cff59bf37d8d47dfdbce21
Reviewed-on: https://webrtc-review.googlesource.com/4340
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20025}

modules/rtp_rtcp/include/rtp_rtcp_defines.h

@@ -51,9 +51,39 @@ struct VideoPayload {
   H264::Profile h264_profile;
 };
 
-union PayloadUnion {
+class PayloadUnion {
-    AudioPayload Audio;
+ public:
-    VideoPayload Video;
+  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 };

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;
+  return {audio_codec.plname,
-  payload.typeSpecific.Audio.channels = audio_codec.channels;
+          PayloadUnion(
-  payload.typeSpecific.Audio.rate = 0;
+              AudioPayload{rtc::dchecked_cast<uint32_t>(audio_codec.plfreq),
-  payload.audio = true;
+                           audio_codec.channels, 0})};
-  return payload;
 }
 
 RtpVideoCodecTypes ConvertToRtpVideoCodecType(VideoCodecType type) {
@@ -74,15 +70,11 @@ RtpVideoCodecTypes ConvertToRtpVideoCodecType(VideoCodecType type) {
 }
 
 RtpUtility::Payload CreatePayloadType(const VideoCodec& video_codec) {
-  RtpUtility::Payload payload;
+  VideoPayload p;
-  payload.name[RTP_PAYLOAD_NAME_SIZE - 1] = 0;
+  p.videoCodecType = ConvertToRtpVideoCodecType(video_codec.codecType);
-  strncpy(payload.name, video_codec.plName, RTP_PAYLOAD_NAME_SIZE - 1);
-  payload.typeSpecific.Video.videoCodecType =
-      ConvertToRtpVideoCodecType(video_codec.codecType);
   if (video_codec.codecType == kVideoCodecH264)
-    payload.typeSpecific.Video.h264_profile = video_codec.H264().profile;
+    p.h264_profile = video_codec.H264().profile;
-  payload.audio = false;
+  return {video_codec.plName, PayloadUnion(p)};
-  return payload;
 }
 
 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.

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_));
 }
 

modules/rtp_rtcp/source/rtp_receiver_strategy.cc

@@ -15,20 +15,20 @@
 namespace webrtc {
 
 RTPReceiverStrategy::RTPReceiverStrategy(RtpData* data_callback)
-    : data_callback_(data_callback) {
+    : data_callback_(data_callback) {}
-  memset(&last_payload_, 0, sizeof(last_payload_));
-}
 
 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,

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<PayloadUnion> last_payload_;
   RtpData* data_callback_;
 };
 }  // namespace webrtc

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));

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 = new RtpUtility::Payload(
-  (*payload)->typeSpecific.Audio.frequency = frequency;
+      payloadName, PayloadUnion(AudioPayload{frequency, channels, rate}));
-  (*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);
   return 0;
 }
 

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();
+  VideoPayload vp;
-  payload->name[RTP_PAYLOAD_NAME_SIZE - 1] = 0;
+  vp.videoCodecType = video_type;
-  strncpy(payload->name, payload_name, RTP_PAYLOAD_NAME_SIZE - 1);
+  return new RtpUtility::Payload(payload_name, PayloadUnion(vp));
-  payload->typeSpecific.Video.videoCodecType = video_type;
-  payload->audio = false;
-  return payload;
 }
 
 void RTPSenderVideo::SendVideoPacket(std::unique_ptr<RtpPacketToSend> packet,

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 <cstring>
 #include <map>
 
 #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;