From 190a7606321b53c861142f97fc39f679b295f31c Mon Sep 17 00:00:00 2001
From: Danil Chapovalov <[email protected]>
Date: Thu, 23 Jul 2026 09:30:33 +0200
Subject: [PATCH] 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 <[email protected]>
Commit-Queue: Danil Chapovalov <[email protected]>
Cr-Commit-Position: refs/heads/main@{#48214}
---
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_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;
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/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_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;