Enable padding bit in TransportFeedback packets

Set padding bit if the TransportFeedback packet contains zero padding.
Also write number of padding elements at the last position of the packet.

Bug: webrtc:10263
Change-Id: I8d17bc0e889f658ac383dec64ddcb95d438c9702
Reviewed-on: https://webrtc-review.googlesource.com/c/122240
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Johannes Kron <kron@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26646}
This commit is contained in:
Johannes Kron 2019-02-12 10:51:18 +01:00 committed by Commit Bot
parent 2ce0cb0e00
commit 99b9149cee
5 changed files with 100 additions and 32 deletions

View File

@ -52,7 +52,8 @@ bool RtcpPacket::OnBufferFull(uint8_t* packet,
size_t RtcpPacket::HeaderLength() const {
size_t length_in_bytes = BlockLength();
RTC_DCHECK_GT(length_in_bytes, 0);
RTC_DCHECK_EQ(length_in_bytes % 4, 0) << "Padding not supported";
RTC_DCHECK_EQ(length_in_bytes % 4, 0)
<< "Padding must be handled by each subclass.";
// Length in 32-bit words without common header.
return (length_in_bytes - kHeaderLength) / 4;
}
@ -71,12 +72,23 @@ void RtcpPacket::CreateHeader(
size_t length,
uint8_t* buffer,
size_t* pos) {
CreateHeader(count_or_format, packet_type, length, /*padding=*/false, buffer,
pos);
}
void RtcpPacket::CreateHeader(
size_t count_or_format, // Depends on packet type.
uint8_t packet_type,
size_t length,
bool padding,
uint8_t* buffer,
size_t* pos) {
RTC_DCHECK_LE(length, 0xffffU);
RTC_DCHECK_LE(count_or_format, 0x1f);
constexpr uint8_t kVersionBits = 2 << 6;
constexpr uint8_t kNoPaddingBit = 0 << 5;
uint8_t padding_bit = padding ? 1 << 5 : 0;
buffer[*pos + 0] =
kVersionBits | kNoPaddingBit | static_cast<uint8_t>(count_or_format);
kVersionBits | padding_bit | static_cast<uint8_t>(count_or_format);
buffer[*pos + 1] = packet_type;
buffer[*pos + 2] = (length >> 8) & 0xff;
buffer[*pos + 3] = length & 0xff;

View File

@ -87,6 +87,13 @@ class RtcpPacket {
uint8_t* buffer,
size_t* pos);
static void CreateHeader(size_t count_or_format,
uint8_t packet_type,
size_t block_length, // Payload size in 32bit words.
bool padding, // True if there are padding bytes.
uint8_t* buffer,
size_t* pos);
bool OnBufferFull(uint8_t* packet,
size_t* index,
PacketReadyCallback callback) const;

View File

@ -521,6 +521,10 @@ size_t TransportFeedback::BlockLength() const {
return (size_bytes_ + 3) & (~static_cast<size_t>(3));
}
size_t TransportFeedback::PaddingLength() const {
return BlockLength() - size_bytes_;
}
// Serialize packet.
bool TransportFeedback::Create(uint8_t* packet,
size_t* position,
@ -534,9 +538,10 @@ bool TransportFeedback::Create(uint8_t* packet,
return false;
}
const size_t position_end = *position + BlockLength();
CreateHeader(kFeedbackMessageType, kPacketType, HeaderLength(), packet,
position);
const size_t padding_length = PaddingLength();
bool has_padding = padding_length > 0;
CreateHeader(kFeedbackMessageType, kPacketType, HeaderLength(), has_padding,
packet, position);
CreateCommonFeedback(packet + *position);
*position += kCommonFeedbackLength;
@ -571,9 +576,12 @@ bool TransportFeedback::Create(uint8_t* packet,
}
}
while ((*position % 4) != 0)
packet[(*position)++] = 0;
if (padding_length > 0) {
for (size_t i = 0; i < padding_length - 1; ++i) {
packet[(*position)++] = 0;
}
packet[(*position)++] = padding_length;
}
RTC_DCHECK_EQ(*position, position_end);
return true;
}

View File

@ -73,6 +73,7 @@ class TransportFeedback : public Rtpfb {
bool IsConsistent() const;
size_t BlockLength() const override;
size_t PaddingLength() const;
bool Create(uint8_t* packet,
size_t* position,

View File

@ -82,7 +82,7 @@ class FeedbackTester {
feedback_ =
TransportFeedback::ParseFrom(serialized_.data(), serialized_.size());
ASSERT_TRUE(feedback_->IsConsistent());
ASSERT_NE(nullptr, feedback_.get());
ASSERT_NE(nullptr, feedback_);
VerifyInternal();
}
@ -130,7 +130,7 @@ class FeedbackTester {
rtc::Buffer serialized_;
};
TEST(RtcpPacketTest, TransportFeedback_OneBitVector) {
TEST(RtcpPacketTest, TransportFeedbackOneBitVector) {
const uint16_t kReceived[] = {1, 2, 7, 8, 9, 10, 13};
const size_t kLength = sizeof(kReceived) / sizeof(uint16_t);
const size_t kExpectedSizeBytes =
@ -142,7 +142,7 @@ TEST(RtcpPacketTest, TransportFeedback_OneBitVector) {
test.VerifyPacket();
}
TEST(RtcpPacketTest, TransportFeedback_FullOneBitVector) {
TEST(RtcpPacketTest, TransportFeedbackFullOneBitVector) {
const uint16_t kReceived[] = {1, 2, 7, 8, 9, 10, 13, 14};
const size_t kLength = sizeof(kReceived) / sizeof(uint16_t);
const size_t kExpectedSizeBytes =
@ -154,7 +154,7 @@ TEST(RtcpPacketTest, TransportFeedback_FullOneBitVector) {
test.VerifyPacket();
}
TEST(RtcpPacketTest, TransportFeedback_OneBitVector_WrapReceived) {
TEST(RtcpPacketTest, TransportFeedbackOneBitVectorWrapReceived) {
const uint16_t kMax = 0xFFFF;
const uint16_t kReceived[] = {kMax - 2, kMax - 1, kMax, 0, 1, 2};
const size_t kLength = sizeof(kReceived) / sizeof(uint16_t);
@ -167,7 +167,7 @@ TEST(RtcpPacketTest, TransportFeedback_OneBitVector_WrapReceived) {
test.VerifyPacket();
}
TEST(RtcpPacketTest, TransportFeedback_OneBitVector_WrapMissing) {
TEST(RtcpPacketTest, TransportFeedbackOneBitVectorWrapMissing) {
const uint16_t kMax = 0xFFFF;
const uint16_t kReceived[] = {kMax - 2, kMax - 1, 1, 2};
const size_t kLength = sizeof(kReceived) / sizeof(uint16_t);
@ -180,7 +180,7 @@ TEST(RtcpPacketTest, TransportFeedback_OneBitVector_WrapMissing) {
test.VerifyPacket();
}
TEST(RtcpPacketTest, TransportFeedback_TwoBitVector) {
TEST(RtcpPacketTest, TransportFeedbackTwoBitVector) {
const uint16_t kReceived[] = {1, 2, 6, 7};
const size_t kLength = sizeof(kReceived) / sizeof(uint16_t);
const size_t kExpectedSizeBytes =
@ -193,7 +193,7 @@ TEST(RtcpPacketTest, TransportFeedback_TwoBitVector) {
test.VerifyPacket();
}
TEST(RtcpPacketTest, TransportFeedback_TwoBitVectorFull) {
TEST(RtcpPacketTest, TransportFeedbackTwoBitVectorFull) {
const uint16_t kReceived[] = {1, 2, 6, 7, 8};
const size_t kLength = sizeof(kReceived) / sizeof(uint16_t);
const size_t kExpectedSizeBytes =
@ -206,7 +206,7 @@ TEST(RtcpPacketTest, TransportFeedback_TwoBitVectorFull) {
test.VerifyPacket();
}
TEST(RtcpPacketTest, TransportFeedback_LargeAndNegativeDeltas) {
TEST(RtcpPacketTest, TransportFeedbackLargeAndNegativeDeltas) {
const uint16_t kReceived[] = {1, 2, 6, 7, 8};
const int64_t kReceiveTimes[] = {
2000, 1000, 4000, 3000,
@ -221,7 +221,7 @@ TEST(RtcpPacketTest, TransportFeedback_LargeAndNegativeDeltas) {
test.VerifyPacket();
}
TEST(RtcpPacketTest, TransportFeedback_MaxRle) {
TEST(RtcpPacketTest, TransportFeedbackMaxRle) {
// Expected chunks created:
// * 1-bit vector chunk (1xreceived + 13xdropped)
// * RLE chunk of max length for dropped symbol
@ -240,7 +240,7 @@ TEST(RtcpPacketTest, TransportFeedback_MaxRle) {
test.VerifyPacket();
}
TEST(RtcpPacketTest, TransportFeedback_MinRle) {
TEST(RtcpPacketTest, TransportFeedbackMinRle) {
// Expected chunks created:
// * 1-bit vector chunk (1xreceived + 13xdropped)
// * RLE chunk of length 15 for dropped symbol
@ -258,7 +258,7 @@ TEST(RtcpPacketTest, TransportFeedback_MinRle) {
test.VerifyPacket();
}
TEST(RtcpPacketTest, TransportFeedback_OneToTwoBitVector) {
TEST(RtcpPacketTest, TransportFeedbackOneToTwoBitVector) {
const size_t kTwoBitVectorCapacity = 7;
const uint16_t kReceived[] = {0, kTwoBitVectorCapacity - 1};
const int64_t kReceiveTimes[] = {
@ -273,7 +273,7 @@ TEST(RtcpPacketTest, TransportFeedback_OneToTwoBitVector) {
test.VerifyPacket();
}
TEST(RtcpPacketTest, TransportFeedback_OneToTwoBitVectorSimpleSplit) {
TEST(RtcpPacketTest, TransportFeedbackOneToTwoBitVectorSimpleSplit) {
const size_t kTwoBitVectorCapacity = 7;
const uint16_t kReceived[] = {0, kTwoBitVectorCapacity};
const int64_t kReceiveTimes[] = {
@ -288,7 +288,7 @@ TEST(RtcpPacketTest, TransportFeedback_OneToTwoBitVectorSimpleSplit) {
test.VerifyPacket();
}
TEST(RtcpPacketTest, TransportFeedback_OneToTwoBitVectorSplit) {
TEST(RtcpPacketTest, TransportFeedbackOneToTwoBitVectorSplit) {
// With received small delta = S, received large delta = L, use input
// SSSSSSSSLSSSSSSSSSSSS. This will cause a 1:2 split at the L.
// After split there will be two symbols in symbol_vec: SL.
@ -316,7 +316,7 @@ TEST(RtcpPacketTest, TransportFeedback_OneToTwoBitVectorSplit) {
test.VerifyPacket();
}
TEST(RtcpPacketTest, TransportFeedback_Aliasing) {
TEST(RtcpPacketTest, TransportFeedbackAliasing) {
TransportFeedback feedback;
feedback.SetBase(0, 0);
@ -340,7 +340,7 @@ TEST(RtcpPacketTest, TransportFeedback_Aliasing) {
}
}
TEST(RtcpPacketTest, TransportFeedback_Limits) {
TEST(RtcpPacketTest, TransportFeedbackLimits) {
// Sequence number wrap above 0x8000.
std::unique_ptr<TransportFeedback> packet(new TransportFeedback());
packet->SetBase(0, 0);
@ -404,10 +404,12 @@ TEST(RtcpPacketTest, TransportFeedback_Limits) {
// add back test for max size in bytes.
}
TEST(RtcpPacketTest, TransportFeedback_Padding) {
TEST(RtcpPacketTest, TransportFeedbackPadding) {
const size_t kExpectedSizeBytes =
kHeaderSize + kStatusChunkSize + kSmallDeltaSize;
const size_t kExpectedSizeWords = (kExpectedSizeBytes + 3) / 4;
const size_t kExpectedPaddingSizeBytes =
4 * kExpectedSizeWords - kExpectedSizeBytes;
TransportFeedback feedback;
feedback.SetBase(0, 0);
@ -416,8 +418,10 @@ TEST(RtcpPacketTest, TransportFeedback_Padding) {
rtc::Buffer packet = feedback.Build();
EXPECT_EQ(kExpectedSizeWords * 4, packet.size());
ASSERT_GT(kExpectedSizeWords * 4, kExpectedSizeBytes);
for (size_t i = kExpectedSizeBytes; i < kExpectedSizeWords * 4; ++i)
EXPECT_EQ(0u, packet.data()[i]);
for (size_t i = kExpectedSizeBytes; i < (kExpectedSizeWords * 4 - 1); ++i)
EXPECT_EQ(0u, packet[i]);
EXPECT_EQ(kExpectedPaddingSizeBytes, packet[kExpectedSizeWords * 4 - 1]);
// Modify packet by adding 4 bytes of padding at the end. Not currently used
// when we're sending, but need to be able to handle it when receiving.
@ -428,7 +432,8 @@ TEST(RtcpPacketTest, TransportFeedback_Padding) {
uint8_t mod_buffer[kExpectedSizeWithPadding];
memcpy(mod_buffer, packet.data(), kExpectedSizeWords * 4);
memset(&mod_buffer[kExpectedSizeWords * 4], 0, kPaddingBytes - 1);
mod_buffer[kExpectedSizeWithPadding - 1] = kPaddingBytes;
mod_buffer[kExpectedSizeWithPadding - 1] =
kPaddingBytes + kExpectedPaddingSizeBytes;
const uint8_t padding_flag = 1 << 5;
mod_buffer[0] |= padding_flag;
ByteWriter<uint16_t>::WriteBigEndian(
@ -437,11 +442,46 @@ TEST(RtcpPacketTest, TransportFeedback_Padding) {
std::unique_ptr<TransportFeedback> parsed_packet(
TransportFeedback::ParseFrom(mod_buffer, kExpectedSizeWithPadding));
ASSERT_TRUE(parsed_packet.get() != nullptr);
ASSERT_TRUE(parsed_packet != nullptr);
EXPECT_EQ(kExpectedSizeWords * 4, packet.size()); // Padding not included.
}
TEST(RtcpPacketTest, TransportFeedback_CorrectlySplitsVectorChunks) {
TEST(RtcpPacketTest, TransportFeedbackPaddingBackwardsCompatibility) {
const size_t kExpectedSizeBytes =
kHeaderSize + kStatusChunkSize + kSmallDeltaSize;
const size_t kExpectedSizeWords = (kExpectedSizeBytes + 3) / 4;
const size_t kExpectedPaddingSizeBytes =
4 * kExpectedSizeWords - kExpectedSizeBytes;
TransportFeedback feedback;
feedback.SetBase(0, 0);
EXPECT_TRUE(feedback.AddReceivedPacket(0, 0));
rtc::Buffer packet = feedback.Build();
EXPECT_EQ(kExpectedSizeWords * 4, packet.size());
ASSERT_GT(kExpectedSizeWords * 4, kExpectedSizeBytes);
for (size_t i = kExpectedSizeBytes; i < (kExpectedSizeWords * 4 - 1); ++i)
EXPECT_EQ(0u, packet[i]);
EXPECT_GT(kExpectedPaddingSizeBytes, 0u);
EXPECT_EQ(kExpectedPaddingSizeBytes, packet[kExpectedSizeWords * 4 - 1]);
// Modify packet by removing padding bit and writing zero at the last padding
// byte to verify that we can parse packets from old clients, where zero
// padding of up to three bytes was used without the padding bit being set.
uint8_t mod_buffer[kExpectedSizeWords * 4];
memcpy(mod_buffer, packet.data(), kExpectedSizeWords * 4);
mod_buffer[kExpectedSizeWords * 4 - 1] = 0;
const uint8_t padding_flag = 1 << 5;
mod_buffer[0] &= ~padding_flag; // Unset padding flag.
std::unique_ptr<TransportFeedback> parsed_packet(
TransportFeedback::ParseFrom(mod_buffer, kExpectedSizeWords * 4));
ASSERT_TRUE(parsed_packet != nullptr);
EXPECT_EQ(kExpectedSizeWords * 4, packet.size());
}
TEST(RtcpPacketTest, TransportFeedbackCorrectlySplitsVectorChunks) {
const int kOneBitVectorCapacity = 14;
const int64_t kLargeTimeDelta =
TransportFeedback::kDeltaScaleFactor * (1 << 8);
@ -460,11 +500,11 @@ TEST(RtcpPacketTest, TransportFeedback_CorrectlySplitsVectorChunks) {
std::unique_ptr<TransportFeedback> deserialized_packet =
TransportFeedback::ParseFrom(serialized_packet.data(),
serialized_packet.size());
EXPECT_TRUE(deserialized_packet.get() != nullptr);
EXPECT_TRUE(deserialized_packet != nullptr);
}
}
TEST(RtcpPacketTest, TransportFeedback_MoveConstructor) {
TEST(RtcpPacketTest, TransportFeedbackMoveConstructor) {
const int kSamples = 100;
const int64_t kDelta = TransportFeedback::kDeltaScaleFactor;
const uint16_t kBaseSeqNo = 7531;