From 45f58d7fccd2660f8489923e36db1a277a683063 Mon Sep 17 00:00:00 2001 From: Pete Makeev Date: Mon, 2 Dec 2024 17:06:05 +0300 Subject: [PATCH] Fixed counting of index 'send_codec_position' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For-loop has a 'continue' statement that skips increment of the index. Added such an increment before 'continue' for the index to keep up with the for-loop. I guess current implementation will bug on codecs sequence like: 'red, unknown, opus' since the subsequent for-loop (the 'red_codec' one) will not be able to find 'opus'. Seems like adding second increment statement is the easiest way to fix it. Bug: None Change-Id: Iab9cc66cf569458af9fd9ba5b938d83186c78c73 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/369700 Reviewed-by: Erik Språng Reviewed-by: Harald Alvestrand Commit-Queue: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#43488} --- AUTHORS | 1 + media/engine/webrtc_voice_engine.cc | 1 + media/engine/webrtc_voice_engine_unittest.cc | 20 +++++++++++++++++++- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/AUTHORS b/AUTHORS index f4dd64134d..46587ac6f3 100644 --- a/AUTHORS +++ b/AUTHORS @@ -106,6 +106,7 @@ Olivier Crête Pali Rohar Paul Kapustin Peng Yu +Pete Makeev Philipp Hancke Piasy Xu Rafael Lopez Diez diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index bf652830f5..ecc2225417 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -1449,6 +1449,7 @@ bool WebRtcVoiceSendChannel::SetSendCodecs( voice_codec_info = engine()->encoder_factory_->QueryAudioEncoder(format); if (!voice_codec_info) { RTC_LOG(LS_WARNING) << "Unknown codec " << ToString(voice_codec); + send_codec_position++; continue; } diff --git a/media/engine/webrtc_voice_engine_unittest.cc b/media/engine/webrtc_voice_engine_unittest.cc index 8a208d7b88..fa027a78a2 100644 --- a/media/engine/webrtc_voice_engine_unittest.cc +++ b/media/engine/webrtc_voice_engine_unittest.cc @@ -117,6 +117,8 @@ const cricket::Codec kTelephoneEventCodec1 = cricket::CreateAudioCodec(106, "telephone-event", 8000, 1); const cricket::Codec kTelephoneEventCodec2 = cricket::CreateAudioCodec(107, "telephone-event", 32000, 1); +const cricket::Codec kUnknownCodec = + cricket::CreateAudioCodec(127, "XYZ", 32000, 1); const uint32_t kSsrc0 = 0; const uint32_t kSsrc1 = 1; @@ -983,7 +985,7 @@ TEST_P(WebRtcVoiceEngineTestFake, SetRecvCodecsUnsupportedCodec) { EXPECT_TRUE(SetupChannel()); cricket::AudioReceiverParameters parameters; parameters.codecs.push_back(kOpusCodec); - parameters.codecs.push_back(cricket::CreateAudioCodec(127, "XYZ", 32000, 1)); + parameters.codecs.push_back(kUnknownCodec); EXPECT_FALSE(receive_channel_->SetReceiverParameters(parameters)); } @@ -1733,6 +1735,22 @@ TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecsRedFmtpAmountOfRedundancy) { EXPECT_EQ(std::nullopt, send_codec_spec3.red_payload_type); } +// Test that we use Opus/Red by default if an unknown codec +// is before RED and Opus. +TEST_P(WebRtcVoiceEngineTestFake, SetSendCodecRedWithUnknownCodec) { + EXPECT_TRUE(SetupSendStream()); + cricket::AudioSenderParameter parameters; + parameters.codecs.push_back(kUnknownCodec); + parameters.codecs.push_back(kRed48000Codec); + parameters.codecs.back().params[""] = "111/111"; + parameters.codecs.push_back(kOpusCodec); + SetSenderParameters(parameters); + const auto& send_codec_spec = *GetSendStreamConfig(kSsrcX).send_codec_spec; + EXPECT_EQ(111, send_codec_spec.payload_type); + EXPECT_STRCASEEQ("opus", send_codec_spec.format.name.c_str()); + EXPECT_EQ(112, send_codec_spec.red_payload_type); +} + // Test that WebRtcVoiceEngine reconfigures, rather than recreates its // AudioSendStream. TEST_P(WebRtcVoiceEngineTestFake, DontRecreateSendStream) {