From 413ccc49ecfe459f9c0db96169668571d1485e9b Mon Sep 17 00:00:00 2001 From: Bjorn A Mellem Date: Fri, 26 Apr 2019 15:41:05 -0700 Subject: [PATCH] Stop DCHECK which occurs in ANA BitrateController when overhead is zero. https://webrtc-review.googlesource.com/c/src/+/119121 added two calls to set the observed overhead. Both SetupSendCodec() and ReconfigureSendCodec() update the encoder's overhead. However, these calls happen before RTP has issued any callbacks to set the overhead, so they tell the encoder that the overhead is zero. This change checks whether the overhead has been set to a non-zero value before each of the new calls and adds a DCHECK to quickly catch future cases which attempt to set overhead to zero. Bug: webrtc:10150 Change-Id: Ieb3345ecfcda1cf25538d5d424383df17a71b4a2 TBR: solenberg@webrtc.org Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/134260 Commit-Queue: Bjorn Mellem Reviewed-by: Anton Sukhanov Reviewed-by: Minyue Li Cr-Commit-Position: refs/heads/master@{#27793} --- audio/audio_send_stream.cc | 7 ++++++- audio/audio_send_stream_unittest.cc | 3 --- .../audio_network_adaptor/bitrate_controller.cc | 4 +++- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc index 283fd9a603..c1503bbb80 100644 --- a/audio/audio_send_stream.cc +++ b/audio/audio_send_stream.cc @@ -517,6 +517,9 @@ void AudioSendStream::OnOverheadChanged( 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. + } channel_send_->CallEncoder([&](AudioEncoder* encoder) { encoder->OnReceivedOverhead(overhead_per_packet_bytes); }); @@ -626,7 +629,9 @@ bool AudioSendStream::SetupSendCodec(AudioSendStream* stream, // If overhead changes later, it will be updated in UpdateOverheadForEncoder. { rtc::CritScope cs(&stream->overhead_per_packet_lock_); - encoder->OnReceivedOverhead(stream->GetPerPacketOverheadBytes()); + if (stream->GetPerPacketOverheadBytes() > 0) { + encoder->OnReceivedOverhead(stream->GetPerPacketOverheadBytes()); + } } stream->StoreEncoderProperties(encoder->SampleRateHz(), diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc index 701df1c9ee..54c4d9b3a2 100644 --- a/audio/audio_send_stream_unittest.cc +++ b/audio/audio_send_stream_unittest.cc @@ -536,9 +536,6 @@ TEST(AudioSendStreamTest, ReconfigureTransportCcResetsFirst) { .Times(1); } - // CallEncoder will be called to re-set overhead. - EXPECT_CALL(*helper.channel_send(), CallEncoder(::testing::_)).Times(1); - send_stream->Reconfigure(new_config); } diff --git a/modules/audio_coding/audio_network_adaptor/bitrate_controller.cc b/modules/audio_coding/audio_network_adaptor/bitrate_controller.cc index 6850926c27..eee6f403aa 100644 --- a/modules/audio_coding/audio_network_adaptor/bitrate_controller.cc +++ b/modules/audio_coding/audio_network_adaptor/bitrate_controller.cc @@ -43,8 +43,10 @@ void BitrateController::UpdateNetworkMetrics( const NetworkMetrics& network_metrics) { if (network_metrics.target_audio_bitrate_bps) target_audio_bitrate_bps_ = network_metrics.target_audio_bitrate_bps; - if (network_metrics.overhead_bytes_per_packet) + if (network_metrics.overhead_bytes_per_packet) { + RTC_DCHECK_GT(*network_metrics.overhead_bytes_per_packet, 0); overhead_bytes_per_packet_ = network_metrics.overhead_bytes_per_packet; + } } void BitrateController::MakeDecision(AudioEncoderRuntimeConfig* config) {