diff --git a/api/rtp_parameters.cc b/api/rtp_parameters.cc index 6de14da10f..b51ea7db3c 100644 --- a/api/rtp_parameters.cc +++ b/api/rtp_parameters.cc @@ -135,6 +135,10 @@ const char RtpExtension::kFrameMarkingUri[] = "http://tools.ietf.org/html/draft-ietf-avtext-framemarking-07"; const int RtpExtension::kFrameMarkingDefaultId = 10; +const char RtpExtension::kGenericFrameDescriptorUri00[] = + "http://www.webrtc.org/experiments/rtp-hdrext/generic-frame-descriptor-00"; +const char RtpExtension::kGenericFrameDescriptorUri01[] = + "http://www.webrtc.org/experiments/rtp-hdrext/generic-frame-descriptor-01"; const char RtpExtension::kGenericFrameDescriptorUri[] = "http://www.webrtc.org/experiments/rtp-hdrext/generic-frame-descriptor-00"; const int RtpExtension::kGenericFrameDescriptorDefaultId = 11; @@ -178,7 +182,8 @@ bool RtpExtension::IsSupportedForVideo(const std::string& uri) { uri == webrtc::RtpExtension::kVideoTimingUri || uri == webrtc::RtpExtension::kMidUri || uri == webrtc::RtpExtension::kFrameMarkingUri || - uri == webrtc::RtpExtension::kGenericFrameDescriptorUri || + uri == webrtc::RtpExtension::kGenericFrameDescriptorUri00 || + uri == webrtc::RtpExtension::kGenericFrameDescriptorUri01 || uri == webrtc::RtpExtension::kColorSpaceUri || uri == webrtc::RtpExtension::kRidUri || uri == webrtc::RtpExtension::kRepairedRidUri; diff --git a/api/rtp_parameters.h b/api/rtp_parameters.h index 6138c37320..1cf4f36091 100644 --- a/api/rtp_parameters.h +++ b/api/rtp_parameters.h @@ -297,6 +297,9 @@ struct RtpExtension { RTC_DEPRECATED static const int kFrameMarkingDefaultId; // Experimental codec agnostic frame descriptor. + static const char kGenericFrameDescriptorUri00[]; + static const char kGenericFrameDescriptorUri01[]; + // TODO(bugs.webrtc.org/10243): Remove once dependencies have been updated. static const char kGenericFrameDescriptorUri[]; // TODO(bugs.webrtc.org/10288): Remove once dependencies have been updated. RTC_DEPRECATED static const int kGenericFrameDescriptorDefaultId; diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 09d37b3f2c..a51c11ac3a 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -494,7 +494,9 @@ RtpCapabilities WebRtcVideoEngine::GetCapabilities() const { webrtc::RtpExtension(webrtc::RtpExtension::kColorSpaceUri, id++)); if (webrtc::field_trial::IsEnabled("WebRTC-GenericDescriptorAdvertised")) { capabilities.header_extensions.push_back(webrtc::RtpExtension( - webrtc::RtpExtension::kGenericFrameDescriptorUri, id++)); + webrtc::RtpExtension::kGenericFrameDescriptorUri00, id++)); + capabilities.header_extensions.push_back(webrtc::RtpExtension( + webrtc::RtpExtension::kGenericFrameDescriptorUri01, id++)); } return capabilities; diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index 482da39fb4..f8341c1ac1 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -313,8 +313,12 @@ TEST_F(WebRtcVideoEngineTest, SupportsColorSpaceHeaderExtension) { ExpectRtpCapabilitySupport(RtpExtension::kColorSpaceUri, true); } -TEST_F(WebRtcVideoEngineTest, AdvertiseGenericDescriptor) { - ExpectRtpCapabilitySupport(RtpExtension::kGenericFrameDescriptorUri, false); +TEST_F(WebRtcVideoEngineTest, AdvertiseGenericDescriptor00) { + ExpectRtpCapabilitySupport(RtpExtension::kGenericFrameDescriptorUri00, false); +} + +TEST_F(WebRtcVideoEngineTest, AdvertiseGenericDescriptor01) { + ExpectRtpCapabilitySupport(RtpExtension::kGenericFrameDescriptorUri01, false); } class WebRtcVideoEngineTestWithGenericDescriptor @@ -324,8 +328,14 @@ class WebRtcVideoEngineTestWithGenericDescriptor : WebRtcVideoEngineTest("WebRTC-GenericDescriptorAdvertised/Enabled/") {} }; -TEST_F(WebRtcVideoEngineTestWithGenericDescriptor, AdvertiseGenericDescriptor) { - ExpectRtpCapabilitySupport(RtpExtension::kGenericFrameDescriptorUri, true); +TEST_F(WebRtcVideoEngineTestWithGenericDescriptor, + AdvertiseGenericDescriptor00) { + ExpectRtpCapabilitySupport(RtpExtension::kGenericFrameDescriptorUri00, true); +} + +TEST_F(WebRtcVideoEngineTestWithGenericDescriptor, + AdvertiseGenericDescriptor01) { + ExpectRtpCapabilitySupport(RtpExtension::kGenericFrameDescriptorUri01, true); } TEST_F(WebRtcVideoEngineTest, CVOSetHeaderExtensionBeforeCapturer) { diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h index 1e80bb2fab..82328980e7 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h +++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h @@ -62,7 +62,9 @@ enum RTPExtensionType : int { kRtpExtensionRtpStreamId, kRtpExtensionRepairedRtpStreamId, kRtpExtensionMid, - kRtpExtensionGenericFrameDescriptor, + kRtpExtensionGenericFrameDescriptor00, + kRtpExtensionGenericFrameDescriptor = kRtpExtensionGenericFrameDescriptor00, + kRtpExtensionGenericFrameDescriptor01, kRtpExtensionColorSpace, kRtpExtensionNumberOfExtensions // Must be the last entity in the enum. }; diff --git a/modules/rtp_rtcp/source/rtp_generic_frame_descriptor.h b/modules/rtp_rtcp/source/rtp_generic_frame_descriptor.h index 52d401fedd..47a2a7468c 100644 --- a/modules/rtp_rtcp/source/rtp_generic_frame_descriptor.h +++ b/modules/rtp_rtcp/source/rtp_generic_frame_descriptor.h @@ -14,6 +14,7 @@ #include #include +#include "absl/types/optional.h" #include "api/array_view.h" namespace webrtc { @@ -36,8 +37,14 @@ class RtpGenericFrameDescriptor { bool LastPacketInSubFrame() const { return end_of_subframe_; } void SetLastPacketInSubFrame(bool last) { end_of_subframe_ = last; } - bool FirstSubFrameInFrame() const { return beginning_of_frame_; } - bool LastSubFrameInFrame() const { return end_of_frame_; } + // Denotes whether the frame is discardable. That is, whether skipping it + // would have no effect on the decodability of subsequent frames. + // An absl::optional is used because version 0 of the extension did not + // support this flag. (The optional aspect is relevant only when parsing.) + // TODO(bugs.webrtc.org/10243): Make this into a plain bool when v00 of + // the extension is deprecated. + absl::optional Discardable() const { return discardable_; } + void SetDiscardable(bool discardable) { discardable_ = discardable; } // Properties below undefined if !FirstPacketInSubFrame() // Valid range for temporal layer: [0, 7] @@ -66,14 +73,10 @@ class RtpGenericFrameDescriptor { rtc::ArrayView GetByteRepresentation(); private: - friend class RtpGenericFrameDescriptorExtension; - void SetFirstSubFrameInFrame(bool first) { beginning_of_frame_ = first; } - void SetLastSubFrameInFrame(bool last) { end_of_frame_ = last; } - bool beginning_of_subframe_ = false; bool end_of_subframe_ = false; - bool beginning_of_frame_ = true; - bool end_of_frame_ = true; + + absl::optional discardable_; uint16_t frame_id_ = 0; uint8_t spatial_layers_ = 1; diff --git a/modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension.cc b/modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension.cc index 7cd120d9f4..a705b5aa7e 100644 --- a/modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension.cc +++ b/modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension.cc @@ -17,15 +17,21 @@ namespace { constexpr uint8_t kFlagBeginOfSubframe = 0x80; constexpr uint8_t kFlagEndOfSubframe = 0x40; -constexpr uint8_t kFlagFirstSubframe = 0x20; -constexpr uint8_t kFlagLastSubframe = 0x10; + +// In version 00, the flags F and L in the first byte correspond to +// kFlagFirstSubframeV00 and kFlagLastSubframeV00. In practice, they were +// always set to |true|. In version 01, these flags are deprecated, and we use +// one of their bits for the discardability flag. +constexpr uint8_t kFlagFirstSubframeV00 = 0x20; +constexpr uint8_t kFlagLastSubframeV00 = 0x10; +constexpr uint8_t kFlagDiscardableV01 = 0x10; + constexpr uint8_t kFlagDependencies = 0x08; constexpr uint8_t kMaskTemporalLayer = 0x07; constexpr uint8_t kFlagMoreDependencies = 0x01; constexpr uint8_t kFlageXtendedOffset = 0x02; -} // namespace // 0 1 2 3 4 5 6 7 // +-+-+-+-+-+-+-+-+ // |B|E|F|L|D| T | @@ -52,10 +58,9 @@ constexpr uint8_t kFlageXtendedOffset = 0x02; // +---------------+ // | ... | // +-+-+-+-+-+-+-+-+ -constexpr RTPExtensionType RtpGenericFrameDescriptorExtension::kId; -constexpr char RtpGenericFrameDescriptorExtension::kUri[]; -bool RtpGenericFrameDescriptorExtension::Parse( +bool RtpGenericFrameDescriptorExtensionParse( + size_t version, rtc::ArrayView data, RtpGenericFrameDescriptor* descriptor) { if (data.empty()) { @@ -65,8 +70,10 @@ bool RtpGenericFrameDescriptorExtension::Parse( bool begins_subframe = (data[0] & kFlagBeginOfSubframe) != 0; descriptor->SetFirstPacketInSubFrame(begins_subframe); descriptor->SetLastPacketInSubFrame((data[0] & kFlagEndOfSubframe) != 0); - descriptor->SetFirstSubFrameInFrame((data[0] & kFlagFirstSubframe) != 0); - descriptor->SetLastSubFrameInFrame((data[0] & kFlagLastSubframe) != 0); + + if (version >= 1) { + descriptor->SetDiscardable((data[0] & kFlagDiscardableV01) != 0); + } // Parse Subframe details provided in 1st packet of subframe. if (!begins_subframe) { @@ -108,7 +115,7 @@ bool RtpGenericFrameDescriptorExtension::Parse( return true; } -size_t RtpGenericFrameDescriptorExtension::ValueSize( +size_t RtpGenericFrameDescriptorExtensionValueSize( const RtpGenericFrameDescriptor& descriptor) { if (!descriptor.FirstPacketInSubFrame()) return 1; @@ -125,15 +132,24 @@ size_t RtpGenericFrameDescriptorExtension::ValueSize( return size; } -bool RtpGenericFrameDescriptorExtension::Write( +bool RtpGenericFrameDescriptorExtensionWrite( + size_t version, rtc::ArrayView data, const RtpGenericFrameDescriptor& descriptor) { - RTC_CHECK_EQ(data.size(), ValueSize(descriptor)); + RTC_CHECK_EQ(data.size(), + + RtpGenericFrameDescriptorExtensionValueSize(descriptor)); uint8_t base_header = (descriptor.FirstPacketInSubFrame() ? kFlagBeginOfSubframe : 0) | - (descriptor.LastPacketInSubFrame() ? kFlagEndOfSubframe : 0) | - (descriptor.FirstSubFrameInFrame() ? kFlagFirstSubframe : 0) | - (descriptor.LastSubFrameInFrame() ? kFlagLastSubframe : 0); + (descriptor.LastPacketInSubFrame() ? kFlagEndOfSubframe : 0); + if (version == 0) { + base_header |= kFlagFirstSubframeV00; + base_header |= kFlagLastSubframeV00; + } else if (version >= 1) { + const absl::optional discardable = descriptor.Discardable(); + base_header |= (discardable.value_or(false) ? kFlagDiscardableV01 : 0); + } + if (!descriptor.FirstPacketInSubFrame()) { data[0] = base_header; return true; @@ -168,4 +184,48 @@ bool RtpGenericFrameDescriptorExtension::Write( return true; } +} // namespace + +constexpr RTPExtensionType RtpGenericFrameDescriptorExtension00::kId; +constexpr char RtpGenericFrameDescriptorExtension00::kUri[]; + +bool RtpGenericFrameDescriptorExtension00::Parse( + rtc::ArrayView data, + RtpGenericFrameDescriptor* descriptor) { + return RtpGenericFrameDescriptorExtensionParse(0, data, descriptor); +} + +size_t RtpGenericFrameDescriptorExtension00::ValueSize( + const RtpGenericFrameDescriptor& descriptor) { + // No difference between existing versions. + return RtpGenericFrameDescriptorExtensionValueSize(descriptor); +} + +bool RtpGenericFrameDescriptorExtension00::Write( + rtc::ArrayView data, + const RtpGenericFrameDescriptor& descriptor) { + return RtpGenericFrameDescriptorExtensionWrite(0, data, descriptor); +} + +constexpr RTPExtensionType RtpGenericFrameDescriptorExtension01::kId; +constexpr char RtpGenericFrameDescriptorExtension01::kUri[]; + +bool RtpGenericFrameDescriptorExtension01::Parse( + rtc::ArrayView data, + RtpGenericFrameDescriptor* descriptor) { + return RtpGenericFrameDescriptorExtensionParse(1, data, descriptor); +} + +size_t RtpGenericFrameDescriptorExtension01::ValueSize( + const RtpGenericFrameDescriptor& descriptor) { + // No difference between existing versions. + return RtpGenericFrameDescriptorExtensionValueSize(descriptor); +} + +bool RtpGenericFrameDescriptorExtension01::Write( + rtc::ArrayView data, + const RtpGenericFrameDescriptor& descriptor) { + return RtpGenericFrameDescriptorExtensionWrite(1, data, descriptor); +} + } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension.h b/modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension.h index 5c1bc52fcd..a52588ee3a 100644 --- a/modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension.h +++ b/modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension.h @@ -19,14 +19,10 @@ namespace webrtc { -// TODO(bugs.webrtc.org/10243): Remove when downstream projects stop using -// RtpGenericFrameDescriptorExtension. -using RtpGenericFrameDescriptorExtension00 = RtpGenericFrameDescriptorExtension; - -class RtpGenericFrameDescriptorExtension { +class RtpGenericFrameDescriptorExtension00 { public: using value_type = RtpGenericFrameDescriptor; - static constexpr RTPExtensionType kId = kRtpExtensionGenericFrameDescriptor; + static constexpr RTPExtensionType kId = kRtpExtensionGenericFrameDescriptor00; static constexpr char kUri[] = "http://www.webrtc.org/experiments/rtp-hdrext/" "generic-frame-descriptor-00"; @@ -34,7 +30,23 @@ class RtpGenericFrameDescriptorExtension { static bool Parse(rtc::ArrayView data, RtpGenericFrameDescriptor* descriptor); - static size_t ValueSize(const RtpGenericFrameDescriptor&); + static size_t ValueSize(const RtpGenericFrameDescriptor& descriptor); + static bool Write(rtc::ArrayView data, + const RtpGenericFrameDescriptor& descriptor); +}; + +class RtpGenericFrameDescriptorExtension01 { + public: + using value_type = RtpGenericFrameDescriptor; + static constexpr RTPExtensionType kId = kRtpExtensionGenericFrameDescriptor01; + static constexpr char kUri[] = + "http://www.webrtc.org/experiments/rtp-hdrext/" + "generic-frame-descriptor-01"; + static constexpr int kMaxSizeBytes = 16; + + static bool Parse(rtc::ArrayView data, + RtpGenericFrameDescriptor* descriptor); + static size_t ValueSize(const RtpGenericFrameDescriptor& descriptor); static bool Write(rtc::ArrayView data, const RtpGenericFrameDescriptor& descriptor); }; diff --git a/modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension_unittest.cc b/modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension_unittest.cc index 5e6c6cdaf8..13cacb5471 100644 --- a/modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension_unittest.cc @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012 The WebRTC project authors. All Rights Reserved. + * 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 @@ -23,28 +23,87 @@ constexpr uint8_t kDeprecatedFlags = 0x30; // TODO(danilchap): Add fuzzer to test for various invalid inputs. -TEST(RtpGenericFrameDescriptorExtensionTest, - ParseFirstPacketOfIndependenSubFrame) { +class RtpGenericFrameDescriptorExtensionTest + : public ::testing::Test, + public ::testing::WithParamInterface { + public: + RtpGenericFrameDescriptorExtensionTest() : version_(GetParam()) {} + + bool Parse(rtc::ArrayView data, + RtpGenericFrameDescriptor* descriptor) const { + switch (version_) { + case 0: + return RtpGenericFrameDescriptorExtension00::Parse(data, descriptor); + case 1: + return RtpGenericFrameDescriptorExtension01::Parse(data, descriptor); + } + RTC_NOTREACHED(); + return false; + } + + size_t ValueSize(const RtpGenericFrameDescriptor& descriptor) const { + switch (version_) { + case 0: + return RtpGenericFrameDescriptorExtension00::ValueSize(descriptor); + case 1: + return RtpGenericFrameDescriptorExtension01::ValueSize(descriptor); + } + RTC_NOTREACHED(); + return 0; + } + + bool Write(rtc::ArrayView data, + const RtpGenericFrameDescriptor& descriptor) const { + switch (version_) { + case 0: + return RtpGenericFrameDescriptorExtension00::Write(data, descriptor); + case 1: + return RtpGenericFrameDescriptorExtension01::Write(data, descriptor); + } + RTC_NOTREACHED(); + return false; + } + + protected: + const int version_; +}; + +INSTANTIATE_TEST_SUITE_P(, + RtpGenericFrameDescriptorExtensionTest, + ::testing::Values(0, 1)); + +TEST_P(RtpGenericFrameDescriptorExtensionTest, + ParseFirstPacketOfIndependenSubFrame) { const int kTemporalLayer = 5; constexpr uint8_t kRaw[] = {0x80 | kTemporalLayer, 0x49, 0x12, 0x34}; RtpGenericFrameDescriptor descriptor; - ASSERT_TRUE(RtpGenericFrameDescriptorExtension::Parse(kRaw, &descriptor)); + ASSERT_TRUE(Parse(kRaw, &descriptor)); EXPECT_TRUE(descriptor.FirstPacketInSubFrame()); EXPECT_FALSE(descriptor.LastPacketInSubFrame()); - EXPECT_FALSE(descriptor.FirstSubFrameInFrame()); - EXPECT_FALSE(descriptor.LastSubFrameInFrame()); + + const absl::optional discardable = descriptor.Discardable(); + if (version_ == 0) { + ASSERT_FALSE(discardable.has_value()); + } else { + ASSERT_TRUE(discardable.has_value()); + EXPECT_FALSE(discardable.value()); + } + EXPECT_THAT(descriptor.FrameDependenciesDiffs(), IsEmpty()); EXPECT_EQ(descriptor.TemporalLayer(), kTemporalLayer); EXPECT_EQ(descriptor.SpatialLayersBitmask(), 0x49); EXPECT_EQ(descriptor.FrameId(), 0x3412); } -TEST(RtpGenericFrameDescriptorExtensionTest, - WriteFirstPacketOfIndependenSubFrame) { +TEST_P(RtpGenericFrameDescriptorExtensionTest, + WriteFirstPacketOfIndependenSubFrame) { const int kTemporalLayer = 5; - constexpr uint8_t kRaw[] = {0xb0 | kTemporalLayer, 0x49, 0x12, 0x34}; + uint8_t kRaw[] = {0x80 | kTemporalLayer, 0x49, 0x12, 0x34}; + if (version_ == 0) { + kRaw[0] |= kDeprecatedFlags; + } RtpGenericFrameDescriptor descriptor; descriptor.SetFirstPacketInSubFrame(true); @@ -52,240 +111,263 @@ TEST(RtpGenericFrameDescriptorExtensionTest, descriptor.SetSpatialLayersBitmask(0x49); descriptor.SetFrameId(0x3412); - ASSERT_EQ(RtpGenericFrameDescriptorExtension::ValueSize(descriptor), - sizeof(kRaw)); + ASSERT_EQ(ValueSize(descriptor), sizeof(kRaw)); uint8_t buffer[sizeof(kRaw)]; - EXPECT_TRUE(RtpGenericFrameDescriptorExtension::Write(buffer, descriptor)); + EXPECT_TRUE(Write(buffer, descriptor)); + EXPECT_THAT(buffer, ElementsAreArray(kRaw)); } -TEST(RtpGenericFrameDescriptorExtensionTest, ParseLastPacketOfSubFrame) { +TEST_P(RtpGenericFrameDescriptorExtensionTest, ParseLastPacketOfSubFrame) { constexpr uint8_t kRaw[] = {0x40}; RtpGenericFrameDescriptor descriptor; - ASSERT_TRUE(RtpGenericFrameDescriptorExtension::Parse(kRaw, &descriptor)); + ASSERT_TRUE(Parse(kRaw, &descriptor)); EXPECT_FALSE(descriptor.FirstPacketInSubFrame()); - EXPECT_FALSE(descriptor.FirstSubFrameInFrame()); - EXPECT_FALSE(descriptor.LastSubFrameInFrame()); + + const absl::optional discardable = descriptor.Discardable(); + if (version_ == 0) { + ASSERT_FALSE(discardable.has_value()); + } else { + ASSERT_TRUE(discardable.has_value()); + EXPECT_FALSE(discardable.value()); + } EXPECT_TRUE(descriptor.LastPacketInSubFrame()); } -TEST(RtpGenericFrameDescriptorExtensionTest, WriteLastPacketOfSubFrame) { - constexpr uint8_t kRaw[] = {0x40 | kDeprecatedFlags}; +TEST_P(RtpGenericFrameDescriptorExtensionTest, WriteLastPacketOfSubFrame) { + uint8_t kRaw[] = {0x40}; + if (version_ == 0) { + kRaw[0] |= kDeprecatedFlags; + } + RtpGenericFrameDescriptor descriptor; descriptor.SetLastPacketInSubFrame(true); - ASSERT_EQ(RtpGenericFrameDescriptorExtension::ValueSize(descriptor), - sizeof(kRaw)); + ASSERT_EQ(ValueSize(descriptor), sizeof(kRaw)); uint8_t buffer[sizeof(kRaw)]; - EXPECT_TRUE(RtpGenericFrameDescriptorExtension::Write(buffer, descriptor)); + EXPECT_TRUE(Write(buffer, descriptor)); EXPECT_THAT(buffer, ElementsAreArray(kRaw)); } -TEST(RtpGenericFrameDescriptorExtensionTest, ParseFirstSubFrameInFrame) { - constexpr uint8_t kRaw[] = {0x20}; - RtpGenericFrameDescriptor descriptor; +TEST_P(RtpGenericFrameDescriptorExtensionTest, ParseDiscardable) { + if (version_ == 0) { + return; + } - ASSERT_TRUE(RtpGenericFrameDescriptorExtension::Parse(kRaw, &descriptor)); - - EXPECT_FALSE(descriptor.FirstPacketInSubFrame()); - EXPECT_FALSE(descriptor.LastPacketInSubFrame()); - EXPECT_FALSE(descriptor.LastSubFrameInFrame()); - - EXPECT_TRUE(descriptor.FirstSubFrameInFrame()); -} - -TEST(RtpGenericFrameDescriptorExtensionTest, ParseLastSubFrameInFrame) { constexpr uint8_t kRaw[] = {0x10}; RtpGenericFrameDescriptor descriptor; - - ASSERT_TRUE(RtpGenericFrameDescriptorExtension::Parse(kRaw, &descriptor)); - - EXPECT_FALSE(descriptor.FirstPacketInSubFrame()); - EXPECT_FALSE(descriptor.LastPacketInSubFrame()); - EXPECT_FALSE(descriptor.FirstSubFrameInFrame()); - - EXPECT_TRUE(descriptor.LastSubFrameInFrame()); + ASSERT_TRUE(Parse(kRaw, &descriptor)); + const absl::optional discardable = descriptor.Discardable(); + ASSERT_TRUE(discardable.has_value()); + EXPECT_TRUE(discardable.value()); } -TEST(RtpGenericFrameDescriptorExtensionTest, ParseMinShortFrameDependencies) { +TEST_P(RtpGenericFrameDescriptorExtensionTest, WriteDiscardable) { + if (version_ == 0) { + return; + } + + constexpr uint8_t kRaw[] = {0x10}; + RtpGenericFrameDescriptor descriptor; + descriptor.SetDiscardable(true); + ASSERT_EQ(ValueSize(descriptor), sizeof(kRaw)); + uint8_t buffer[sizeof(kRaw)]; + EXPECT_TRUE(Write(buffer, descriptor)); + EXPECT_THAT(buffer, ElementsAreArray(kRaw)); +} + +TEST_P(RtpGenericFrameDescriptorExtensionTest, ParseMinShortFrameDependencies) { constexpr uint16_t kDiff = 1; constexpr uint8_t kRaw[] = {0x88, 0x01, 0x00, 0x00, 0x04}; RtpGenericFrameDescriptor descriptor; - ASSERT_TRUE(RtpGenericFrameDescriptorExtension::Parse(kRaw, &descriptor)); + ASSERT_TRUE(Parse(kRaw, &descriptor)); ASSERT_TRUE(descriptor.FirstPacketInSubFrame()); EXPECT_THAT(descriptor.FrameDependenciesDiffs(), ElementsAre(kDiff)); } -TEST(RtpGenericFrameDescriptorExtensionTest, WriteMinShortFrameDependencies) { +TEST_P(RtpGenericFrameDescriptorExtensionTest, WriteMinShortFrameDependencies) { constexpr uint16_t kDiff = 1; - constexpr uint8_t kRaw[] = {0xb8, 0x01, 0x00, 0x00, 0x04}; + uint8_t kRaw[] = {0x88, 0x01, 0x00, 0x00, 0x04}; + if (version_ == 0) { + kRaw[0] |= kDeprecatedFlags; + } RtpGenericFrameDescriptor descriptor; descriptor.SetFirstPacketInSubFrame(true); descriptor.AddFrameDependencyDiff(kDiff); - ASSERT_EQ(RtpGenericFrameDescriptorExtension::ValueSize(descriptor), - sizeof(kRaw)); + ASSERT_EQ(ValueSize(descriptor), sizeof(kRaw)); uint8_t buffer[sizeof(kRaw)]; - EXPECT_TRUE(RtpGenericFrameDescriptorExtension::Write(buffer, descriptor)); + EXPECT_TRUE(Write(buffer, descriptor)); EXPECT_THAT(buffer, ElementsAreArray(kRaw)); } -TEST(RtpGenericFrameDescriptorExtensionTest, ParseMaxShortFrameDependencies) { +TEST_P(RtpGenericFrameDescriptorExtensionTest, ParseMaxShortFrameDependencies) { constexpr uint16_t kDiff = 0x3f; constexpr uint8_t kRaw[] = {0xb8, 0x01, 0x00, 0x00, 0xfc}; RtpGenericFrameDescriptor descriptor; - ASSERT_TRUE(RtpGenericFrameDescriptorExtension::Parse(kRaw, &descriptor)); + ASSERT_TRUE(Parse(kRaw, &descriptor)); ASSERT_TRUE(descriptor.FirstPacketInSubFrame()); EXPECT_THAT(descriptor.FrameDependenciesDiffs(), ElementsAre(kDiff)); } -TEST(RtpGenericFrameDescriptorExtensionTest, WriteMaxShortFrameDependencies) { +TEST_P(RtpGenericFrameDescriptorExtensionTest, WriteMaxShortFrameDependencies) { constexpr uint16_t kDiff = 0x3f; - constexpr uint8_t kRaw[] = {0xb8, 0x01, 0x00, 0x00, 0xfc}; + uint8_t kRaw[] = {0x88, 0x01, 0x00, 0x00, 0xfc}; + if (version_ == 0) { + kRaw[0] |= kDeprecatedFlags; + } RtpGenericFrameDescriptor descriptor; descriptor.SetFirstPacketInSubFrame(true); descriptor.AddFrameDependencyDiff(kDiff); - ASSERT_EQ(RtpGenericFrameDescriptorExtension::ValueSize(descriptor), - sizeof(kRaw)); + ASSERT_EQ(ValueSize(descriptor), sizeof(kRaw)); uint8_t buffer[sizeof(kRaw)]; - EXPECT_TRUE(RtpGenericFrameDescriptorExtension::Write(buffer, descriptor)); + EXPECT_TRUE(Write(buffer, descriptor)); EXPECT_THAT(buffer, ElementsAreArray(kRaw)); } -TEST(RtpGenericFrameDescriptorExtensionTest, ParseMinLongFrameDependencies) { +TEST_P(RtpGenericFrameDescriptorExtensionTest, ParseMinLongFrameDependencies) { constexpr uint16_t kDiff = 0x40; constexpr uint8_t kRaw[] = {0xb8, 0x01, 0x00, 0x00, 0x02, 0x01}; RtpGenericFrameDescriptor descriptor; - ASSERT_TRUE(RtpGenericFrameDescriptorExtension::Parse(kRaw, &descriptor)); + ASSERT_TRUE(Parse(kRaw, &descriptor)); ASSERT_TRUE(descriptor.FirstPacketInSubFrame()); EXPECT_THAT(descriptor.FrameDependenciesDiffs(), ElementsAre(kDiff)); } -TEST(RtpGenericFrameDescriptorExtensionTest, WriteMinLongFrameDependencies) { +TEST_P(RtpGenericFrameDescriptorExtensionTest, WriteMinLongFrameDependencies) { constexpr uint16_t kDiff = 0x40; - constexpr uint8_t kRaw[] = {0xb8, 0x01, 0x00, 0x00, 0x02, 0x01}; + uint8_t kRaw[] = {0x88, 0x01, 0x00, 0x00, 0x02, 0x01}; + if (version_ == 0) { + kRaw[0] |= kDeprecatedFlags; + } RtpGenericFrameDescriptor descriptor; descriptor.SetFirstPacketInSubFrame(true); descriptor.AddFrameDependencyDiff(kDiff); - ASSERT_EQ(RtpGenericFrameDescriptorExtension::ValueSize(descriptor), - sizeof(kRaw)); + ASSERT_EQ(ValueSize(descriptor), sizeof(kRaw)); uint8_t buffer[sizeof(kRaw)]; - EXPECT_TRUE(RtpGenericFrameDescriptorExtension::Write(buffer, descriptor)); + EXPECT_TRUE(Write(buffer, descriptor)); EXPECT_THAT(buffer, ElementsAreArray(kRaw)); } -TEST(RtpGenericFrameDescriptorExtensionTest, - ParseLongFrameDependenciesAsBigEndian) { +TEST_P(RtpGenericFrameDescriptorExtensionTest, + ParseLongFrameDependenciesAsBigEndian) { constexpr uint16_t kDiff = 0x7654 >> 2; constexpr uint8_t kRaw[] = {0xb8, 0x01, 0x00, 0x00, 0x54 | 0x02, 0x76}; RtpGenericFrameDescriptor descriptor; - ASSERT_TRUE(RtpGenericFrameDescriptorExtension::Parse(kRaw, &descriptor)); + ASSERT_TRUE(Parse(kRaw, &descriptor)); ASSERT_TRUE(descriptor.FirstPacketInSubFrame()); EXPECT_THAT(descriptor.FrameDependenciesDiffs(), ElementsAre(kDiff)); } -TEST(RtpGenericFrameDescriptorExtensionTest, - WriteLongFrameDependenciesAsBigEndian) { +TEST_P(RtpGenericFrameDescriptorExtensionTest, + WriteLongFrameDependenciesAsBigEndian) { constexpr uint16_t kDiff = 0x7654 >> 2; - constexpr uint8_t kRaw[] = {0xb8, 0x01, 0x00, 0x00, 0x54 | 0x02, 0x76}; + uint8_t kRaw[] = {0x88, 0x01, 0x00, 0x00, 0x54 | 0x02, 0x76}; + if (version_ == 0) { + kRaw[0] |= kDeprecatedFlags; + } RtpGenericFrameDescriptor descriptor; descriptor.SetFirstPacketInSubFrame(true); descriptor.AddFrameDependencyDiff(kDiff); - ASSERT_EQ(RtpGenericFrameDescriptorExtension::ValueSize(descriptor), - sizeof(kRaw)); + ASSERT_EQ(ValueSize(descriptor), sizeof(kRaw)); uint8_t buffer[sizeof(kRaw)]; - EXPECT_TRUE(RtpGenericFrameDescriptorExtension::Write(buffer, descriptor)); + EXPECT_TRUE(Write(buffer, descriptor)); EXPECT_THAT(buffer, ElementsAreArray(kRaw)); } -TEST(RtpGenericFrameDescriptorExtensionTest, ParseMaxLongFrameDependencies) { +TEST_P(RtpGenericFrameDescriptorExtensionTest, ParseMaxLongFrameDependencies) { constexpr uint16_t kDiff = 0x3fff; constexpr uint8_t kRaw[] = {0xb8, 0x01, 0x00, 0x00, 0xfe, 0xff}; RtpGenericFrameDescriptor descriptor; - ASSERT_TRUE(RtpGenericFrameDescriptorExtension::Parse(kRaw, &descriptor)); + ASSERT_TRUE(Parse(kRaw, &descriptor)); ASSERT_TRUE(descriptor.FirstPacketInSubFrame()); EXPECT_THAT(descriptor.FrameDependenciesDiffs(), ElementsAre(kDiff)); } -TEST(RtpGenericFrameDescriptorExtensionTest, WriteMaxLongFrameDependencies) { +TEST_P(RtpGenericFrameDescriptorExtensionTest, WriteMaxLongFrameDependencies) { constexpr uint16_t kDiff = 0x3fff; - constexpr uint8_t kRaw[] = {0xb8, 0x01, 0x00, 0x00, 0xfe, 0xff}; + uint8_t kRaw[] = {0x88, 0x01, 0x00, 0x00, 0xfe, 0xff}; + if (version_ == 0) { + kRaw[0] |= kDeprecatedFlags; + } RtpGenericFrameDescriptor descriptor; descriptor.SetFirstPacketInSubFrame(true); descriptor.AddFrameDependencyDiff(kDiff); - ASSERT_EQ(RtpGenericFrameDescriptorExtension::ValueSize(descriptor), - sizeof(kRaw)); + ASSERT_EQ(ValueSize(descriptor), sizeof(kRaw)); uint8_t buffer[sizeof(kRaw)]; - EXPECT_TRUE(RtpGenericFrameDescriptorExtension::Write(buffer, descriptor)); + EXPECT_TRUE(Write(buffer, descriptor)); EXPECT_THAT(buffer, ElementsAreArray(kRaw)); } -TEST(RtpGenericFrameDescriptorExtensionTest, ParseTwoFrameDependencies) { +TEST_P(RtpGenericFrameDescriptorExtensionTest, ParseTwoFrameDependencies) { constexpr uint16_t kDiff1 = 9; constexpr uint16_t kDiff2 = 15; constexpr uint8_t kRaw[] = { 0xb8, 0x01, 0x00, 0x00, (kDiff1 << 2) | 0x01, kDiff2 << 2}; RtpGenericFrameDescriptor descriptor; - ASSERT_TRUE(RtpGenericFrameDescriptorExtension::Parse(kRaw, &descriptor)); + ASSERT_TRUE(Parse(kRaw, &descriptor)); ASSERT_TRUE(descriptor.FirstPacketInSubFrame()); EXPECT_THAT(descriptor.FrameDependenciesDiffs(), ElementsAre(kDiff1, kDiff2)); } -TEST(RtpGenericFrameDescriptorExtensionTest, WriteTwoFrameDependencies) { +TEST_P(RtpGenericFrameDescriptorExtensionTest, WriteTwoFrameDependencies) { constexpr uint16_t kDiff1 = 9; constexpr uint16_t kDiff2 = 15; - constexpr uint8_t kRaw[] = { - 0xb8, 0x01, 0x00, 0x00, (kDiff1 << 2) | 0x01, kDiff2 << 2}; + uint8_t kRaw[] = {0x88, 0x01, 0x00, 0x00, (kDiff1 << 2) | 0x01, kDiff2 << 2}; + if (version_ == 0) { + kRaw[0] |= kDeprecatedFlags; + } RtpGenericFrameDescriptor descriptor; descriptor.SetFirstPacketInSubFrame(true); descriptor.AddFrameDependencyDiff(kDiff1); descriptor.AddFrameDependencyDiff(kDiff2); - ASSERT_EQ(RtpGenericFrameDescriptorExtension::ValueSize(descriptor), - sizeof(kRaw)); + ASSERT_EQ(ValueSize(descriptor), sizeof(kRaw)); uint8_t buffer[sizeof(kRaw)]; - EXPECT_TRUE(RtpGenericFrameDescriptorExtension::Write(buffer, descriptor)); + EXPECT_TRUE(Write(buffer, descriptor)); EXPECT_THAT(buffer, ElementsAreArray(kRaw)); } -TEST(RtpGenericFrameDescriptorExtensionTest, - ParseResolutionOnIndependentFrame) { +TEST_P(RtpGenericFrameDescriptorExtensionTest, + ParseResolutionOnIndependentFrame) { constexpr int kWidth = 0x2468; constexpr int kHeight = 0x6543; constexpr uint8_t kRaw[] = {0xb0, 0x01, 0x00, 0x00, 0x24, 0x68, 0x65, 0x43}; RtpGenericFrameDescriptor descriptor; - ASSERT_TRUE(RtpGenericFrameDescriptorExtension::Parse(kRaw, &descriptor)); + ASSERT_TRUE(Parse(kRaw, &descriptor)); EXPECT_EQ(descriptor.Width(), kWidth); EXPECT_EQ(descriptor.Height(), kHeight); } -TEST(RtpGenericFrameDescriptorExtensionTest, - WriteResolutionOnIndependentFrame) { +TEST_P(RtpGenericFrameDescriptorExtensionTest, + WriteResolutionOnIndependentFrame) { constexpr int kWidth = 0x2468; constexpr int kHeight = 0x6543; - constexpr uint8_t kRaw[] = {0xb0, 0x01, 0x00, 0x00, 0x24, 0x68, 0x65, 0x43}; + uint8_t kRaw[] = {0x80, 0x01, 0x00, 0x00, 0x24, 0x68, 0x65, 0x43}; + if (version_ == 0) { + kRaw[0] |= kDeprecatedFlags; + } RtpGenericFrameDescriptor descriptor; descriptor.SetFirstPacketInSubFrame(true); descriptor.SetResolution(kWidth, kHeight); - ASSERT_EQ(RtpGenericFrameDescriptorExtension::ValueSize(descriptor), - sizeof(kRaw)); + ASSERT_EQ(ValueSize(descriptor), sizeof(kRaw)); uint8_t buffer[sizeof(kRaw)]; - EXPECT_TRUE(RtpGenericFrameDescriptorExtension::Write(buffer, descriptor)); + EXPECT_TRUE(Write(buffer, descriptor)); EXPECT_THAT(buffer, ElementsAreArray(kRaw)); } } // namespace diff --git a/modules/rtp_rtcp/source/rtp_header_extension_map.cc b/modules/rtp_rtcp/source/rtp_header_extension_map.cc index 8e0a484d97..4eec8d3cb8 100644 --- a/modules/rtp_rtcp/source/rtp_header_extension_map.cc +++ b/modules/rtp_rtcp/source/rtp_header_extension_map.cc @@ -42,7 +42,8 @@ constexpr ExtensionInfo kExtensions[] = { CreateExtensionInfo(), CreateExtensionInfo(), CreateExtensionInfo(), - CreateExtensionInfo(), + CreateExtensionInfo(), + CreateExtensionInfo(), CreateExtensionInfo(), }; diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index 73d6413c2d..ce10848138 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -79,8 +79,10 @@ constexpr RtpExtensionSize kVideoExtensionSizes[] = { CreateMaxExtensionSize(), CreateMaxExtensionSize(), CreateMaxExtensionSize(), - {RtpGenericFrameDescriptorExtension::kId, - RtpGenericFrameDescriptorExtension::kMaxSizeBytes}, + {RtpGenericFrameDescriptorExtension00::kId, + RtpGenericFrameDescriptorExtension00::kMaxSizeBytes}, + {RtpGenericFrameDescriptorExtension01::kId, + RtpGenericFrameDescriptorExtension01::kMaxSizeBytes}, }; } // namespace diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 0b881f877a..85566c4178 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -43,7 +43,8 @@ namespace { enum : int { // The first valid value is 1. kAbsoluteSendTimeExtensionId = 1, kAudioLevelExtensionId, - kGenericDescriptorId, + kGenericDescriptorId00, + kGenericDescriptorId01, kMidExtensionId, kRepairedRidExtensionId, kRidExtensionId, @@ -92,8 +93,10 @@ class LoopbackTransportTest : public webrtc::Transport { receivers_extensions_.Register(kRtpExtensionVideoTiming, kVideoTimingExtensionId); receivers_extensions_.Register(kRtpExtensionMid, kMidExtensionId); - receivers_extensions_.Register(kRtpExtensionGenericFrameDescriptor, - kGenericDescriptorId); + receivers_extensions_.Register(kRtpExtensionGenericFrameDescriptor00, + kGenericDescriptorId00); + receivers_extensions_.Register(kRtpExtensionGenericFrameDescriptor01, + kGenericDescriptorId01); receivers_extensions_.Register(kRtpExtensionRtpStreamId, kRidExtensionId); receivers_extensions_.Register(kRtpExtensionRepairedRtpStreamId, kRepairedRidExtensionId); diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index 3adc98aece..cc1c58b8ad 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -97,6 +97,7 @@ void AddRtpHeaderExtensions(const RTPVideoHeader& video_header, RtpGenericFrameDescriptor generic_descriptor; generic_descriptor.SetFirstPacketInSubFrame(first_packet); generic_descriptor.SetLastPacketInSubFrame(last_packet); + generic_descriptor.SetDiscardable(video_header.generic->discardable); if (first_packet) { generic_descriptor.SetFrameId( @@ -121,8 +122,13 @@ void AddRtpHeaderExtensions(const RTPVideoHeader& video_header, video_header.height); } } - packet->SetExtension( - generic_descriptor); + if (!packet->SetExtension( + generic_descriptor) && + !packet->SetExtension( + generic_descriptor)) { + RTC_LOG(LS_ERROR) + << "Could not set RTP extension - Generic Frame Descriptor"; + } } } @@ -539,8 +545,21 @@ bool RTPSenderVideo::SendVideo(FrameType frame_type, RTPVideoHeader minimized_video_header; const RTPVideoHeader* packetize_video_header = video_header; + + rtc::ArrayView generic_descriptor_raw_00 = + first_packet->GetRawExtension(); + rtc::ArrayView generic_descriptor_raw_01 = + first_packet->GetRawExtension(); + + if (!generic_descriptor_raw_00.empty() && + !generic_descriptor_raw_01.empty()) { + RTC_LOG(LS_WARNING) << "Two versions of GFD extension used."; + return false; + } + rtc::ArrayView generic_descriptor_raw = - first_packet->GetRawExtension(); + !generic_descriptor_raw_01.empty() ? generic_descriptor_raw_01 + : generic_descriptor_raw_00; if (!generic_descriptor_raw.empty()) { if (MinimizeDescriptor(*video_header, &minimized_video_header)) { packetize_video_header = &minimized_video_header; diff --git a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc index 0cd0572e30..6dd5353c3d 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc @@ -37,7 +37,8 @@ using ::testing::ElementsAre; enum : int { // The first valid value is 1. kAbsoluteSendTimeExtensionId = 1, kFrameMarkingExtensionId, - kGenericDescriptorId, + kGenericDescriptorId00, + kGenericDescriptorId01, kTransmissionTimeOffsetExtensionId, kTransportSequenceNumberExtensionId, kVideoRotationExtensionId, @@ -65,8 +66,10 @@ class LoopbackTransportTest : public webrtc::Transport { kVideoRotationExtensionId); receivers_extensions_.Register(kRtpExtensionVideoTiming, kVideoTimingExtensionId); - receivers_extensions_.Register(kRtpExtensionGenericFrameDescriptor, - kGenericDescriptorId); + receivers_extensions_.Register(kRtpExtensionGenericFrameDescriptor00, + kGenericDescriptorId00); + receivers_extensions_.Register(kRtpExtensionGenericFrameDescriptor01, + kGenericDescriptorId01); receivers_extensions_.Register(kRtpExtensionFrameMarking, kFrameMarkingExtensionId); } @@ -139,6 +142,11 @@ class RtpSenderVideoTest : public ::testing::TestWithParam { rtp_sender_video_.RegisterPayloadType(kPayload, "generic"); } + void PopulateGenericFrameDescriptor(int version); + + void UsesMinimalVp8DescriptorWhenGenericFrameDescriptorExtensionIsUsed( + int version); + protected: test::ScopedFieldTrials field_trials_; SimulatedClock fake_clock_; @@ -512,11 +520,16 @@ TEST_P(RtpSenderVideoTest, ConditionalRetransmitLimit) { rtp_sender_video_.GetStorageType(header, kSettings, kRttMs)); } -TEST_P(RtpSenderVideoTest, PopulateGenericFrameDescriptor) { +void RtpSenderVideoTest::PopulateGenericFrameDescriptor(int version) { + const RTPExtensionType ext_type = + (version == 0) ? RTPExtensionType::kRtpExtensionGenericFrameDescriptor00 + : RTPExtensionType::kRtpExtensionGenericFrameDescriptor01; + const int ext_id = + (version == 0) ? kGenericDescriptorId00 : kGenericDescriptorId01; + const int64_t kFrameId = 100000; uint8_t kFrame[100]; - EXPECT_EQ(0, rtp_sender_.RegisterRtpHeaderExtension( - kRtpExtensionGenericFrameDescriptor, kGenericDescriptorId)); + EXPECT_EQ(0, rtp_sender_.RegisterRtpHeaderExtension(ext_type, ext_id)); RTPVideoHeader hdr; RTPVideoHeader::GenericDescriptorInfo& generic = hdr.generic.emplace(); @@ -532,9 +545,15 @@ TEST_P(RtpSenderVideoTest, PopulateGenericFrameDescriptor) { RtpGenericFrameDescriptor descriptor_wire; EXPECT_EQ(1, transport_.packets_sent()); - EXPECT_TRUE( - transport_.last_sent_packet() - .GetExtension(&descriptor_wire)); + if (version == 0) { + ASSERT_TRUE(transport_.last_sent_packet() + .GetExtension( + &descriptor_wire)); + } else { + ASSERT_TRUE(transport_.last_sent_packet() + .GetExtension( + &descriptor_wire)); + } EXPECT_EQ(static_cast(generic.frame_id), descriptor_wire.FrameId()); EXPECT_EQ(generic.temporal_index, descriptor_wire.TemporalLayer()); EXPECT_THAT(descriptor_wire.FrameDependenciesDiffs(), ElementsAre(1, 500)); @@ -542,13 +561,28 @@ TEST_P(RtpSenderVideoTest, PopulateGenericFrameDescriptor) { EXPECT_EQ(spatial_bitmask, descriptor_wire.SpatialLayersBitmask()); } -TEST_P(RtpSenderVideoTest, - UsesMinimalVp8DescriptorWhenGenericFrameDescriptorExtensionIsUsed) { +TEST_P(RtpSenderVideoTest, PopulateGenericFrameDescriptor00) { + PopulateGenericFrameDescriptor(0); +} + +TEST_P(RtpSenderVideoTest, PopulateGenericFrameDescriptor01) { + PopulateGenericFrameDescriptor(1); +} + +void RtpSenderVideoTest:: + UsesMinimalVp8DescriptorWhenGenericFrameDescriptorExtensionIsUsed( + int version) { const int64_t kFrameId = 100000; const size_t kFrameSize = 100; uint8_t kFrame[kFrameSize]; - ASSERT_TRUE(rtp_sender_.RegisterRtpHeaderExtension( - RtpGenericFrameDescriptorExtension::kUri, kGenericDescriptorId)); + + if (version == 0) { + ASSERT_TRUE(rtp_sender_.RegisterRtpHeaderExtension( + RtpGenericFrameDescriptorExtension00::kUri, kGenericDescriptorId00)); + } else { + ASSERT_TRUE(rtp_sender_.RegisterRtpHeaderExtension( + RtpGenericFrameDescriptorExtension01::kUri, kGenericDescriptorId01)); + } RTPVideoHeader hdr; hdr.codec = kVideoCodecVP8; @@ -569,6 +603,16 @@ TEST_P(RtpSenderVideoTest, EXPECT_EQ(transport_.last_sent_packet().payload_size(), 1 + kFrameSize); } +TEST_P(RtpSenderVideoTest, + UsesMinimalVp8DescriptorWhenGenericFrameDescriptorExtensionIsUsed00) { + UsesMinimalVp8DescriptorWhenGenericFrameDescriptorExtensionIsUsed(0); +} + +TEST_P(RtpSenderVideoTest, + UsesMinimalVp8DescriptorWhenGenericFrameDescriptorExtensionIsUsed01) { + UsesMinimalVp8DescriptorWhenGenericFrameDescriptorExtensionIsUsed(1); +} + INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead, RtpSenderVideoTest, ::testing::Bool()); diff --git a/modules/rtp_rtcp/source/rtp_utility.cc b/modules/rtp_rtcp/source/rtp_utility.cc index 9db5ed914a..12038237b7 100644 --- a/modules/rtp_rtcp/source/rtp_utility.cc +++ b/modules/rtp_rtcp/source/rtp_utility.cc @@ -502,7 +502,8 @@ void RtpHeaderParser::ParseOneByteExtensionHeader( header->extension.mid.Set(rtc::MakeArrayView(ptr, len + 1)); break; } - case kRtpExtensionGenericFrameDescriptor: + case kRtpExtensionGenericFrameDescriptor00: + case kRtpExtensionGenericFrameDescriptor01: RTC_LOG(WARNING) << "RtpGenericFrameDescriptor unsupported by rtp header parser."; break; diff --git a/modules/rtp_rtcp/source/rtp_video_header.h b/modules/rtp_rtcp/source/rtp_video_header.h index 49d8c28ed2..417c38c655 100644 --- a/modules/rtp_rtcp/source/rtp_video_header.h +++ b/modules/rtp_rtcp/source/rtp_video_header.h @@ -43,6 +43,7 @@ struct RTPVideoHeader { int temporal_index = 0; absl::InlinedVector dependencies; absl::InlinedVector higher_spatial_layers; + bool discardable = false; }; RTPVideoHeader(); diff --git a/test/call_test.cc b/test/call_test.cc index ca0d7bbec3..b8334b548f 100644 --- a/test/call_test.cc +++ b/test/call_test.cc @@ -262,7 +262,9 @@ void CallTest::CreateVideoSendConfig(VideoSendStream::Config* video_config, &video_config->rtp.extensions); AddRtpExtensionByUri(RtpExtension::kVideoContentTypeUri, &video_config->rtp.extensions); - AddRtpExtensionByUri(RtpExtension::kGenericFrameDescriptorUri, + AddRtpExtensionByUri(RtpExtension::kGenericFrameDescriptorUri00, + &video_config->rtp.extensions); + AddRtpExtensionByUri(RtpExtension::kGenericFrameDescriptorUri01, &video_config->rtp.extensions); if (video_encoder_configs_.empty()) { video_encoder_configs_.emplace_back(); diff --git a/test/fuzzers/rtp_packet_fuzzer.cc b/test/fuzzers/rtp_packet_fuzzer.cc index f774c0cb8b..c65809630d 100644 --- a/test/fuzzers/rtp_packet_fuzzer.cc +++ b/test/fuzzers/rtp_packet_fuzzer.cc @@ -116,9 +116,14 @@ void FuzzOneInput(const uint8_t* data, size_t size) { packet.GetExtension(&mid); break; } - case kRtpExtensionGenericFrameDescriptor: { + case kRtpExtensionGenericFrameDescriptor00: { RtpGenericFrameDescriptor descriptor; - packet.GetExtension(&descriptor); + packet.GetExtension(&descriptor); + break; + } + case kRtpExtensionGenericFrameDescriptor01: { + RtpGenericFrameDescriptor descriptor; + packet.GetExtension(&descriptor); break; } case kRtpExtensionColorSpace: { diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index 656dd7a990..aec6ecdb83 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -531,17 +531,31 @@ void RtpVideoStreamReceiver::ReceivePacket(const RtpPacketReceived& packet) { absl::optional generic_descriptor_wire; generic_descriptor_wire.emplace(); - if (packet.GetExtension( - &generic_descriptor_wire.value())) { - generic_descriptor_wire->SetByteRepresentation( - packet.GetRawExtension()); + const bool generic_descriptor_v00 = + packet.GetExtension( + &generic_descriptor_wire.value()); + const bool generic_descriptor_v01 = + packet.GetExtension( + &generic_descriptor_wire.value()); + if (generic_descriptor_v00 && generic_descriptor_v01) { + RTC_LOG(LS_WARNING) << "RTP packet had two different GFD versions."; + return; + } + + if (generic_descriptor_v00 || generic_descriptor_v01) { + if (generic_descriptor_v00) { + generic_descriptor_wire->SetByteRepresentation( + packet.GetRawExtension()); + } else { + generic_descriptor_wire->SetByteRepresentation( + packet.GetRawExtension()); + } + webrtc_rtp_header.video_header().is_first_packet_in_frame = - generic_descriptor_wire->FirstSubFrameInFrame() && generic_descriptor_wire->FirstPacketInSubFrame(); webrtc_rtp_header.video_header().is_last_packet_in_frame = webrtc_rtp_header.header.markerBit || - (generic_descriptor_wire->LastSubFrameInFrame() && - generic_descriptor_wire->LastPacketInSubFrame()); + generic_descriptor_wire->LastPacketInSubFrame(); if (generic_descriptor_wire->FirstPacketInSubFrame()) { webrtc_rtp_header.frameType = diff --git a/video/rtp_video_stream_receiver_unittest.cc b/video/rtp_video_stream_receiver_unittest.cc index d80e55147c..c6fb13d2d0 100644 --- a/video/rtp_video_stream_receiver_unittest.cc +++ b/video/rtp_video_stream_receiver_unittest.cc @@ -32,6 +32,7 @@ using ::testing::_; using ::testing::Invoke; +using ::testing::Values; namespace webrtc { @@ -284,10 +285,9 @@ class RtpVideoStreamReceiverTestH264 RtpVideoStreamReceiverTestH264() : RtpVideoStreamReceiverTest(GetParam()) {} }; -INSTANTIATE_TEST_SUITE_P( - SpsPpsIdrIsKeyframe, - RtpVideoStreamReceiverTestH264, - ::testing::Values("", "WebRTC-SpsPpsIdrIsH264Keyframe/Enabled/")); +INSTANTIATE_TEST_SUITE_P(SpsPpsIdrIsKeyframe, + RtpVideoStreamReceiverTestH264, + Values("", "WebRTC-SpsPpsIdrIsH264Keyframe/Enabled/")); TEST_P(RtpVideoStreamReceiverTestH264, InBandSpsPps) { std::vector sps_data; @@ -500,7 +500,51 @@ TEST_F(RtpVideoStreamReceiverTest, rtp_video_stream_receiver_->RemoveSecondarySink(&secondary_sink); } -TEST_F(RtpVideoStreamReceiverTest, ParseGenericDescriptorOnePacket) { +class RtpVideoStreamReceiverGenericDescriptorTest + : public RtpVideoStreamReceiverTest, + public ::testing::WithParamInterface { + public: + void RegisterRtpGenericFrameDescriptorExtension( + RtpHeaderExtensionMap* extension_map, + int version) { + constexpr int kId00 = 5; + constexpr int kId01 = 6; + switch (version) { + case 0: + extension_map->Register(kId00); + return; + case 1: + extension_map->Register(kId01); + return; + } + RTC_NOTREACHED(); + } + + bool SetExtensionRtpGenericFrameDescriptorExtension( + const RtpGenericFrameDescriptor& generic_descriptor, + RtpPacketReceived* rtp_packet, + int version) { + switch (version) { + case 0: + return rtp_packet->SetExtension( + generic_descriptor); + case 1: + return rtp_packet->SetExtension( + generic_descriptor); + } + RTC_NOTREACHED(); + return false; + } +}; + +INSTANTIATE_TEST_SUITE_P(, + RtpVideoStreamReceiverGenericDescriptorTest, + Values(0, 1)); + +TEST_P(RtpVideoStreamReceiverGenericDescriptorTest, + ParseGenericDescriptorOnePacket) { + const int version = GetParam(); + const std::vector data = {0, 1, 2, 3, 4}; const int kPayloadType = 123; const int kSpatialIndex = 1; @@ -511,7 +555,7 @@ TEST_F(RtpVideoStreamReceiverTest, ParseGenericDescriptorOnePacket) { rtp_video_stream_receiver_->StartReceive(); RtpHeaderExtensionMap extension_map; - extension_map.Register(5); + RegisterRtpGenericFrameDescriptorExtension(&extension_map, version); RtpPacketReceived rtp_packet(&extension_map); RtpGenericFrameDescriptor generic_descriptor; @@ -521,8 +565,8 @@ TEST_F(RtpVideoStreamReceiverTest, ParseGenericDescriptorOnePacket) { generic_descriptor.SetSpatialLayersBitmask(1 << kSpatialIndex); generic_descriptor.AddFrameDependencyDiff(90); generic_descriptor.AddFrameDependencyDiff(80); - EXPECT_TRUE(rtp_packet.SetExtension( - generic_descriptor)); + ASSERT_TRUE(SetExtensionRtpGenericFrameDescriptorExtension( + generic_descriptor, &rtp_packet, version)); uint8_t* payload = rtp_packet.SetPayloadSize(data.size()); memcpy(payload, data.data(), data.size()); @@ -545,7 +589,10 @@ TEST_F(RtpVideoStreamReceiverTest, ParseGenericDescriptorOnePacket) { rtp_video_stream_receiver_->OnRtpPacket(rtp_packet); } -TEST_F(RtpVideoStreamReceiverTest, ParseGenericDescriptorTwoPackets) { +TEST_P(RtpVideoStreamReceiverGenericDescriptorTest, + ParseGenericDescriptorTwoPackets) { + const int version = GetParam(); + const std::vector data = {0, 1, 2, 3, 4}; const int kPayloadType = 123; const int kSpatialIndex = 1; @@ -556,7 +603,7 @@ TEST_F(RtpVideoStreamReceiverTest, ParseGenericDescriptorTwoPackets) { rtp_video_stream_receiver_->StartReceive(); RtpHeaderExtensionMap extension_map; - extension_map.Register(5); + RegisterRtpGenericFrameDescriptorExtension(&extension_map, version); RtpPacketReceived first_packet(&extension_map); RtpGenericFrameDescriptor first_packet_descriptor; @@ -565,8 +612,8 @@ TEST_F(RtpVideoStreamReceiverTest, ParseGenericDescriptorTwoPackets) { first_packet_descriptor.SetFrameId(100); first_packet_descriptor.SetSpatialLayersBitmask(1 << kSpatialIndex); first_packet_descriptor.SetResolution(480, 360); - EXPECT_TRUE(first_packet.SetExtension( - first_packet_descriptor)); + ASSERT_TRUE(SetExtensionRtpGenericFrameDescriptorExtension( + first_packet_descriptor, &first_packet, version)); uint8_t* first_packet_payload = first_packet.SetPayloadSize(data.size()); memcpy(first_packet_payload, data.data(), data.size()); @@ -582,8 +629,8 @@ TEST_F(RtpVideoStreamReceiverTest, ParseGenericDescriptorTwoPackets) { RtpGenericFrameDescriptor second_packet_descriptor; second_packet_descriptor.SetFirstPacketInSubFrame(false); second_packet_descriptor.SetLastPacketInSubFrame(true); - EXPECT_TRUE(second_packet.SetExtension( - second_packet_descriptor)); + ASSERT_TRUE(SetExtensionRtpGenericFrameDescriptorExtension( + second_packet_descriptor, &second_packet, version)); second_packet.SetMarker(true); second_packet.SetPayloadType(kPayloadType); @@ -606,6 +653,45 @@ TEST_F(RtpVideoStreamReceiverTest, ParseGenericDescriptorTwoPackets) { rtp_video_stream_receiver_->OnRtpPacket(second_packet); } +TEST_F(RtpVideoStreamReceiverGenericDescriptorTest, + DropPacketsWithMultipleVersionsOfExtension) { + const std::vector data = {0, 1, 2, 3, 4}; + const int kPayloadType = 123; + + VideoCodec codec; + codec.plType = kPayloadType; + rtp_video_stream_receiver_->AddReceiveCodec(codec, {}); + rtp_video_stream_receiver_->StartReceive(); + + RtpHeaderExtensionMap extension_map; + RegisterRtpGenericFrameDescriptorExtension(&extension_map, 0); + RegisterRtpGenericFrameDescriptorExtension(&extension_map, 1); + RtpPacketReceived rtp_packet(&extension_map); + + RtpGenericFrameDescriptor generic_descriptors[2]; + for (size_t i = 0; i < 2; ++i) { + generic_descriptors[i].SetFirstPacketInSubFrame(true); + generic_descriptors[i].SetLastPacketInSubFrame(true); + generic_descriptors[i].SetFrameId(100); + ASSERT_TRUE(SetExtensionRtpGenericFrameDescriptorExtension( + generic_descriptors[i], &rtp_packet, i)); + } + + uint8_t* payload = rtp_packet.SetPayloadSize(data.size()); + memcpy(payload, data.data(), data.size()); + // The first byte is the header, so we ignore the first byte of |data|. + mock_on_complete_frame_callback_.AppendExpectedBitstream(data.data() + 1, + data.size() - 1); + + rtp_packet.SetMarker(true); + rtp_packet.SetPayloadType(kPayloadType); + rtp_packet.SetSequenceNumber(1); + + EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame).Times(0); + + rtp_video_stream_receiver_->OnRtpPacket(rtp_packet); +} + #if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) TEST_F(RtpVideoStreamReceiverTest, RepeatedSecondarySinkDisallowed) { MockRtpPacketSink secondary_sink; diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index 0ac5cc2233..3627918b94 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -49,7 +49,8 @@ namespace webrtc { namespace { enum : int { // The first valid value is 1. kAbsSendTimeExtensionId = 1, - kGenericFrameDescriptorExtensionId, + kGenericFrameDescriptorExtensionId00, + kGenericFrameDescriptorExtensionId01, kTransportSequenceNumberExtensionId, kVideoContentTypeExtensionId, kVideoTimingExtensionId, @@ -732,8 +733,11 @@ void VideoQualityTest::SetupVideo(Transport* send_transport, RTC_CHECK(field_trial::IsEnabled("WebRTC-GenericDescriptor")); video_send_configs_[video_idx].rtp.extensions.emplace_back( - RtpExtension::kGenericFrameDescriptorUri, - kGenericFrameDescriptorExtensionId); + RtpExtension::kGenericFrameDescriptorUri00, + kGenericFrameDescriptorExtensionId00); + video_send_configs_[video_idx].rtp.extensions.emplace_back( + RtpExtension::kGenericFrameDescriptorUri01, + kGenericFrameDescriptorExtensionId01); } video_send_configs_[video_idx].rtp.extensions.emplace_back(