diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h index 03dcda3c3c..82328980e7 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h +++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h @@ -55,7 +55,6 @@ enum RTPExtensionType : int { kRtpExtensionAbsoluteSendTime, kRtpExtensionVideoRotation, kRtpExtensionTransportSequenceNumber, - kRtpExtensionTransportSequenceNumber02, kRtpExtensionPlayoutDelay, kRtpExtensionVideoContentType, kRtpExtensionVideoTiming, diff --git a/modules/rtp_rtcp/source/rtp_header_extension_map.cc b/modules/rtp_rtcp/source/rtp_header_extension_map.cc index b4aaf3ff58..4eec8d3cb8 100644 --- a/modules/rtp_rtcp/source/rtp_header_extension_map.cc +++ b/modules/rtp_rtcp/source/rtp_header_extension_map.cc @@ -35,7 +35,6 @@ constexpr ExtensionInfo kExtensions[] = { CreateExtensionInfo(), CreateExtensionInfo(), CreateExtensionInfo(), - CreateExtensionInfo(), CreateExtensionInfo(), CreateExtensionInfo(), CreateExtensionInfo(), diff --git a/modules/rtp_rtcp/source/rtp_header_extensions.cc b/modules/rtp_rtcp/source/rtp_header_extensions.cc index e531a886c6..38c2d5ac7d 100644 --- a/modules/rtp_rtcp/source/rtp_header_extensions.cc +++ b/modules/rtp_rtcp/source/rtp_header_extensions.cc @@ -126,93 +126,27 @@ bool TransmissionOffset::Write(rtc::ArrayView data, int32_t rtp_time) { return true; } -// TransportSequenceNumber -// // 0 1 2 // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ -// | ID | L=1 |transport-wide sequence number | +// | ID | L=1 |transport wide sequence number | // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ constexpr RTPExtensionType TransportSequenceNumber::kId; constexpr uint8_t TransportSequenceNumber::kValueSizeBytes; constexpr const char TransportSequenceNumber::kUri[]; bool TransportSequenceNumber::Parse(rtc::ArrayView data, - uint16_t* transport_sequence_number) { - if (data.size() != kValueSizeBytes) + uint16_t* value) { + if (data.size() != 2) return false; - *transport_sequence_number = ByteReader::ReadBigEndian(data.data()); + *value = ByteReader::ReadBigEndian(data.data()); return true; } bool TransportSequenceNumber::Write(rtc::ArrayView data, - uint16_t transport_sequence_number) { - RTC_DCHECK_EQ(data.size(), ValueSize(transport_sequence_number)); - ByteWriter::WriteBigEndian(data.data(), transport_sequence_number); - return true; -} - -// TransportSequenceNumberV2 -// -// In addition to the format used for TransportSequencNumber, V2 also supports -// the following packet format where two extra bytes are used to specify that -// the sender requests immediate feedback. -// 0 1 2 3 -// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 -// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ -// | ID | L=3 |transport-wide sequence number |T| seq count | -// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ -// |seq count cont.| -// +-+-+-+-+-+-+-+-+ -// -// The bit |T| determines whether the feedback should include timing information -// or not and |seq_count| determines how many additional packets the feedback -// packet should cover. -constexpr RTPExtensionType TransportSequenceNumberV2::kId; -constexpr uint8_t TransportSequenceNumberV2::kValueSizeBytesWithFeedbackRequest; -constexpr const char TransportSequenceNumberV2::kUri[]; -constexpr uint16_t TransportSequenceNumberV2::kIncludeTimestampsBit; - -bool TransportSequenceNumberV2::Parse( - rtc::ArrayView data, - uint16_t* transport_sequence_number, - absl::optional* feedback_request) { - if (data.size() != TransportSequenceNumber::kValueSizeBytes && - data.size() != kValueSizeBytesWithFeedbackRequest) - return false; - - *transport_sequence_number = ByteReader::ReadBigEndian(data.data()); - - if (data.size() == kValueSizeBytesWithFeedbackRequest) { - uint16_t feedback_request_raw = - ByteReader::ReadBigEndian(data.data() + 2); - bool include_timestamps = - (feedback_request_raw & kIncludeTimestampsBit) != 0; - uint16_t sequence_count = feedback_request_raw & ~kIncludeTimestampsBit; - *feedback_request = {include_timestamps, sequence_count}; - } else { - *feedback_request = absl::nullopt; - } - return true; -} - -bool TransportSequenceNumberV2::Write( - rtc::ArrayView data, - uint16_t transport_sequence_number, - const absl::optional& feedback_request) { - RTC_DCHECK_EQ(data.size(), - ValueSize(transport_sequence_number, feedback_request)); - - ByteWriter::WriteBigEndian(data.data(), transport_sequence_number); - - if (feedback_request) { - RTC_DCHECK_GE(feedback_request->sequence_count, 0); - RTC_DCHECK_LT(feedback_request->sequence_count, kIncludeTimestampsBit); - uint16_t feedback_request_raw = - feedback_request->sequence_count | - (feedback_request->include_timestamps ? kIncludeTimestampsBit : 0); - ByteWriter::WriteBigEndian(data.data() + 2, feedback_request_raw); - } + uint16_t value) { + RTC_DCHECK_EQ(data.size(), 2); + ByteWriter::WriteBigEndian(data.data(), value); return true; } diff --git a/modules/rtp_rtcp/source/rtp_header_extensions.h b/modules/rtp_rtcp/source/rtp_header_extensions.h index e37388a3cd..9f4a28b701 100644 --- a/modules/rtp_rtcp/source/rtp_header_extensions.h +++ b/modules/rtp_rtcp/source/rtp_header_extensions.h @@ -81,38 +81,9 @@ class TransportSequenceNumber { static constexpr const char kUri[] = "http://www.ietf.org/id/" "draft-holmer-rmcat-transport-wide-cc-extensions-01"; - static bool Parse(rtc::ArrayView data, - uint16_t* transport_sequence_number); - static size_t ValueSize(uint16_t /*transport_sequence_number*/) { - return kValueSizeBytes; - } - static bool Write(rtc::ArrayView data, - uint16_t transport_sequence_number); -}; - -class TransportSequenceNumberV2 { - public: - static constexpr RTPExtensionType kId = - kRtpExtensionTransportSequenceNumber02; - static constexpr uint8_t kValueSizeBytesWithFeedbackRequest = 4; - static constexpr const char kUri[] = - "http://www.ietf.org/id/" - "draft-holmer-rmcat-transport-wide-cc-extensions-02"; - static bool Parse(rtc::ArrayView data, - uint16_t* transport_sequence_number, - absl::optional* feedback_request); - static size_t ValueSize( - uint16_t /*transport_sequence_number*/, - const absl::optional& feedback_request) { - return feedback_request ? kValueSizeBytesWithFeedbackRequest - : TransportSequenceNumber::kValueSizeBytes; - } - static bool Write(rtc::ArrayView data, - uint16_t transport_sequence_number, - const absl::optional& feedback_request); - - private: - static constexpr uint16_t kIncludeTimestampsBit = 1 << 15; + static bool Parse(rtc::ArrayView data, uint16_t* value); + static size_t ValueSize(uint16_t value) { return kValueSizeBytes; } + static bool Write(rtc::ArrayView data, uint16_t value); }; class VideoOrientation { diff --git a/modules/rtp_rtcp/source/rtp_packet_received.cc b/modules/rtp_rtcp/source/rtp_packet_received.cc index 2a36b7bd1b..f80fad68e0 100644 --- a/modules/rtp_rtcp/source/rtp_packet_received.cc +++ b/modules/rtp_rtcp/source/rtp_packet_received.cc @@ -52,9 +52,6 @@ void RtpPacketReceived::GetHeader(RTPHeader* header) const { header->extension.hasAbsoluteSendTime = GetExtension(&header->extension.absoluteSendTime); header->extension.hasTransportSequenceNumber = - GetExtension( - &header->extension.transportSequenceNumber, - &header->extension.feedback_request) || GetExtension( &header->extension.transportSequenceNumber); header->extension.hasAudioLevel = GetExtension( diff --git a/modules/rtp_rtcp/source/rtp_packet_unittest.cc b/modules/rtp_rtcp/source/rtp_packet_unittest.cc index 4700a776f5..0d24272f5f 100644 --- a/modules/rtp_rtcp/source/rtp_packet_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_packet_unittest.cc @@ -827,62 +827,4 @@ TEST(RtpPacketTest, CreateAndParseColorSpaceExtensionWithoutHdrMetadata) { TestCreateAndParseColorSpaceExtension(/*with_hdr_metadata=*/false); } -TEST(RtpPacketTest, CreateAndParseTransportSequenceNumber) { - // Create a packet with transport sequence number extension populated. - RtpPacketToSend::ExtensionManager extensions; - constexpr int kExtensionId = 1; - extensions.Register(kExtensionId); - RtpPacketToSend send_packet(&extensions); - send_packet.SetPayloadType(kPayloadType); - send_packet.SetSequenceNumber(kSeqNum); - send_packet.SetTimestamp(kTimestamp); - send_packet.SetSsrc(kSsrc); - - constexpr int kTransportSequenceNumber = 12345; - send_packet.SetExtension(kTransportSequenceNumber); - - // Serialize the packet and then parse it again. - RtpPacketReceived receive_packet(&extensions); - EXPECT_TRUE(receive_packet.Parse(send_packet.Buffer())); - - uint16_t received_transport_sequeunce_number; - EXPECT_TRUE(receive_packet.GetExtension( - &received_transport_sequeunce_number)); - EXPECT_EQ(received_transport_sequeunce_number, kTransportSequenceNumber); -} - -TEST(RtpPacketTest, CreateAndParseTransportSequenceNumberWithFeedbackRequest) { - // Create a packet with TransportSequenceNumberV2 extension populated. - RtpPacketToSend::ExtensionManager extensions; - constexpr int kExtensionId = 1; - extensions.Register(kExtensionId); - RtpPacketToSend send_packet(&extensions); - send_packet.SetPayloadType(kPayloadType); - send_packet.SetSequenceNumber(kSeqNum); - send_packet.SetTimestamp(kTimestamp); - send_packet.SetSsrc(kSsrc); - - constexpr int kTransportSequenceNumber = 12345; - constexpr absl::optional kFeedbackRequest = - FeedbackRequest{/*include_timestamps=*/true, /*sequence_count=*/3}; - send_packet.SetExtension(kTransportSequenceNumber, - kFeedbackRequest); - - // Serialize the packet and then parse it again. - RtpPacketReceived receive_packet(&extensions); - EXPECT_TRUE(receive_packet.Parse(send_packet.Buffer())); - - // Parse transport sequence number and feedback request. - uint16_t received_transport_sequeunce_number; - absl::optional received_feedback_request; - EXPECT_TRUE(receive_packet.GetExtension( - &received_transport_sequeunce_number, &received_feedback_request)); - EXPECT_EQ(received_transport_sequeunce_number, kTransportSequenceNumber); - ASSERT_TRUE(received_feedback_request); - EXPECT_EQ(received_feedback_request->include_timestamps, - kFeedbackRequest->include_timestamps); - EXPECT_EQ(received_feedback_request->sequence_count, - kFeedbackRequest->sequence_count); -} - } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_utility.cc b/modules/rtp_rtcp/source/rtp_utility.cc index 8811bf1d21..12038237b7 100644 --- a/modules/rtp_rtcp/source/rtp_utility.cc +++ b/modules/rtp_rtcp/source/rtp_utility.cc @@ -433,10 +433,6 @@ void RtpHeaderParser::ParseOneByteExtensionHeader( header->extension.hasTransportSequenceNumber = true; break; } - case kRtpExtensionTransportSequenceNumber02: - RTC_LOG(WARNING) << "TransportSequenceNumberV2 unsupported by rtp " - "header parser."; - break; case kRtpExtensionPlayoutDelay: { if (len != 2) { RTC_LOG(LS_WARNING) << "Incorrect playout delay len: " << len; diff --git a/test/fuzzers/BUILD.gn b/test/fuzzers/BUILD.gn index 6e723e1548..797c27d917 100644 --- a/test/fuzzers/BUILD.gn +++ b/test/fuzzers/BUILD.gn @@ -230,7 +230,6 @@ webrtc_fuzzer_test("rtp_packet_fuzzer") { ] deps = [ "../../modules/rtp_rtcp:rtp_rtcp_format", - "//third_party/abseil-cpp/absl/types:optional", ] seed_corpus = "corpora/rtp-corpus" } diff --git a/test/fuzzers/rtp_packet_fuzzer.cc b/test/fuzzers/rtp_packet_fuzzer.cc index 08865cb4f1..c65809630d 100644 --- a/test/fuzzers/rtp_packet_fuzzer.cc +++ b/test/fuzzers/rtp_packet_fuzzer.cc @@ -10,7 +10,6 @@ #include -#include "absl/types/optional.h" #include "modules/rtp_rtcp/include/rtp_header_extension_map.h" #include "modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension.h" #include "modules/rtp_rtcp/source/rtp_header_extensions.h" @@ -86,13 +85,6 @@ void FuzzOneInput(const uint8_t* data, size_t size) { uint16_t seqnum; packet.GetExtension(&seqnum); break; - case kRtpExtensionTransportSequenceNumber02: { - uint16_t seqnum; - absl::optional feedback_request; - packet.GetExtension(&seqnum, - &feedback_request); - break; - } case kRtpExtensionPlayoutDelay: PlayoutDelay playout; packet.GetExtension(&playout);