Reland "Extend TransportSequenceNumber RTP header extension"
This reverts commit 109b5fb5f5b2f46e1798c91c4a024ce26f57f0b0.
Reason for revert: The failing libfuzzer was fixed in commit d6c6f16063b81fc60206618ba06198e34ee0eacb
Original change's description:
> Revert "Extend TransportSequenceNumber RTP header extension"
>
> This reverts commit 28c7362bc485d22bdc8c744bc725022780187a96.
>
> Reason for revert: It breaks Linux64 Release (libfuzzer):
> https://logs.chromium.org/logs/webrtc/buildbucket/cr-buildbucket.appspot.com/8921003137877469920/+/steps/compile/0/stdout
>
> Original change's description:
> > Extend TransportSequenceNumber RTP header extension
> >
> > Extend TransportSequenceNumber RTP header extension to support
> > feedback on sender request.
> >
> > Bug: webrtc:10262
> > Change-Id: Ibc1cf18162d15cd102e951c9dc697d8ea536ebb6
> > Reviewed-on: https://webrtc-review.googlesource.com/c/123233
> > Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
> > Reviewed-by: Alex Loiko <aleloi@webrtc.org>
> > Commit-Queue: Johannes Kron <kron@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#26766}
>
> TBR=danilchap@webrtc.org,aleloi@webrtc.org,kron@webrtc.org
>
> Change-Id: Ie8a73f5fdffd99919ceaa1ae8911a1645f2077e9
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: webrtc:10262
> Reviewed-on: https://webrtc-review.googlesource.com/c/123522
> Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#26767}
TBR=danilchap@webrtc.org,mbonadei@webrtc.org,aleloi@webrtc.org,kron@webrtc.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: webrtc:10262
Change-Id: I0f854299a46c042cfbdf8b8cc8cd965a228142c8
Reviewed-on: https://webrtc-review.googlesource.com/c/123764
Reviewed-by: Johannes Kron <kron@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Alex Loiko <aleloi@webrtc.org>
Commit-Queue: Johannes Kron <kron@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26798}diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h
index 8232898..03dcda3 100644
--- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h
+++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h
@@ -55,6 +55,7 @@
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 4eec8d3..b4aaf3f 100644
--- a/modules/rtp_rtcp/source/rtp_header_extension_map.cc
+++ b/modules/rtp_rtcp/source/rtp_header_extension_map.cc
@@ -35,6 +35,7 @@
CreateExtensionInfo<AbsoluteSendTime>(),
CreateExtensionInfo<VideoOrientation>(),
CreateExtensionInfo<TransportSequenceNumber>(),
+ CreateExtensionInfo<TransportSequenceNumberV2>(),
CreateExtensionInfo<PlayoutDelayLimits>(),
CreateExtensionInfo<VideoContentTypeExtension>(),
CreateExtensionInfo<VideoTimingExtension>(),
diff --git a/modules/rtp_rtcp/source/rtp_header_extensions.cc b/modules/rtp_rtcp/source/rtp_header_extensions.cc
index 38c2d5a..e531a88 100644
--- a/modules/rtp_rtcp/source/rtp_header_extensions.cc
+++ b/modules/rtp_rtcp/source/rtp_header_extensions.cc
@@ -126,27 +126,93 @@
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<const uint8_t> data,
- uint16_t* value) {
- if (data.size() != 2)
+ uint16_t* transport_sequence_number) {
+ if (data.size() != kValueSizeBytes)
return false;
- *value = ByteReader<uint16_t>::ReadBigEndian(data.data());
+ *transport_sequence_number = ByteReader<uint16_t>::ReadBigEndian(data.data());
return true;
}
bool TransportSequenceNumber::Write(rtc::ArrayView<uint8_t> data,
- uint16_t value) {
- RTC_DCHECK_EQ(data.size(), 2);
- ByteWriter<uint16_t>::WriteBigEndian(data.data(), value);
+ uint16_t transport_sequence_number) {
+ RTC_DCHECK_EQ(data.size(), ValueSize(transport_sequence_number));
+ ByteWriter<uint16_t>::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<const uint8_t> data,
+ uint16_t* transport_sequence_number,
+ absl::optional<FeedbackRequest>* feedback_request) {
+ if (data.size() != TransportSequenceNumber::kValueSizeBytes &&
+ data.size() != kValueSizeBytesWithFeedbackRequest)
+ return false;
+
+ *transport_sequence_number = ByteReader<uint16_t>::ReadBigEndian(data.data());
+
+ if (data.size() == kValueSizeBytesWithFeedbackRequest) {
+ uint16_t feedback_request_raw =
+ ByteReader<uint16_t>::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<uint8_t> data,
+ uint16_t transport_sequence_number,
+ const absl::optional<FeedbackRequest>& feedback_request) {
+ RTC_DCHECK_EQ(data.size(),
+ ValueSize(transport_sequence_number, feedback_request));
+
+ ByteWriter<uint16_t>::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<uint16_t>::WriteBigEndian(data.data() + 2, feedback_request_raw);
+ }
return true;
}
diff --git a/modules/rtp_rtcp/source/rtp_header_extensions.h b/modules/rtp_rtcp/source/rtp_header_extensions.h
index 9f4a28b..e37388a 100644
--- a/modules/rtp_rtcp/source/rtp_header_extensions.h
+++ b/modules/rtp_rtcp/source/rtp_header_extensions.h
@@ -81,9 +81,38 @@
static constexpr const char kUri[] =
"http://www.ietf.org/id/"
"draft-holmer-rmcat-transport-wide-cc-extensions-01";
- static bool Parse(rtc::ArrayView<const uint8_t> data, uint16_t* value);
- static size_t ValueSize(uint16_t value) { return kValueSizeBytes; }
- static bool Write(rtc::ArrayView<uint8_t> data, uint16_t value);
+ static bool Parse(rtc::ArrayView<const uint8_t> data,
+ uint16_t* transport_sequence_number);
+ static size_t ValueSize(uint16_t /*transport_sequence_number*/) {
+ return kValueSizeBytes;
+ }
+ static bool Write(rtc::ArrayView<uint8_t> 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<const uint8_t> data,
+ uint16_t* transport_sequence_number,
+ absl::optional<FeedbackRequest>* feedback_request);
+ static size_t ValueSize(
+ uint16_t /*transport_sequence_number*/,
+ const absl::optional<FeedbackRequest>& feedback_request) {
+ return feedback_request ? kValueSizeBytesWithFeedbackRequest
+ : TransportSequenceNumber::kValueSizeBytes;
+ }
+ static bool Write(rtc::ArrayView<uint8_t> data,
+ uint16_t transport_sequence_number,
+ const absl::optional<FeedbackRequest>& feedback_request);
+
+ private:
+ static constexpr uint16_t kIncludeTimestampsBit = 1 << 15;
};
class VideoOrientation {
diff --git a/modules/rtp_rtcp/source/rtp_packet_received.cc b/modules/rtp_rtcp/source/rtp_packet_received.cc
index f80fad6..2a36b7b 100644
--- a/modules/rtp_rtcp/source/rtp_packet_received.cc
+++ b/modules/rtp_rtcp/source/rtp_packet_received.cc
@@ -52,6 +52,9 @@
header->extension.hasAbsoluteSendTime =
GetExtension<AbsoluteSendTime>(&header->extension.absoluteSendTime);
header->extension.hasTransportSequenceNumber =
+ GetExtension<TransportSequenceNumberV2>(
+ &header->extension.transportSequenceNumber,
+ &header->extension.feedback_request) ||
GetExtension<TransportSequenceNumber>(
&header->extension.transportSequenceNumber);
header->extension.hasAudioLevel = GetExtension<AudioLevel>(
diff --git a/modules/rtp_rtcp/source/rtp_packet_unittest.cc b/modules/rtp_rtcp/source/rtp_packet_unittest.cc
index 0d24272..4700a77 100644
--- a/modules/rtp_rtcp/source/rtp_packet_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_packet_unittest.cc
@@ -827,4 +827,62 @@
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<TransportSequenceNumber>(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<TransportSequenceNumber>(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<TransportSequenceNumber>(
+ &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<TransportSequenceNumberV2>(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<FeedbackRequest> kFeedbackRequest =
+ FeedbackRequest{/*include_timestamps=*/true, /*sequence_count=*/3};
+ send_packet.SetExtension<TransportSequenceNumberV2>(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<FeedbackRequest> received_feedback_request;
+ EXPECT_TRUE(receive_packet.GetExtension<TransportSequenceNumberV2>(
+ &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 1203823..8811bf1 100644
--- a/modules/rtp_rtcp/source/rtp_utility.cc
+++ b/modules/rtp_rtcp/source/rtp_utility.cc
@@ -433,6 +433,10 @@
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 797c27d..6e723e1 100644
--- a/test/fuzzers/BUILD.gn
+++ b/test/fuzzers/BUILD.gn
@@ -230,6 +230,7 @@
]
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 d2de72d..730124c 100644
--- a/test/fuzzers/rtp_packet_fuzzer.cc
+++ b/test/fuzzers/rtp_packet_fuzzer.cc
@@ -10,6 +10,7 @@
#include <bitset>
+#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,6 +87,13 @@
uint16_t seqnum;
packet.GetExtension<TransportSequenceNumber>(&seqnum);
break;
+ case kRtpExtensionTransportSequenceNumber02: {
+ uint16_t seqnum;
+ absl::optional<FeedbackRequest> feedback_request;
+ packet.GetExtension<TransportSequenceNumberV2>(&seqnum,
+ &feedback_request);
+ break;
+ }
case kRtpExtensionPlayoutDelay:
PlayoutDelay playout;
packet.GetExtension<PlayoutDelayLimits>(&playout);
