Remove stringstream fallback from MakeVal in logging.h This change removes the generic MakeVal fallback in logging.h that relied on std::ostringstream. This hasn't been used in webrtc for years but now known downstream dependencies have been removed in favor of more efficient alternatives like StringBuilder and Abseil's string utilities. Existing call sites and related code in tests and tools have similarly been migrated: * Replaced std::ostringstream and std::stringstream with StringBuilder in SDP, Opus, and OpenSSL unit tests. * Refactored string parsing in neteq_quality_test and bwe_rtp to use absl::StrSplit and Abseil's numeric conversion functions. * Updated RTC_LOG calls in cross_traffic_unittest to explicitly load atomic counters (and not rely on stringstream to do it). * Updated BUILD.gn and DEPS to include required Abseil and string utility dependencies. Bug: webrtc:42234461 Change-Id: I7fbe4b68478fb1aca08c6126979ae577ef5418da Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/466441 Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org> Reviewed-by: Jakob Ivarsson‎ <jakobi@webrtc.org> Cr-Commit-Position: refs/heads/main@{#47577}
diff --git a/api/webrtc_sdp_unittest.cc b/api/webrtc_sdp_unittest.cc index 108b530..b6b009d 100644 --- a/api/webrtc_sdp_unittest.cc +++ b/api/webrtc_sdp_unittest.cc
@@ -14,7 +14,6 @@ #include <map> #include <memory> #include <optional> -#include <sstream> #include <string> #include <utility> #include <vector> @@ -1856,8 +1855,8 @@ "a=rtpmap:105 telephone-event/8000\r\n" "a=fmtp:105 0-15,66,70\r\n" "a=fmtp:111 "; - std::ostringstream os; - os << "minptime=" << params.min_ptime << "; stereo=" << params.stereo + StringBuilder sb; + sb << "minptime=" << params.min_ptime << "; stereo=" << params.stereo << "; sprop-stereo=" << params.sprop_stereo << "; useinbandfec=" << params.useinband << "; maxaveragebitrate=" << params.maxaveragebitrate @@ -1867,17 +1866,15 @@ << "\r\n" "a=maxptime:" << params.max_ptime << "\r\n"; - sdp += os.str(); + sdp += sb.Release(); - os.clear(); - os.str(""); // Pl type 100 preferred. - os << "m=video 9 RTP/SAVPF 99 95 96\r\n" - "a=rtpmap:96 VP9/90000\r\n" // out-of-order wrt the m= line. - "a=rtpmap:99 VP8/90000\r\n" - "a=rtpmap:95 RTX/90000\r\n" - "a=fmtp:95 apt=99;\r\n"; - sdp += os.str(); + sdp += + "m=video 9 RTP/SAVPF 99 95 96\r\n" + "a=rtpmap:96 VP9/90000\r\n" // out-of-order wrt the m= line. + "a=rtpmap:99 VP8/90000\r\n" + "a=rtpmap:95 RTX/90000\r\n" + "a=fmtp:95 apt=99;\r\n"; // Deserialize SdpParseError error; @@ -1949,18 +1946,18 @@ "a=rtcp-fb:101 nack\r\n" "a=rtcp-fb:101 nack pli\r\n" "a=rtcp-fb:101 goog-remb\r\n"; - std::ostringstream os; - os << sdp_session_and_audio; - os << "a=rtcp-fb:" << (use_wildcard ? "*" : "111") << " nack\r\n"; + StringBuilder sb; + sb << sdp_session_and_audio; + sb << "a=rtcp-fb:" << (use_wildcard ? "*" : "111") << " nack\r\n"; if (use_ccfb) { - os << "a=rtcp-fb:" << (use_wildcard ? "*" : "111") << " ack ccfb\r\n"; + sb << "a=rtcp-fb:" << (use_wildcard ? "*" : "111") << " ack ccfb\r\n"; } - os << sdp_video; - os << "a=rtcp-fb:" << (use_wildcard ? "*" : "101") << " ccm fir\r\n"; + sb << sdp_video; + sb << "a=rtcp-fb:" << (use_wildcard ? "*" : "101") << " ccm fir\r\n"; if (use_ccfb) { - os << "a=rtcp-fb:" << (use_wildcard ? "*" : "101") << " ack ccfb\r\n"; + sb << "a=rtcp-fb:" << (use_wildcard ? "*" : "101") << " ack ccfb\r\n"; } - std::string sdp = os.str(); + std::string sdp = sb.Release(); // Deserialize SdpParseError error; jdesc_output = SdpDeserialize(sdp, &error);
diff --git a/modules/audio_coding/BUILD.gn b/modules/audio_coding/BUILD.gn index 337f9b1..dd4e7bd 100644 --- a/modules/audio_coding/BUILD.gn +++ b/modules/audio_coding/BUILD.gn
@@ -1138,6 +1138,7 @@ "../../test:test_support", "//testing/gtest", "//third_party/abseil-cpp/absl/flags:flag", + "//third_party/abseil-cpp/absl/strings", "//third_party/abseil-cpp/absl/strings:string_view", ] }
diff --git a/modules/audio_coding/neteq/tools/DEPS b/modules/audio_coding/neteq/tools/DEPS index 4db1e1d..a196a56 100644 --- a/modules/audio_coding/neteq/tools/DEPS +++ b/modules/audio_coding/neteq/tools/DEPS
@@ -1,3 +1,5 @@ include_rules = [ + "+absl/strings/numbers.h", + "+absl/strings/str_split.h", "+logging/rtc_event_log", ]
diff --git a/modules/audio_coding/neteq/tools/neteq_quality_test.cc b/modules/audio_coding/neteq/tools/neteq_quality_test.cc index 666d2e9..8246f89 100644 --- a/modules/audio_coding/neteq/tools/neteq_quality_test.cc +++ b/modules/audio_coding/neteq/tools/neteq_quality_test.cc
@@ -10,24 +10,23 @@ #include "modules/audio_coding/neteq/tools/neteq_quality_test.h" -#include <algorithm> #include <climits> #include <cmath> #include <cstdint> #include <cstdio> #include <cstdlib> #include <fstream> -#include <iterator> #include <memory> #include <ostream> #include <set> #include <span> -#include <sstream> #include <string> #include <utility> #include <vector> #include "absl/flags/flag.h" +#include "absl/strings/numbers.h" +#include "absl/strings/str_split.h" #include "absl/strings/string_view.h" #include "api/audio_codecs/audio_decoder_factory.h" #include "api/audio_codecs/audio_format.h" @@ -36,7 +35,6 @@ #include "api/neteq/neteq.h" #include "api/scoped_refptr.h" #include "api/units/timestamp.h" -#include "modules/audio_coding/neteq/tools/neteq_quality_test.h" #include "modules/audio_coding/neteq/tools/output_audio_file.h" #include "modules/audio_coding/neteq/tools/output_wav_file.h" #include "modules/audio_coding/neteq/tools/resample_input_audio_file.h" @@ -376,15 +374,17 @@ break; } case kFixedLoss: { - std::istringstream loss_events_stream(absl::GetFlag(FLAGS_loss_events)); - std::string loss_event_string; std::set<FixedLossEvent, FixedLossEventCmp> loss_events; - while (std::getline(loss_events_stream, loss_event_string, ',')) { + for (absl::string_view loss_event_string : absl::StrSplit( + absl::GetFlag(FLAGS_loss_events), ',', absl::SkipEmpty())) { std::vector<int> loss_event_params; - std::istringstream loss_event_params_stream(loss_event_string); - std::copy(std::istream_iterator<int>(loss_event_params_stream), - std::istream_iterator<int>(), - std::back_inserter(loss_event_params)); + for (absl::string_view param : + absl::StrSplit(loss_event_string, ' ', absl::SkipEmpty())) { + int val; + if (absl::SimpleAtoi(param, &val)) { + loss_event_params.push_back(val); + } + } RTC_CHECK_EQ(loss_event_params.size(), 2); auto result = loss_events.insert( FixedLossEvent(loss_event_params[0], loss_event_params[1]));
diff --git a/modules/audio_coding/test/opus_test.cc b/modules/audio_coding/test/opus_test.cc index 6b511c2..eaa5623 100644 --- a/modules/audio_coding/test/opus_test.cc +++ b/modules/audio_coding/test/opus_test.cc
@@ -13,7 +13,6 @@ #include <cmath> #include <cstddef> #include <cstdint> -#include <sstream> #include <string> #include "api/audio/audio_frame.h" @@ -26,6 +25,7 @@ #include "modules/audio_coding/codecs/opus/opus_interface.h" #include "modules/audio_coding/include/audio_coding_module_typedefs.h" #include "modules/audio_coding/test/TestStereo.h" +#include "rtc_base/strings/string_builder.h" #include "test/gtest.h" #include "test/testsupport/file_utils.h" @@ -393,17 +393,14 @@ } void OpusTest::OpenOutFile(int test_number) { - std::string file_name; - std::stringstream file_stream; - file_stream << test::OutputPath() << "opustest_out_" << test_number << ".pcm"; - file_name = file_stream.str(); - out_file_.Open(file_name, 48000, "wb"); - file_stream.str(""); - file_name = file_stream.str(); - file_stream << test::OutputPath() << "opusstandalone_out_" << test_number - << ".pcm"; - file_name = file_stream.str(); - out_file_standalone_.Open(file_name, 48000, "wb"); + StringBuilder sb; + sb << test::OutputPath() << "opustest_out_" << test_number << ".pcm"; + out_file_.Open(sb.Release(), 48000, "wb"); + + StringBuilder sb_standalone; + sb_standalone << test::OutputPath() << "opusstandalone_out_" << test_number + << ".pcm"; + out_file_standalone_.Open(sb_standalone.Release(), 48000, "wb"); } } // namespace webrtc
diff --git a/modules/remote_bitrate_estimator/BUILD.gn b/modules/remote_bitrate_estimator/BUILD.gn index f4a7acd..765ebda 100644 --- a/modules/remote_bitrate_estimator/BUILD.gn +++ b/modules/remote_bitrate_estimator/BUILD.gn
@@ -134,6 +134,7 @@ "../rtp_rtcp:rtp_rtcp_format", "//third_party/abseil-cpp/absl/flags:flag", "//third_party/abseil-cpp/absl/flags:parse", + "//third_party/abseil-cpp/absl/strings", ] }
diff --git a/modules/remote_bitrate_estimator/DEPS b/modules/remote_bitrate_estimator/DEPS index 35a6211..2386e6f 100644 --- a/modules/remote_bitrate_estimator/DEPS +++ b/modules/remote_bitrate_estimator/DEPS
@@ -1,4 +1,6 @@ include_rules = [ + "+absl/strings/numbers.h", + "+absl/strings/str_split.h", "+logging/rtc_event_log", "+system_wrappers", # Avoid directly using field_trial. Instead use FieldTrialsView.
diff --git a/modules/remote_bitrate_estimator/tools/bwe_rtp.cc b/modules/remote_bitrate_estimator/tools/bwe_rtp.cc index c38f510..7f05e17 100644 --- a/modules/remote_bitrate_estimator/tools/bwe_rtp.cc +++ b/modules/remote_bitrate_estimator/tools/bwe_rtp.cc
@@ -12,14 +12,15 @@ #include <cstdint> #include <cstdio> -#include <ios> #include <memory> #include <set> -#include <sstream> #include <string> #include "absl/flags/flag.h" #include "absl/flags/parse.h" +#include "absl/strings/numbers.h" +#include "absl/strings/str_split.h" +#include "absl/strings/string_view.h" #include "modules/rtp_rtcp/include/rtp_header_extension_map.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "test/rtp_file_reader.h" @@ -52,16 +53,15 @@ std::string ssrc_filter_string = absl::GetFlag(FLAGS_ssrc_filter); if (ssrc_filter_string.empty()) return std::set<uint32_t>(); - std::stringstream ss; - std::string ssrc_filter = ssrc_filter_string; std::set<uint32_t> ssrcs; // Parse the ssrcs in hexadecimal format. - ss << std::hex << ssrc_filter; - uint32_t ssrc; - while (ss >> ssrc) { - ssrcs.insert(ssrc); - ss.ignore(1, ','); + for (absl::string_view s : + absl::StrSplit(ssrc_filter_string, ',', absl::SkipEmpty())) { + uint32_t ssrc; + if (absl::SimpleHexAtoi(s, &ssrc)) { + ssrcs.insert(ssrc); + } } return ssrcs; }
diff --git a/modules/video_capture/BUILD.gn b/modules/video_capture/BUILD.gn index ef4c4ff..5a6fab0 100644 --- a/modules/video_capture/BUILD.gn +++ b/modules/video_capture/BUILD.gn
@@ -194,6 +194,7 @@ "../../common_video", "../../rtc_base:checks", "../../rtc_base:macromagic", + "../../rtc_base:stringutils", "../../rtc_base:timeutils", "../../rtc_base/synchronization:mutex", "../../system_wrappers",
diff --git a/modules/video_capture/test/video_capture_unittest.cc b/modules/video_capture/test/video_capture_unittest.cc index 01a4738..9cdbd96 100644 --- a/modules/video_capture/test/video_capture_unittest.cc +++ b/modules/video_capture/test/video_capture_unittest.cc
@@ -16,7 +16,6 @@ #include <cstring> #include <map> #include <memory> -#include <sstream> #include <string> #include <utility> #include <vector> @@ -32,6 +31,7 @@ #include "modules/video_capture/video_capture_defines.h" #include "modules/video_capture/video_capture_factory.h" #include "rtc_base/checks.h" +#include "rtc_base/strings/string_builder.h" #include "rtc_base/synchronization/mutex.h" #include "rtc_base/thread_annotations.h" #include "system_wrappers/include/clock.h" @@ -357,10 +357,9 @@ VideoCaptureCapability capability; EXPECT_EQ(0, device_info_->GetCapability(module->CurrentDeviceName(), i, capability)); - std::ostringstream resolutionStream; - resolutionStream << capability.width << "x" << capability.height; - resolutionStream.flush(); - std::string resolution = resolutionStream.str(); + StringBuilder sb; + sb << capability.width << "x" << capability.height; + std::string resolution = sb.Release(); frame_rates_by_resolution[resolution].push_back(capability.maxFPS); // Since Android presents so many resolution/FPS combinations and the test
diff --git a/rtc_base/logging.h b/rtc_base/logging.h index d7521b3..67c398a 100644 --- a/rtc_base/logging.h +++ b/rtc_base/logging.h
@@ -54,7 +54,6 @@ #include <memory> #include <optional> #include <span> -#include <sstream> // no-presubmit-check TODO(webrtc:8982) #include <string> #include <type_traits> #include <utility> @@ -417,24 +416,6 @@ return {absl::StrCat(x)}; } -// Handle arbitrary types other than the above by falling back to stringstream. -// TODO(bugs.webrtc.org/9278): Get rid of this overload when callers don't need -// it anymore. No in-tree caller does, but some external callers still do. -template <typename T, - typename T1 = std::decay_t<T>, - std::enable_if_t<std::is_class<T1>::value && // - !std::is_same<T1, std::string>::value && // - !std::is_same<T1, LogMetadata>::value && // - !absl::HasAbslStringify<T1>::value && -#ifdef WEBRTC_ANDROID - !std::is_same<T1, LogMetadataTag>::value && // -#endif - !std::is_same<T1, LogMetadataErr>::value>* = nullptr> -ToStringVal MakeVal(const T& x) { - std::ostringstream os; // no-presubmit-check TODO(webrtc:8982) - os << x; - return {os.str()}; -} #if RTC_LOG_ENABLED() void Log(const LogArgType* fmt, ...);
diff --git a/rtc_base/openssl_adapter_unittest.cc b/rtc_base/openssl_adapter_unittest.cc index 8d40fd8..4a3dbb2 100644 --- a/rtc_base/openssl_adapter_unittest.cc +++ b/rtc_base/openssl_adapter_unittest.cc
@@ -13,7 +13,6 @@ #include <cstddef> #include <cstdint> #include <memory> -#include <sstream> #include <string> #include <vector> @@ -27,6 +26,7 @@ #include "rtc_base/socket_address.h" #include "rtc_base/ssl_certificate.h" #include "rtc_base/ssl_stream_adapter.h" // IWYU pragma: keep +#include "rtc_base/strings/string_builder.h" #include "test/gmock.h" #include "test/gtest.h" #include "test/run_loop.h" @@ -132,14 +132,17 @@ // One protocol test. std::vector<std::string> alpn_protos{"h2"}; - std::stringstream expected_response; - expected_response << static_cast<char>(2) << "h2"; - EXPECT_EQ(expected_response.str(), TransformAlpnProtocols(alpn_protos)); + StringBuilder sb; + sb << static_cast<char>(2) << "h2"; + std::string expected_response = sb.Release(); + EXPECT_EQ(expected_response, TransformAlpnProtocols(alpn_protos)); // Standard protocols test (h2,http/1.1). alpn_protos.push_back("http/1.1"); - expected_response << static_cast<char>(8) << "http/1.1"; - EXPECT_EQ(expected_response.str(), TransformAlpnProtocols(alpn_protos)); + StringBuilder sb2; + sb2 << static_cast<char>(2) << "h2" << static_cast<char>(8) << "http/1.1"; + expected_response = sb2.Release(); + EXPECT_EQ(expected_response, TransformAlpnProtocols(alpn_protos)); } // Verifies that SSLStart works when OpenSSLAdapter is started in standalone
diff --git a/test/network/cross_traffic_unittest.cc b/test/network/cross_traffic_unittest.cc index 592d0d9..1224706 100644 --- a/test/network/cross_traffic_unittest.cc +++ b/test/network/cross_traffic_unittest.cc
@@ -91,8 +91,8 @@ fixture.clock.AdvanceTimeMilliseconds(1); } - RTC_LOG(LS_INFO) << fixture.counter.packets_count_ << " packets; " - << fixture.counter.total_packets_size_ << " bytes"; + RTC_LOG(LS_INFO) << fixture.counter.packets_count_.load() << " packets; " + << fixture.counter.total_packets_size_.load() << " bytes"; // Using 50% duty cycle. const auto kExpectedDataSent = kRunTime * config.peak_rate * 0.5; EXPECT_NEAR(fixture.counter.total_packets_size_, kExpectedDataSent.bytes(), @@ -119,8 +119,8 @@ fixture.clock.AdvanceTimeMilliseconds(1); } - RTC_LOG(LS_INFO) << fixture.counter.packets_count_ << " packets; " - << fixture.counter.total_packets_size_ << " bytes"; + RTC_LOG(LS_INFO) << fixture.counter.packets_count_.load() << " packets; " + << fixture.counter.total_packets_size_.load() << " bytes"; // Sending at peak rate since bias = 1. const auto kExpectedDataSent = kRunTime * config.peak_rate; EXPECT_NEAR(fixture.counter.total_packets_size_, kExpectedDataSent.bytes(),