Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(55)

Unified Diff: media/base/sinc_resampler_unittest.cc

Issue 10702050: Add SincResampler ported from WebKit. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Zero-Initialize Arrays + Check Max Error. Created 8 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: media/base/sinc_resampler_unittest.cc
diff --git a/media/base/sinc_resampler_unittest.cc b/media/base/sinc_resampler_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..ae773a23716d42ea7dec3458ead723232e495ade
--- /dev/null
+++ b/media/base/sinc_resampler_unittest.cc
@@ -0,0 +1,164 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+// MSVC++ requires this to be set before any other includes to get M_PI.
+#define _USE_MATH_DEFINES
+
+#include <cmath>
+
+#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/stringprintf.h"
+#include "media/base/sinc_resampler.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace media {
+
+// Chosen arbitrarily on what each resampler reported during testing.
Ami GONE FROM CHROMIUM 2012/06/30 20:29:30 s/on/based on/
DaleCurtis 2012/07/03 00:36:34 Done.
+static const double kMaxRMSError = 0.0002;
Chris Rogers 2012/07/02 20:38:14 I'm not sure if the RMS error will be useful, but
DaleCurtis 2012/07/03 00:36:34 Happy to drop it, but I figured there would be cas
+static const double kMaxError = 0.00022;
Chris Rogers 2012/07/02 20:38:14 I'd represent this in decibels (actually dbFS) - s
+
+// Generate a swept sine wave with the given hertz over [0, kCycles * 2 * PI].
+class SweptSineSourceProvider : public SincResampler::AudioSourceProvider {
+ public:
+ // Maximum sine wave cycles.
+ static const int kCycles = 6;
Chris Rogers 2012/07/02 20:38:14 As we discussed in chat, I think we should define
DaleCurtis 2012/07/03 00:36:34 Done.
+
+ // Hz/sec value for the swept sine wave.
+ // TODO(dalecurtis): Should we use a more complex pattern here?
+ static const int kHertzPerSecond = 1;
Chris Rogers 2012/07/02 20:38:14 I don't think we'll need this constant.
DaleCurtis 2012/07/03 00:36:34 Done.
+
+ explicit SweptSineSourceProvider(int max) {
Chris Rogers 2012/07/02 20:38:14 "max"? how about "sample_rate" Then I'd simply g
DaleCurtis 2012/07/03 00:36:34 Done.
+ ResetTimeState(max);
+ }
+
+ virtual ~SweptSineSourceProvider() {}
+
+ virtual void ProvideInput(float* destination, int number_of_frames) OVERRIDE {
+ for (int i = 0; i < number_of_frames; ++i) {
+ destination[i] = sin(kHertzPerSecond * time_state_ * time_state_);
+ time_state_ += time_increment_;
+ }
+ }
+
+ void ResetTimeState(int max) {
+ time_state_ = 0.0;
+ time_increment_ = kCycles * 2 * M_PI / max;
+ }
+
+ protected:
+ double hertz_;
+ double time_increment_;
+ double time_state_;
+
+ DISALLOW_COPY_AND_ASSIGN(SweptSineSourceProvider);
+};
+
+// Used for tests which just need to run without crashing or tooling errors, but
+// which may have undefined behavior for hashing, etc.
Ami GONE FROM CHROMIUM 2012/06/30 20:29:30 wat?
DaleCurtis 2012/07/01 23:31:30 Copy paste fail! (from ffmpeg_regression_tests)
DaleCurtis 2012/07/03 00:36:34 Done.
+struct SincResamplerTestData {
Ami GONE FROM CHROMIUM 2012/06/30 20:29:30 Isn't this kind of overkill for a std::pair<int, i
DaleCurtis 2012/07/01 23:31:30 Good point, I completely forgot about pair.
DaleCurtis 2012/07/03 00:36:34 Switched to using tr1::tuple since gtest uses that
+ SincResamplerTestData(int input_rate, int output_rate)
+ : input_rate(input_rate),
+ output_rate(output_rate) {
+ }
+
+ std::string DebugString() const {
Ami GONE FROM CHROMIUM 2012/06/30 20:29:30 FWIW, since this doesn't touch private data (there
DaleCurtis 2012/07/03 00:36:34 Removed with tuple().
+ return base::StringPrintf(
+ "Resampling test from %dHz to %dHz.", input_rate, output_rate);
+ }
+
+ const int input_rate;
+ const int output_rate;
+};
+
+class SincResamplerTestCase
+ : public testing::TestWithParam<SincResamplerTestData> {
+};
+
+// Define ostream << operator so GTest will print nice error messages instead of
+// "[...], where GetParam() = 8-byte object <44-AC 00-00 00-77 01-00>"
+::std::ostream& operator<<(::std::ostream& os,
+ const SincResamplerTestData& data) {
+ return os << data.DebugString();
+}
+
+INSTANTIATE_TEST_CASE_P(
Ami GONE FROM CHROMIUM 2012/06/30 20:29:30 Traditionally this goes *after* the test case it i
DaleCurtis 2012/07/03 00:36:34 Done.
+ SincResamplerTest, SincResamplerTestCase, testing::Values(
+ // To 44.1kHz
+ SincResamplerTestData(8000, 44100),
Ami GONE FROM CHROMIUM 2012/06/30 20:29:30 Replace the explicit lists (which are prone to acc
DaleCurtis 2012/07/01 23:31:30 Neat! I was looking for that.
DaleCurtis 2012/07/03 00:36:34 Don't think we'll be able to use this since after
+ SincResamplerTestData(11025, 44100),
+ SincResamplerTestData(16000, 44100),
+ SincResamplerTestData(22050, 44100),
+ SincResamplerTestData(32000, 44100),
+ SincResamplerTestData(48000, 44100),
+ SincResamplerTestData(96000, 44100),
+ SincResamplerTestData(192000, 44100),
+
+ // To 48kHz
+ SincResamplerTestData(8000, 48000),
+ SincResamplerTestData(11025, 48000),
+ SincResamplerTestData(16000, 48000),
+ SincResamplerTestData(22050, 48000),
+ SincResamplerTestData(32000, 48000),
+ SincResamplerTestData(44100, 48000),
+ SincResamplerTestData(96000, 48000),
+ SincResamplerTestData(192000, 48000),
+
+ // To 96kHz
+ SincResamplerTestData(8000, 96000),
+ SincResamplerTestData(11025, 96000),
+ SincResamplerTestData(16000, 96000),
+ SincResamplerTestData(22050, 96000),
+ SincResamplerTestData(32000, 96000),
+ SincResamplerTestData(44100, 96000),
+ SincResamplerTestData(48000, 96000),
+ SincResamplerTestData(192000, 96000),
+
+ // To 192kHz
+ SincResamplerTestData(8000, 192000),
+ SincResamplerTestData(11025, 192000),
+ SincResamplerTestData(16000, 192000),
+ SincResamplerTestData(22050, 192000),
+ SincResamplerTestData(32000, 192000),
+ SincResamplerTestData(44100, 192000),
+ SincResamplerTestData(48000, 192000),
+ SincResamplerTestData(96000, 192000)));
+
+// Test callback() works as expected.
+TEST_P(SincResamplerTestCase, Resample) {
+ int input_rate = GetParam().input_rate;
+ int output_rate = GetParam().output_rate;
+
+ scoped_ptr<SweptSineSourceProvider> provider(
Ami GONE FROM CHROMIUM 2012/06/30 20:29:30 hopefully you won't need this post-migration-to-CB
DaleCurtis 2012/07/03 00:36:34 Done.
+ new SweptSineSourceProvider(input_rate));
+ scoped_ptr<SincResampler> resampler(new SincResampler(
+ provider.get(), static_cast<double>(input_rate) / output_rate));
Ami GONE FROM CHROMIUM 2012/06/30 20:29:30 you could avoid this cast by making the input & ou
DaleCurtis 2012/07/03 00:36:34 Then I need two casts for constructing the arrays.
+
+ // TODO(dalecurtis): These will need SSE appropriate allocations.
Ami GONE FROM CHROMIUM 2012/06/30 20:29:30 Do you mean padding/alignment? why not fix now?
DaleCurtis 2012/07/01 23:31:30 Yes, but it's more than just here that needs to be
+ scoped_array<float> resampled_destination(new float[output_rate]);
+ scoped_array<float> pure_destination(new float[output_rate]);
+
+ // Generate resampled signal.
+ resampler->Resample(resampled_destination.get(), output_rate);
Ami GONE FROM CHROMIUM 2012/06/30 20:29:30 It's confusing that output_rate is used for number
DaleCurtis 2012/07/03 00:36:34 Done.
+
+ // Generate pure signal.
+ provider->ResetTimeState(output_rate);
Chris Rogers 2012/07/02 20:38:14 Instead of re-using the existing object and exposi
DaleCurtis 2012/07/03 00:36:34 Done.
+ provider->ProvideInput(pure_destination.get(), output_rate);
+
+ // Calculate Root-Mean-Square-Error for the resampling.
+ double sum_of_squares = 0.0;
Ami GONE FROM CHROMIUM 2012/06/30 20:29:30 .0 here and below unnecessary
DaleCurtis 2012/07/03 00:36:34 Done.
+ double max_error = 0.0;
+ for (int i = 0; i < output_rate; ++i) {
+ double error = fabs(resampled_destination[i] - pure_destination[i]);
+ max_error = std::max(max_error, error);
+ sum_of_squares += error * error;
+ }
+
+ double rms_error = sqrt(sum_of_squares / output_rate);
+
+ // TODO(dalecurtis): Should this be different for each (in, out) pair?
Ami GONE FROM CHROMIUM 2012/06/30 20:29:30 how wide is the spread?
DaleCurtis 2012/07/03 00:36:34 rms error is about 0.17 -> 0.55, max: 0.86 -> 1.86
+ EXPECT_LT(rms_error, kMaxRMSError);
+ EXPECT_LT(max_error, kMaxError);
+}
+
+} // namespace media

Powered by Google App Engine
This is Rietveld 408576698