Use single packet limit when all fragments end up in one H.264 packet
Update RtpPacketizerH264::PacketizeStapA to use
single_packet_reduction_len when all fragments end up in one H.264
packet.
Previous code was using first_packet_reduction_len +
last_packet_reduction_len for this case, which can cause an occasional
RTC_CHECK crash in RtpPacketizerH264::NextAggregatePacket due to
exceeding the available payload capacity of an RTP packet.
Bug: webrtc:15477
Change-Id: Iba1564a6a29618bef22f19d82aba938420994b23
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/319645
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40737}
diff --git a/modules/rtp_rtcp/source/rtp_format_h264.cc b/modules/rtp_rtcp/source/rtp_format_h264.cc
index cc8d1bf..d066baf 100644
--- a/modules/rtp_rtcp/source/rtp_format_h264.cc
+++ b/modules/rtp_rtcp/source/rtp_format_h264.cc
@@ -153,28 +153,25 @@
size_t RtpPacketizerH264::PacketizeStapA(size_t fragment_index) {
// Aggregate fragments into one packet (STAP-A).
size_t payload_size_left = limits_.max_payload_len;
- if (input_fragments_.size() == 1)
- payload_size_left -= limits_.single_packet_reduction_len;
- else if (fragment_index == 0)
- payload_size_left -= limits_.first_packet_reduction_len;
int aggregated_fragments = 0;
size_t fragment_headers_length = 0;
rtc::ArrayView<const uint8_t> fragment = input_fragments_[fragment_index];
RTC_CHECK_GE(payload_size_left, fragment.size());
++num_packets_left_;
+ const bool has_first_fragment = fragment_index == 0;
auto payload_size_needed = [&] {
size_t fragment_size = fragment.size() + fragment_headers_length;
- if (input_fragments_.size() == 1) {
- // Single fragment, single packet, payload_size_left already adjusted
- // with limits_.single_packet_reduction_len.
+ bool has_last_fragment = fragment_index == input_fragments_.size() - 1;
+ if (has_first_fragment && has_last_fragment) {
+ return fragment_size + limits_.single_packet_reduction_len;
+ } else if (has_first_fragment) {
+ return fragment_size + limits_.first_packet_reduction_len;
+ } else if (has_last_fragment) {
+ return fragment_size + limits_.last_packet_reduction_len;
+ } else {
return fragment_size;
}
- if (fragment_index == input_fragments_.size() - 1) {
- // Last fragment, so STAP-A might be the last packet.
- return fragment_size + limits_.last_packet_reduction_len;
- }
- return fragment_size;
};
while (payload_size_left >= payload_size_needed()) {
diff --git a/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc b/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc
index 80d8801..18311c6 100644
--- a/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_format_h264_unittest.cc
@@ -285,14 +285,50 @@
0, 2, nalus[2][0], nalus[2][1]));
}
+TEST(RtpPacketizerH264Test, StapARespectsSinglePacketReduction) {
+ RtpPacketizer::PayloadSizeLimits limits;
+ limits.max_payload_len = 1000;
+ // It is possible for single_packet_reduction_len to be greater than
+ // first_packet_reduction_len + last_packet_reduction_len. Check that the
+ // right limit is used when first and last fragment go to one packet.
+ limits.first_packet_reduction_len = 4;
+ limits.last_packet_reduction_len = 0;
+ limits.single_packet_reduction_len = 8;
+ // 3 fragments of sizes 2 + 2 + 981, plus 7 bytes of headers, is expected to
+ // be packetized to single packet of size 992.
+ rtc::Buffer first_nalus[] = {GenerateNalUnit(/*size=*/2),
+ GenerateNalUnit(/*size=*/2),
+ GenerateNalUnit(/*size=*/981)};
+ rtc::Buffer first_frame = CreateFrame(first_nalus);
+
+ RtpPacketizerH264 first_packetizer(first_frame, limits,
+ H264PacketizationMode::NonInterleaved);
+ std::vector<RtpPacketToSend> packets = FetchAllPackets(&first_packetizer);
+
+ // Expect that everything fits in a single packet.
+ ASSERT_THAT(packets, SizeIs(1));
+ EXPECT_EQ(packets[0].payload_size(), 992u);
+
+ // Increasing the last fragment size by one exceeds
+ // single_packet_reduction_len and produces two packets.
+ rtc::Buffer second_nalus[] = {GenerateNalUnit(/*size=*/2),
+ GenerateNalUnit(/*size=*/2),
+ GenerateNalUnit(/*size=*/982)};
+ rtc::Buffer second_frame = CreateFrame(second_nalus);
+ RtpPacketizerH264 second_packetizer(second_frame, limits,
+ H264PacketizationMode::NonInterleaved);
+ packets = FetchAllPackets(&second_packetizer);
+ ASSERT_THAT(packets, SizeIs(2));
+}
+
TEST(RtpPacketizerH264Test, StapARespectsLastPacketReduction) {
RtpPacketizer::PayloadSizeLimits limits;
limits.max_payload_len = 1000;
limits.last_packet_reduction_len = 100;
+ const size_t kFirstFragmentSize = 1000;
const size_t kLastFragmentSize =
- limits.max_payload_len - limits.last_packet_reduction_len;
- rtc::Buffer nalus[] = {GenerateNalUnit(/*size=*/2),
- GenerateNalUnit(/*size=*/2),
+ limits.max_payload_len - limits.last_packet_reduction_len + 1;
+ rtc::Buffer nalus[] = {GenerateNalUnit(/*size=*/kFirstFragmentSize),
GenerateNalUnit(/*size=*/kLastFragmentSize)};
rtc::Buffer frame = CreateFrame(nalus);
@@ -300,14 +336,13 @@
H264PacketizationMode::NonInterleaved);
std::vector<RtpPacketToSend> packets = FetchAllPackets(&packetizer);
- ASSERT_THAT(packets, SizeIs(2));
- // Expect 1st packet is aggregate of 1st two fragments.
- EXPECT_THAT(packets[0].payload(),
- ElementsAre(kStapA, //
- 0, 2, nalus[0][0], nalus[0][1], //
- 0, 2, nalus[1][0], nalus[1][1]));
- // Expect 2nd packet is single nalu.
- EXPECT_THAT(packets[1].payload(), ElementsAreArray(nalus[2]));
+ ASSERT_THAT(packets, SizeIs(3));
+ // Expect 1st packet contains first fragment.
+ EXPECT_THAT(packets[0].payload()[0], kSlice);
+ // Expect 2nd and 3rd packets to be FU-A since last_packet_reduction_len
+ // was exceeded by one byte.
+ EXPECT_THAT(packets[1].payload()[0], kFuA);
+ EXPECT_THAT(packets[2].payload()[0], kFuA);
}
TEST(RtpPacketizerH264Test, TooSmallForStapAHeaders) {
