Use zero octets for rtp packet padding
RFC3550 Section 4 mention
"Octets designated as padding have the value zero."
Bug: None
Change-Id: Ife4c6226143c79ad7d152bc6099ba1d81f5492dd
Reviewed-on: https://webrtc-review.googlesource.com/c/103983
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25109}diff --git a/logging/rtc_event_log/rtc_event_log_unittest_helper.cc b/logging/rtc_event_log/rtc_event_log_unittest_helper.cc
index 9f6240c..1a405ec 100644
--- a/logging/rtc_event_log/rtc_event_log_unittest_helper.cc
+++ b/logging/rtc_event_log/rtc_event_log_unittest_helper.cc
@@ -299,7 +299,7 @@
for (size_t i = 0; i < payload_size; i++) {
payload[i] = prng_.Rand<uint8_t>();
}
- RTC_CHECK(rtp_packet->SetPadding(padding_size, &prng_));
+ RTC_CHECK(rtp_packet->SetPadding(padding_size));
}
std::unique_ptr<RtcEventRtpPacketIncoming> EventGenerator::NewRtpPacketIncoming(
diff --git a/modules/rtp_rtcp/source/receive_statistics_unittest.cc b/modules/rtp_rtcp/source/receive_statistics_unittest.cc
index 18448cb..578d81f 100644
--- a/modules/rtp_rtcp/source/receive_statistics_unittest.cc
+++ b/modules/rtp_rtcp/source/receive_statistics_unittest.cc
@@ -48,10 +48,7 @@
packet.SetCsrcs(csrcs);
}
packet.SetPayloadSize(payload_size);
- if (padding_size > 0) {
- Random random(17);
- packet.SetPadding(padding_size, &random);
- }
+ packet.SetPadding(padding_size);
return packet;
}
diff --git a/modules/rtp_rtcp/source/rtp_packet.cc b/modules/rtp_rtcp/source/rtp_packet.cc
index 0b9ed80..8cb8fd8 100644
--- a/modules/rtp_rtcp/source/rtp_packet.cc
+++ b/modules/rtp_rtcp/source/rtp_packet.cc
@@ -19,7 +19,6 @@
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "rtc_base/numerics/safe_conversions.h"
-#include "rtc_base/random.h"
namespace webrtc {
namespace {
@@ -359,22 +358,20 @@
return WriteAt(payload_offset_);
}
-bool RtpPacket::SetPadding(uint8_t size_bytes, Random* random) {
- RTC_DCHECK(random);
- if (payload_offset_ + payload_size_ + size_bytes > capacity()) {
- RTC_LOG(LS_WARNING) << "Cannot set padding size " << size_bytes << ", only "
+bool RtpPacket::SetPadding(size_t padding_bytes) {
+ if (payload_offset_ + payload_size_ + padding_bytes > capacity()) {
+ RTC_LOG(LS_WARNING) << "Cannot set padding size " << padding_bytes
+ << ", only "
<< (capacity() - payload_offset_ - payload_size_)
<< " bytes left in buffer.";
return false;
}
- padding_size_ = size_bytes;
+ padding_size_ = rtc::dchecked_cast<uint8_t>(padding_bytes);
buffer_.SetSize(payload_offset_ + payload_size_ + padding_size_);
if (padding_size_ > 0) {
size_t padding_offset = payload_offset_ + payload_size_;
size_t padding_end = padding_offset + padding_size_;
- for (size_t offset = padding_offset; offset < padding_end - 1; ++offset) {
- WriteAt(offset, random->Rand<uint8_t>());
- }
+ memset(WriteAt(padding_offset), 0, padding_size_ - 1);
WriteAt(padding_end - 1, padding_size_);
WriteAt(0, data()[0] | 0x20); // Set padding bit.
} else {
diff --git a/modules/rtp_rtcp/source/rtp_packet.h b/modules/rtp_rtcp/source/rtp_packet.h
index 15c3865..3d7d90c 100644
--- a/modules/rtp_rtcp/source/rtp_packet.h
+++ b/modules/rtp_rtcp/source/rtp_packet.h
@@ -16,6 +16,7 @@
#include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "rtc_base/copyonwritebuffer.h"
+#include "rtc_base/deprecation.h"
namespace webrtc {
class Random;
@@ -111,7 +112,12 @@
uint8_t* SetPayloadSize(size_t size_bytes);
// Same as SetPayloadSize but doesn't guarantee to keep current payload.
uint8_t* AllocatePayload(size_t size_bytes);
- bool SetPadding(uint8_t size_bytes, Random* random);
+ RTC_DEPRECATED
+ bool SetPadding(uint8_t size_bytes, Random* random) {
+ return SetPadding(size_bytes);
+ }
+
+ bool SetPadding(size_t padding_size);
private:
struct ExtensionInfo {
diff --git a/modules/rtp_rtcp/source/rtp_packet_unittest.cc b/modules/rtp_rtcp/source/rtp_packet_unittest.cc
index 125000d..648ddc3 100644
--- a/modules/rtp_rtcp/source/rtp_packet_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_packet_unittest.cc
@@ -19,6 +19,7 @@
namespace webrtc {
namespace {
+using ::testing::Each;
using ::testing::ElementsAre;
using ::testing::ElementsAreArray;
using ::testing::IsEmpty;
@@ -367,11 +368,10 @@
packet.SetSequenceNumber(kSeqNum);
packet.SetTimestamp(kTimestamp);
packet.SetSsrc(kSsrc);
- Random random(0x123456789);
EXPECT_LT(packet.size(), packet.capacity());
- EXPECT_FALSE(packet.SetPadding(kPaddingSize + 1, &random));
- EXPECT_TRUE(packet.SetPadding(kPaddingSize, &random));
+ EXPECT_FALSE(packet.SetPadding(kPaddingSize + 1));
+ EXPECT_TRUE(packet.SetPadding(kPaddingSize));
EXPECT_EQ(packet.size(), packet.capacity());
}
@@ -383,13 +383,48 @@
packet.SetTimestamp(kTimestamp);
packet.SetSsrc(kSsrc);
packet.SetPayloadSize(kPayloadSize);
- Random r(0x123456789);
EXPECT_LT(packet.size(), packet.capacity());
- EXPECT_TRUE(packet.SetPadding(kMaxPaddingSize, &r));
+ EXPECT_TRUE(packet.SetPadding(kMaxPaddingSize));
EXPECT_EQ(packet.size(), packet.capacity());
}
+TEST(RtpPacketTest, WritesPaddingSizeToLastByte) {
+ const size_t kPaddingSize = 5;
+ RtpPacket packet;
+
+ EXPECT_TRUE(packet.SetPadding(kPaddingSize));
+ EXPECT_EQ(packet.data()[packet.size() - 1], kPaddingSize);
+}
+
+TEST(RtpPacketTest, UsesZerosForPadding) {
+ const size_t kPaddingSize = 5;
+ RtpPacket packet;
+
+ EXPECT_TRUE(packet.SetPadding(kPaddingSize));
+ EXPECT_THAT(rtc::MakeArrayView(packet.data() + 12, kPaddingSize - 1),
+ Each(0));
+}
+
+TEST(RtpPacketTest, CreateOneBytePadding) {
+ size_t kPayloadSize = 123;
+ RtpPacket packet(nullptr, 12 + kPayloadSize + 1);
+ packet.SetPayloadSize(kPayloadSize);
+
+ EXPECT_TRUE(packet.SetPadding(1));
+
+ EXPECT_EQ(packet.size(), 12 + kPayloadSize + 1);
+ EXPECT_EQ(packet.padding_size(), 1u);
+}
+
+TEST(RtpPacketTest, FailsToAddPaddingWithoutCapacity) {
+ size_t kPayloadSize = 123;
+ RtpPacket packet(nullptr, 12 + kPayloadSize);
+ packet.SetPayloadSize(kPayloadSize);
+
+ EXPECT_FALSE(packet.SetPadding(1));
+}
+
TEST(RtpPacketTest, ParseMinimum) {
RtpPacketReceived packet;
EXPECT_TRUE(packet.Parse(kMinimumPacket, sizeof(kMinimumPacket)));
diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc
index 31887bd..1678a03 100644
--- a/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/modules/rtp_rtcp/source/rtp_sender.cc
@@ -622,7 +622,7 @@
has_transport_seq_num || force_part_of_allocation_;
options.included_in_feedback = has_transport_seq_num;
}
- padding_packet.SetPadding(padding_bytes_in_packet, &random_);
+ padding_packet.SetPadding(padding_bytes_in_packet);
if (has_transport_seq_num) {
AddPacketToTransportFeedback(options.packet_id, padding_packet,
pacing_info);
