Change Resampler to not change state on invalid Reset and ResetIfNeeded calls.

Adds a unittest to test this.

A Reset() with unsupported frequencies will fail, but currently leaves the resampler in an illegal state.
Subsequent calls to ResetIfNeeded(), which depends on the internal state, will then have unreliable behavior.

The following sequence of calls demonstrate this: It appears as though the resampler is successfully reinitialized to upsample from 44 kHz to 48 kHz, but will in fact crash on Push().

Resampler::Reset() with in=44000, out=32000           // Returns 0
Resampler::ResetIfNeeded() with in=44000, out=48000   // Returns -1
Resampler::ResetIfNeeded() with in=44000, out=48000   // Returns 0
Resampler::Push() with some data

Bug: webrtc:8426
Change-Id: Id1e0528ffcb7a86702d4c2f4c5103a1db419c07d
Reviewed-on: https://webrtc-review.googlesource.com/16424
Commit-Queue: Sam Zackrisson <saza@webrtc.org>
Reviewed-by: Henrik Lundin <henrik.lundin@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20474}
This commit is contained in:
Sam Zackrisson
2017-10-30 10:05:10 +01:00
committed by Commit Bot
parent 90397d9aa9
commit 9da7c7480b
3 changed files with 143 additions and 83 deletions

View File

@ -64,6 +64,12 @@ class Resampler {
kResamplerMode11To8
};
// Computes the resampler mode for a given sampling frequency pair.
// Returns -1 for unsupported frequency pairs.
static int ComputeResamplerMode(int in_freq_hz,
int out_freq_hz,
ResamplerMode* mode);
// Generic pointers since we don't know what states we'll need
void* state1_;
void* state2_;

View File

@ -18,6 +18,7 @@
#include "common_audio/resampler/include/resampler.h"
#include "common_audio/signal_processing/include/signal_processing_library.h"
#include "rtc_base/logging.h"
namespace webrtc {
@ -83,9 +84,20 @@ int Resampler::ResetIfNeeded(int inFreq, int outFreq, size_t num_channels) {
int Resampler::Reset(int inFreq, int outFreq, size_t num_channels) {
if (num_channels != 1 && num_channels != 2) {
return -1;
LOG(LS_WARNING)
<< "Reset() called with unsupported channel count, num_channels = "
<< num_channels;
return -1;
}
ResamplerMode mode;
if (ComputeResamplerMode(inFreq, outFreq, &mode) != 0) {
LOG(LS_WARNING) << "Reset() called with unsupported sample rates, inFreq = "
<< inFreq << ", outFreq = " << outFreq;
return -1;
}
// Reinitialize internal state for the frequencies and sample rates.
num_channels_ = num_channels;
my_mode_ = mode;
if (state1_) {
free(state1_);
@ -121,98 +133,17 @@ int Resampler::Reset(int inFreq, int outFreq, size_t num_channels) {
in_buffer_size_max_ = 0;
out_buffer_size_max_ = 0;
// Start with a math exercise, Euclid's algorithm to find the gcd:
int a = inFreq;
int b = outFreq;
int c = a % b;
while (c != 0) {
a = b;
b = c;
c = a % b;
}
// b is now the gcd;
// We need to track what domain we're in.
my_in_frequency_khz_ = inFreq / 1000;
my_out_frequency_khz_ = outFreq / 1000;
// Scale with GCD
inFreq = inFreq / b;
outFreq = outFreq / b;
if (num_channels_ == 2) {
// Create two mono resamplers.
slave_left_ = new Resampler(inFreq, outFreq, 1);
slave_right_ = new Resampler(inFreq, outFreq, 1);
}
if (inFreq == outFreq) {
my_mode_ = kResamplerMode1To1;
} else if (inFreq == 1) {
switch (outFreq) {
case 2:
my_mode_ = kResamplerMode1To2;
break;
case 3:
my_mode_ = kResamplerMode1To3;
break;
case 4:
my_mode_ = kResamplerMode1To4;
break;
case 6:
my_mode_ = kResamplerMode1To6;
break;
case 12:
my_mode_ = kResamplerMode1To12;
break;
default:
return -1;
}
} else if (outFreq == 1) {
switch (inFreq) {
case 2:
my_mode_ = kResamplerMode2To1;
break;
case 3:
my_mode_ = kResamplerMode3To1;
break;
case 4:
my_mode_ = kResamplerMode4To1;
break;
case 6:
my_mode_ = kResamplerMode6To1;
break;
case 12:
my_mode_ = kResamplerMode12To1;
break;
default:
return -1;
}
} else if ((inFreq == 2) && (outFreq == 3)) {
my_mode_ = kResamplerMode2To3;
} else if ((inFreq == 2) && (outFreq == 11)) {
my_mode_ = kResamplerMode2To11;
} else if ((inFreq == 4) && (outFreq == 11)) {
my_mode_ = kResamplerMode4To11;
} else if ((inFreq == 8) && (outFreq == 11)) {
my_mode_ = kResamplerMode8To11;
} else if ((inFreq == 3) && (outFreq == 2)) {
my_mode_ = kResamplerMode3To2;
} else if ((inFreq == 11) && (outFreq == 2)) {
my_mode_ = kResamplerMode11To2;
} else if ((inFreq == 11) && (outFreq == 4)) {
my_mode_ = kResamplerMode11To4;
} else if ((inFreq == 11) && (outFreq == 16)) {
my_mode_ = kResamplerMode11To16;
} else if ((inFreq == 11) && (outFreq == 32)) {
my_mode_ = kResamplerMode11To32;
} else if ((inFreq == 11) && (outFreq == 8)) {
my_mode_ = kResamplerMode11To8;
} else {
return -1;
}
// Now create the states we need
// Now create the states we need.
switch (my_mode_) {
case kResamplerMode1To1:
// No state needed;
@ -376,6 +307,92 @@ int Resampler::Reset(int inFreq, int outFreq, size_t num_channels) {
return 0;
}
int Resampler::ComputeResamplerMode(int in_freq_hz,
int out_freq_hz,
ResamplerMode* mode) {
// Start with a math exercise, Euclid's algorithm to find the gcd:
int a = in_freq_hz;
int b = out_freq_hz;
int c = a % b;
while (c != 0) {
a = b;
b = c;
c = a % b;
}
// b is now the gcd;
// Scale with GCD
const int reduced_in_freq = in_freq_hz / b;
const int reduced_out_freq = out_freq_hz / b;
if (reduced_in_freq == reduced_out_freq) {
*mode = kResamplerMode1To1;
} else if (reduced_in_freq == 1) {
switch (reduced_out_freq) {
case 2:
*mode = kResamplerMode1To2;
break;
case 3:
*mode = kResamplerMode1To3;
break;
case 4:
*mode = kResamplerMode1To4;
break;
case 6:
*mode = kResamplerMode1To6;
break;
case 12:
*mode = kResamplerMode1To12;
break;
default:
return -1;
}
} else if (reduced_out_freq == 1) {
switch (reduced_in_freq) {
case 2:
*mode = kResamplerMode2To1;
break;
case 3:
*mode = kResamplerMode3To1;
break;
case 4:
*mode = kResamplerMode4To1;
break;
case 6:
*mode = kResamplerMode6To1;
break;
case 12:
*mode = kResamplerMode12To1;
break;
default:
return -1;
}
} else if ((reduced_in_freq == 2) && (reduced_out_freq == 3)) {
*mode = kResamplerMode2To3;
} else if ((reduced_in_freq == 2) && (reduced_out_freq == 11)) {
*mode = kResamplerMode2To11;
} else if ((reduced_in_freq == 4) && (reduced_out_freq == 11)) {
*mode = kResamplerMode4To11;
} else if ((reduced_in_freq == 8) && (reduced_out_freq == 11)) {
*mode = kResamplerMode8To11;
} else if ((reduced_in_freq == 3) && (reduced_out_freq == 2)) {
*mode = kResamplerMode3To2;
} else if ((reduced_in_freq == 11) && (reduced_out_freq == 2)) {
*mode = kResamplerMode11To2;
} else if ((reduced_in_freq == 11) && (reduced_out_freq == 4)) {
*mode = kResamplerMode11To4;
} else if ((reduced_in_freq == 11) && (reduced_out_freq == 16)) {
*mode = kResamplerMode11To16;
} else if ((reduced_in_freq == 11) && (reduced_out_freq == 32)) {
*mode = kResamplerMode11To32;
} else if ((reduced_in_freq == 11) && (reduced_out_freq == 8)) {
*mode = kResamplerMode11To8;
} else {
return -1;
}
return 0;
}
// Synchronous resampling, all output samples are written to samplesOut
int Resampler::Push(const int16_t * samplesIn, size_t lengthIn,
int16_t* samplesOut, size_t maxLen, size_t& outLen) {

View File

@ -8,6 +8,8 @@
* be found in the AUTHORS file in the root of the source tree.
*/
#include <array>
#include "common_audio/resampler/include/resampler.h"
#include "test/gtest.h"
@ -50,6 +52,8 @@ class ResamplerTest : public testing::Test {
virtual void SetUp();
virtual void TearDown();
void ResetIfNeededAndPush(int in_rate, int out_rate, int num_channels);
Resampler rs_;
int16_t data_in_[kDataSize];
int16_t data_out_[kDataSize];
@ -64,6 +68,26 @@ void ResamplerTest::SetUp() {
void ResamplerTest::TearDown() {}
void ResamplerTest::ResetIfNeededAndPush(int in_rate,
int out_rate,
int num_channels) {
std::ostringstream ss;
ss << "Input rate: " << in_rate << ", output rate: " << out_rate
<< ", channel count: " << num_channels;
SCOPED_TRACE(ss.str());
if (ValidRates(in_rate, out_rate)) {
size_t in_length = static_cast<size_t>(in_rate / 100);
size_t out_length = 0;
EXPECT_EQ(0, rs_.ResetIfNeeded(in_rate, out_rate, num_channels));
EXPECT_EQ(0,
rs_.Push(data_in_, in_length, data_out_, kDataSize, out_length));
EXPECT_EQ(static_cast<size_t>(out_rate / 100), out_length);
} else {
EXPECT_EQ(-1, rs_.ResetIfNeeded(in_rate, out_rate, num_channels));
}
}
TEST_F(ResamplerTest, Reset) {
// The only failure mode for the constructor is if Reset() fails. For the
// time being then (until an Init function is added), we rely on Reset()
@ -134,5 +158,18 @@ TEST_F(ResamplerTest, Stereo) {
}
}
// Try multiple resets between a few supported and unsupported rates.
TEST_F(ResamplerTest, MultipleResets) {
constexpr size_t kNumChanges = 5;
constexpr std::array<int, kNumChanges> kInRates = {
{8000, 44000, 44000, 32000, 32000}};
constexpr std::array<int, kNumChanges> kOutRates = {
{16000, 48000, 48000, 16000, 16000}};
constexpr std::array<int, kNumChanges> kNumChannels = {{2, 2, 2, 2, 1}};
for (size_t i = 0; i < kNumChanges; ++i) {
ResetIfNeededAndPush(kInRates[i], kOutRates[i], kNumChannels[i]);
}
}
} // namespace
} // namespace webrtc