Revert of Adding a some checks and switching out a few assert for RTC_[D]CHECK. (patchset #1 id:1 of https://codereview.webrtc.org/2014183003/ )
Reason for revert:
Alas, the bot magically fails again. Looks like indeed it's a Clang bug :(
https://build.chromium.org/p/client.webrtc/builders/Win64%20Debug%20%28Clang%29/builds/1687
Original issue's description:
> Reland of Adding a some checks and switching out a few assert for RTC_[D]CHECK. (patchset #1 id:1 of https://codereview.webrtc.org/2006313009/ )
>
> Reason for revert:
> Relanding. The errors look like legit disk space problems so I'm going to watch the bots this time.
>
> Original issue's description:
> > Revert of Adding a some checks and switching out a few assert for RTC_[D]CHECK. (patchset #1 id:1 of https://codereview.webrtc.org/2009253004/ )
> >
> > Reason for revert:
> > Fails unexpectedly on multiple commit bots:
> > https://build.chromium.org/p/client.webrtc/builders/Win32%20Debug%20%28Clang%29/builds/1748
> > https://build.chromium.org/p/client.webrtc/builders/Win64%20Debug%20%28Clang%29/builds/1683
> >
> > I filed https://bugs.chromium.org/p/chromium/issues/detail?id=614967 to track the problem. I'll reland if it doesn't solve the problem.
> >
> > Original issue's description:
> > > Reland of Adding a some checks and switching out a few assert for RTC_[D]CHECK. (patchset #1 id:1 of https://codereview.webrtc.org/2006243002/ )
> > >
> > > Original issue's description:
> > > > 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)
> > > >
> > > > Committed: https://crrev.com/d36df89d40bde3c62ee5cbff841933e50b3c007b
> > > > Cr-Commit-Position: refs/heads/master@{#12870}
> > >
> > > TBR=henrik.lundin@webrtc.org
> > > # Not skipping CQ checks because original CL landed more than 1 days ago.
> > > BUG=chromium:613482
> > >
> > > Committed: https://crrev.com/ba189cc4f4f6fe311a815646059e8011ffa53894
> > > Cr-Commit-Position: refs/heads/master@{#12907}
> >
> > TBR=henrik.lundin@webrtc.org,tommi@webrtc.org
> > # Skipping CQ checks because original CL landed less than 1 days ago.
> > NOPRESUBMIT=true
> > NOTREECHECKS=true
> > NOTRY=true
> > BUG=chromium:613482
> >
> > Committed: https://crrev.com/6895d8ca788df29f4d3a4b97fe891f0fb3a6dbec
> > Cr-Commit-Position: refs/heads/master@{#12909}
>
> TBR=henrik.lundin@webrtc.org,kjellander@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=chromium:613482
>
> Committed: https://crrev.com/b7318f1293453d8d319e27473c5bfb72292ebfd2
> Cr-Commit-Position: refs/heads/master@{#12920}
TBR=henrik.lundin@webrtc.org,kjellander@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:613482
Review-Url: https://codereview.webrtc.org/2018553002
Cr-Commit-Position: refs/heads/master@{#12921}
diff --git a/webrtc/common_audio/resampler/push_resampler.cc b/webrtc/common_audio/resampler/push_resampler.cc
index 06fdfb8..f654e9a 100644
--- a/webrtc/common_audio/resampler/push_resampler.cc
+++ b/webrtc/common_audio/resampler/push_resampler.cc
@@ -12,7 +12,6 @@
#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"
@@ -34,22 +33,15 @@
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;
@@ -78,8 +70,6 @@
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;
diff --git a/webrtc/common_audio/resampler/push_resampler_unittest.cc b/webrtc/common_audio/resampler/push_resampler_unittest.cc
index 58880cc..4449f4c 100644
--- a/webrtc/common_audio/resampler/push_resampler_unittest.cc
+++ b/webrtc/common_audio/resampler/push_resampler_unittest.cc
@@ -9,7 +9,6 @@
*/
#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.
@@ -18,32 +17,12 @@
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
diff --git a/webrtc/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc b/webrtc/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc
index eefe0a5..d30a63c 100644
--- a/webrtc/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc
+++ b/webrtc/modules/audio_coding/acm2/audio_coding_module_unittest_oldapi.cc
@@ -309,14 +309,11 @@
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_DEATH(acm_->PlayoutData10Ms(0, &audio_frame, &muted),
- "dst_sample_rate_hz");
+ EXPECT_EQ(-1, acm_->PlayoutData10Ms(0, &audio_frame, &muted));
}
-#endif
// Checks that the transport callback is invoked once for each speech packet.
// Also checks that the frame type is kAudioFrameSpeech.
diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc
index 8ccad33..742e53e 100644
--- a/webrtc/voice_engine/channel.cc
+++ b/webrtc/voice_engine/channel.cc
@@ -2995,7 +2995,6 @@
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 {
diff --git a/webrtc/voice_engine/test/auto_test/standard/external_media_test.cc b/webrtc/voice_engine/test/auto_test/standard/external_media_test.cc
index 4534e12..4f86010 100644
--- a/webrtc/voice_engine/test/auto_test/standard/external_media_test.cc
+++ b/webrtc/voice_engine/test/auto_test/standard/external_media_test.cc
@@ -107,3 +107,20 @@
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();
+}
diff --git a/webrtc/voice_engine/utility.cc b/webrtc/voice_engine/utility.cc
index 37e12ce..605e553 100644
--- a/webrtc/voice_engine/utility.cc
+++ b/webrtc/voice_engine/utility.cc
@@ -10,7 +10,6 @@
#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"
@@ -53,18 +52,21 @@
if (resampler->InitializeIfNeeded(sample_rate_hz, dst_frame->sample_rate_hz_,
audio_ptr_num_channels) == -1) {
- 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;
+ LOG(LS_ERROR) << "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) {
- FATAL() << "Resample failed: audio_ptr = " << audio_ptr
- << ", src_length = " << src_length
- << ", dst_frame->data_ = " << dst_frame->data_;
+ LOG(LS_ERROR) << "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;
@@ -82,10 +84,8 @@
const int16_t source[],
size_t source_channel,
size_t source_len) {
- RTC_DCHECK_GE(target_channel, 1u);
- RTC_DCHECK_LE(target_channel, 2u);
- RTC_DCHECK_GE(source_channel, 1u);
- RTC_DCHECK_LE(source_channel, 2u);
+ assert(target_channel == 1 || target_channel == 2);
+ assert(source_channel == 1 || source_channel == 2);
if (target_channel == 2 && source_channel == 1) {
// Convert source from mono to stereo.
