Adding a some checks and switching out a few assert for RTC_[D]CHECK.

These changes are around use of AudioFrame.data_ to help us catch issues earlier since assert() is left out in release builds, including builds with DCHECK enabled.  I've also added a few full-on CHECKs to avoid reading past buffer boundaries or continuing on in a failed state.

BUG=chromium:613482
NOTRY=true
(using notry due to offline android_arm64_rel bot)

Review-Url: https://codereview.webrtc.org/2007563002
Cr-Commit-Position: refs/heads/master@{#12870}
This commit is contained in:
tommi 2016-05-24 05:49:04 -07:00 committed by Commit bot
parent 32e80e4b2e
commit d36df89d40
6 changed files with 54 additions and 36 deletions

View File

@ -12,6 +12,7 @@
#include <string.h>
#include "webrtc/base/checks.h"
#include "webrtc/common_audio/include/audio_util.h"
#include "webrtc/common_audio/resampler/include/resampler.h"
#include "webrtc/common_audio/resampler/push_sinc_resampler.h"
@ -33,15 +34,22 @@ template <typename T>
int PushResampler<T>::InitializeIfNeeded(int src_sample_rate_hz,
int dst_sample_rate_hz,
size_t num_channels) {
RTC_DCHECK_GT(src_sample_rate_hz, 0);
RTC_DCHECK_GT(dst_sample_rate_hz, 0);
RTC_DCHECK_GT(num_channels, 0u);
RTC_DCHECK_LE(num_channels, 2u);
if (src_sample_rate_hz == src_sample_rate_hz_ &&
dst_sample_rate_hz == dst_sample_rate_hz_ &&
num_channels == num_channels_)
num_channels == num_channels_) {
// No-op if settings haven't changed.
return 0;
}
if (src_sample_rate_hz <= 0 || dst_sample_rate_hz <= 0 ||
num_channels <= 0 || num_channels > 2)
if (src_sample_rate_hz <= 0 || dst_sample_rate_hz <= 0 || num_channels <= 0 ||
num_channels > 2) {
return -1;
}
src_sample_rate_hz_ = src_sample_rate_hz;
dst_sample_rate_hz_ = dst_sample_rate_hz;
@ -70,6 +78,8 @@ int PushResampler<T>::Resample(const T* src, size_t src_length, T* dst,
size_t dst_capacity) {
const size_t src_size_10ms = src_sample_rate_hz_ * num_channels_ / 100;
const size_t dst_size_10ms = dst_sample_rate_hz_ * num_channels_ / 100;
RTC_CHECK_EQ(src_length, src_size_10ms);
RTC_CHECK_GE(dst_capacity, dst_size_10ms);
if (src_length != src_size_10ms || dst_capacity < dst_size_10ms)
return -1;

View File

@ -9,6 +9,7 @@
*/
#include "testing/gtest/include/gtest/gtest.h"
#include "webrtc/base/checks.h" // force defintion of RTC_DCHECK_IS_ON
#include "webrtc/common_audio/resampler/include/push_resampler.h"
// Quality testing of PushResampler is handled through output_mixer_unittest.cc.
@ -17,12 +18,32 @@ namespace webrtc {
TEST(PushResamplerTest, VerifiesInputParameters) {
PushResampler<int16_t> resampler;
EXPECT_EQ(-1, resampler.InitializeIfNeeded(-1, 16000, 1));
EXPECT_EQ(-1, resampler.InitializeIfNeeded(16000, -1, 1));
EXPECT_EQ(-1, resampler.InitializeIfNeeded(16000, 16000, 0));
EXPECT_EQ(-1, resampler.InitializeIfNeeded(16000, 16000, 3));
EXPECT_EQ(0, resampler.InitializeIfNeeded(16000, 16000, 1));
EXPECT_EQ(0, resampler.InitializeIfNeeded(16000, 16000, 2));
}
#if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
TEST(PushResamplerTest, VerifiesBadInputParameters1) {
PushResampler<int16_t> resampler;
EXPECT_DEATH(resampler.InitializeIfNeeded(-1, 16000, 1),
"src_sample_rate_hz");
}
TEST(PushResamplerTest, VerifiesBadInputParameters2) {
PushResampler<int16_t> resampler;
EXPECT_DEATH(resampler.InitializeIfNeeded(16000, -1, 1),
"dst_sample_rate_hz");
}
TEST(PushResamplerTest, VerifiesBadInputParameters3) {
PushResampler<int16_t> resampler;
EXPECT_DEATH(resampler.InitializeIfNeeded(16000, 16000, 0), "num_channels");
}
TEST(PushResamplerTest, VerifiesBadInputParameters4) {
PushResampler<int16_t> resampler;
EXPECT_DEATH(resampler.InitializeIfNeeded(16000, 16000, 3), "num_channels");
}
#endif
} // namespace webrtc

View File

@ -309,11 +309,14 @@ TEST_F(AudioCodingModuleTestOldApi, VerifyOutputFrame) {
EXPECT_EQ(kSampleRateHz, audio_frame.sample_rate_hz_);
}
#if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
TEST_F(AudioCodingModuleTestOldApi, FailOnZeroDesiredFrequency) {
AudioFrame audio_frame;
bool muted;
EXPECT_EQ(-1, acm_->PlayoutData10Ms(0, &audio_frame, &muted));
EXPECT_DEATH(acm_->PlayoutData10Ms(0, &audio_frame, &muted),
"dst_sample_rate_hz");
}
#endif
// Checks that the transport callback is invoked once for each speech packet.
// Also checks that the frame type is kAudioFrameSpeech.

View File

@ -2993,6 +2993,7 @@ uint32_t Channel::PrepareEncodeAndSend(int mixingFrequency) {
if (_includeAudioLevelIndication) {
size_t length =
_audioFrame.samples_per_channel_ * _audioFrame.num_channels_;
RTC_CHECK_LE(length, sizeof(_audioFrame.data_));
if (is_muted && previous_frame_muted_) {
rms_level_.ProcessMuted(length);
} else {

View File

@ -107,20 +107,3 @@ TEST_F(ExternalMediaTest,
EXPECT_EQ(0, voe_xmedia_->SetExternalMixing(channel_, false));
ResumePlaying();
}
TEST_F(ExternalMediaTest,
ExternalMixingResamplingToInvalidFrequenciesFails) {
const int kInvalidFrequencies[] = {-8000, -1};
webrtc::AudioFrame frame;
PausePlaying();
EXPECT_EQ(0, voe_xmedia_->SetExternalMixing(channel_, true));
ResumePlaying();
for (size_t i = 0; i < arraysize(kInvalidFrequencies); i++) {
int f = kInvalidFrequencies[i];
EXPECT_EQ(-1, voe_xmedia_->GetAudioFrame(channel_, f, &frame))
<< "Resampling fails for freq=" << f;
}
PausePlaying();
EXPECT_EQ(0, voe_xmedia_->SetExternalMixing(channel_, false));
ResumePlaying();
}

View File

@ -10,6 +10,7 @@
#include "webrtc/voice_engine/utility.h"
#include "webrtc/base/checks.h"
#include "webrtc/base/logging.h"
#include "webrtc/common_audio/resampler/include/push_resampler.h"
#include "webrtc/common_audio/signal_processing/include/signal_processing_library.h"
@ -52,21 +53,18 @@ void RemixAndResample(const int16_t* src_data,
if (resampler->InitializeIfNeeded(sample_rate_hz, dst_frame->sample_rate_hz_,
audio_ptr_num_channels) == -1) {
LOG(LS_ERROR) << "InitializeIfNeeded failed: sample_rate_hz = "
<< sample_rate_hz << ", dst_frame->sample_rate_hz_ = "
<< dst_frame->sample_rate_hz_
FATAL() << "InitializeIfNeeded failed: sample_rate_hz = " << sample_rate_hz
<< ", dst_frame->sample_rate_hz_ = " << dst_frame->sample_rate_hz_
<< ", audio_ptr_num_channels = " << audio_ptr_num_channels;
assert(false);
}
const size_t src_length = samples_per_channel * audio_ptr_num_channels;
int out_length = resampler->Resample(audio_ptr, src_length, dst_frame->data_,
AudioFrame::kMaxDataSizeSamples);
if (out_length == -1) {
LOG(LS_ERROR) << "Resample failed: audio_ptr = " << audio_ptr
FATAL() << "Resample failed: audio_ptr = " << audio_ptr
<< ", src_length = " << src_length
<< ", dst_frame->data_ = " << dst_frame->data_;
assert(false);
}
dst_frame->samples_per_channel_ = out_length / audio_ptr_num_channels;
@ -84,8 +82,10 @@ void MixWithSat(int16_t target[],
const int16_t source[],
size_t source_channel,
size_t source_len) {
assert(target_channel == 1 || target_channel == 2);
assert(source_channel == 1 || source_channel == 2);
RTC_DCHECK_GE(target_channel, 1u);
RTC_DCHECK_LE(target_channel, 2u);
RTC_DCHECK_GE(source_channel, 1u);
RTC_DCHECK_LE(source_channel, 2u);
if (target_channel == 2 && source_channel == 1) {
// Convert source from mono to stereo.