Use signed integers for limiting packet size in video packetizers

Using signed integers allows to centralize checking of edge cases
in RtpPacketizer::SplitAboutEqually and
reduce chance of hitting issues with size_t underflow

Bug: webrtc:9680
Change-Id: Ic05bf0a9565a277c4608f43061ca46cf44e82d08
Reviewed-on: https://webrtc-review.googlesource.com/98602
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24618}
This commit is contained in:
Danil Chapovalov
2018-09-07 10:57:26 +02:00
committed by Commit Bot
parent 5e2e66d8a0
commit fa5ec8d20d
8 changed files with 125 additions and 79 deletions

View File

@ -59,26 +59,42 @@ std::unique_ptr<RtpPacketizer> RtpPacketizer::Create(
}
}
std::vector<size_t> RtpPacketizer::SplitAboutEqually(
size_t payload_len,
std::vector<int> RtpPacketizer::SplitAboutEqually(
int payload_len,
const PayloadSizeLimits& limits) {
RTC_CHECK_GT(limits.max_payload_len, limits.first_packet_reduction_len);
RTC_CHECK_GT(limits.max_payload_len, limits.last_packet_reduction_len);
RTC_DCHECK_GT(payload_len, 0);
// First or last packet larger than normal are unsupported.
RTC_DCHECK_GE(limits.first_packet_reduction_len, 0);
RTC_DCHECK_GE(limits.last_packet_reduction_len, 0);
std::vector<int> result;
if (limits.max_payload_len - limits.first_packet_reduction_len < 1 ||
limits.max_payload_len - limits.last_packet_reduction_len < 1) {
// Capacity is not enough to put a single byte into one of the packets.
return result;
}
// First and last packet of the frame can be smaller. Pretend that it's
// the same size, but we must write more payload to it.
// Assume frame fits in single packet if packet has extra space for sum
// of first and last packets reductions.
size_t total_bytes = payload_len + limits.first_packet_reduction_len +
limits.last_packet_reduction_len;
int total_bytes = payload_len + limits.first_packet_reduction_len +
limits.last_packet_reduction_len;
// Integer divisions with rounding up.
size_t num_packets_left =
int num_packets_left =
(total_bytes + limits.max_payload_len - 1) / limits.max_payload_len;
size_t bytes_per_packet = total_bytes / num_packets_left;
size_t num_larger_packets = total_bytes % num_packets_left;
size_t remaining_data = payload_len;
std::vector<size_t> result;
if (payload_len < num_packets_left) {
// Edge case where limits force to have more packets than there are payload
// bytes. This may happen when there is single byte of payload that can't be
// put into single packet if
// first_packet_reduction + last_packet_reduction >= max_payload_len.
return result;
}
int bytes_per_packet = total_bytes / num_packets_left;
int num_larger_packets = total_bytes % num_packets_left;
int remaining_data = payload_len;
result.reserve(num_packets_left);
bool first_packet = true;
while (remaining_data > 0) {
@ -86,7 +102,7 @@ std::vector<size_t> RtpPacketizer::SplitAboutEqually(
// per-packet payload size when needed.
if (num_packets_left == num_larger_packets)
++bytes_per_packet;
size_t current_packet_bytes = bytes_per_packet;
int current_packet_bytes = bytes_per_packet;
if (first_packet) {
if (current_packet_bytes > limits.first_packet_reduction_len + 1)
current_packet_bytes -= limits.first_packet_reduction_len;

View File

@ -27,9 +27,9 @@ class RtpPacketToSend;
class RtpPacketizer {
public:
struct PayloadSizeLimits {
size_t max_payload_len = 1200;
size_t first_packet_reduction_len = 0;
size_t last_packet_reduction_len = 0;
int max_payload_len = 1200;
int first_packet_reduction_len = 0;
int last_packet_reduction_len = 0;
};
static std::unique_ptr<RtpPacketizer> Create(
VideoCodecType type,
@ -51,8 +51,9 @@ class RtpPacketizer {
virtual bool NextPacket(RtpPacketToSend* packet) = 0;
// Split payload_len into sum of integers with respect to |limits|.
static std::vector<size_t> SplitAboutEqually(size_t payload_len,
const PayloadSizeLimits& limits);
// Returns empty vector on failure.
static std::vector<int> SplitAboutEqually(int payload_len,
const PayloadSizeLimits& limits);
};
// TODO(sprang): Update the depacketizer to return a std::unqie_ptr with a copy

View File

@ -31,7 +31,7 @@ using ::testing::SizeIs;
// adjustement provided by limits,
// i.e. last packet expected to be smaller than 'average' by reduction_len.
int EffectivePacketsSizeDifference(
std::vector<size_t> sizes,
std::vector<int> sizes,
const RtpPacketizer::PayloadSizeLimits& limits) {
// Account for larger last packet header.
sizes.back() += limits.last_packet_reduction_len;
@ -41,7 +41,7 @@ int EffectivePacketsSizeDifference(
return *minmax.second - *minmax.first;
}
size_t Sum(const std::vector<size_t>& sizes) {
int Sum(const std::vector<int>& sizes) {
return std::accumulate(sizes.begin(), sizes.end(), 0);
}
@ -50,8 +50,7 @@ TEST(RtpPacketizerSplitAboutEqually, AllPacketsAreEqualSumToPayloadLen) {
limits.max_payload_len = 5;
limits.last_packet_reduction_len = 2;
std::vector<size_t> payload_sizes =
RtpPacketizer::SplitAboutEqually(13, limits);
std::vector<int> payload_sizes = RtpPacketizer::SplitAboutEqually(13, limits);
EXPECT_THAT(Sum(payload_sizes), 13);
}
@ -61,8 +60,7 @@ TEST(RtpPacketizerSplitAboutEqually, AllPacketsAreEqualRespectsMaxPayloadSize) {
limits.max_payload_len = 5;
limits.last_packet_reduction_len = 2;
std::vector<size_t> payload_sizes =
RtpPacketizer::SplitAboutEqually(13, limits);
std::vector<int> payload_sizes = RtpPacketizer::SplitAboutEqually(13, limits);
EXPECT_THAT(payload_sizes, Each(Le(limits.max_payload_len)));
}
@ -73,8 +71,7 @@ TEST(RtpPacketizerSplitAboutEqually,
limits.max_payload_len = 5;
limits.first_packet_reduction_len = 2;
std::vector<size_t> payload_sizes =
RtpPacketizer::SplitAboutEqually(13, limits);
std::vector<int> payload_sizes = RtpPacketizer::SplitAboutEqually(13, limits);
ASSERT_THAT(payload_sizes, Not(IsEmpty()));
EXPECT_EQ(payload_sizes.front() + limits.first_packet_reduction_len,
@ -87,8 +84,7 @@ TEST(RtpPacketizerSplitAboutEqually,
limits.max_payload_len = 5;
limits.last_packet_reduction_len = 2;
std::vector<size_t> payload_sizes =
RtpPacketizer::SplitAboutEqually(13, limits);
std::vector<int> payload_sizes = RtpPacketizer::SplitAboutEqually(13, limits);
ASSERT_THAT(payload_sizes, Not(IsEmpty()));
EXPECT_LE(payload_sizes.back() + limits.last_packet_reduction_len,
@ -100,8 +96,7 @@ TEST(RtpPacketizerSplitAboutEqually, AllPacketsAreEqualInSize) {
limits.max_payload_len = 5;
limits.last_packet_reduction_len = 2;
std::vector<size_t> payload_sizes =
RtpPacketizer::SplitAboutEqually(13, limits);
std::vector<int> payload_sizes = RtpPacketizer::SplitAboutEqually(13, limits);
EXPECT_EQ(EffectivePacketsSizeDifference(payload_sizes, limits), 0);
}
@ -112,8 +107,7 @@ TEST(RtpPacketizerSplitAboutEqually,
limits.max_payload_len = 5;
limits.last_packet_reduction_len = 2;
std::vector<size_t> payload_sizes =
RtpPacketizer::SplitAboutEqually(13, limits);
std::vector<int> payload_sizes = RtpPacketizer::SplitAboutEqually(13, limits);
// Computed by hand. 3 packets would have exactly capacity 3*5-2=13
// (max length - for each packet minus last packet reduction).
EXPECT_THAT(payload_sizes, SizeIs(3));
@ -124,8 +118,7 @@ TEST(RtpPacketizerSplitAboutEqually, SomePacketsAreSmallerSumToPayloadLen) {
limits.max_payload_len = 7;
limits.last_packet_reduction_len = 5;
std::vector<size_t> payload_sizes =
RtpPacketizer::SplitAboutEqually(28, limits);
std::vector<int> payload_sizes = RtpPacketizer::SplitAboutEqually(28, limits);
EXPECT_THAT(Sum(payload_sizes), 28);
}
@ -136,8 +129,7 @@ TEST(RtpPacketizerSplitAboutEqually,
limits.max_payload_len = 7;
limits.last_packet_reduction_len = 5;
std::vector<size_t> payload_sizes =
RtpPacketizer::SplitAboutEqually(28, limits);
std::vector<int> payload_sizes = RtpPacketizer::SplitAboutEqually(28, limits);
EXPECT_THAT(payload_sizes, Each(Le(limits.max_payload_len)));
}
@ -148,8 +140,7 @@ TEST(RtpPacketizerSplitAboutEqually,
limits.max_payload_len = 7;
limits.first_packet_reduction_len = 5;
std::vector<size_t> payload_sizes =
RtpPacketizer::SplitAboutEqually(28, limits);
std::vector<int> payload_sizes = RtpPacketizer::SplitAboutEqually(28, limits);
EXPECT_LE(payload_sizes.front() + limits.first_packet_reduction_len,
limits.max_payload_len);
@ -161,8 +152,7 @@ TEST(RtpPacketizerSplitAboutEqually,
limits.max_payload_len = 7;
limits.last_packet_reduction_len = 5;
std::vector<size_t> payload_sizes =
RtpPacketizer::SplitAboutEqually(28, limits);
std::vector<int> payload_sizes = RtpPacketizer::SplitAboutEqually(28, limits);
EXPECT_LE(payload_sizes.back(),
limits.max_payload_len - limits.last_packet_reduction_len);
@ -174,8 +164,7 @@ TEST(RtpPacketizerSplitAboutEqually,
limits.max_payload_len = 7;
limits.last_packet_reduction_len = 5;
std::vector<size_t> payload_sizes =
RtpPacketizer::SplitAboutEqually(28, limits);
std::vector<int> payload_sizes = RtpPacketizer::SplitAboutEqually(28, limits);
EXPECT_LE(EffectivePacketsSizeDifference(payload_sizes, limits), 1);
}
@ -186,8 +175,7 @@ TEST(RtpPacketizerSplitAboutEqually,
limits.max_payload_len = 7;
limits.last_packet_reduction_len = 5;
std::vector<size_t> payload_sizes =
RtpPacketizer::SplitAboutEqually(24, limits);
std::vector<int> payload_sizes = RtpPacketizer::SplitAboutEqually(24, limits);
// Computed by hand. 4 packets would have capacity 4*7-5=23 (max length -
// for each packet minus last packet reduction).
// 5 packets is enough for kPayloadSize.
@ -203,11 +191,11 @@ TEST(RtpPacketizerSplitAboutEqually, GivesNonZeroPayloadLengthEachPacket) {
// Naive implementation would split 1450 payload + 1050 reduction bytes into 5
// packets 500 bytes each, thus leaving first packet zero bytes and even less
// to last packet.
std::vector<size_t> payload_sizes =
std::vector<int> payload_sizes =
RtpPacketizer::SplitAboutEqually(1450, limits);
EXPECT_EQ(Sum(payload_sizes), 1450u);
EXPECT_THAT(payload_sizes, Each(Gt(0u)));
EXPECT_EQ(Sum(payload_sizes), 1450);
EXPECT_THAT(payload_sizes, Each(Gt(0)));
}
TEST(RtpPacketizerSplitAboutEqually,
@ -232,5 +220,55 @@ TEST(RtpPacketizerSplitAboutEqually,
EXPECT_THAT(RtpPacketizer::SplitAboutEqually(20, limits), ElementsAre(9, 11));
}
TEST(RtpPacketizerSplitAboutEqually, RejectsZeroMaxPayloadLen) {
RtpPacketizer::PayloadSizeLimits limits;
limits.max_payload_len = 0;
EXPECT_THAT(RtpPacketizer::SplitAboutEqually(20, limits), IsEmpty());
}
TEST(RtpPacketizerSplitAboutEqually, RejectsZeroFirstPacketLen) {
RtpPacketizer::PayloadSizeLimits limits;
limits.max_payload_len = 5;
limits.first_packet_reduction_len = 5;
EXPECT_THAT(RtpPacketizer::SplitAboutEqually(20, limits), IsEmpty());
}
TEST(RtpPacketizerSplitAboutEqually, RejectsZeroLastPacketLen) {
RtpPacketizer::PayloadSizeLimits limits;
limits.max_payload_len = 5;
limits.last_packet_reduction_len = 5;
EXPECT_THAT(RtpPacketizer::SplitAboutEqually(20, limits), IsEmpty());
}
TEST(RtpPacketizerSplitAboutEqually, CantPutSinglePayloadByteInTwoPackets) {
RtpPacketizer::PayloadSizeLimits limits;
limits.max_payload_len = 10;
limits.first_packet_reduction_len = 6;
limits.last_packet_reduction_len = 4;
EXPECT_THAT(RtpPacketizer::SplitAboutEqually(1, limits), IsEmpty());
}
TEST(RtpPacketizerSplitAboutEqually, CanPutTwoPayloadBytesInTwoPackets) {
RtpPacketizer::PayloadSizeLimits limits;
limits.max_payload_len = 10;
limits.first_packet_reduction_len = 6;
limits.last_packet_reduction_len = 4;
EXPECT_THAT(RtpPacketizer::SplitAboutEqually(2, limits), ElementsAre(1, 1));
}
TEST(RtpPacketizerSplitAboutEqually, CanPutSinglePayloadByteInOnePacket) {
RtpPacketizer::PayloadSizeLimits limits;
limits.max_payload_len = 11;
limits.first_packet_reduction_len = 6;
limits.last_packet_reduction_len = 4;
EXPECT_THAT(RtpPacketizer::SplitAboutEqually(1, limits), ElementsAre(1));
}
} // namespace
} // namespace webrtc

View File

@ -52,8 +52,8 @@ class RtpPacketizerGeneric : public RtpPacketizer {
uint8_t header_[3];
size_t header_size_;
rtc::ArrayView<const uint8_t> remaining_payload_;
std::vector<size_t> payload_sizes_;
std::vector<size_t>::const_iterator current_packet_;
std::vector<int> payload_sizes_;
std::vector<int>::const_iterator current_packet_;
RTC_DISALLOW_COPY_AND_ASSIGN(RtpPacketizerGeneric);
};

View File

@ -33,10 +33,9 @@ using ::testing::Contains;
constexpr RtpPacketizer::PayloadSizeLimits kNoSizeLimits;
std::vector<size_t> NextPacketFillPayloadSizes(
RtpPacketizerGeneric* packetizer) {
std::vector<int> NextPacketFillPayloadSizes(RtpPacketizerGeneric* packetizer) {
RtpPacketToSend packet(nullptr);
std::vector<size_t> result;
std::vector<int> result;
while (packetizer->NextPacket(&packet)) {
result.push_back(packet.payload_size());
}
@ -52,7 +51,7 @@ TEST(RtpPacketizerVideoGeneric, RespectsMaxPayloadSize) {
RtpPacketizerGeneric packetizer(kPayload, limits, RTPVideoHeader(),
kVideoFrameKey);
std::vector<size_t> payload_sizes = NextPacketFillPayloadSizes(&packetizer);
std::vector<int> payload_sizes = NextPacketFillPayloadSizes(&packetizer);
EXPECT_THAT(payload_sizes, Each(Le(limits.max_payload_len)));
}
@ -66,7 +65,7 @@ TEST(RtpPacketizerVideoGeneric, UsesMaxPayloadSize) {
RtpPacketizerGeneric packetizer(kPayload, limits, RTPVideoHeader(),
kVideoFrameKey);
std::vector<size_t> payload_sizes = NextPacketFillPayloadSizes(&packetizer);
std::vector<int> payload_sizes = NextPacketFillPayloadSizes(&packetizer);
// With kPayloadSize > max_payload_len^2, there should be packets that use
// all the payload, otherwise it is possible to use less packets.
@ -94,7 +93,7 @@ TEST(RtpPacketizerVideoGeneric, WritesExtendedHeaderWhenPictureIdIsSet) {
}
TEST(RtpPacketizerVideoGeneric, RespectsMaxPayloadSizeWithExtendedHeader) {
const size_t kPayloadSize = 50;
const int kPayloadSize = 50;
const uint8_t kPayload[kPayloadSize] = {};
RtpPacketizer::PayloadSizeLimits limits;
@ -104,13 +103,13 @@ TEST(RtpPacketizerVideoGeneric, RespectsMaxPayloadSizeWithExtendedHeader) {
RtpPacketizerGeneric packetizer(kPayload, limits, rtp_video_header,
kVideoFrameKey);
std::vector<size_t> payload_sizes = NextPacketFillPayloadSizes(&packetizer);
std::vector<int> payload_sizes = NextPacketFillPayloadSizes(&packetizer);
EXPECT_THAT(payload_sizes, Each(Le(limits.max_payload_len)));
}
TEST(RtpPacketizerVideoGeneric, UsesMaxPayloadSizeWithExtendedHeader) {
const size_t kPayloadSize = 50;
const int kPayloadSize = 50;
const uint8_t kPayload[kPayloadSize] = {};
RtpPacketizer::PayloadSizeLimits limits;
@ -119,7 +118,7 @@ TEST(RtpPacketizerVideoGeneric, UsesMaxPayloadSizeWithExtendedHeader) {
rtp_video_header.generic.emplace().frame_id = 37;
RtpPacketizerGeneric packetizer(kPayload, limits, rtp_video_header,
kVideoFrameKey);
std::vector<size_t> payload_sizes = NextPacketFillPayloadSizes(&packetizer);
std::vector<int> payload_sizes = NextPacketFillPayloadSizes(&packetizer);
// With kPayloadSize > max_payload_len^2, there should be packets that use
// all the payload, otherwise it is possible to use less packets.
@ -127,7 +126,7 @@ TEST(RtpPacketizerVideoGeneric, UsesMaxPayloadSizeWithExtendedHeader) {
}
TEST(RtpPacketizerVideoGeneric, FrameIdOver15bitsWrapsAround) {
const size_t kPayloadSize = 13;
const int kPayloadSize = 13;
const uint8_t kPayload[kPayloadSize] = {};
RTPVideoHeader rtp_video_header;
@ -146,7 +145,7 @@ TEST(RtpPacketizerVideoGeneric, FrameIdOver15bitsWrapsAround) {
}
TEST(RtpPacketizerVideoGeneric, NoFrameIdDoesNotWriteExtendedHeader) {
const size_t kPayloadSize = 13;
const int kPayloadSize = 13;
const uint8_t kPayload[kPayloadSize] = {};
RtpPacketizerGeneric packetizer(kPayload, kNoSizeLimits, RTPVideoHeader(),

View File

@ -173,13 +173,6 @@ RtpPacketizerVp8::RtpPacketizerVp8(rtc::ArrayView<const uint8_t> payload,
PayloadSizeLimits limits,
const RTPVideoHeaderVP8& hdr_info)
: hdr_(BuildHeader(hdr_info)), remaining_payload_(payload) {
if (limits.max_payload_len - limits.last_packet_reduction_len <
hdr_.size() + 1) {
// The provided payload length is not long enough for the payload
// descriptor and one payload byte in the last packet.
current_packet_ = payload_sizes_.begin();
return;
}
limits.max_payload_len -= hdr_.size();
payload_sizes_ = SplitAboutEqually(payload.size(), limits);
current_packet_ = payload_sizes_.begin();

View File

@ -61,8 +61,8 @@ class RtpPacketizerVp8 : public RtpPacketizer {
RawHeader hdr_;
rtc::ArrayView<const uint8_t> remaining_payload_;
std::vector<size_t> payload_sizes_;
std::vector<size_t>::const_iterator current_packet_;
std::vector<int> payload_sizes_;
std::vector<int>::const_iterator current_packet_;
RTC_DISALLOW_COPY_AND_ASSIGN(RtpPacketizerVp8);
};

View File

@ -335,20 +335,18 @@ bool RTPSenderVideo::SendVideo(enum VideoCodecType video_type,
retransmission_settings = retransmission_settings_;
}
size_t packet_capacity = rtp_sender_->MaxRtpPacketSize() -
fec_packet_overhead -
(rtp_sender_->RtxStatus() ? kRtxHeaderSize : 0);
int packet_capacity = rtp_sender_->MaxRtpPacketSize() - fec_packet_overhead -
(rtp_sender_->RtxStatus() ? kRtxHeaderSize : 0);
RTC_DCHECK_LE(packet_capacity, rtp_header->capacity());
RTC_DCHECK_GT(packet_capacity, rtp_header->headers_size());
RTC_DCHECK_GT(packet_capacity, last_packet->headers_size());
size_t max_data_payload_length = packet_capacity - rtp_header->headers_size();
RtpPacketizer::PayloadSizeLimits limits;
limits.max_payload_len = packet_capacity - rtp_header->headers_size();
RTC_DCHECK_GE(last_packet->headers_size(), rtp_header->headers_size());
size_t last_packet_reduction_len =
limits.last_packet_reduction_len =
last_packet->headers_size() - rtp_header->headers_size();
RtpPacketizer::PayloadSizeLimits limits;
limits.max_payload_len = max_data_payload_length;
limits.last_packet_reduction_len = last_packet_reduction_len;
std::unique_ptr<RtpPacketizer> packetizer = RtpPacketizer::Create(
video_type, rtc::MakeArrayView(payload_data, payload_size), limits,
*video_header, frame_type, fragmentation);
@ -368,9 +366,10 @@ bool RTPSenderVideo::SendVideo(enum VideoCodecType video_type,
: absl::make_unique<RtpPacketToSend>(*rtp_header);
if (!packetizer->NextPacket(packet.get()))
return false;
RTC_DCHECK_LE(packet->payload_size(),
last ? max_data_payload_length - last_packet_reduction_len
: max_data_payload_length);
RTC_DCHECK_LE(
packet->payload_size(),
last ? limits.max_payload_len - limits.last_packet_reduction_len
: limits.max_payload_len);
if (!rtp_sender_->AssignSequenceNumber(packet.get()))
return false;