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}
This commit is contained in:

committed by
Commit Bot

parent
1eb3d7ea0f
commit
54047bea1b
@ -55,6 +55,7 @@ enum RTPExtensionType : int {
|
||||
kRtpExtensionAbsoluteSendTime,
|
||||
kRtpExtensionVideoRotation,
|
||||
kRtpExtensionTransportSequenceNumber,
|
||||
kRtpExtensionTransportSequenceNumber02,
|
||||
kRtpExtensionPlayoutDelay,
|
||||
kRtpExtensionVideoContentType,
|
||||
kRtpExtensionVideoTiming,
|
||||
|
@ -35,6 +35,7 @@ constexpr ExtensionInfo kExtensions[] = {
|
||||
CreateExtensionInfo<AbsoluteSendTime>(),
|
||||
CreateExtensionInfo<VideoOrientation>(),
|
||||
CreateExtensionInfo<TransportSequenceNumber>(),
|
||||
CreateExtensionInfo<TransportSequenceNumberV2>(),
|
||||
CreateExtensionInfo<PlayoutDelayLimits>(),
|
||||
CreateExtensionInfo<VideoContentTypeExtension>(),
|
||||
CreateExtensionInfo<VideoTimingExtension>(),
|
||||
|
@ -126,27 +126,93 @@ bool TransmissionOffset::Write(rtc::ArrayView<uint8_t> 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<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;
|
||||
}
|
||||
|
||||
|
@ -81,9 +81,38 @@ 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<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 {
|
||||
|
@ -52,6 +52,9 @@ void RtpPacketReceived::GetHeader(RTPHeader* header) const {
|
||||
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>(
|
||||
|
@ -827,4 +827,62 @@ 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<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
|
||||
|
@ -433,6 +433,10 @@ 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;
|
||||
|
@ -230,6 +230,7 @@ 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"
|
||||
}
|
||||
|
@ -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 @@ void FuzzOneInput(const uint8_t* data, size_t size) {
|
||||
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);
|
||||
|
Reference in New Issue
Block a user