From bc3be2e781abb356eee95f3cea3afa5be8b10c7e Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Mon, 16 Jun 2025 19:38:22 +0000 Subject: [PATCH] Switch video_{send,recv}_stream to StringBuilder It's been shown that it's hard to estimate the size of the debug messages that these functions build; fixed size buffers are not worth the effort to maintain. Example CL that triggered an overflow: https://webrtc-review.googlesource.com/c/src/+/396640 - possibly because a debug message logged an extra parameter. Bug: None Change-Id: I47d924bb5d490d2165cbc441c307cb2396c7ce37 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/396721 Commit-Queue: Harald Alvestrand Reviewed-by: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#44950} --- call/video_receive_stream.cc | 34 +++++++++++++++------------------- call/video_send_stream.cc | 15 ++++++--------- 2 files changed, 21 insertions(+), 28 deletions(-) diff --git a/call/video_receive_stream.cc b/call/video_receive_stream.cc index 3bfb35297f..c16cd59b4d 100644 --- a/call/video_receive_stream.cc +++ b/call/video_receive_stream.cc @@ -36,8 +36,7 @@ bool VideoReceiveStreamInterface::Decoder::operator==( } std::string VideoReceiveStreamInterface::Decoder::ToString() const { - char buf[1024]; - SimpleStringBuilder ss(buf); + StringBuilder ss; ss << "{payload_type: " << payload_type; ss << ", payload_name: " << video_format.name; ss << ", codec_params: {"; @@ -48,8 +47,8 @@ std::string VideoReceiveStreamInterface::Decoder::ToString() const { } ss << it->first << ": " << it->second; } - ss << '}'; - ss << '}'; + ss << "}"; + ss << "}"; return ss.str(); } @@ -59,8 +58,7 @@ VideoReceiveStreamInterface::Stats::~Stats() = default; std::string VideoReceiveStreamInterface::Stats::ToString( int64_t time_ms) const { - char buf[2048]; - SimpleStringBuilder ss(buf); + StringBuilder ss; ss << "VideoReceiveStreamInterface stats: " << time_ms << ", {ssrc: " << ssrc << ", "; ss << "total_bps: " << total_bitrate_bps << ", "; @@ -102,7 +100,7 @@ std::string VideoReceiveStreamInterface::Stats::ToString( ss << "nackCount: " << rtcp_packet_type_counts.nack_packets << ", "; ss << "firCount: " << rtcp_packet_type_counts.fir_packets << ", "; ss << "pliCount: " << rtcp_packet_type_counts.pli_packets; - ss << '}'; + ss << "}"; return ss.str(); } @@ -119,21 +117,20 @@ VideoReceiveStreamInterface::Config::operator=(Config&&) = default; VideoReceiveStreamInterface::Config::Config::~Config() = default; std::string VideoReceiveStreamInterface::Config::ToString() const { - char buf[4 * 1024]; - SimpleStringBuilder ss(buf); + StringBuilder ss; ss << "{decoders: ["; for (size_t i = 0; i < decoders.size(); ++i) { ss << decoders[i].ToString(); if (i != decoders.size() - 1) ss << ", "; } - ss << ']'; + ss << "]"; ss << ", rtp: " << rtp.ToString(); ss << ", renderer: " << (renderer ? "(renderer)" : "nullptr"); ss << ", render_delay_ms: " << render_delay_ms; if (!sync_group.empty()) ss << ", sync_group: " << sync_group; - ss << '}'; + ss << "}"; return ss.str(); } @@ -143,8 +140,7 @@ VideoReceiveStreamInterface::Config::Rtp::Rtp(const Rtp&) = default; VideoReceiveStreamInterface::Config::Rtp::~Rtp() = default; std::string VideoReceiveStreamInterface::Config::Rtp::ToString() const { - char buf[2 * 1024]; - SimpleStringBuilder ss(buf); + StringBuilder ss; ss << "{remote_ssrc: " << remote_ssrc; ss << ", local_ssrc: " << local_ssrc; ss << ", rtcp_mode: " @@ -153,9 +149,9 @@ std::string VideoReceiveStreamInterface::Config::Rtp::ToString() const { ss << ", rtcp_xr: "; ss << "{receiver_reference_time_report: " << (rtcp_xr.receiver_reference_time_report ? "on" : "off"); - ss << '}'; - ss << ", lntf: {enabled: " << (lntf.enabled ? "true" : "false") << '}'; - ss << ", nack: {rtp_history_ms: " << nack.rtp_history_ms << '}'; + ss << "}"; + ss << ", lntf: {enabled: " << (lntf.enabled ? "true" : "false") << "}"; + ss << ", nack: {rtp_history_ms: " << nack.rtp_history_ms << "}"; ss << ", ulpfec_payload_type: " << ulpfec_payload_type; ss << ", red_type: " << red_payload_type; ss << ", rtx_ssrc: " << rtx_ssrc; @@ -163,13 +159,13 @@ std::string VideoReceiveStreamInterface::Config::Rtp::ToString() const { for (auto& kv : rtx_associated_payload_types) { ss << kv.first << " (pt) -> " << kv.second << " (apt), "; } - ss << '}'; + ss << "}"; ss << ", raw_payload_types: {"; for (const auto& pt : raw_payload_types) { ss << pt << ", "; } - ss << '}'; - ss << '}'; + ss << "}"; + ss << "}"; return ss.str(); } diff --git a/call/video_send_stream.cc b/call/video_send_stream.cc index 79a911c828..e68f73bbe8 100644 --- a/call/video_send_stream.cc +++ b/call/video_send_stream.cc @@ -42,8 +42,7 @@ VideoSendStream::StreamStats::StreamStats() = default; VideoSendStream::StreamStats::~StreamStats() = default; std::string VideoSendStream::StreamStats::ToString() const { - char buf[1024]; - SimpleStringBuilder ss(buf); + StringBuilder ss; ss << "type: " << StreamTypeToString(type); if (referenced_media_ssrc.has_value()) ss << " (for: " << referenced_media_ssrc.value() << ")"; @@ -71,8 +70,7 @@ VideoSendStream::Stats::Stats() = default; VideoSendStream::Stats::~Stats() = default; std::string VideoSendStream::Stats::ToString(int64_t time_ms) const { - char buf[2048]; - SimpleStringBuilder ss(buf); + StringBuilder ss; ss << "VideoSendStream stats: " << time_ms << ", {"; ss << "input_fps: " << StringFormat("%.1f", input_frame_rate) << ", "; ss << "encode_fps: " << encode_frame_rate << ", "; @@ -90,13 +88,13 @@ std::string VideoSendStream::Stats::ToString(int64_t time_ms) const { << ", "; ss << "#cpu_adaptations: " << number_of_cpu_adapt_changes << ", "; ss << "#quality_adaptations: " << number_of_quality_adapt_changes; - ss << '}'; + ss << "}"; for (const auto& substream : substreams) { if (substream.second.type == VideoSendStream::StreamStats::StreamType::kMedia) { ss << " {ssrc: " << substream.first << ", "; ss << substream.second.ToString(); - ss << '}'; + ss << "}"; } } return ss.str(); @@ -113,8 +111,7 @@ VideoSendStream::Config& VideoSendStream::Config::operator=(Config&&) = default; VideoSendStream::Config::Config::~Config() = default; std::string VideoSendStream::Config::ToString() const { - char buf[2 * 1024]; - SimpleStringBuilder ss(buf); + StringBuilder ss; ss << "{encoder_settings: { experiment_cpu_load_estimator: " << (encoder_settings.experiment_cpu_load_estimator ? "on" : "off") << "}}"; ss << ", rtp: " << rtp.ToString(); @@ -124,7 +121,7 @@ std::string VideoSendStream::Config::ToString() const { ss << ", target_delay_ms: " << target_delay_ms; ss << ", suspend_below_min_bitrate: " << (suspend_below_min_bitrate ? "on" : "off"); - ss << '}'; + ss << "}"; return ss.str(); }