From 884e49f9d69968c124f082999a57bab4019230de Mon Sep 17 00:00:00 2001 From: Karl Wiberg Date: Mon, 2 Oct 2017 09:54:48 +0200 Subject: [PATCH] Convert PayloadUnion from a union to a class, step 3 Remove PayloadUnion's public member variables, so that the outside world has to go through the accessors. This is good code hygiene in general. For example, it makes it possible to make the audio and video states Optional, so that exactly one of them can be live at any one time. BUG=webrtc:8159 Change-Id: Ie617b9038f961b329bd67b45478ff33d97148447 Reviewed-on: https://webrtc-review.googlesource.com/4428 Commit-Queue: Karl Wiberg Reviewed-by: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#20064} --- modules/rtp_rtcp/BUILD.gn | 1 + modules/rtp_rtcp/include/rtp_rtcp_defines.cc | 26 +++++++++++++ modules/rtp_rtcp/include/rtp_rtcp_defines.h | 40 ++++++++++---------- modules/rtp_rtcp/source/rtp_utility.h | 4 +- 4 files changed, 48 insertions(+), 23 deletions(-) create mode 100644 modules/rtp_rtcp/include/rtp_rtcp_defines.cc diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn index c66f67261e..6e3fb91ba6 100644 --- a/modules/rtp_rtcp/BUILD.gn +++ b/modules/rtp_rtcp/BUILD.gn @@ -45,6 +45,7 @@ rtc_source_set("rtp_rtcp_format") { "source/rtp_packet_to_send.h", ] sources = [ + "include/rtp_rtcp_defines.cc", "source/rtcp_packet.cc", "source/rtcp_packet/app.cc", "source/rtcp_packet/bye.cc", diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.cc b/modules/rtp_rtcp/include/rtp_rtcp_defines.cc new file mode 100644 index 0000000000..af3714b25e --- /dev/null +++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.cc @@ -0,0 +1,26 @@ +/* + * Copyright (c) 2017 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/rtp_rtcp/include/rtp_rtcp_defines.h" + +namespace webrtc { + +PayloadUnion::PayloadUnion(const AudioPayload& payload) + : audio_payload_(payload) {} +PayloadUnion::PayloadUnion(const VideoPayload& payload) + : video_payload_(payload) {} +PayloadUnion::PayloadUnion(const PayloadUnion&) = default; +PayloadUnion::PayloadUnion(PayloadUnion&&) = default; +PayloadUnion::~PayloadUnion() = default; + +PayloadUnion& PayloadUnion::operator=(const PayloadUnion&) = default; +PayloadUnion& PayloadUnion::operator=(PayloadUnion&&) = default; + +} // namespace webrtc diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h index 64980051c4..0b850fe946 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h +++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h @@ -53,37 +53,37 @@ struct VideoPayload { class PayloadUnion { public: - explicit PayloadUnion(const AudioPayload& payload) - : Audio(payload), is_audio_(true) {} - explicit PayloadUnion(const VideoPayload& payload) - : Video(payload), is_audio_(false) {} + explicit PayloadUnion(const AudioPayload& payload); + explicit PayloadUnion(const VideoPayload& payload); + PayloadUnion(const PayloadUnion&); + PayloadUnion(PayloadUnion&&); + ~PayloadUnion(); - bool is_audio() const { return is_audio_; } - bool is_video() const { return !is_audio_; } + PayloadUnion& operator=(const PayloadUnion&); + PayloadUnion& operator=(PayloadUnion&&); + + bool is_audio() const { return audio_payload_.has_value(); } + bool is_video() const { return video_payload_.has_value(); } const AudioPayload& audio_payload() const { - RTC_DCHECK(is_audio_); - return Audio; + RTC_DCHECK(audio_payload_); + return *audio_payload_; } const VideoPayload& video_payload() const { - RTC_DCHECK(!is_audio_); - return Video; + RTC_DCHECK(video_payload_); + return *video_payload_; } AudioPayload& audio_payload() { - RTC_DCHECK(is_audio_); - return Audio; + RTC_DCHECK(audio_payload_); + return *audio_payload_; } VideoPayload& video_payload() { - RTC_DCHECK(!is_audio_); - return Video; + RTC_DCHECK(video_payload_); + return *video_payload_; } - public: - // These two are public for backwards compatibilty. They'll go private soon. - AudioPayload Audio; - VideoPayload Video; - private: - bool is_audio_; + rtc::Optional audio_payload_; + rtc::Optional video_payload_; }; enum RTPAliveType { kRtpDead = 0, kRtpNoRtp = 1, kRtpAlive = 2 }; diff --git a/modules/rtp_rtcp/source/rtp_utility.h b/modules/rtp_rtcp/source/rtp_utility.h index 04eb4383d3..b71b9dca16 100644 --- a/modules/rtp_rtcp/source/rtp_utility.h +++ b/modules/rtp_rtcp/source/rtp_utility.h @@ -30,13 +30,11 @@ RtpFeedback* NullObjectRtpFeedback(); namespace RtpUtility { struct Payload { - Payload(const char* name, const PayloadUnion& pu) - : audio(pu.is_audio()), typeSpecific(pu) { + Payload(const char* name, const PayloadUnion& pu) : 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; };