From 665e6817d1f60090e97ef39fe463f869a5ee5b7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Wed, 18 Oct 2023 14:25:33 +0200 Subject: [PATCH] Add field trial to control network socket receive buffer size. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In some very high-bandwidth application there have been observations of packet loss in the socket implementation (not on the network itself) due to large bursts of packets arriving. Allocating too big buffers can of course lead to issue as well, so this flag is intended to find a good tradeoff. Bug: webrtc:15585 Change-Id: I63eccb1a9f34d852d80c286fc27bffd17818f0ef Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/324021 Auto-Submit: Erik Språng Reviewed-by: Per Kjellander Commit-Queue: Per Kjellander Cr-Commit-Position: refs/heads/main@{#40963} --- experiments/field_trials.py | 3 ++ media/engine/webrtc_video_engine.cc | 18 ++++++++- media/engine/webrtc_video_engine.h | 2 + media/engine/webrtc_video_engine_unittest.cc | 39 ++++++++++++++++++++ 4 files changed, 60 insertions(+), 2 deletions(-) diff --git a/experiments/field_trials.py b/experiments/field_trials.py index 0e922f9d7d..e39b53eb47 100755 --- a/experiments/field_trials.py +++ b/experiments/field_trials.py @@ -715,6 +715,9 @@ POLICY_EXEMPT_FIELD_TRIALS: FrozenSet[FieldTrial] = frozenset([ FieldTrial('WebRTC-SendBufferSizeBytes', 'webrtc:11905', date(2024, 4, 1)), + FieldTrial('WebRTC-ReceiveBufferSize', + 'webrtc:15585', + date(2024, 4, 1)), FieldTrial('WebRTC-SendNackDelayMs', 'webrtc:9953', date(2024, 4, 1)), diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index fbf9226c4e..b2f8ae658a 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -745,6 +745,19 @@ void ExtractCodecInformation( } } +int ParseReceiveBufferSize(const webrtc::FieldTrialsView& trials) { + webrtc::FieldTrialParameter size_bytes("size_bytes", + kVideoRtpRecvBufferSize); + webrtc::ParseFieldTrial({&size_bytes}, + trials.Lookup("WebRTC-ReceiveBufferSize")); + if (size_bytes.Get() < 10'000 || size_bytes.Get() > 10'000'000) { + RTC_LOG(LS_WARNING) << "WebRTC-ReceiveBufferSize out of bounds: " + << size_bytes.Get(); + return kVideoRtpRecvBufferSize; + } + return size_bytes.Get(); +} + } // namespace // --------------- WebRtcVideoEngine --------------------------- @@ -2585,7 +2598,8 @@ WebRtcVideoReceiveChannel::WebRtcVideoReceiveChannel( discard_unknown_ssrc_packets_( IsEnabled(call_->trials(), "WebRTC-Video-DiscardPacketsWithUnknownSsrc")), - crypto_options_(crypto_options) { + crypto_options_(crypto_options), + receive_buffer_size_(ParseReceiveBufferSize(call_->trials())) { RTC_DCHECK_RUN_ON(&thread_checker_); rtcp_receiver_report_ssrc_ = kDefaultRtcpReceiverReportSsrc; recv_codecs_ = MapCodecs(GetPayloadTypesAndDefaultCodecs( @@ -3182,7 +3196,7 @@ void WebRtcVideoReceiveChannel::SetInterface( MediaChannelUtil::SetInterface(iface); // Set the RTP recv/send buffer to a bigger size. MediaChannelUtil::SetOption(MediaChannelNetworkInterface::ST_RTP, - rtc::Socket::OPT_RCVBUF, kVideoRtpRecvBufferSize); + rtc::Socket::OPT_RCVBUF, receive_buffer_size_); } void WebRtcVideoReceiveChannel::SetFrameDecryptor( diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h index 11f1b99ac2..e58c2f926c 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -891,6 +891,8 @@ class WebRtcVideoReceiveChannel : public MediaChannelUtil, // Callback invoked whenever the list of SSRCs changes. absl::AnyInvocable&)> ssrc_list_changed_callback_; + + const int receive_buffer_size_; }; // Keeping the old name "WebRtcVideoChannel" around because some external diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index b98d8e481d..9ed1de7e15 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -977,6 +977,45 @@ TEST_F(WebRtcVideoEngineTest, SendsFeedbackAfterUnsignaledRtxPacket) { receive_channel->SetInterface(nullptr); } +TEST_F(WebRtcVideoEngineTest, ReceiveBufferSizeViaFieldTrial) { + webrtc::test::ScopedKeyValueConfig override_field_trials( + field_trials_, "WebRTC-ReceiveBufferSize/size_bytes:10000/"); + std::unique_ptr receive_channel = + engine_.CreateReceiveChannel(call_.get(), GetMediaConfig(), + VideoOptions(), webrtc::CryptoOptions()); + cricket::FakeNetworkInterface network; + receive_channel->SetInterface(&network); + EXPECT_EQ(10000, network.recvbuf_size()); + receive_channel->SetInterface(nullptr); +} + +TEST_F(WebRtcVideoEngineTest, TooLowReceiveBufferSizeViaFieldTrial) { + // 10000001 is too high, it will revert to the default + // kVideoRtpRecvBufferSize. + webrtc::test::ScopedKeyValueConfig override_field_trials( + field_trials_, "WebRTC-ReceiveBufferSize/size_bytes:10000001/"); + std::unique_ptr receive_channel = + engine_.CreateReceiveChannel(call_.get(), GetMediaConfig(), + VideoOptions(), webrtc::CryptoOptions()); + cricket::FakeNetworkInterface network; + receive_channel->SetInterface(&network); + EXPECT_EQ(kVideoRtpRecvBufferSize, network.recvbuf_size()); + receive_channel->SetInterface(nullptr); +} + +TEST_F(WebRtcVideoEngineTest, TooHighReceiveBufferSizeViaFieldTrial) { + // 9999 is too low, it will revert to the default kVideoRtpRecvBufferSize. + webrtc::test::ScopedKeyValueConfig override_field_trials( + field_trials_, "WebRTC-ReceiveBufferSize/size_bytes:9999/"); + std::unique_ptr receive_channel = + engine_.CreateReceiveChannel(call_.get(), GetMediaConfig(), + VideoOptions(), webrtc::CryptoOptions()); + cricket::FakeNetworkInterface network; + receive_channel->SetInterface(&network); + EXPECT_EQ(kVideoRtpRecvBufferSize, network.recvbuf_size()); + receive_channel->SetInterface(nullptr); +} + TEST_F(WebRtcVideoEngineTest, UpdatesUnsignaledRtxSsrcAndRecoversPayload) { // Setup a channel with VP8, RTX and transport sequence number header // extension. Receive stream is not explicitly configured.