From cf6544aef7ba7febc43f2584c4e375e366bcc4fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Wed, 13 May 2020 14:43:11 +0200 Subject: [PATCH] Avoids unnecessary calls to audio encoder. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As of this CL: https://webrtc-review.googlesource.com/c/src/+/173704 ...we now call AudioEncoder::OnReceivedOverhead() too often, since we don't check if overhead has actually changed. This CL rectifies that. Bug: webrtc:10809 Change-Id: I0b86e0296a7860dde3e62e817f9b941fa82afe4a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175009 Reviewed-by: Per Åhgren Commit-Queue: Erik Språng Cr-Commit-Position: refs/heads/master@{#31279} --- audio/audio_send_stream.cc | 8 +++++--- audio/audio_send_stream.h | 1 + audio/audio_send_stream_unittest.cc | 23 ++++++++++++++++++++++- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc index 8730c45258..cdeea1a107 100644 --- a/audio/audio_send_stream.cc +++ b/audio/audio_send_stream.cc @@ -544,10 +544,12 @@ void AudioSendStream::SetTransportOverhead( } void AudioSendStream::UpdateOverheadForEncoder() { - const size_t overhead_per_packet_bytes = GetPerPacketOverheadBytes(); - if (overhead_per_packet_bytes == 0) { - return; // Overhead is not known yet, do not tell the encoder. + size_t overhead_per_packet_bytes = GetPerPacketOverheadBytes(); + if (overhead_per_packet_ == overhead_per_packet_bytes) { + return; } + overhead_per_packet_ = overhead_per_packet_bytes; + channel_send_->CallEncoder([&](AudioEncoder* encoder) { encoder->OnReceivedOverhead(overhead_per_packet_bytes); }); diff --git a/audio/audio_send_stream.h b/audio/audio_send_stream.h index 92e9a7fb16..5f8a936aab 100644 --- a/audio/audio_send_stream.h +++ b/audio/audio_send_stream.h @@ -195,6 +195,7 @@ class AudioSendStream final : public webrtc::AudioSendStream, static int TransportSeqNumId(const Config& config); rtc::CriticalSection overhead_per_packet_lock_; + size_t overhead_per_packet_ RTC_GUARDED_BY(overhead_per_packet_lock_) = 0; // Current transport overhead (ICE, TURN, etc.) size_t transport_overhead_per_packet_bytes_ diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc index 60655e9b00..89f94cdb0c 100644 --- a/audio/audio_send_stream_unittest.cc +++ b/audio/audio_send_stream_unittest.cc @@ -794,7 +794,7 @@ TEST(AudioSendStreamTest, OnTransportOverheadChanged) { auto new_config = helper.config(); // CallEncoder will be called on overhead change. - EXPECT_CALL(*helper.channel_send(), CallEncoder(::testing::_)).Times(1); + EXPECT_CALL(*helper.channel_send(), CallEncoder); const size_t transport_overhead_per_packet_bytes = 333; send_stream->SetTransportOverhead(transport_overhead_per_packet_bytes); @@ -804,6 +804,27 @@ TEST(AudioSendStreamTest, OnTransportOverheadChanged) { } } +TEST(AudioSendStreamTest, DoesntCallEncoderWhenOverheadUnchanged) { + for (bool use_null_audio_processing : {false, true}) { + ConfigHelper helper(false, true, use_null_audio_processing); + auto send_stream = helper.CreateAudioSendStream(); + auto new_config = helper.config(); + + // CallEncoder will be called on overhead change. + EXPECT_CALL(*helper.channel_send(), CallEncoder); + const size_t transport_overhead_per_packet_bytes = 333; + send_stream->SetTransportOverhead(transport_overhead_per_packet_bytes); + + // Set the same overhead again, CallEncoder should not be called again. + EXPECT_CALL(*helper.channel_send(), CallEncoder).Times(0); + send_stream->SetTransportOverhead(transport_overhead_per_packet_bytes); + + // New overhead, call CallEncoder again + EXPECT_CALL(*helper.channel_send(), CallEncoder); + send_stream->SetTransportOverhead(transport_overhead_per_packet_bytes + 1); + } +} + TEST(AudioSendStreamTest, AudioOverheadChanged) { for (bool use_null_audio_processing : {false, true}) { ConfigHelper helper(false, true, use_null_audio_processing);