Always call IsOk() to ensure audio codec configuration is valid when negotiating.
We should avoid creating codecs with invalid parameters, since this can
expose security issues. For many codecs the IsOk() method to check the
codec config is only called in DCHECKs. This CL ensures IsOk() is always
called, also in non-debug builds.
Bug: chromium:1265806
Change-Id: Ibd3c6c65d3bb547cd2603e11808ac40ac693a8b1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/238801
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Ivo Creusen <ivoc@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35422}
diff --git a/api/audio_codecs/L16/audio_decoder_L16.cc b/api/audio_codecs/L16/audio_decoder_L16.cc
index 57c9e76..93863f1 100644
--- a/api/audio_codecs/L16/audio_decoder_L16.cc
+++ b/api/audio_codecs/L16/audio_decoder_L16.cc
@@ -24,9 +24,10 @@
Config config;
config.sample_rate_hz = format.clockrate_hz;
config.num_channels = rtc::checked_cast<int>(format.num_channels);
- return absl::EqualsIgnoreCase(format.name, "L16") && config.IsOk()
- ? absl::optional<Config>(config)
- : absl::nullopt;
+ if (absl::EqualsIgnoreCase(format.name, "L16") && config.IsOk()) {
+ return config;
+ }
+ return absl::nullopt;
}
void AudioDecoderL16::AppendSupportedDecoders(
@@ -37,9 +38,11 @@
std::unique_ptr<AudioDecoder> AudioDecoderL16::MakeAudioDecoder(
const Config& config,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
- return config.IsOk() ? std::make_unique<AudioDecoderPcm16B>(
- config.sample_rate_hz, config.num_channels)
- : nullptr;
+ if (!config.IsOk()) {
+ return nullptr;
+ }
+ return std::make_unique<AudioDecoderPcm16B>(config.sample_rate_hz,
+ config.num_channels);
}
} // namespace webrtc
diff --git a/api/audio_codecs/L16/audio_encoder_L16.cc b/api/audio_codecs/L16/audio_encoder_L16.cc
index 507c8d7..590d3e3 100644
--- a/api/audio_codecs/L16/audio_encoder_L16.cc
+++ b/api/audio_codecs/L16/audio_encoder_L16.cc
@@ -24,6 +24,7 @@
absl::optional<AudioEncoderL16::Config> AudioEncoderL16::SdpToConfig(
const SdpAudioFormat& format) {
if (!rtc::IsValueInRangeForNumericType<int>(format.num_channels)) {
+ RTC_DCHECK_NOTREACHED();
return absl::nullopt;
}
Config config;
@@ -36,9 +37,10 @@
config.frame_size_ms = rtc::SafeClamp(10 * (*ptime / 10), 10, 60);
}
}
- return absl::EqualsIgnoreCase(format.name, "L16") && config.IsOk()
- ? absl::optional<Config>(config)
- : absl::nullopt;
+ if (absl::EqualsIgnoreCase(format.name, "L16") && config.IsOk()) {
+ return config;
+ }
+ return absl::nullopt;
}
void AudioEncoderL16::AppendSupportedEncoders(
@@ -58,12 +60,15 @@
const AudioEncoderL16::Config& config,
int payload_type,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
- RTC_DCHECK(config.IsOk());
AudioEncoderPcm16B::Config c;
c.sample_rate_hz = config.sample_rate_hz;
c.num_channels = config.num_channels;
c.frame_size_ms = config.frame_size_ms;
c.payload_type = payload_type;
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return nullptr;
+ }
return std::make_unique<AudioEncoderPcm16B>(c);
}
diff --git a/api/audio_codecs/g711/audio_decoder_g711.cc b/api/audio_codecs/g711/audio_decoder_g711.cc
index 57e3741..f3d3378 100644
--- a/api/audio_codecs/g711/audio_decoder_g711.cc
+++ b/api/audio_codecs/g711/audio_decoder_g711.cc
@@ -28,7 +28,10 @@
Config config;
config.type = is_pcmu ? Config::Type::kPcmU : Config::Type::kPcmA;
config.num_channels = rtc::dchecked_cast<int>(format.num_channels);
- RTC_DCHECK(config.IsOk());
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return absl::nullopt;
+ }
return config;
} else {
return absl::nullopt;
@@ -45,13 +48,17 @@
std::unique_ptr<AudioDecoder> AudioDecoderG711::MakeAudioDecoder(
const Config& config,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
- RTC_DCHECK(config.IsOk());
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return nullptr;
+ }
switch (config.type) {
case Config::Type::kPcmU:
return std::make_unique<AudioDecoderPcmU>(config.num_channels);
case Config::Type::kPcmA:
return std::make_unique<AudioDecoderPcmA>(config.num_channels);
default:
+ RTC_DCHECK_NOTREACHED();
return nullptr;
}
}
diff --git a/api/audio_codecs/g711/audio_encoder_g711.cc b/api/audio_codecs/g711/audio_encoder_g711.cc
index ab95ad4..4c1ce0f 100644
--- a/api/audio_codecs/g711/audio_encoder_g711.cc
+++ b/api/audio_codecs/g711/audio_encoder_g711.cc
@@ -38,7 +38,10 @@
config.frame_size_ms = rtc::SafeClamp(10 * (*ptime / 10), 10, 60);
}
}
- RTC_DCHECK(config.IsOk());
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return absl::nullopt;
+ }
return config;
} else {
return absl::nullopt;
@@ -62,7 +65,10 @@
const Config& config,
int payload_type,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
- RTC_DCHECK(config.IsOk());
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return nullptr;
+ }
switch (config.type) {
case Config::Type::kPcmU: {
AudioEncoderPcmU::Config impl_config;
@@ -79,6 +85,7 @@
return std::make_unique<AudioEncoderPcmA>(impl_config);
}
default: {
+ RTC_DCHECK_NOTREACHED();
return nullptr;
}
}
diff --git a/api/audio_codecs/g722/audio_decoder_g722.cc b/api/audio_codecs/g722/audio_decoder_g722.cc
index 29b6d5d..0049e5a 100644
--- a/api/audio_codecs/g722/audio_decoder_g722.cc
+++ b/api/audio_codecs/g722/audio_decoder_g722.cc
@@ -21,12 +21,12 @@
absl::optional<AudioDecoderG722::Config> AudioDecoderG722::SdpToConfig(
const SdpAudioFormat& format) {
- return absl::EqualsIgnoreCase(format.name, "G722") &&
- format.clockrate_hz == 8000 &&
- (format.num_channels == 1 || format.num_channels == 2)
- ? absl::optional<Config>(
- Config{rtc::dchecked_cast<int>(format.num_channels)})
- : absl::nullopt;
+ if (absl::EqualsIgnoreCase(format.name, "G722") &&
+ format.clockrate_hz == 8000 &&
+ (format.num_channels == 1 || format.num_channels == 2)) {
+ return Config{rtc::dchecked_cast<int>(format.num_channels)};
+ }
+ return absl::nullopt;
}
void AudioDecoderG722::AppendSupportedDecoders(
@@ -37,12 +37,17 @@
std::unique_ptr<AudioDecoder> AudioDecoderG722::MakeAudioDecoder(
Config config,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return nullptr;
+ }
switch (config.num_channels) {
case 1:
return std::make_unique<AudioDecoderG722Impl>();
case 2:
return std::make_unique<AudioDecoderG722StereoImpl>();
default:
+ RTC_DCHECK_NOTREACHED();
return nullptr;
}
}
diff --git a/api/audio_codecs/g722/audio_encoder_g722.cc b/api/audio_codecs/g722/audio_encoder_g722.cc
index 12c1746..66cf9e1 100644
--- a/api/audio_codecs/g722/audio_encoder_g722.cc
+++ b/api/audio_codecs/g722/audio_encoder_g722.cc
@@ -38,8 +38,11 @@
config.frame_size_ms = rtc::SafeClamp<int>(whole_packets * 10, 10, 60);
}
}
- return config.IsOk() ? absl::optional<AudioEncoderG722Config>(config)
- : absl::nullopt;
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return absl::nullopt;
+ }
+ return config;
}
void AudioEncoderG722::AppendSupportedEncoders(
@@ -60,7 +63,10 @@
const AudioEncoderG722Config& config,
int payload_type,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
- RTC_DCHECK(config.IsOk());
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return nullptr;
+ }
return std::make_unique<AudioEncoderG722Impl>(config, payload_type);
}
diff --git a/api/audio_codecs/ilbc/audio_decoder_ilbc.cc b/api/audio_codecs/ilbc/audio_decoder_ilbc.cc
index d0aae90..237cef2 100644
--- a/api/audio_codecs/ilbc/audio_decoder_ilbc.cc
+++ b/api/audio_codecs/ilbc/audio_decoder_ilbc.cc
@@ -20,10 +20,11 @@
absl::optional<AudioDecoderIlbc::Config> AudioDecoderIlbc::SdpToConfig(
const SdpAudioFormat& format) {
- return absl::EqualsIgnoreCase(format.name, "ILBC") &&
- format.clockrate_hz == 8000 && format.num_channels == 1
- ? absl::optional<Config>(Config())
- : absl::nullopt;
+ if (absl::EqualsIgnoreCase(format.name, "ILBC") &&
+ format.clockrate_hz == 8000 && format.num_channels == 1) {
+ return Config();
+ }
+ return absl::nullopt;
}
void AudioDecoderIlbc::AppendSupportedDecoders(
diff --git a/api/audio_codecs/ilbc/audio_encoder_ilbc.cc b/api/audio_codecs/ilbc/audio_encoder_ilbc.cc
index 035b0dc..52ba8f6 100644
--- a/api/audio_codecs/ilbc/audio_encoder_ilbc.cc
+++ b/api/audio_codecs/ilbc/audio_encoder_ilbc.cc
@@ -53,8 +53,11 @@
config.frame_size_ms = rtc::SafeClamp<int>(whole_packets * 10, 20, 60);
}
}
- return config.IsOk() ? absl::optional<AudioEncoderIlbcConfig>(config)
- : absl::nullopt;
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return absl::nullopt;
+ }
+ return config;
}
void AudioEncoderIlbc::AppendSupportedEncoders(
@@ -74,7 +77,10 @@
const AudioEncoderIlbcConfig& config,
int payload_type,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
- RTC_DCHECK(config.IsOk());
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return nullptr;
+ }
return std::make_unique<AudioEncoderIlbcImpl>(config, payload_type);
}
diff --git a/api/audio_codecs/isac/audio_decoder_isac_fix.cc b/api/audio_codecs/isac/audio_decoder_isac_fix.cc
index 21d0da3..305e15a 100644
--- a/api/audio_codecs/isac/audio_decoder_isac_fix.cc
+++ b/api/audio_codecs/isac/audio_decoder_isac_fix.cc
@@ -19,10 +19,11 @@
absl::optional<AudioDecoderIsacFix::Config> AudioDecoderIsacFix::SdpToConfig(
const SdpAudioFormat& format) {
- return absl::EqualsIgnoreCase(format.name, "ISAC") &&
- format.clockrate_hz == 16000 && format.num_channels == 1
- ? absl::optional<Config>(Config())
- : absl::nullopt;
+ if (absl::EqualsIgnoreCase(format.name, "ISAC") &&
+ format.clockrate_hz == 16000 && format.num_channels == 1) {
+ return Config();
+ }
+ return absl::nullopt;
}
void AudioDecoderIsacFix::AppendSupportedDecoders(
diff --git a/api/audio_codecs/isac/audio_decoder_isac_float.cc b/api/audio_codecs/isac/audio_decoder_isac_float.cc
index 4efc2ea..683eb6c 100644
--- a/api/audio_codecs/isac/audio_decoder_isac_float.cc
+++ b/api/audio_codecs/isac/audio_decoder_isac_float.cc
@@ -24,6 +24,10 @@
format.num_channels == 1) {
Config config;
config.sample_rate_hz = format.clockrate_hz;
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return absl::nullopt;
+ }
return config;
} else {
return absl::nullopt;
@@ -39,9 +43,12 @@
std::unique_ptr<AudioDecoder> AudioDecoderIsacFloat::MakeAudioDecoder(
Config config,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
- RTC_DCHECK(config.IsOk());
AudioDecoderIsacFloatImpl::Config c;
c.sample_rate_hz = config.sample_rate_hz;
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return nullptr;
+ }
return std::make_unique<AudioDecoderIsacFloatImpl>(c);
}
diff --git a/api/audio_codecs/isac/audio_encoder_isac_fix.cc b/api/audio_codecs/isac/audio_encoder_isac_fix.cc
index 7cf55b9..b590be1 100644
--- a/api/audio_codecs/isac/audio_encoder_isac_fix.cc
+++ b/api/audio_codecs/isac/audio_encoder_isac_fix.cc
@@ -30,6 +30,10 @@
config.frame_size_ms = 60;
}
}
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return absl::nullopt;
+ }
return config;
} else {
return absl::nullopt;
@@ -53,11 +57,14 @@
AudioEncoderIsacFix::Config config,
int payload_type,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
- RTC_DCHECK(config.IsOk());
AudioEncoderIsacFixImpl::Config c;
c.frame_size_ms = config.frame_size_ms;
c.bit_rate = config.bit_rate;
c.payload_type = payload_type;
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return nullptr;
+ }
return std::make_unique<AudioEncoderIsacFixImpl>(c);
}
diff --git a/api/audio_codecs/isac/audio_encoder_isac_float.cc b/api/audio_codecs/isac/audio_encoder_isac_float.cc
index 6f684c8..e2afeae 100644
--- a/api/audio_codecs/isac/audio_encoder_isac_float.cc
+++ b/api/audio_codecs/isac/audio_encoder_isac_float.cc
@@ -37,6 +37,10 @@
}
}
}
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return absl::nullopt;
+ }
return config;
} else {
return absl::nullopt;
@@ -65,12 +69,15 @@
const AudioEncoderIsacFloat::Config& config,
int payload_type,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
- RTC_DCHECK(config.IsOk());
AudioEncoderIsacFloatImpl::Config c;
c.payload_type = payload_type;
c.sample_rate_hz = config.sample_rate_hz;
c.frame_size_ms = config.frame_size_ms;
c.bit_rate = config.bit_rate;
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return nullptr;
+ }
return std::make_unique<AudioEncoderIsacFloatImpl>(c);
}
diff --git a/api/audio_codecs/opus/audio_decoder_opus.cc b/api/audio_codecs/opus/audio_decoder_opus.cc
index 6b4e0d3..7e0d88b 100644
--- a/api/audio_codecs/opus/audio_decoder_opus.cc
+++ b/api/audio_codecs/opus/audio_decoder_opus.cc
@@ -51,7 +51,10 @@
num_channels) {
Config config;
config.num_channels = *num_channels;
- RTC_DCHECK(config.IsOk());
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return absl::nullopt;
+ }
return config;
} else {
return absl::nullopt;
@@ -71,7 +74,10 @@
std::unique_ptr<AudioDecoder> AudioDecoderOpus::MakeAudioDecoder(
Config config,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
- RTC_DCHECK(config.IsOk());
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return nullptr;
+ }
return std::make_unique<AudioDecoderOpusImpl>(config.num_channels,
config.sample_rate_hz);
}
diff --git a/api/audio_codecs/opus/audio_encoder_opus.cc b/api/audio_codecs/opus/audio_encoder_opus.cc
index 36d82b3..6d950c5 100644
--- a/api/audio_codecs/opus/audio_encoder_opus.cc
+++ b/api/audio_codecs/opus/audio_encoder_opus.cc
@@ -33,6 +33,10 @@
const AudioEncoderOpusConfig& config,
int payload_type,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return nullptr;
+ }
return AudioEncoderOpusImpl::MakeAudioEncoder(config, payload_type);
}
diff --git a/modules/audio_coding/codecs/opus/audio_decoder_multi_channel_opus_impl.cc b/modules/audio_coding/codecs/opus/audio_decoder_multi_channel_opus_impl.cc
index c8fd176..285ea89 100644
--- a/modules/audio_coding/codecs/opus/audio_decoder_multi_channel_opus_impl.cc
+++ b/modules/audio_coding/codecs/opus/audio_decoder_multi_channel_opus_impl.cc
@@ -26,6 +26,7 @@
AudioDecoderMultiChannelOpusImpl::MakeAudioDecoder(
AudioDecoderMultiChannelOpusConfig config) {
if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
return nullptr;
}
// Fill the pointer with a working decoder through the C interface. This
@@ -78,6 +79,9 @@
return absl::nullopt;
}
config.channel_mapping = *channel_mapping;
+ if (!config.IsOk()) {
+ return absl::nullopt;
+ }
return config;
}
diff --git a/modules/audio_coding/codecs/opus/audio_decoder_multi_channel_opus_unittest.cc b/modules/audio_coding/codecs/opus/audio_decoder_multi_channel_opus_unittest.cc
index 66eecb7..57e2107 100644
--- a/modules/audio_coding/codecs/opus/audio_decoder_multi_channel_opus_unittest.cc
+++ b/modules/audio_coding/codecs/opus/audio_decoder_multi_channel_opus_unittest.cc
@@ -45,13 +45,7 @@
{"num_streams", "2"}});
const absl::optional<AudioDecoderMultiChannelOpus::Config> decoder_config =
AudioDecoderMultiChannelOpus::SdpToConfig(sdp_format);
- ASSERT_TRUE(decoder_config.has_value());
- EXPECT_FALSE(decoder_config->IsOk());
-
- const std::unique_ptr<AudioDecoder> opus_decoder =
- AudioDecoderMultiChannelOpus::MakeAudioDecoder(*decoder_config);
-
- EXPECT_FALSE(opus_decoder);
+ EXPECT_FALSE(decoder_config.has_value());
}
{
// The mapping is too long. There are only 5 channels, but 6 elements in the
@@ -62,13 +56,7 @@
{"num_streams", "2"}});
const absl::optional<AudioDecoderMultiChannelOpus::Config> decoder_config =
AudioDecoderMultiChannelOpus::SdpToConfig(sdp_format);
- ASSERT_TRUE(decoder_config.has_value());
- EXPECT_FALSE(decoder_config->IsOk());
-
- const std::unique_ptr<AudioDecoder> opus_decoder =
- AudioDecoderMultiChannelOpus::MakeAudioDecoder(*decoder_config);
-
- EXPECT_FALSE(opus_decoder);
+ EXPECT_FALSE(decoder_config.has_value());
}
{
// The mapping doesn't parse correctly.
diff --git a/modules/audio_coding/codecs/opus/audio_encoder_multi_channel_opus_impl.cc b/modules/audio_coding/codecs/opus/audio_encoder_multi_channel_opus_impl.cc
index 1feef3d..38a11c1 100644
--- a/modules/audio_coding/codecs/opus/audio_encoder_multi_channel_opus_impl.cc
+++ b/modules/audio_coding/codecs/opus/audio_encoder_multi_channel_opus_impl.cc
@@ -131,6 +131,7 @@
const AudioEncoderMultiChannelOpusConfig& config,
int payload_type) {
if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
return nullptr;
}
return std::make_unique<AudioEncoderMultiChannelOpusImpl>(config,
@@ -280,6 +281,9 @@
}
config.channel_mapping = *channel_mapping;
+ if (!config.IsOk()) {
+ return absl::nullopt;
+ }
return config;
}
diff --git a/modules/audio_coding/codecs/opus/audio_encoder_multi_channel_opus_unittest.cc b/modules/audio_coding/codecs/opus/audio_encoder_multi_channel_opus_unittest.cc
index 44da7d7..92f6f2c 100644
--- a/modules/audio_coding/codecs/opus/audio_encoder_multi_channel_opus_unittest.cc
+++ b/modules/audio_coding/codecs/opus/audio_encoder_multi_channel_opus_unittest.cc
@@ -28,10 +28,9 @@
{"num_streams", "2"}});
const absl::optional<AudioEncoderMultiChannelOpus::Config> encoder_config =
AudioEncoderMultiChannelOpus::SdpToConfig(sdp_format);
- ASSERT_TRUE(encoder_config.has_value());
// Maps input channel 0 to coded channel 3, which doesn't exist.
- EXPECT_FALSE(encoder_config->IsOk());
+ EXPECT_FALSE(encoder_config.has_value());
}
{
@@ -41,10 +40,9 @@
{"num_streams", "2"}});
const absl::optional<AudioEncoderMultiChannelOpus::Config> encoder_config =
AudioEncoderMultiChannelOpus::SdpToConfig(sdp_format);
- ASSERT_TRUE(encoder_config.has_value());
// The mapping is too short.
- EXPECT_FALSE(encoder_config->IsOk());
+ EXPECT_FALSE(encoder_config.has_value());
}
{
const SdpAudioFormat sdp_format("multiopus", 48000, 3,
@@ -53,10 +51,9 @@
{"num_streams", "1"}});
const absl::optional<AudioEncoderMultiChannelOpus::Config> encoder_config =
AudioEncoderMultiChannelOpus::SdpToConfig(sdp_format);
- ASSERT_TRUE(encoder_config.has_value());
// Coded channel 0 comes from both input channels 0, 1 and 2.
- EXPECT_FALSE(encoder_config->IsOk());
+ EXPECT_FALSE(encoder_config.has_value());
}
{
const SdpAudioFormat sdp_format("multiopus", 48000, 3,
@@ -77,11 +74,10 @@
{"num_streams", "2"}});
const absl::optional<AudioEncoderMultiChannelOpus::Config> encoder_config =
AudioEncoderMultiChannelOpus::SdpToConfig(sdp_format);
- ASSERT_TRUE(encoder_config.has_value());
// This is NOT fine, because channels nothing says how coded channel 1
// should be coded.
- EXPECT_FALSE(encoder_config->IsOk());
+ EXPECT_FALSE(encoder_config.has_value());
}
}
@@ -105,7 +101,7 @@
testing::ContainerEq(std::vector<unsigned char>({0, 4, 1, 2, 3, 5})));
}
-TEST(AudioEncoderMultiOpusTest, CreateFromValidOrInvalidConfig) {
+TEST(AudioEncoderMultiOpusTest, CreateFromValidConfig) {
{
const SdpAudioFormat sdp_format("multiopus", 48000, 3,
{{"channel_mapping", "0,255,255"},
@@ -113,19 +109,7 @@
{"num_streams", "2"}});
const absl::optional<AudioEncoderMultiChannelOpus::Config> encoder_config =
AudioEncoderMultiChannelOpus::SdpToConfig(sdp_format);
- ASSERT_TRUE(encoder_config.has_value());
-
- // Invalid config from the ConfigValidity test. It's not allowed by our
- // checks, but Opus is more forgiving.
- EXPECT_FALSE(encoder_config->IsOk());
-
- const std::unique_ptr<AudioEncoder> opus_encoder =
- AudioEncoderMultiChannelOpus::MakeAudioEncoder(*encoder_config,
- kOpusPayloadType);
-
- // Shouldn't be possible (but shouldn't result in a crash) to create an
- // Encoder from an invalid config.
- EXPECT_FALSE(opus_encoder);
+ ASSERT_FALSE(encoder_config.has_value());
}
{
const SdpAudioFormat sdp_format("multiopus", 48000, 3,
diff --git a/modules/audio_coding/codecs/opus/audio_encoder_opus.cc b/modules/audio_coding/codecs/opus/audio_encoder_opus.cc
index e4d3b9e..fe6d852 100644
--- a/modules/audio_coding/codecs/opus/audio_encoder_opus.cc
+++ b/modules/audio_coding/codecs/opus/audio_encoder_opus.cc
@@ -229,7 +229,10 @@
std::unique_ptr<AudioEncoder> AudioEncoderOpusImpl::MakeAudioEncoder(
const AudioEncoderOpusConfig& config,
int payload_type) {
- RTC_DCHECK(config.IsOk());
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return nullptr;
+ }
return std::make_unique<AudioEncoderOpusImpl>(config, payload_type);
}
@@ -268,7 +271,10 @@
FindSupportedFrameLengths(min_frame_length_ms, max_frame_length_ms,
&config.supported_frame_lengths_ms);
- RTC_DCHECK(config.IsOk());
+ if (!config.IsOk()) {
+ RTC_DCHECK_NOTREACHED();
+ return absl::nullopt;
+ }
return config;
}
