From 7b1899224bbf7b059dee701ada60ae42083222ea Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Wed, 3 Oct 2018 10:15:36 +0200 Subject: [PATCH] Move RtpHeaderExtensionMap::GetTotalLengthInBytes into own file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename to better match what it does, Adjust to support two-byte header extension Bug: webrtc:7990 Change-Id: I2786d70e7cf9cd3d722f54fb1d07c9cfaafab947 Reviewed-on: https://webrtc-review.googlesource.com/103201 Reviewed-by: Erik Språng Reviewed-by: Johannes Kron Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#24958} --- modules/rtp_rtcp/BUILD.gn | 3 + modules/rtp_rtcp/include/flexfec_sender.h | 1 + .../include/rtp_header_extension_map.h | 8 -- modules/rtp_rtcp/source/flexfec_sender.cc | 2 +- .../source/rtp_header_extension_map.cc | 21 ----- .../rtp_header_extension_map_unittest.cc | 11 --- .../source/rtp_header_extension_size.cc | 49 ++++++++++ .../source/rtp_header_extension_size.h | 32 +++++++ .../rtp_header_extension_size_unittest.cc | 92 +++++++++++++++++++ modules/rtp_rtcp/source/rtp_sender.cc | 4 +- video/video_send_stream.cc | 7 +- 11 files changed, 184 insertions(+), 46 deletions(-) create mode 100644 modules/rtp_rtcp/source/rtp_header_extension_size.cc create mode 100644 modules/rtp_rtcp/source/rtp_header_extension_size.h create mode 100644 modules/rtp_rtcp/source/rtp_header_extension_size_unittest.cc diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn index 278ece9359..39f88cd293 100644 --- a/modules/rtp_rtcp/BUILD.gn +++ b/modules/rtp_rtcp/BUILD.gn @@ -154,6 +154,8 @@ rtc_static_library("rtp_rtcp") { "source/rtp_format_vp8.h", "source/rtp_format_vp9.cc", "source/rtp_format_vp9.h", + "source/rtp_header_extension_size.cc", + "source/rtp_header_extension_size.h", "source/rtp_header_parser.cc", "source/rtp_packet_history.cc", "source/rtp_packet_history.h", @@ -405,6 +407,7 @@ if (rtc_include_tests) { "source/rtp_format_vp9_unittest.cc", "source/rtp_generic_frame_descriptor_extension_unittest.cc", "source/rtp_header_extension_map_unittest.cc", + "source/rtp_header_extension_size_unittest.cc", "source/rtp_packet_history_unittest.cc", "source/rtp_packet_unittest.cc", "source/rtp_payload_registry_unittest.cc", diff --git a/modules/rtp_rtcp/include/flexfec_sender.h b/modules/rtp_rtcp/include/flexfec_sender.h index 57ba84124f..12277e128f 100644 --- a/modules/rtp_rtcp/include/flexfec_sender.h +++ b/modules/rtp_rtcp/include/flexfec_sender.h @@ -21,6 +21,7 @@ #include "modules/rtp_rtcp/include/flexfec_sender.h" #include "modules/rtp_rtcp/include/rtp_header_extension_map.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" +#include "modules/rtp_rtcp/source/rtp_header_extension_size.h" #include "modules/rtp_rtcp/source/rtp_packet_to_send.h" #include "modules/rtp_rtcp/source/ulpfec_generator.h" #include "rtc_base/random.h" diff --git a/modules/rtp_rtcp/include/rtp_header_extension_map.h b/modules/rtp_rtcp/include/rtp_header_extension_map.h index e76650d8ee..75a6720bb9 100644 --- a/modules/rtp_rtcp/include/rtp_header_extension_map.h +++ b/modules/rtp_rtcp/include/rtp_header_extension_map.h @@ -20,11 +20,6 @@ namespace webrtc { -struct RtpExtensionSize { - RTPExtensionType type; - uint8_t value_size; -}; - class RtpHeaderExtensionMap { public: static constexpr RTPExtensionType kInvalidType = kRtpExtensionNone; @@ -52,9 +47,6 @@ class RtpHeaderExtensionMap { return ids_[type]; } - size_t GetTotalLengthInBytes( - rtc::ArrayView extensions) const; - // TODO(danilchap): Remove use of the functions below. int32_t Register(RTPExtensionType type, int id) { return RegisterByType(id, type) ? 0 : -1; diff --git a/modules/rtp_rtcp/source/flexfec_sender.cc b/modules/rtp_rtcp/source/flexfec_sender.cc index 9704096ff7..286f47c5fe 100644 --- a/modules/rtp_rtcp/source/flexfec_sender.cc +++ b/modules/rtp_rtcp/source/flexfec_sender.cc @@ -90,7 +90,7 @@ FlexfecSender::FlexfecSender( rtp_header_extension_map_( RegisterSupportedExtensions(rtp_header_extensions)), header_extensions_size_( - rtp_header_extension_map_.GetTotalLengthInBytes(extension_sizes)) { + RtpHeaderExtensionSize(extension_sizes, rtp_header_extension_map_)) { // This object should not have been instantiated if FlexFEC is disabled. RTC_DCHECK_GE(payload_type, 0); RTC_DCHECK_LE(payload_type, 127); diff --git a/modules/rtp_rtcp/source/rtp_header_extension_map.cc b/modules/rtp_rtcp/source/rtp_header_extension_map.cc index 093ffb3ce2..338cd9edb8 100644 --- a/modules/rtp_rtcp/source/rtp_header_extension_map.cc +++ b/modules/rtp_rtcp/source/rtp_header_extension_map.cc @@ -97,27 +97,6 @@ RTPExtensionType RtpHeaderExtensionMap::GetType(int id) const { return kInvalidType; } -size_t RtpHeaderExtensionMap::GetTotalLengthInBytes( - rtc::ArrayView extensions) const { - // TODO(webrtc:7990): This function must be updated when we start to send - // two-byte header extensions. - // Header size of the extension block, see RFC3550 Section 5.3.1 - static constexpr size_t kRtpOneByteHeaderLength = 4; - // Header size of each individual extension, see RFC8285 Section 4.2-4.3. - static constexpr size_t kOneByteExtensionHeaderLength = 1; - size_t values_size = 0; - for (const RtpExtensionSize& extension : extensions) { - if (IsRegistered(extension.type)) - values_size += extension.value_size + kOneByteExtensionHeaderLength; - } - if (values_size == 0) - return 0; - size_t size = kRtpOneByteHeaderLength + values_size; - // Round up to the nearest size that is a multiple of 4. - // Which is same as round down (size + 3). - return size + 3 - (size + 3) % 4; -} - int32_t RtpHeaderExtensionMap::Deregister(RTPExtensionType type) { if (IsRegistered(type)) { ids_[type] = kInvalidId; diff --git a/modules/rtp_rtcp/source/rtp_header_extension_map_unittest.cc b/modules/rtp_rtcp/source/rtp_header_extension_map_unittest.cc index 5dc16cec18..f5bb1bb324 100644 --- a/modules/rtp_rtcp/source/rtp_header_extension_map_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_header_extension_map_unittest.cc @@ -89,17 +89,6 @@ TEST(RtpHeaderExtensionTest, NonUniqueId) { EXPECT_TRUE(map.Register(4)); } -TEST(RtpHeaderExtensionTest, GetTotalLength) { - RtpHeaderExtensionMap map; - constexpr RtpExtensionSize kExtensionSizes[] = { - {TransmissionOffset::kId, TransmissionOffset::kValueSizeBytes}}; - EXPECT_EQ(0u, map.GetTotalLengthInBytes(kExtensionSizes)); - EXPECT_TRUE(map.Register(3)); - static constexpr size_t kRtpOneByteHeaderLength = 4; - EXPECT_EQ(kRtpOneByteHeaderLength + (TransmissionOffset::kValueSizeBytes + 1), - map.GetTotalLengthInBytes(kExtensionSizes)); -} - TEST(RtpHeaderExtensionTest, GetType) { RtpHeaderExtensionMap map; EXPECT_EQ(RtpHeaderExtensionMap::kInvalidType, map.GetType(3)); diff --git a/modules/rtp_rtcp/source/rtp_header_extension_size.cc b/modules/rtp_rtcp/source/rtp_header_extension_size.cc new file mode 100644 index 0000000000..5edf120b56 --- /dev/null +++ b/modules/rtp_rtcp/source/rtp_header_extension_size.cc @@ -0,0 +1,49 @@ +/* + * 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_header_extension_size.h" + +#include "api/rtpparameters.h" + +namespace webrtc { + +int RtpHeaderExtensionSize(rtc::ArrayView extensions, + const RtpHeaderExtensionMap& registered_extensions) { + // RFC3550 Section 5.3.1 + static constexpr int kExtensionBlockHeaderSize = 4; + // RFC8285 Section 4.2-4.3. + static constexpr int kOneByteHeaderExtensionMaxValueSize = 16; + + int values_size = 0; + int num_extensions = 0; + int each_extension_header_size = 1; + for (const RtpExtensionSize& extension : extensions) { + int id = registered_extensions.GetId(extension.type); + if (id == RtpHeaderExtensionMap::kInvalidId) + continue; + // All extensions should use same size header. Check if the |extension| + // forces to switch to two byte header that allows larger id and value size. + if (id > RtpExtension::kOneByteHeaderExtensionMaxId || + extension.value_size > kOneByteHeaderExtensionMaxValueSize) { + each_extension_header_size = 2; + } + values_size += extension.value_size; + num_extensions++; + } + if (values_size == 0) + return 0; + int size = kExtensionBlockHeaderSize + + each_extension_header_size * num_extensions + values_size; + // Extension size specified in 32bit words, + // so result must be multiple of 4 bytes. Round up. + return size + 3 - (size + 3) % 4; +} + +} // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_header_extension_size.h b/modules/rtp_rtcp/source/rtp_header_extension_size.h new file mode 100644 index 0000000000..8047fcc721 --- /dev/null +++ b/modules/rtp_rtcp/source/rtp_header_extension_size.h @@ -0,0 +1,32 @@ +/* + * 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_HEADER_EXTENSION_SIZE_H_ +#define MODULES_RTP_RTCP_SOURCE_RTP_HEADER_EXTENSION_SIZE_H_ + +#include "api/array_view.h" +#include "modules/rtp_rtcp/include/rtp_header_extension_map.h" +#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" + +namespace webrtc { + +struct RtpExtensionSize { + RTPExtensionType type; + int value_size; +}; + +// Calculates rtp header extension size in bytes assuming packet contain +// all |extensions| with provided |value_size|. +// Counts only extensions present among |registered_extensions|. +int RtpHeaderExtensionSize(rtc::ArrayView extensions, + const RtpHeaderExtensionMap& registered_extensions); + +} // namespace webrtc + +#endif // MODULES_RTP_RTCP_SOURCE_RTP_HEADER_EXTENSION_SIZE_H_ diff --git a/modules/rtp_rtcp/source/rtp_header_extension_size_unittest.cc b/modules/rtp_rtcp/source/rtp_header_extension_size_unittest.cc new file mode 100644 index 0000000000..5cc26bc652 --- /dev/null +++ b/modules/rtp_rtcp/source/rtp_header_extension_size_unittest.cc @@ -0,0 +1,92 @@ +/* + * 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_header_extension_size.h" + +#include "modules/rtp_rtcp/source/rtp_header_extensions.h" +#include "test/gtest.h" + +namespace { + +using ::webrtc::RtpExtensionSize; +using ::webrtc::RtpHeaderExtensionMap; +using ::webrtc::RtpHeaderExtensionSize; +using ::webrtc::RtpMid; +using ::webrtc::RtpStreamId; + +// id for 1-byte header extension. actual value is irrelevant for these tests. +constexpr int kId = 1; +// id that forces to use 2-byte header extension. +constexpr int kIdForceTwoByteHeader = 15; + +TEST(RtpHeaderExtensionSizeTest, ReturnsZeroIfNoExtensionsAreRegistered) { + constexpr RtpExtensionSize kExtensionSizes[] = {{RtpMid::kId, 3}}; + // Register different extension than ask size for. + RtpHeaderExtensionMap registered; + registered.Register(kId); + + EXPECT_EQ(RtpHeaderExtensionSize(kExtensionSizes, registered), 0); +} + +TEST(RtpHeaderExtensionSizeTest, IncludesSizeOfExtensionHeaders) { + constexpr RtpExtensionSize kExtensionSizes[] = {{RtpMid::kId, 3}}; + RtpHeaderExtensionMap registered; + registered.Register(kId); + + // 4 bytes for extension block header + 1 byte for individual extension header + // + 3 bytes for the value. + EXPECT_EQ(RtpHeaderExtensionSize(kExtensionSizes, registered), 8); +} + +TEST(RtpHeaderExtensionSizeTest, RoundsUpTo32bitAlignmant) { + constexpr RtpExtensionSize kExtensionSizes[] = {{RtpMid::kId, 5}}; + RtpHeaderExtensionMap registered; + registered.Register(kId); + + // 10 bytes of data including headers + 2 bytes of padding. + EXPECT_EQ(RtpHeaderExtensionSize(kExtensionSizes, registered), 12); +} + +TEST(RtpHeaderExtensionSizeTest, SumsSeveralExtensions) { + constexpr RtpExtensionSize kExtensionSizes[] = {{RtpMid::kId, 16}, + {RtpStreamId::kId, 2}}; + RtpHeaderExtensionMap registered; + registered.Register(kId); + registered.Register(14); + + // 4 bytes for extension block header + 18 bytes of value + + // 2 bytes for two headers + EXPECT_EQ(RtpHeaderExtensionSize(kExtensionSizes, registered), 24); +} + +TEST(RtpHeaderExtensionSizeTest, LargeIdForce2BytesHeader) { + constexpr RtpExtensionSize kExtensionSizes[] = {{RtpMid::kId, 3}, + {RtpStreamId::kId, 2}}; + RtpHeaderExtensionMap registered; + registered.Register(kId); + registered.Register(kIdForceTwoByteHeader); + + // 4 bytes for extension block header + 5 bytes of value + + // 2*2 bytes for two headers + 3 bytes of padding. + EXPECT_EQ(RtpHeaderExtensionSize(kExtensionSizes, registered), 16); +} + +TEST(RtpHeaderExtensionSizeTest, LargeValueForce2BytesHeader) { + constexpr RtpExtensionSize kExtensionSizes[] = {{RtpMid::kId, 17}, + {RtpStreamId::kId, 4}}; + RtpHeaderExtensionMap registered; + registered.Register(1); + registered.Register(2); + + // 4 bytes for extension block header + 21 bytes of value + + // 2*2 bytes for two headers + 3 byte of padding. + EXPECT_EQ(RtpHeaderExtensionSize(kExtensionSizes, registered), 32); +} + +} // namespace diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index 62ac066ffa..9f10c40e57 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -1120,8 +1120,8 @@ size_t RTPSender::RtpHeaderLength() const { rtc::CritScope lock(&send_critsect_); size_t rtp_header_length = kRtpHeaderLength; rtp_header_length += sizeof(uint32_t) * csrcs_.size(); - rtp_header_length += rtp_header_extension_map_.GetTotalLengthInBytes( - kFecOrPaddingExtensionSizes); + rtp_header_length += RtpHeaderExtensionSize(kFecOrPaddingExtensionSizes, + rtp_header_extension_map_); return rtp_header_length; } diff --git a/video/video_send_stream.cc b/video/video_send_stream.cc index f666f84245..2a47b9d18d 100644 --- a/video/video_send_stream.cc +++ b/video/video_send_stream.cc @@ -12,6 +12,7 @@ #include #include "api/video/video_stream_encoder_create.h" +#include "modules/rtp_rtcp/source/rtp_header_extension_size.h" #include "modules/rtp_rtcp/source/rtp_sender.h" #include "rtc_base/logging.h" #include "video/video_send_stream_impl.h" @@ -26,10 +27,10 @@ size_t CalculateMaxHeaderSize(const RtpConfig& config) { size_t fec_extensions_size = 0; if (config.extensions.size() > 0) { RtpHeaderExtensionMap extensions_map(config.extensions); - extensions_size = - extensions_map.GetTotalLengthInBytes(RTPSender::VideoExtensionSizes()); + extensions_size = RtpHeaderExtensionSize(RTPSender::VideoExtensionSizes(), + extensions_map); fec_extensions_size = - extensions_map.GetTotalLengthInBytes(RTPSender::FecExtensionSizes()); + RtpHeaderExtensionSize(RTPSender::FecExtensionSizes(), extensions_map); } header_size += extensions_size; if (config.flexfec.payload_type >= 0) {