From bcde776340c1eb3f2e1703966619c6015a5f7f4e Mon Sep 17 00:00:00 2001 From: "bjornv@webrtc.org" Date: Fri, 20 Apr 2012 09:35:20 +0000 Subject: [PATCH] Changed Delay Estimator create call Unit tests updated and runs. Change made w.r.t. issue 441. BUG=Issue441 TEST=None Review URL: https://webrtc-codereview.appspot.com/498001 git-svn-id: http://webrtc.googlecode.com/svn/trunk@2079 4adac7df-926f-26a2-2b94-8c16560cd09d --- src/modules/audio_processing/aec/aec_core.c | 8 +-- src/modules/audio_processing/aecm/aecm_core.c | 7 +- .../utility/delay_estimator_unittest.cc | 16 ++--- .../utility/delay_estimator_wrapper.c | 68 ++++++++----------- .../utility/delay_estimator_wrapper.h | 15 ++-- 5 files changed, 49 insertions(+), 65 deletions(-) diff --git a/src/modules/audio_processing/aec/aec_core.c b/src/modules/audio_processing/aec/aec_core.c index c5ec5592f8..f43c9246ec 100644 --- a/src/modules/audio_processing/aec/aec_core.c +++ b/src/modules/audio_processing/aec/aec_core.c @@ -206,10 +206,10 @@ int WebRtcAec_CreateAec(aec_t **aecInst) return -1; } #endif - if (WebRtc_CreateDelayEstimator(&aec->delay_estimator, - PART_LEN1, - kMaxDelayBlocks, - kLookaheadBlocks) == -1) { + aec->delay_estimator = WebRtc_CreateDelayEstimator(PART_LEN1, + kMaxDelayBlocks, + kLookaheadBlocks); + if (aec->delay_estimator == NULL) { WebRtcAec_FreeAec(aec); aec = NULL; return -1; diff --git a/src/modules/audio_processing/aecm/aecm_core.c b/src/modules/audio_processing/aecm/aecm_core.c index 4a51d56a8d..f57e434e71 100644 --- a/src/modules/audio_processing/aecm/aecm_core.c +++ b/src/modules/audio_processing/aecm/aecm_core.c @@ -312,10 +312,9 @@ int WebRtcAecm_CreateCore(AecmCore_t **aecmInst) return -1; } - if (WebRtc_CreateDelayEstimator(&aecm->delay_estimator, - PART_LEN1, - MAX_DELAY, - 0) == -1) { + aecm->delay_estimator = WebRtc_CreateDelayEstimator(PART_LEN1, MAX_DELAY, + 0); + if (aecm->delay_estimator == NULL) { WebRtcAecm_FreeCore(aecm); aecm = NULL; return -1; diff --git a/src/modules/audio_processing/utility/delay_estimator_unittest.cc b/src/modules/audio_processing/utility/delay_estimator_unittest.cc index 3ebecca651..49a82dbaf0 100644 --- a/src/modules/audio_processing/utility/delay_estimator_unittest.cc +++ b/src/modules/audio_processing/utility/delay_estimator_unittest.cc @@ -55,7 +55,7 @@ DelayEstimatorTest::DelayEstimatorTest() } void DelayEstimatorTest::SetUp() { - ASSERT_EQ(0, WebRtc_CreateDelayEstimator(&handle_, kSpectrumSize, 100, 10)); + handle_ = WebRtc_CreateDelayEstimator(kSpectrumSize, 100, 10); ASSERT_TRUE(handle_ != NULL); self_ = reinterpret_cast(handle_); binary_handle_ = self_->binary_handle; @@ -87,23 +87,21 @@ void DelayEstimatorTest::InitBinary() { TEST_F(DelayEstimatorTest, CorrectErrorReturnsOfWrapper) { // In this test we verify correct error returns on invalid API calls. - // WebRtc_CreateDelayEstimator() should return -1 if we have a NULL pointer - // as |handle| or invalid input values. Upon failure, the handle should be - // NULL. + // WebRtc_CreateDelayEstimator() should return a NULL pointer on invalid input + // values. // Make sure we have a non-NULL value at start, so we can detect NULL after // create failure. void* handle = handle_; - EXPECT_EQ(-1, WebRtc_CreateDelayEstimator(NULL, kSpectrumSize, 100, 10)); - EXPECT_EQ(-1, WebRtc_CreateDelayEstimator(&handle, 33, 100, 10)); + handle = WebRtc_CreateDelayEstimator(33, 100, 10); EXPECT_TRUE(handle == NULL); handle = handle_; - EXPECT_EQ(-1, WebRtc_CreateDelayEstimator(&handle, kSpectrumSize, -1, 10)); + handle = WebRtc_CreateDelayEstimator(kSpectrumSize, -1, 10); EXPECT_TRUE(handle == NULL); handle = handle_; - EXPECT_EQ(-1, WebRtc_CreateDelayEstimator(&handle, kSpectrumSize, 100, -1)); + handle = WebRtc_CreateDelayEstimator(kSpectrumSize, 100, -1); EXPECT_TRUE(handle == NULL); handle = handle_; - EXPECT_EQ(-1, WebRtc_CreateDelayEstimator(&handle, kSpectrumSize, 0, 0)); + handle = WebRtc_CreateDelayEstimator(kSpectrumSize, 0, 0); EXPECT_TRUE(handle == NULL); // WebRtc_InitDelayEstimator() should return -1 if we have a NULL pointer as diff --git a/src/modules/audio_processing/utility/delay_estimator_wrapper.c b/src/modules/audio_processing/utility/delay_estimator_wrapper.c index 006bdf16dc..74918076cc 100644 --- a/src/modules/audio_processing/utility/delay_estimator_wrapper.c +++ b/src/modules/audio_processing/utility/delay_estimator_wrapper.c @@ -141,57 +141,45 @@ void WebRtc_FreeDelayEstimator(void* handle) { free(self); } -int WebRtc_CreateDelayEstimator(void** handle, - int spectrum_size, - int max_delay, - int lookahead) { +void* WebRtc_CreateDelayEstimator(int spectrum_size, int max_delay, + int lookahead) { DelayEstimator* self = NULL; - int return_value = 0; // TODO(bjornv): Make this a static assert. // Check if the sub band used in the delay estimation is small enough to fit // the binary spectra in a uint32_t. assert(kBandLast - kBandFirst < 32); - if (handle == NULL) { - return -1; - } - if (spectrum_size < kBandLast) { - *handle = NULL; - return -1; + if (spectrum_size >= kBandLast) { + self = malloc(sizeof(DelayEstimator)); } - self = malloc(sizeof(DelayEstimator)); - *handle = self; - if (self == NULL) { - return -1; + if (self != NULL) { + int memory_fail = 0; + + self->mean_far_spectrum = NULL; + self->mean_near_spectrum = NULL; + + self->binary_handle = WebRtc_CreateBinaryDelayEstimator(max_delay, + lookahead); + memory_fail |= (self->binary_handle == NULL); + + // Allocate memory for spectrum buffers. + self->mean_far_spectrum = malloc(spectrum_size * sizeof(SpectrumType)); + memory_fail |= (self->mean_far_spectrum == NULL); + + self->mean_near_spectrum = malloc(spectrum_size * sizeof(SpectrumType)); + memory_fail |= (self->mean_near_spectrum == NULL); + + self->spectrum_size = spectrum_size; + + if (memory_fail) { + WebRtc_FreeDelayEstimator(self); + self = NULL; + } } - self->mean_far_spectrum = NULL; - self->mean_near_spectrum = NULL; - - self->binary_handle = WebRtc_CreateBinaryDelayEstimator(max_delay, lookahead); - if (self->binary_handle == NULL) { - return_value = -1; - } - - // Allocate memory for spectrum buffers. - self->mean_far_spectrum = malloc(spectrum_size * sizeof(SpectrumType)); - if (self->mean_far_spectrum == NULL) { - return_value = -1; - } - self->mean_near_spectrum = malloc(spectrum_size * sizeof(SpectrumType)); - if (self->mean_near_spectrum == NULL) { - return_value = -1; - } - - self->spectrum_size = spectrum_size; - - if (return_value < 0) { - WebRtc_FreeDelayEstimator(self); - *handle = NULL; - } - return return_value; + return self; } int WebRtc_InitDelayEstimator(void* handle) { diff --git a/src/modules/audio_processing/utility/delay_estimator_wrapper.h b/src/modules/audio_processing/utility/delay_estimator_wrapper.h index 5d3f0689db..4591e4bf79 100644 --- a/src/modules/audio_processing/utility/delay_estimator_wrapper.h +++ b/src/modules/audio_processing/utility/delay_estimator_wrapper.h @@ -26,7 +26,6 @@ void WebRtc_FreeDelayEstimator(void* handle); // initialized separately through WebRtc_InitDelayEstimator(...). // // Inputs: -// - handle : Instance that should be created. // - spectrum_size : Size of the spectrum used both in far-end and // near-end. Used to allocate memory for spectrum // specific buffers. @@ -42,15 +41,15 @@ void WebRtc_FreeDelayEstimator(void* handle); // This also represents the minimum delay which can be // estimated. // -// Output: -// - handle : Created instance. +// Return value: +// - void* : Created |handle|. If the memory can't be allocated or +// if any of the input parameters are invalid NULL is +// returned. // -int WebRtc_CreateDelayEstimator(void** handle, - int spectrum_size, - int max_delay, - int lookahead); +void* WebRtc_CreateDelayEstimator(int spectrum_size, int max_delay, + int lookahead); -// Initializes the delay estimation instance created with +// Initializes the delay estimation instance returned by // WebRtc_CreateDelayEstimator(...) // Input: // - handle : Pointer to the delay estimation instance.