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}
This commit is contained in:
Ivo Creusen
2021-11-24 19:29:10 +00:00
committed by WebRTC LUCI CQ
parent 789a0f361f
commit deb1b1bc70
19 changed files with 135 additions and 77 deletions

View File

@ -24,9 +24,10 @@ absl::optional<AudioDecoderL16::Config> AudioDecoderL16::SdpToConfig(
Config config; Config config;
config.sample_rate_hz = format.clockrate_hz; config.sample_rate_hz = format.clockrate_hz;
config.num_channels = rtc::checked_cast<int>(format.num_channels); config.num_channels = rtc::checked_cast<int>(format.num_channels);
return absl::EqualsIgnoreCase(format.name, "L16") && config.IsOk() if (absl::EqualsIgnoreCase(format.name, "L16") && config.IsOk()) {
? absl::optional<Config>(config) return config;
: absl::nullopt; }
return absl::nullopt;
} }
void AudioDecoderL16::AppendSupportedDecoders( void AudioDecoderL16::AppendSupportedDecoders(
@ -37,9 +38,11 @@ void AudioDecoderL16::AppendSupportedDecoders(
std::unique_ptr<AudioDecoder> AudioDecoderL16::MakeAudioDecoder( std::unique_ptr<AudioDecoder> AudioDecoderL16::MakeAudioDecoder(
const Config& config, const Config& config,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) { absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
return config.IsOk() ? std::make_unique<AudioDecoderPcm16B>( if (!config.IsOk()) {
config.sample_rate_hz, config.num_channels) return nullptr;
: nullptr; }
return std::make_unique<AudioDecoderPcm16B>(config.sample_rate_hz,
config.num_channels);
} }
} // namespace webrtc } // namespace webrtc

View File

@ -24,6 +24,7 @@ namespace webrtc {
absl::optional<AudioEncoderL16::Config> AudioEncoderL16::SdpToConfig( absl::optional<AudioEncoderL16::Config> AudioEncoderL16::SdpToConfig(
const SdpAudioFormat& format) { const SdpAudioFormat& format) {
if (!rtc::IsValueInRangeForNumericType<int>(format.num_channels)) { if (!rtc::IsValueInRangeForNumericType<int>(format.num_channels)) {
RTC_DCHECK_NOTREACHED();
return absl::nullopt; return absl::nullopt;
} }
Config config; Config config;
@ -36,9 +37,10 @@ absl::optional<AudioEncoderL16::Config> AudioEncoderL16::SdpToConfig(
config.frame_size_ms = rtc::SafeClamp(10 * (*ptime / 10), 10, 60); config.frame_size_ms = rtc::SafeClamp(10 * (*ptime / 10), 10, 60);
} }
} }
return absl::EqualsIgnoreCase(format.name, "L16") && config.IsOk() if (absl::EqualsIgnoreCase(format.name, "L16") && config.IsOk()) {
? absl::optional<Config>(config) return config;
: absl::nullopt; }
return absl::nullopt;
} }
void AudioEncoderL16::AppendSupportedEncoders( void AudioEncoderL16::AppendSupportedEncoders(
@ -58,12 +60,15 @@ std::unique_ptr<AudioEncoder> AudioEncoderL16::MakeAudioEncoder(
const AudioEncoderL16::Config& config, const AudioEncoderL16::Config& config,
int payload_type, int payload_type,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) { absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
RTC_DCHECK(config.IsOk());
AudioEncoderPcm16B::Config c; AudioEncoderPcm16B::Config c;
c.sample_rate_hz = config.sample_rate_hz; c.sample_rate_hz = config.sample_rate_hz;
c.num_channels = config.num_channels; c.num_channels = config.num_channels;
c.frame_size_ms = config.frame_size_ms; c.frame_size_ms = config.frame_size_ms;
c.payload_type = payload_type; c.payload_type = payload_type;
if (!config.IsOk()) {
RTC_DCHECK_NOTREACHED();
return nullptr;
}
return std::make_unique<AudioEncoderPcm16B>(c); return std::make_unique<AudioEncoderPcm16B>(c);
} }

View File

@ -28,7 +28,10 @@ absl::optional<AudioDecoderG711::Config> AudioDecoderG711::SdpToConfig(
Config config; Config config;
config.type = is_pcmu ? Config::Type::kPcmU : Config::Type::kPcmA; config.type = is_pcmu ? Config::Type::kPcmU : Config::Type::kPcmA;
config.num_channels = rtc::dchecked_cast<int>(format.num_channels); 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; return config;
} else { } else {
return absl::nullopt; return absl::nullopt;
@ -45,13 +48,17 @@ void AudioDecoderG711::AppendSupportedDecoders(
std::unique_ptr<AudioDecoder> AudioDecoderG711::MakeAudioDecoder( std::unique_ptr<AudioDecoder> AudioDecoderG711::MakeAudioDecoder(
const Config& config, const Config& config,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) { absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
RTC_DCHECK(config.IsOk()); if (!config.IsOk()) {
RTC_DCHECK_NOTREACHED();
return nullptr;
}
switch (config.type) { switch (config.type) {
case Config::Type::kPcmU: case Config::Type::kPcmU:
return std::make_unique<AudioDecoderPcmU>(config.num_channels); return std::make_unique<AudioDecoderPcmU>(config.num_channels);
case Config::Type::kPcmA: case Config::Type::kPcmA:
return std::make_unique<AudioDecoderPcmA>(config.num_channels); return std::make_unique<AudioDecoderPcmA>(config.num_channels);
default: default:
RTC_DCHECK_NOTREACHED();
return nullptr; return nullptr;
} }
} }

View File

@ -38,7 +38,10 @@ absl::optional<AudioEncoderG711::Config> AudioEncoderG711::SdpToConfig(
config.frame_size_ms = rtc::SafeClamp(10 * (*ptime / 10), 10, 60); 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; return config;
} else { } else {
return absl::nullopt; return absl::nullopt;
@ -62,7 +65,10 @@ std::unique_ptr<AudioEncoder> AudioEncoderG711::MakeAudioEncoder(
const Config& config, const Config& config,
int payload_type, int payload_type,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) { absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
RTC_DCHECK(config.IsOk()); if (!config.IsOk()) {
RTC_DCHECK_NOTREACHED();
return nullptr;
}
switch (config.type) { switch (config.type) {
case Config::Type::kPcmU: { case Config::Type::kPcmU: {
AudioEncoderPcmU::Config impl_config; AudioEncoderPcmU::Config impl_config;
@ -79,6 +85,7 @@ std::unique_ptr<AudioEncoder> AudioEncoderG711::MakeAudioEncoder(
return std::make_unique<AudioEncoderPcmA>(impl_config); return std::make_unique<AudioEncoderPcmA>(impl_config);
} }
default: { default: {
RTC_DCHECK_NOTREACHED();
return nullptr; return nullptr;
} }
} }

View File

@ -21,12 +21,12 @@ namespace webrtc {
absl::optional<AudioDecoderG722::Config> AudioDecoderG722::SdpToConfig( absl::optional<AudioDecoderG722::Config> AudioDecoderG722::SdpToConfig(
const SdpAudioFormat& format) { const SdpAudioFormat& format) {
return absl::EqualsIgnoreCase(format.name, "G722") && if (absl::EqualsIgnoreCase(format.name, "G722") &&
format.clockrate_hz == 8000 && format.clockrate_hz == 8000 &&
(format.num_channels == 1 || format.num_channels == 2) (format.num_channels == 1 || format.num_channels == 2)) {
? absl::optional<Config>( return Config{rtc::dchecked_cast<int>(format.num_channels)};
Config{rtc::dchecked_cast<int>(format.num_channels)}) }
: absl::nullopt; return absl::nullopt;
} }
void AudioDecoderG722::AppendSupportedDecoders( void AudioDecoderG722::AppendSupportedDecoders(
@ -37,12 +37,17 @@ void AudioDecoderG722::AppendSupportedDecoders(
std::unique_ptr<AudioDecoder> AudioDecoderG722::MakeAudioDecoder( std::unique_ptr<AudioDecoder> AudioDecoderG722::MakeAudioDecoder(
Config config, Config config,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) { absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
if (!config.IsOk()) {
RTC_DCHECK_NOTREACHED();
return nullptr;
}
switch (config.num_channels) { switch (config.num_channels) {
case 1: case 1:
return std::make_unique<AudioDecoderG722Impl>(); return std::make_unique<AudioDecoderG722Impl>();
case 2: case 2:
return std::make_unique<AudioDecoderG722StereoImpl>(); return std::make_unique<AudioDecoderG722StereoImpl>();
default: default:
RTC_DCHECK_NOTREACHED();
return nullptr; return nullptr;
} }
} }

View File

@ -38,8 +38,11 @@ absl::optional<AudioEncoderG722Config> AudioEncoderG722::SdpToConfig(
config.frame_size_ms = rtc::SafeClamp<int>(whole_packets * 10, 10, 60); config.frame_size_ms = rtc::SafeClamp<int>(whole_packets * 10, 10, 60);
} }
} }
return config.IsOk() ? absl::optional<AudioEncoderG722Config>(config) if (!config.IsOk()) {
: absl::nullopt; RTC_DCHECK_NOTREACHED();
return absl::nullopt;
}
return config;
} }
void AudioEncoderG722::AppendSupportedEncoders( void AudioEncoderG722::AppendSupportedEncoders(
@ -60,7 +63,10 @@ std::unique_ptr<AudioEncoder> AudioEncoderG722::MakeAudioEncoder(
const AudioEncoderG722Config& config, const AudioEncoderG722Config& config,
int payload_type, int payload_type,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) { 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); return std::make_unique<AudioEncoderG722Impl>(config, payload_type);
} }

View File

@ -20,10 +20,11 @@ namespace webrtc {
absl::optional<AudioDecoderIlbc::Config> AudioDecoderIlbc::SdpToConfig( absl::optional<AudioDecoderIlbc::Config> AudioDecoderIlbc::SdpToConfig(
const SdpAudioFormat& format) { const SdpAudioFormat& format) {
return absl::EqualsIgnoreCase(format.name, "ILBC") && if (absl::EqualsIgnoreCase(format.name, "ILBC") &&
format.clockrate_hz == 8000 && format.num_channels == 1 format.clockrate_hz == 8000 && format.num_channels == 1) {
? absl::optional<Config>(Config()) return Config();
: absl::nullopt; }
return absl::nullopt;
} }
void AudioDecoderIlbc::AppendSupportedDecoders( void AudioDecoderIlbc::AppendSupportedDecoders(

View File

@ -53,8 +53,11 @@ absl::optional<AudioEncoderIlbcConfig> AudioEncoderIlbc::SdpToConfig(
config.frame_size_ms = rtc::SafeClamp<int>(whole_packets * 10, 20, 60); config.frame_size_ms = rtc::SafeClamp<int>(whole_packets * 10, 20, 60);
} }
} }
return config.IsOk() ? absl::optional<AudioEncoderIlbcConfig>(config) if (!config.IsOk()) {
: absl::nullopt; RTC_DCHECK_NOTREACHED();
return absl::nullopt;
}
return config;
} }
void AudioEncoderIlbc::AppendSupportedEncoders( void AudioEncoderIlbc::AppendSupportedEncoders(
@ -74,7 +77,10 @@ std::unique_ptr<AudioEncoder> AudioEncoderIlbc::MakeAudioEncoder(
const AudioEncoderIlbcConfig& config, const AudioEncoderIlbcConfig& config,
int payload_type, int payload_type,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) { 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); return std::make_unique<AudioEncoderIlbcImpl>(config, payload_type);
} }

View File

@ -19,10 +19,11 @@ namespace webrtc {
absl::optional<AudioDecoderIsacFix::Config> AudioDecoderIsacFix::SdpToConfig( absl::optional<AudioDecoderIsacFix::Config> AudioDecoderIsacFix::SdpToConfig(
const SdpAudioFormat& format) { const SdpAudioFormat& format) {
return absl::EqualsIgnoreCase(format.name, "ISAC") && if (absl::EqualsIgnoreCase(format.name, "ISAC") &&
format.clockrate_hz == 16000 && format.num_channels == 1 format.clockrate_hz == 16000 && format.num_channels == 1) {
? absl::optional<Config>(Config()) return Config();
: absl::nullopt; }
return absl::nullopt;
} }
void AudioDecoderIsacFix::AppendSupportedDecoders( void AudioDecoderIsacFix::AppendSupportedDecoders(

View File

@ -24,6 +24,10 @@ AudioDecoderIsacFloat::SdpToConfig(const SdpAudioFormat& format) {
format.num_channels == 1) { format.num_channels == 1) {
Config config; Config config;
config.sample_rate_hz = format.clockrate_hz; config.sample_rate_hz = format.clockrate_hz;
if (!config.IsOk()) {
RTC_DCHECK_NOTREACHED();
return absl::nullopt;
}
return config; return config;
} else { } else {
return absl::nullopt; return absl::nullopt;
@ -39,9 +43,12 @@ void AudioDecoderIsacFloat::AppendSupportedDecoders(
std::unique_ptr<AudioDecoder> AudioDecoderIsacFloat::MakeAudioDecoder( std::unique_ptr<AudioDecoder> AudioDecoderIsacFloat::MakeAudioDecoder(
Config config, Config config,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) { absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
RTC_DCHECK(config.IsOk());
AudioDecoderIsacFloatImpl::Config c; AudioDecoderIsacFloatImpl::Config c;
c.sample_rate_hz = config.sample_rate_hz; c.sample_rate_hz = config.sample_rate_hz;
if (!config.IsOk()) {
RTC_DCHECK_NOTREACHED();
return nullptr;
}
return std::make_unique<AudioDecoderIsacFloatImpl>(c); return std::make_unique<AudioDecoderIsacFloatImpl>(c);
} }

View File

@ -30,6 +30,10 @@ absl::optional<AudioEncoderIsacFix::Config> AudioEncoderIsacFix::SdpToConfig(
config.frame_size_ms = 60; config.frame_size_ms = 60;
} }
} }
if (!config.IsOk()) {
RTC_DCHECK_NOTREACHED();
return absl::nullopt;
}
return config; return config;
} else { } else {
return absl::nullopt; return absl::nullopt;
@ -53,11 +57,14 @@ std::unique_ptr<AudioEncoder> AudioEncoderIsacFix::MakeAudioEncoder(
AudioEncoderIsacFix::Config config, AudioEncoderIsacFix::Config config,
int payload_type, int payload_type,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) { absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
RTC_DCHECK(config.IsOk());
AudioEncoderIsacFixImpl::Config c; AudioEncoderIsacFixImpl::Config c;
c.frame_size_ms = config.frame_size_ms; c.frame_size_ms = config.frame_size_ms;
c.bit_rate = config.bit_rate; c.bit_rate = config.bit_rate;
c.payload_type = payload_type; c.payload_type = payload_type;
if (!config.IsOk()) {
RTC_DCHECK_NOTREACHED();
return nullptr;
}
return std::make_unique<AudioEncoderIsacFixImpl>(c); return std::make_unique<AudioEncoderIsacFixImpl>(c);
} }

View File

@ -37,6 +37,10 @@ AudioEncoderIsacFloat::SdpToConfig(const SdpAudioFormat& format) {
} }
} }
} }
if (!config.IsOk()) {
RTC_DCHECK_NOTREACHED();
return absl::nullopt;
}
return config; return config;
} else { } else {
return absl::nullopt; return absl::nullopt;
@ -65,12 +69,15 @@ std::unique_ptr<AudioEncoder> AudioEncoderIsacFloat::MakeAudioEncoder(
const AudioEncoderIsacFloat::Config& config, const AudioEncoderIsacFloat::Config& config,
int payload_type, int payload_type,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) { absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
RTC_DCHECK(config.IsOk());
AudioEncoderIsacFloatImpl::Config c; AudioEncoderIsacFloatImpl::Config c;
c.payload_type = payload_type; c.payload_type = payload_type;
c.sample_rate_hz = config.sample_rate_hz; c.sample_rate_hz = config.sample_rate_hz;
c.frame_size_ms = config.frame_size_ms; c.frame_size_ms = config.frame_size_ms;
c.bit_rate = config.bit_rate; c.bit_rate = config.bit_rate;
if (!config.IsOk()) {
RTC_DCHECK_NOTREACHED();
return nullptr;
}
return std::make_unique<AudioEncoderIsacFloatImpl>(c); return std::make_unique<AudioEncoderIsacFloatImpl>(c);
} }

View File

@ -51,7 +51,10 @@ absl::optional<AudioDecoderOpus::Config> AudioDecoderOpus::SdpToConfig(
num_channels) { num_channels) {
Config config; Config config;
config.num_channels = *num_channels; config.num_channels = *num_channels;
RTC_DCHECK(config.IsOk()); if (!config.IsOk()) {
RTC_DCHECK_NOTREACHED();
return absl::nullopt;
}
return config; return config;
} else { } else {
return absl::nullopt; return absl::nullopt;
@ -71,7 +74,10 @@ void AudioDecoderOpus::AppendSupportedDecoders(
std::unique_ptr<AudioDecoder> AudioDecoderOpus::MakeAudioDecoder( std::unique_ptr<AudioDecoder> AudioDecoderOpus::MakeAudioDecoder(
Config config, Config config,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) { 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, return std::make_unique<AudioDecoderOpusImpl>(config.num_channels,
config.sample_rate_hz); config.sample_rate_hz);
} }

View File

@ -33,6 +33,10 @@ std::unique_ptr<AudioEncoder> AudioEncoderOpus::MakeAudioEncoder(
const AudioEncoderOpusConfig& config, const AudioEncoderOpusConfig& config,
int payload_type, int payload_type,
absl::optional<AudioCodecPairId> /*codec_pair_id*/) { absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
if (!config.IsOk()) {
RTC_DCHECK_NOTREACHED();
return nullptr;
}
return AudioEncoderOpusImpl::MakeAudioEncoder(config, payload_type); return AudioEncoderOpusImpl::MakeAudioEncoder(config, payload_type);
} }

View File

@ -26,6 +26,7 @@ std::unique_ptr<AudioDecoderMultiChannelOpusImpl>
AudioDecoderMultiChannelOpusImpl::MakeAudioDecoder( AudioDecoderMultiChannelOpusImpl::MakeAudioDecoder(
AudioDecoderMultiChannelOpusConfig config) { AudioDecoderMultiChannelOpusConfig config) {
if (!config.IsOk()) { if (!config.IsOk()) {
RTC_DCHECK_NOTREACHED();
return nullptr; return nullptr;
} }
// Fill the pointer with a working decoder through the C interface. This // Fill the pointer with a working decoder through the C interface. This
@ -78,6 +79,9 @@ AudioDecoderMultiChannelOpusImpl::SdpToConfig(const SdpAudioFormat& format) {
return absl::nullopt; return absl::nullopt;
} }
config.channel_mapping = *channel_mapping; config.channel_mapping = *channel_mapping;
if (!config.IsOk()) {
return absl::nullopt;
}
return config; return config;
} }

View File

@ -45,13 +45,7 @@ TEST(AudioDecoderMultiOpusTest, InvalidChannelMappings) {
{"num_streams", "2"}}); {"num_streams", "2"}});
const absl::optional<AudioDecoderMultiChannelOpus::Config> decoder_config = const absl::optional<AudioDecoderMultiChannelOpus::Config> decoder_config =
AudioDecoderMultiChannelOpus::SdpToConfig(sdp_format); AudioDecoderMultiChannelOpus::SdpToConfig(sdp_format);
ASSERT_TRUE(decoder_config.has_value()); EXPECT_FALSE(decoder_config.has_value());
EXPECT_FALSE(decoder_config->IsOk());
const std::unique_ptr<AudioDecoder> opus_decoder =
AudioDecoderMultiChannelOpus::MakeAudioDecoder(*decoder_config);
EXPECT_FALSE(opus_decoder);
} }
{ {
// The mapping is too long. There are only 5 channels, but 6 elements in the // The mapping is too long. There are only 5 channels, but 6 elements in the
@ -62,13 +56,7 @@ TEST(AudioDecoderMultiOpusTest, InvalidChannelMappings) {
{"num_streams", "2"}}); {"num_streams", "2"}});
const absl::optional<AudioDecoderMultiChannelOpus::Config> decoder_config = const absl::optional<AudioDecoderMultiChannelOpus::Config> decoder_config =
AudioDecoderMultiChannelOpus::SdpToConfig(sdp_format); AudioDecoderMultiChannelOpus::SdpToConfig(sdp_format);
ASSERT_TRUE(decoder_config.has_value()); EXPECT_FALSE(decoder_config.has_value());
EXPECT_FALSE(decoder_config->IsOk());
const std::unique_ptr<AudioDecoder> opus_decoder =
AudioDecoderMultiChannelOpus::MakeAudioDecoder(*decoder_config);
EXPECT_FALSE(opus_decoder);
} }
{ {
// The mapping doesn't parse correctly. // The mapping doesn't parse correctly.

View File

@ -131,6 +131,7 @@ AudioEncoderMultiChannelOpusImpl::MakeAudioEncoder(
const AudioEncoderMultiChannelOpusConfig& config, const AudioEncoderMultiChannelOpusConfig& config,
int payload_type) { int payload_type) {
if (!config.IsOk()) { if (!config.IsOk()) {
RTC_DCHECK_NOTREACHED();
return nullptr; return nullptr;
} }
return std::make_unique<AudioEncoderMultiChannelOpusImpl>(config, return std::make_unique<AudioEncoderMultiChannelOpusImpl>(config,
@ -280,6 +281,9 @@ AudioEncoderMultiChannelOpusImpl::SdpToConfig(const SdpAudioFormat& format) {
} }
config.channel_mapping = *channel_mapping; config.channel_mapping = *channel_mapping;
if (!config.IsOk()) {
return absl::nullopt;
}
return config; return config;
} }

View File

@ -28,10 +28,9 @@ TEST(AudioEncoderMultiOpusTest, CheckConfigValidity) {
{"num_streams", "2"}}); {"num_streams", "2"}});
const absl::optional<AudioEncoderMultiChannelOpus::Config> encoder_config = const absl::optional<AudioEncoderMultiChannelOpus::Config> encoder_config =
AudioEncoderMultiChannelOpus::SdpToConfig(sdp_format); AudioEncoderMultiChannelOpus::SdpToConfig(sdp_format);
ASSERT_TRUE(encoder_config.has_value());
// Maps input channel 0 to coded channel 3, which doesn't exist. // 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 @@ TEST(AudioEncoderMultiOpusTest, CheckConfigValidity) {
{"num_streams", "2"}}); {"num_streams", "2"}});
const absl::optional<AudioEncoderMultiChannelOpus::Config> encoder_config = const absl::optional<AudioEncoderMultiChannelOpus::Config> encoder_config =
AudioEncoderMultiChannelOpus::SdpToConfig(sdp_format); AudioEncoderMultiChannelOpus::SdpToConfig(sdp_format);
ASSERT_TRUE(encoder_config.has_value());
// The mapping is too short. // The mapping is too short.
EXPECT_FALSE(encoder_config->IsOk()); EXPECT_FALSE(encoder_config.has_value());
} }
{ {
const SdpAudioFormat sdp_format("multiopus", 48000, 3, const SdpAudioFormat sdp_format("multiopus", 48000, 3,
@ -53,10 +51,9 @@ TEST(AudioEncoderMultiOpusTest, CheckConfigValidity) {
{"num_streams", "1"}}); {"num_streams", "1"}});
const absl::optional<AudioEncoderMultiChannelOpus::Config> encoder_config = const absl::optional<AudioEncoderMultiChannelOpus::Config> encoder_config =
AudioEncoderMultiChannelOpus::SdpToConfig(sdp_format); AudioEncoderMultiChannelOpus::SdpToConfig(sdp_format);
ASSERT_TRUE(encoder_config.has_value());
// Coded channel 0 comes from both input channels 0, 1 and 2. // 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, const SdpAudioFormat sdp_format("multiopus", 48000, 3,
@ -77,11 +74,10 @@ TEST(AudioEncoderMultiOpusTest, CheckConfigValidity) {
{"num_streams", "2"}}); {"num_streams", "2"}});
const absl::optional<AudioEncoderMultiChannelOpus::Config> encoder_config = const absl::optional<AudioEncoderMultiChannelOpus::Config> encoder_config =
AudioEncoderMultiChannelOpus::SdpToConfig(sdp_format); AudioEncoderMultiChannelOpus::SdpToConfig(sdp_format);
ASSERT_TRUE(encoder_config.has_value());
// This is NOT fine, because channels nothing says how coded channel 1 // This is NOT fine, because channels nothing says how coded channel 1
// should be coded. // should be coded.
EXPECT_FALSE(encoder_config->IsOk()); EXPECT_FALSE(encoder_config.has_value());
} }
} }
@ -105,7 +101,7 @@ TEST(AudioEncoderMultiOpusTest, ConfigValuesAreParsedCorrectly) {
testing::ContainerEq(std::vector<unsigned char>({0, 4, 1, 2, 3, 5}))); 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, const SdpAudioFormat sdp_format("multiopus", 48000, 3,
{{"channel_mapping", "0,255,255"}, {{"channel_mapping", "0,255,255"},
@ -113,19 +109,7 @@ TEST(AudioEncoderMultiOpusTest, CreateFromValidOrInvalidConfig) {
{"num_streams", "2"}}); {"num_streams", "2"}});
const absl::optional<AudioEncoderMultiChannelOpus::Config> encoder_config = const absl::optional<AudioEncoderMultiChannelOpus::Config> encoder_config =
AudioEncoderMultiChannelOpus::SdpToConfig(sdp_format); AudioEncoderMultiChannelOpus::SdpToConfig(sdp_format);
ASSERT_TRUE(encoder_config.has_value()); ASSERT_FALSE(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);
} }
{ {
const SdpAudioFormat sdp_format("multiopus", 48000, 3, const SdpAudioFormat sdp_format("multiopus", 48000, 3,

View File

@ -229,7 +229,10 @@ AudioCodecInfo AudioEncoderOpusImpl::QueryAudioEncoder(
std::unique_ptr<AudioEncoder> AudioEncoderOpusImpl::MakeAudioEncoder( std::unique_ptr<AudioEncoder> AudioEncoderOpusImpl::MakeAudioEncoder(
const AudioEncoderOpusConfig& config, const AudioEncoderOpusConfig& config,
int payload_type) { int payload_type) {
RTC_DCHECK(config.IsOk()); if (!config.IsOk()) {
RTC_DCHECK_NOTREACHED();
return nullptr;
}
return std::make_unique<AudioEncoderOpusImpl>(config, payload_type); return std::make_unique<AudioEncoderOpusImpl>(config, payload_type);
} }
@ -268,7 +271,10 @@ absl::optional<AudioEncoderOpusConfig> AudioEncoderOpusImpl::SdpToConfig(
FindSupportedFrameLengths(min_frame_length_ms, max_frame_length_ms, FindSupportedFrameLengths(min_frame_length_ms, max_frame_length_ms,
&config.supported_frame_lengths_ms); &config.supported_frame_lengths_ms);
RTC_DCHECK(config.IsOk()); if (!config.IsOk()) {
RTC_DCHECK_NOTREACHED();
return absl::nullopt;
}
return config; return config;
} }