Limit size of a video frame when packetizing it To avoid various corner cases when frame is too large. Such large frames are impractical for RTP. Add a throttled log when a video frame is dropped by a packetizer. Bug: chromium:503013378 Change-Id: Ie8c55d7c3976f0cf7420163e30e4f95f21a8b3d2 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/490600 Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org> Commit-Queue: Danil Chapovalov <danilchap@webrtc.org> Cr-Commit-Position: refs/heads/main@{#48214}
diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn index c73c903..d496d99 100644 --- a/modules/rtp_rtcp/BUILD.gn +++ b/modules/rtp_rtcp/BUILD.gn
@@ -340,6 +340,7 @@ "../../rtc_base:rate_limiter", "../../rtc_base:rtc_numerics", "../../rtc_base:rtp_to_ntp_estimator", + "../../rtc_base:safe_compare", "../../rtc_base:safe_conversions", "../../rtc_base:safe_minmax", "../../rtc_base:timeutils",
diff --git a/modules/rtp_rtcp/source/rtp_format.cc b/modules/rtp_rtcp/source/rtp_format.cc index 5d3c880..75d00b1 100644 --- a/modules/rtp_rtcp/source/rtp_format.cc +++ b/modules/rtp_rtcp/source/rtp_format.cc
@@ -10,6 +10,7 @@ #include "modules/rtp_rtcp/source/rtp_format.h" +#include <cstddef> #include <cstdint> #include <memory> #include <span> @@ -24,6 +25,8 @@ #include "modules/video_coding/codecs/vp8/include/vp8_globals.h" #include "modules/video_coding/codecs/vp9/include/vp9_globals.h" #include "rtc_base/checks.h" +#include "rtc_base/numerics/safe_compare.h" + #ifdef RTC_ENABLE_H265 #include "modules/rtp_rtcp/source/rtp_packetizer_h265.h" #endif @@ -77,14 +80,22 @@ } std::vector<int> RtpPacketizer::SplitAboutEqually( - int payload_len, + size_t payload_size, const PayloadSizeLimits& limits) { - 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 (payload_size == 0 || + SafeGt(payload_size, limits.max_payload_len * 0x7000)) { + // Do not support frames that are so large they need almost half of the RTP + // sequence number space. With MTU ~= 1.2KB that puts a limit of ~34MB on a + // single frame. Sending such large frames over RTP is likely impractical. + return result; + } + int payload_len = static_cast<int>(payload_size); + if (limits.max_payload_len >= limits.single_packet_reduction_len + payload_len) { result.push_back(payload_len);
diff --git a/modules/rtp_rtcp/source/rtp_format.h b/modules/rtp_rtcp/source/rtp_format.h index e329109..214594b 100644 --- a/modules/rtp_rtcp/source/rtp_format.h +++ b/modules/rtp_rtcp/source/rtp_format.h
@@ -60,9 +60,9 @@ // Returns true on success, false otherwise. virtual bool NextPacket(RtpPacketToSend* packet) = 0; - // Split payload_len into sum of integers with respect to `limits`. + // Splits `payload_size` into sum of integers with respect to `limits`. // Returns empty vector on failure. - static std::vector<int> SplitAboutEqually(int payload_len, + static std::vector<int> SplitAboutEqually(size_t payload_size, const PayloadSizeLimits& limits); }; } // namespace webrtc
diff --git a/modules/rtp_rtcp/source/rtp_format_unittest.cc b/modules/rtp_rtcp/source/rtp_format_unittest.cc index d67e17a..4cb5153 100644 --- a/modules/rtp_rtcp/source/rtp_format_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_format_unittest.cc
@@ -231,6 +231,21 @@ EXPECT_THAT(RtpPacketizer::SplitAboutEqually(20, limits), ElementsAre(9, 11)); } +TEST(RtpPacketizerSplitAboutEqually, RejectsZeroSize) { + RtpPacketizer::PayloadSizeLimits limits; + limits.max_payload_len = 1200; + + EXPECT_THAT(RtpPacketizer::SplitAboutEqually(0, limits), IsEmpty()); +} + +TEST(RtpPacketizerSplitAboutEqually, RejectsHugeSize) { + RtpPacketizer::PayloadSizeLimits limits; + limits.max_payload_len = 1200; + + EXPECT_THAT(RtpPacketizer::SplitAboutEqually(0xFFFF'FFFF, limits), IsEmpty()); + EXPECT_THAT(RtpPacketizer::SplitAboutEqually(40'000'000, limits), IsEmpty()); +} + TEST(RtpPacketizerSplitAboutEqually, RejectsZeroMaxPayloadLen) { RtpPacketizer::PayloadSizeLimits limits; limits.max_payload_len = 0;
diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index d5d2fa0..1bb3c8a 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc
@@ -710,8 +710,16 @@ const size_t num_packets = packetizer->NumPackets(); - if (num_packets == 0) + if (num_packets == 0) { + Timestamp now = clock_->CurrentTime(); + if (now >= last_fail_packetize_log_ + TimeDelta::Seconds(10)) { + RTC_LOG(LS_WARNING) << "Failed to packetize " << codec_type + << " video frame of size " << payload.size() + << ". Frame is dropped."; + last_fail_packetize_log_ = now; + } return false; + } bool first_frame = first_frame_sent_(); std::vector<std::unique_ptr<RtpPacketToSend>> rtp_packets;
diff --git a/modules/rtp_rtcp/source/rtp_sender_video.h b/modules/rtp_rtcp/source/rtp_sender_video.h index 396591a..91d0360 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.h +++ b/modules/rtp_rtcp/source/rtp_sender_video.h
@@ -240,6 +240,8 @@ RTC_GUARDED_BY(stats_mutex_); OneTimeEvent first_frame_sent_; + Timestamp last_fail_packetize_log_ RTC_GUARDED_BY(send_checker_) = + Timestamp::MinusInfinity(); // E2EE Custom Video Frame Encryptor (optional) FrameEncryptorInterface* const frame_encryptor_ = nullptr;