From 918eb19303851b0d070dbb85874c9bd016cc1b4d Mon Sep 17 00:00:00 2001 From: Jakob Ivarsson Date: Mon, 21 Nov 2022 19:07:10 +0100 Subject: [PATCH] Fix crash when Opus maxptime < 20ms. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A follow up cl will be created to better handle nullopt frame length range in AudioSendStream. Note that maxptime is still not used for setting the frame length (only ptime is). Bug: chromium:1109337 Change-Id: Id21fd8c76a6c4a0c85719a955116f8d16001a3d4 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/284501 Commit-Queue: Jakob Ivarsson‎ Reviewed-by: Ivo Creusen Cr-Commit-Position: refs/heads/main@{#38702} --- .../audio_coding/codecs/opus/audio_encoder_opus.cc | 7 ++++--- .../codecs/opus/audio_encoder_opus_unittest.cc | 11 +++++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/modules/audio_coding/codecs/opus/audio_encoder_opus.cc b/modules/audio_coding/codecs/opus/audio_encoder_opus.cc index dcd2ce0344..19afca1bdb 100644 --- a/modules/audio_coding/codecs/opus/audio_encoder_opus.cc +++ b/modules/audio_coding/codecs/opus/audio_encoder_opus.cc @@ -813,9 +813,10 @@ ANAStats AudioEncoderOpusImpl::GetANAStats() const { absl::optional > AudioEncoderOpusImpl::GetFrameLengthRange() const { - if (config_.supported_frame_lengths_ms.empty()) { - return absl::nullopt; - } else if (audio_network_adaptor_) { + if (audio_network_adaptor_) { + if (config_.supported_frame_lengths_ms.empty()) { + return absl::nullopt; + } return {{TimeDelta::Millis(config_.supported_frame_lengths_ms.front()), TimeDelta::Millis(config_.supported_frame_lengths_ms.back())}}; } else { diff --git a/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc b/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc index 43e8a7a80f..673aad734b 100644 --- a/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc +++ b/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc @@ -670,6 +670,17 @@ TEST(AudioEncoderOpusTest, TestConfigFromInvalidParams) { config.supported_frame_lengths_ms); } +TEST(AudioEncoderOpusTest, GetFrameLenghtRange) { + AudioEncoderOpusConfig config = + CreateConfigWithParameters({{"maxptime", "10"}, {"ptime", "10"}}); + std::unique_ptr encoder = + AudioEncoderOpus::MakeAudioEncoder(config, kDefaultOpusPayloadType); + auto ptime = webrtc::TimeDelta::Millis(10); + absl::optional> range = { + {ptime, ptime}}; + EXPECT_EQ(encoder->GetFrameLengthRange(), range); +} + // Test that bitrate will be overridden by the "maxaveragebitrate" parameter. // Also test that the "maxaveragebitrate" can't be set to values outside the // range of 6000 and 510000