Change RTPVideoTypeHeader to absl::variant and move RTPVideoHeader in…

…to its own h/cc file. Bug: none Change-Id: If28f57c5ae250afbb47c5d20c9854e9a11182642 Reviewed-on: https://webrtc-review.googlesource.com/87561 Reviewed-by: Stefan Holmer <[email protected]> Reviewed-by: Danil Chapovalov <[email protected]> Commit-Queue: Philip Eliasson <[email protected]> Cr-Commit-Position: refs/heads/master@{#23904}
cneyoo · Jul 10, 2018 · 1a4746a · 1a4746a
1 parent d4c5d63
commit 1a4746a
Show file tree

Hide file tree

Showing 16 changed files with 152 additions and 50 deletions.
diff --git a/modules/BUILD.gn b/modules/BUILD.gn
@@ -56,6 +56,7 @@ rtc_source_set("module_api") {
     "../api/transport:network_control",
     "../api/video:video_frame",
     "../api/video:video_frame_i420",
+    "../modules/rtp_rtcp:rtp_video_header",
     "../rtc_base:deprecation",
     "../rtc_base:rtc_base_approved",
     "video_coding:codec_globals_headers",

diff --git a/modules/include/module_common_types.h b/modules/include/module_common_types.h
@@ -24,9 +24,7 @@
 #include "common_types.h"  // NOLINT(build/include)
 #include "modules/include/module_common_types_public.h"
 #include "modules/include/module_fec_types.h"
-#include "modules/video_coding/codecs/h264/include/h264_globals.h"
-#include "modules/video_coding/codecs/vp8/include/vp8_globals.h"
-#include "modules/video_coding/codecs/vp9/include/vp9_globals.h"
+#include "modules/rtp_rtcp/source/rtp_video_header.h"
 #include "rtc_base/constructormagic.h"
 #include "rtc_base/deprecation.h"
 #include "rtc_base/numerics/safe_conversions.h"
@@ -38,40 +36,6 @@ namespace webrtc {
 // TODO(nisse): Deprecated, use webrtc::VideoCodecType instead.
 using RtpVideoCodecTypes = VideoCodecType;
 
-// TODO(philipel): Change from union to abls::variant.
-union RTPVideoTypeHeader {
-  RTPVideoHeaderVP8 VP8;
-  RTPVideoHeaderVP9 VP9;
-  RTPVideoHeaderH264 H264;
-};
-
-// Since RTPVideoHeader is used as a member of a union, it can't have a
-// non-trivial default constructor.
-struct RTPVideoHeader {
-  RTPVideoHeaderVP8& vp8() { return codecHeader.VP8; }
-  const RTPVideoHeaderVP8& vp8() const { return codecHeader.VP8; }
-  RTPVideoHeaderVP9& vp9() { return codecHeader.VP9; }
-  const RTPVideoHeaderVP9& vp9() const { return codecHeader.VP9; }
-  RTPVideoHeaderH264& h264() { return codecHeader.H264; }
-  const RTPVideoHeaderH264& h264() const { return codecHeader.H264; }
-
-  uint16_t width;  // size
-  uint16_t height;
-  VideoRotation rotation;
-
-  PlayoutDelay playout_delay;
-
-  VideoContentType content_type;
-
-  VideoSendTiming video_timing;
-
-  bool is_first_packet_in_frame;
-  uint8_t simulcastIdx;  // Index if the simulcast encoder creating
-                         // this frame, 0 if not using simulcast.
-  VideoCodecType codec;
-  RTPVideoTypeHeader codecHeader;
-};
-
 struct WebRtcRTPHeader {
   RTPVideoHeader& video_header() { return video; }
   const RTPVideoHeader& video_header() const { return video; }

diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn
@@ -258,6 +258,20 @@ rtc_source_set("rtcp_transceiver") {
   ]
 }
 
+rtc_source_set("rtp_video_header") {
+  visibility = [ "*" ]
+  sources = [
+    "source/rtp_video_header.cc",
+    "source/rtp_video_header.h",
+  ]
+  deps = [
+    "../../:webrtc_common",
+    "../../api/video:video_frame",
+    "../../modules/video_coding:codec_globals_headers",
+    "//third_party/abseil-cpp/absl/types:variant",
+  ]
+}
+
 rtc_source_set("fec_test_helper") {
   testonly = true
   sources = [

diff --git a/modules/rtp_rtcp/source/fec_test_helper.cc b/modules/rtp_rtcp/source/fec_test_helper.cc
@@ -123,7 +123,6 @@ std::unique_ptr<AugmentedPacket> AugmentedPacketGenerator::NextPacket(
   for (size_t i = 0; i < length; ++i)
     packet->data[i + kRtpHeaderSize] = offset + i;
   packet->length = length + kRtpHeaderSize;
-  memset(&packet->header, 0, sizeof(WebRtcRTPHeader));
   packet->header.frameType = kVideoFrameDelta;
   packet->header.header.headerLength = kRtpHeaderSize;
   packet->header.header.markerBit = (num_packets_ == 1);

diff --git a/modules/rtp_rtcp/source/rtp_receiver_impl.cc b/modules/rtp_rtcp/source/rtp_receiver_impl.cc
@@ -159,8 +159,7 @@ bool RtpReceiverImpl::IncomingRtpPacket(const RTPHeader& rtp_header,
     return false;
   }
 
-  WebRtcRTPHeader webrtc_rtp_header;
-  memset(&webrtc_rtp_header, 0, sizeof(webrtc_rtp_header));
+  WebRtcRTPHeader webrtc_rtp_header{};
   webrtc_rtp_header.header = rtp_header;
   CheckCSRC(webrtc_rtp_header);
 

diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -1682,7 +1682,7 @@ TEST_P(RtpSenderVideoTest, KeyFrameHasCVO) {
   EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(
                    kRtpExtensionVideoRotation, kVideoRotationExtensionId));
 
-  RTPVideoHeader hdr = {0};
+  RTPVideoHeader hdr;
   hdr.rotation = kVideoRotation_0;
   rtp_sender_video_->SendVideo(kVideoCodecGeneric, kVideoFrameKey, kPayload,
                                kTimestamp, 0, kFrame, sizeof(kFrame), nullptr,
@@ -1704,7 +1704,7 @@ TEST_P(RtpSenderVideoTest, TimingFrameHasPacketizationTimstampSet) {
 
   const int64_t kCaptureTimestamp = fake_clock_.TimeInMilliseconds();
 
-  RTPVideoHeader hdr = {0};
+  RTPVideoHeader hdr;
   hdr.video_timing.flags = VideoSendTiming::kTriggeredByTimer;
   hdr.video_timing.encode_start_delta_ms = kEncodeStartDeltaMs;
   hdr.video_timing.encode_finish_delta_ms = kEncodeFinishDeltaMs;
@@ -1727,7 +1727,7 @@ TEST_P(RtpSenderVideoTest, DeltaFrameHasCVOWhenChanged) {
   EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(
                    kRtpExtensionVideoRotation, kVideoRotationExtensionId));
 
-  RTPVideoHeader hdr = {0};
+  RTPVideoHeader hdr;
   hdr.rotation = kVideoRotation_90;
   EXPECT_TRUE(rtp_sender_video_->SendVideo(
       kVideoCodecGeneric, kVideoFrameKey, kPayload, kTimestamp, 0, kFrame,
@@ -1749,7 +1749,7 @@ TEST_P(RtpSenderVideoTest, DeltaFrameHasCVOWhenNonZero) {
   EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(
                    kRtpExtensionVideoRotation, kVideoRotationExtensionId));
 
-  RTPVideoHeader hdr = {0};
+  RTPVideoHeader hdr;
   hdr.rotation = kVideoRotation_90;
   EXPECT_TRUE(rtp_sender_video_->SendVideo(
       kVideoCodecGeneric, kVideoFrameKey, kPayload, kTimestamp, 0, kFrame,

diff --git a/modules/rtp_rtcp/source/rtp_video_header.cc b/modules/rtp_rtcp/source/rtp_video_header.cc
@@ -0,0 +1,27 @@
+/*
+ *  Copyright (c) 2018 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/source/rtp_video_header.h"
+
+namespace webrtc {
+
+RTPVideoHeader::RTPVideoHeader()
+    : width(),
+      height(),
+      rotation(),
+      playout_delay(),
+      content_type(),
+      video_timing(),
+      is_first_packet_in_frame(),
+      simulcastIdx(),
+      codec() {}
+RTPVideoHeader::RTPVideoHeader(const RTPVideoHeader& other) = default;
+
+}  // namespace webrtc
diff --git a/modules/rtp_rtcp/source/rtp_video_header.h b/modules/rtp_rtcp/source/rtp_video_header.h
@@ -0,0 +1,88 @@
+/*
+ *  Copyright (c) 2018 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.
+ */
+#ifndef MODULES_RTP_RTCP_SOURCE_RTP_VIDEO_HEADER_H_
+#define MODULES_RTP_RTCP_SOURCE_RTP_VIDEO_HEADER_H_
+
+#include "absl/types/variant.h"
+#include "api/video/video_content_type.h"
+#include "api/video/video_rotation.h"
+#include "api/video/video_timing.h"
+#include "common_types.h"  // NOLINT(build/include)
+#include "modules/video_coding/codecs/h264/include/h264_globals.h"
+#include "modules/video_coding/codecs/vp8/include/vp8_globals.h"
+#include "modules/video_coding/codecs/vp9/include/vp9_globals.h"
+
+namespace webrtc {
+using RTPVideoTypeHeader =
+    absl::variant<RTPVideoHeaderVP8, RTPVideoHeaderVP9, RTPVideoHeaderH264>;
+
+struct RTPVideoHeader {
+  RTPVideoHeader();
+  RTPVideoHeader(const RTPVideoHeader& other);
+
+  // TODO(philipel): Remove when downstream projects have been updated.
+  RTPVideoHeaderVP8& vp8() {
+    if (!absl::holds_alternative<RTPVideoHeaderVP8>(video_type_header))
+      video_type_header.emplace<RTPVideoHeaderVP8>();
+
+    return absl::get<RTPVideoHeaderVP8>(video_type_header);
+  }
+  // TODO(philipel): Remove when downstream projects have been updated.
+  const RTPVideoHeaderVP8& vp8() const {
+    if (!absl::holds_alternative<RTPVideoHeaderVP8>(video_type_header))
+      video_type_header.emplace<RTPVideoHeaderVP8>();
+
+    return absl::get<RTPVideoHeaderVP8>(video_type_header);
+  }
+  // TODO(philipel): Remove when downstream projects have been updated.
+  RTPVideoHeaderVP9& vp9() {
+    if (!absl::holds_alternative<RTPVideoHeaderVP9>(video_type_header))
+      video_type_header.emplace<RTPVideoHeaderVP9>();
+
+    return absl::get<RTPVideoHeaderVP9>(video_type_header);
+  }
+  // TODO(philipel): Remove when downstream projects have been updated.
+  const RTPVideoHeaderVP9& vp9() const {
+    if (!absl::holds_alternative<RTPVideoHeaderVP9>(video_type_header))
+      video_type_header.emplace<RTPVideoHeaderVP9>();
+
+    return absl::get<RTPVideoHeaderVP9>(video_type_header);
+  }
+  // TODO(philipel): Remove when downstream projects have been updated.
+  RTPVideoHeaderH264& h264() {
+    if (!absl::holds_alternative<RTPVideoHeaderH264>(video_type_header))
+      video_type_header.emplace<RTPVideoHeaderH264>();
+
+    return absl::get<RTPVideoHeaderH264>(video_type_header);
+  }
+  // TODO(philipel): Remove when downstream projects have been updated.
+  const RTPVideoHeaderH264& h264() const {
+    if (!absl::holds_alternative<RTPVideoHeaderH264>(video_type_header))
+      video_type_header.emplace<RTPVideoHeaderH264>();
+
+    return absl::get<RTPVideoHeaderH264>(video_type_header);
+  }
+
+  uint16_t width;
+  uint16_t height;
+  VideoRotation rotation;
+  PlayoutDelay playout_delay;
+  VideoContentType content_type;
+  VideoSendTiming video_timing;
+  bool is_first_packet_in_frame;
+  uint8_t simulcastIdx;
+  VideoCodecType codec;
+  // TODO(philipel): remove mutable when downstream projects have been updated.
+  mutable RTPVideoTypeHeader video_type_header;
+};
+
+}  // namespace webrtc
+
+#endif  // MODULES_RTP_RTCP_SOURCE_RTP_VIDEO_HEADER_H_
diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn
@@ -179,6 +179,7 @@ rtc_static_library("video_coding") {
     "../rtp_rtcp:rtp_rtcp_format",
     "../utility:utility",
     "//third_party/abseil-cpp/absl/types:optional",
+    "//third_party/abseil-cpp/absl/types:variant",
   ]
 }
 
@@ -221,6 +222,10 @@ rtc_source_set("codec_globals_headers") {
     "codecs/vp8/include/vp8_globals.h",
     "codecs/vp9/include/vp9_globals.h",
   ]
+
+  deps = [
+    "../../rtc_base:checks",
+  ]
 }
 
 rtc_source_set("video_coding_utility") {

diff --git a/modules/video_coding/codecs/h264/include/h264_globals.h b/modules/video_coding/codecs/h264/include/h264_globals.h
@@ -16,6 +16,8 @@
 
 #include <string>
 
+#include "rtc_base/checks.h"
+
 namespace webrtc {
 
 // The packetization types that we support: single, aggregated, and fragmented.

diff --git a/modules/video_coding/frame_object.cc b/modules/video_coding/frame_object.cc
@@ -161,7 +161,7 @@ absl::optional<RTPVideoTypeHeader> RtpFrameObject::GetCodecHeader() const {
   VCMPacket* packet = packet_buffer_->GetPacket(first_seq_num_);
   if (!packet)
     return absl::nullopt;
-  return packet->video_header.codecHeader;
+  return packet->video_header.video_type_header;
 }
 
 }  // namespace video_coding

diff --git a/modules/video_coding/packet.cc b/modules/video_coding/packet.cc
@@ -86,7 +86,7 @@ void VCMPacket::Reset() {
   insertStartCode = false;
   width = 0;
   height = 0;
-  memset(&video_header, 0, sizeof(RTPVideoHeader));
+  video_header = {};
 }
 
 void VCMPacket::CopyCodecSpecifics(const RTPVideoHeader& videoHeader) {

diff --git a/modules/video_coding/rtp_frame_reference_finder.cc b/modules/video_coding/rtp_frame_reference_finder.cc
@@ -13,6 +13,7 @@
 #include <algorithm>
 #include <limits>
 
+#include "absl/types/variant.h"
 #include "modules/video_coding/frame_object.h"
 #include "modules/video_coding/packet_buffer.h"
 #include "rtc_base/checks.h"
@@ -247,7 +248,8 @@ RtpFrameReferenceFinder::FrameDecision RtpFrameReferenceFinder::ManageFrameVp8(
     return kDrop;
   }
 
-  const RTPVideoHeaderVP8& codec_header = rtp_codec_header->VP8;
+  const RTPVideoHeaderVP8& codec_header =
+      absl::get<RTPVideoHeaderVP8>(*rtp_codec_header);
 
   if (codec_header.pictureId == kNoPictureId ||
       codec_header.temporalIdx == kNoTemporalIdx ||
@@ -398,7 +400,8 @@ RtpFrameReferenceFinder::FrameDecision RtpFrameReferenceFinder::ManageFrameVp9(
     return kDrop;
   }
 
-  const RTPVideoHeaderVP9& codec_header = rtp_codec_header->VP9;
+  const RTPVideoHeaderVP9& codec_header =
+      absl::get<RTPVideoHeaderVP9>(*rtp_codec_header);
 
   if (codec_header.picture_id == kNoPictureId ||
       codec_header.temporal_idx == kNoTemporalIdx ||

diff --git a/video/BUILD.gn b/video/BUILD.gn
@@ -72,6 +72,7 @@ rtc_static_library("video") {
     "../call:rtp_interfaces",
     "../call:video_stream_api",
     "../modules/rtp_rtcp:rtp_rtcp_format",
+    "../modules/rtp_rtcp:rtp_video_header",
     "../modules/video_coding:codec_globals_headers",
     "../modules/video_coding:nack_module",
     "../modules/video_coding:packet",

diff --git a/video/payload_router.cc b/video/payload_router.cc
@@ -230,7 +230,6 @@ EncodedImageCallback::Result PayloadRouter::OnEncodedImage(
     return Result(Result::ERROR_SEND_FAILED);
 
   RTPVideoHeader rtp_video_header;
-  memset(&rtp_video_header, 0, sizeof(RTPVideoHeader));
   if (codec_specific_info)
     CopyCodecSpecific(codec_specific_info, &rtp_video_header);
 

diff --git a/video/payload_router.h b/video/payload_router.h
@@ -16,6 +16,7 @@
 
 #include "api/video_codecs/video_encoder.h"
 #include "common_types.h"  // NOLINT(build/include)
+#include "modules/rtp_rtcp/source/rtp_video_header.h"
 #include "rtc_base/constructormagic.h"
 #include "rtc_base/criticalsection.h"
 #include "rtc_base/thread_annotations.h"
@@ -24,7 +25,6 @@ namespace webrtc {
 
 class RTPFragmentationHeader;
 class RtpRtcp;
-struct RTPVideoHeader;
 
 // Currently only VP8/VP9 specific.
 struct RtpPayloadState {