Fix crash when setting RTCRtpEncodingParameter.maxBitrate to a non-positive value.

This will result in using a default value instead.

The crash is caused by configuring audio send stream with a non-positive max_bitrate_bps and it expects a positive value (or special value -1).

Also remove support for setting min bitrate since it is not in the spec and there are no tests.

Bug: chromium:1377286
Change-Id: I19af29bff79eda5c0fbb1e528fced7976f5c2511
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/284640
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Jakob Ivarsson‎ <jakobi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38719}
This commit is contained in:
Jakob Ivarsson
2022-11-23 16:48:49 +01:00
committed by WebRTC LUCI CQ
parent 4bee365c82
commit 514dff834b
3 changed files with 58 additions and 8 deletions

View File

@ -619,6 +619,7 @@ if (rtc_include_tests) {
"../call:call_interfaces",
"../common_video",
"../modules/audio_device:mock_audio_device",
"../modules/audio_mixer:audio_mixer_impl",
"../modules/audio_processing",
"../modules/audio_processing:api",
"../modules/audio_processing:mocks",

View File

@ -1041,7 +1041,6 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream
// - a reasonable default of 32kbps min/max
// - fixed target bitrate from codec spec
// - lower min bitrate if adaptive ptime is enabled
// - bitrate configured in the rtp_parameter encodings settings
const int kDefaultBitrateBps = 32000;
config_.min_bitrate_bps = kDefaultBitrateBps;
config_.max_bitrate_bps = kDefaultBitrateBps;
@ -1057,13 +1056,6 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream
config_.min_bitrate_bps,
static_cast<int>(adaptive_ptime_config_.min_encoder_bitrate.bps()));
}
if (rtp_parameters_.encodings[0].min_bitrate_bps) {
config_.min_bitrate_bps = *rtp_parameters_.encodings[0].min_bitrate_bps;
}
if (rtp_parameters_.encodings[0].max_bitrate_bps) {
config_.max_bitrate_bps = *rtp_parameters_.encodings[0].max_bitrate_bps;
}
}
void UpdateSendCodecSpec(

View File

@ -29,6 +29,7 @@
#include "media/base/media_constants.h"
#include "media/engine/fake_webrtc_call.h"
#include "modules/audio_device/include/mock_audio_device.h"
#include "modules/audio_mixer/audio_mixer_impl.h"
#include "modules/audio_processing/include/mock_audio_processing.h"
#include "rtc_base/arraysize.h"
#include "rtc_base/byte_order.h"
@ -444,6 +445,10 @@ class WebRtcVoiceEngineTestFake : public ::testing::TestWithParam<bool> {
return GetSendStreamConfig(ssrc).send_codec_spec->target_bitrate_bps;
}
int GetMaxBitrate(int32_t ssrc) {
return GetSendStreamConfig(ssrc).max_bitrate_bps;
}
const absl::optional<std::string>& GetAudioNetworkAdaptorConfig(
int32_t ssrc) {
return GetSendStreamConfig(ssrc).audio_network_adaptor_config;
@ -471,6 +476,7 @@ class WebRtcVoiceEngineTestFake : public ::testing::TestWithParam<bool> {
// Verify that the codec settings have the expected bitrate.
EXPECT_EQ(expected_codec_bitrate, GetCodecBitrate(kSsrcX));
EXPECT_EQ(expected_codec_bitrate, GetMaxBitrate(kSsrcX));
}
void SetSendCodecsShouldWorkForBitrates(const char* min_bitrate_kbps,
@ -3741,6 +3747,57 @@ TEST(WebRtcVoiceEngineTest, SetRecvCodecs) {
}
}
TEST(WebRtcVoiceEngineTest, SetRtpSendParametersMaxBitrate) {
rtc::AutoThread main_thread;
std::unique_ptr<webrtc::TaskQueueFactory> task_queue_factory =
webrtc::CreateDefaultTaskQueueFactory();
rtc::scoped_refptr<webrtc::test::MockAudioDeviceModule> adm =
webrtc::test::MockAudioDeviceModule::CreateNice();
webrtc::FieldTrialBasedConfig field_trials;
FakeAudioSource source;
cricket::WebRtcVoiceEngine engine(task_queue_factory.get(), adm.get(),
webrtc::CreateBuiltinAudioEncoderFactory(),
webrtc::CreateBuiltinAudioDecoderFactory(),
nullptr, nullptr, nullptr, field_trials);
engine.Init();
webrtc::RtcEventLogNull event_log;
webrtc::Call::Config call_config(&event_log);
call_config.trials = &field_trials;
call_config.task_queue_factory = task_queue_factory.get();
{
webrtc::AudioState::Config config;
config.audio_mixer = webrtc::AudioMixerImpl::Create();
config.audio_device_module =
webrtc::test::MockAudioDeviceModule::CreateNice();
call_config.audio_state = webrtc::AudioState::Create(config);
}
auto call = absl::WrapUnique(webrtc::Call::Create(call_config));
cricket::WebRtcVoiceMediaChannel channel(&engine, cricket::MediaConfig(),
cricket::AudioOptions(),
webrtc::CryptoOptions(), call.get());
{
cricket::AudioSendParameters params;
params.codecs.push_back(cricket::AudioCodec(1, "opus", 48000, 32000, 2));
params.extensions.push_back(webrtc::RtpExtension(
webrtc::RtpExtension::kTransportSequenceNumberUri, 1));
EXPECT_TRUE(channel.SetSendParameters(params));
}
constexpr int kSsrc = 1234;
{
cricket::StreamParams params;
params.add_ssrc(kSsrc);
channel.AddSendStream(params);
}
channel.SetAudioSend(kSsrc, true, nullptr, &source);
channel.SetSend(true);
webrtc::RtpParameters params = channel.GetRtpSendParameters(kSsrc);
for (int max_bitrate : {-10, -1, 0, 10000}) {
params.encodings[0].max_bitrate_bps = max_bitrate;
channel.SetRtpSendParameters(
kSsrc, params, [](webrtc::RTCError error) { EXPECT_TRUE(error.ok()); });
}
}
TEST(WebRtcVoiceEngineTest, CollectRecvCodecs) {
for (bool use_null_apm : {false, true}) {
std::vector<webrtc::AudioCodecSpec> specs;