Remove upcasting from AudioBus interleave methods where possible.

Upcasting isn't necessary without a bias since the overflow
check is performed on the float value.

No change on x86, but avoids using int64 types on arm which makes
the new methods 3x faster for int32 ToInterleave.  Also fixes a
sign bug which never manifested as a problem due to overflow.

For reference, in aggregate, Daisy is only 3x slower than my Z620,
which isn't too shabby.

BUG=none
TEST=media_unittests --gtest_filter=AudioBus* --gtest_also_run_disabled_tests

Review URL: https://chromiumcodereview.appspot.com/13868019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@196946 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 70 insertions, 47 deletions

diff --git a/media/base/audio_bus.cc b/media/base/audio_bus.cc
index 06ed414..a53fdaf 100644
--- a/media/base/audio_bus.cc
+++ b/media/base/audio_bus.cc
@@ -4,8 +4,6 @@
 
 #include "media/base/audio_bus.h"
 
-#include <limits>
-
 #include "base/logging.h"
 #include "media/audio/audio_parameters.h"
 #include "media/base/limits.h"
@@ -14,8 +12,6 @@
 namespace media {
 
 static const uint8 kUint8Bias = 128;
-static const int16 kUint8Min = -kUint8Bias;
-static const int16 kUint8Max = kUint8Bias - 1;
 
 static bool IsAligned(void* ptr) {
   return (reinterpret_cast<uintptr_t>(ptr) &
@@ -38,30 +34,35 @@ static int CalculateMemorySizeInternal(int channels, int frames,
   return sizeof(float) * channels * aligned_frames;
 }
 
-// |Format| is the destination type, |Fixed| is a type larger than |Format|
-// such that operations can be made without overflowing.
-template<class Format, class Fixed>
+// |Format| is the destination type.  If a bias is present, |Fixed| must be a
+// type larger than |Format| such that operations can be made without
+// overflowing.  Without a bias |Fixed| must be the same as |Format|.
+template<class Format, class Fixed, Format Bias>
 static void FromInterleavedInternal(const void* src, int start_frame,
                                     int frames, AudioBus* dest,
-                                    Format bias, float min, float max) {
+                                    float min, float max) {
+  COMPILE_ASSERT((Bias == 0 && sizeof(Fixed) == sizeof(Format)) ||
+                 sizeof(Fixed) > sizeof(Format), invalid_deinterleave_types);
   const Format* source = static_cast<const Format*>(src);
   const int channels = dest->channels();
   for (int ch = 0; ch < channels; ++ch) {
     float* channel_data = dest->channel(ch);
     for (int i = start_frame, offset = ch; i < start_frame + frames;
          ++i, offset += channels) {
-      const Fixed v = static_cast<Fixed>(source[offset]) - bias;
+      const Fixed v = static_cast<Fixed>(source[offset]) - Bias;
       channel_data[i] = v * (v < 0 ? -min : max);
     }
   }
 }
 
-// |Format| is the destination type, |Fixed| is a type larger than |Format|
-// such that operations can be made without overflowing.
-template<class Format, class Fixed>
+// |Format| is the destination type.  If a bias is present, |Fixed| must be a
+// type larger than |Format| such that operations can be made without
+// overflowing.  Without a bias |Fixed| must be the same as |Format|.
+template<class Format, class Fixed, Format Bias>
 static void ToInterleavedInternal(const AudioBus* source, int start_frame,
-                                  int frames, void* dst,
-                                  Format bias, Fixed min, Fixed max) {
+                                  int frames, void* dst, Fixed min, Fixed max) {
+  COMPILE_ASSERT((Bias == 0 && sizeof(Fixed) == sizeof(Format)) ||
+                 sizeof(Fixed) > sizeof(Format), invalid_interleave_types);
   Format* dest = static_cast<Format*>(dst);
   const int channels = source->channels();
   for (int ch = 0; ch < channels; ++ch) {
@@ -72,11 +73,11 @@ static void ToInterleavedInternal(const AudioBus* source, int start_frame,
 
       Fixed sample;
       if (v < 0)
-        sample = v <= -1 ? -min : static_cast<Fixed>(v * -min);
+        sample = v <= -1 ? min : static_cast<Fixed>(-v * min);
       else
         sample = v >= 1 ? max : static_cast<Fixed>(v * max);
 
-      dest[offset] = static_cast<Format>(sample) + bias;
+      dest[offset] = static_cast<Format>(sample) + Bias;
     }
   }
 }
@@ -236,19 +237,19 @@ void AudioBus::FromInterleavedPartial(const void* source, int start_frame,
   CheckOverflow(start_frame, frames, frames_);
   switch (bytes_per_sample) {
     case 1:
-      FromInterleavedInternal<uint8, int16>(
+      FromInterleavedInternal<uint8, int16, kUint8Bias>(
           source, start_frame, frames, this,
-          kUint8Bias, 1.0f / kUint8Min, 1.0f / kUint8Max);
+          1.0f / kint8min, 1.0f / kint8max);
       break;
     case 2:
-      FromInterleavedInternal<int16, int32>(
+      FromInterleavedInternal<int16, int16, 0>(
           source, start_frame, frames, this,
-          0, 1.0f / kint16min, 1.0f / kint16max);
+          1.0f / kint16min, 1.0f / kint16max);
       break;
     case 4:
-      FromInterleavedInternal<int32, int64>(
+      FromInterleavedInternal<int32, int32, 0>(
           source, start_frame, frames, this,
-          0, 1.0f / kint32min, 1.0f / kint32max);
+          1.0f / kint32min, 1.0f / kint32max);
       break;
     default:
       NOTREACHED() << "Unsupported bytes per sample encountered.";
@@ -279,17 +280,16 @@ void AudioBus::ToInterleavedPartial(int start_frame, int frames,
   CheckOverflow(start_frame, frames, frames_);
   switch (bytes_per_sample) {
     case 1:
-      ToInterleavedInternal<uint8, int16>(
-          this, start_frame, frames, dest,
-          kUint8Bias, kUint8Min, kUint8Max);
+      ToInterleavedInternal<uint8, int16, kUint8Bias>(
+          this, start_frame, frames, dest, kint8min, kint8max);
       break;
     case 2:
-      ToInterleavedInternal<int16, int32>(
-          this, start_frame, frames, dest, 0, kint16min, kint16max);
+      ToInterleavedInternal<int16, int16, 0>(
+          this, start_frame, frames, dest, kint16min, kint16max);
       break;
     case 4:
-      ToInterleavedInternal<int32, int64>(
-          this, start_frame, frames, dest, 0, kint32min, kint32max);
+      ToInterleavedInternal<int32, int32, 0>(
+          this, start_frame, frames, dest, kint32min, kint32max);
       break;
     default:
       NOTREACHED() << "Unsupported bytes per sample encountered.";
diff --git a/media/base/audio_bus_unittest.cc b/media/base/audio_bus_unittest.cc
index f5443cc..a8ab5ae 100644
--- a/media/base/audio_bus_unittest.cc
+++ b/media/base/audio_bus_unittest.cc
@@ -2,10 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <limits>
-
 #include "base/stringprintf.h"
 #include "base/time.h"
+#include "build/build_config.h"
 #include "media/audio/audio_parameters.h"
 #include "media/base/audio_bus.h"
 #include "media/base/channel_layout.h"
@@ -39,18 +38,30 @@ class AudioBusTest : public testing::Test {
       ASSERT_FLOAT_EQ(value, data[i]) << "i=" << i;
   }
 
-  // Verify values for each channel in |result| against |expected|.
-  void VerifyBus(const AudioBus* result, const AudioBus* expected) {
+  // Verify values for each channel in |result| are within |epsilon| of
+  // |expected|.  If |epsilon| exactly equals 0, uses FLOAT_EQ macro.
+  void VerifyBusWithEpsilon(const AudioBus* result, const AudioBus* expected,
+                            float epsilon) {
     ASSERT_EQ(expected->channels(), result->channels());
     ASSERT_EQ(expected->frames(), result->frames());
     for (int ch = 0; ch < result->channels(); ++ch) {
       for (int i = 0; i < result->frames(); ++i) {
         SCOPED_TRACE(base::StringPrintf("ch=%d, i=%d", ch, i));
-        ASSERT_FLOAT_EQ(expected->channel(ch)[i], result->channel(ch)[i]);
+        if (epsilon == 0) {
+          ASSERT_FLOAT_EQ(expected->channel(ch)[i], result->channel(ch)[i]);
+        } else {
+          ASSERT_NEAR(expected->channel(ch)[i], result->channel(ch)[i],
+                      epsilon);
+        }
       }
     }
   }
 
+  // Verify values for each channel in |result| against |expected|.
+  void VerifyBus(const AudioBus* result, const AudioBus* expected) {
+    VerifyBusWithEpsilon(result, expected, 0);
+  }
+
   // Read and write to the full extent of the allocated channel data.  Also test
   // the Zero() method and verify it does as advertised.  Also test data if data
   // is 16-byte aligned as advertised (see kChannelAlignment in audio_bus.h).
@@ -230,19 +241,29 @@ TEST_F(AudioBusTest, Zero) {
 }
 
 // Each test vector represents two channels of data in the following arbitrary
-// layout: <min, zero, max, min, zero, max, zero, zero>.
-static const int kTestVectorSize = 8;
+// layout: <min, zero, max, min, max / 2, min / 2, zero, max, zero, zero>.
+static const int kTestVectorSize = 10;
 static const uint8 kTestVectorUint8[kTestVectorSize] = {
-    0, -kint8min, kuint8max, 0, -kint8min, kuint8max, -kint8min, -kint8min };
+    0, -kint8min, kuint8max, 0, kint8max / 2 + 128, kint8min / 2 + 128,
+    -kint8min, kuint8max, -kint8min, -kint8min };
 static const int16 kTestVectorInt16[kTestVectorSize] = {
-    kint16min, 0, kint16max, kint16min, 0, kint16max, 0, 0 };
+    kint16min, 0, kint16max, kint16min, kint16max / 2, kint16min / 2,
+    0, kint16max, 0, 0 };
 static const int32 kTestVectorInt32[kTestVectorSize] = {
-    kint32min, 0, kint32max, kint32min, 0, kint32max, 0, 0 };
+    kint32min, 0, kint32max, kint32min,
+// MSVC somehow gets better precision during conversion, so adjust the half-max
+// appropriately based on the compiler.
+#if defined(COMPILER_MSVC)
+    kint32max / 2,
+#else
+    kint32max / 2 + 1,
+#endif
+    kint32min / 2, 0, kint32max, 0, 0 };
 
 // Expected results.
 static const int kTestVectorFrames = kTestVectorSize / 2;
 static const float kTestVectorResult[][kTestVectorFrames] = {
-    { -1, 1, 0, 0 }, { 0, -1, 1, 0 }};
+    { -1, 1, 0.5, 0, 0 }, { 0, -1, -0.5, 1, 0 }};
 static const int kTestVectorChannels = arraysize(kTestVectorResult);
 
 // Verify FromInterleaved() deinterleaves audio in supported formats correctly.
@@ -260,21 +281,23 @@ TEST_F(AudioBusTest, FromInterleaved) {
     bus->Zero();
     bus->FromInterleaved(
         kTestVectorUint8, kTestVectorFrames, sizeof(*kTestVectorUint8));
-    VerifyBus(bus.get(), expected.get());
+    // Biased uint8 calculations have poor precision, so the epsilon here is
+    // slightly more permissive than int16 and int32 calculations.
+    VerifyBusWithEpsilon(bus.get(), expected.get(), 1.0f / (kuint8max - 1));
   }
   {
     SCOPED_TRACE("int16");
     bus->Zero();
     bus->FromInterleaved(
         kTestVectorInt16, kTestVectorFrames, sizeof(*kTestVectorInt16));
-    VerifyBus(bus.get(), expected.get());
+    VerifyBusWithEpsilon(bus.get(), expected.get(), 1.0f / (kuint16max + 1.0f));
   }
   {
     SCOPED_TRACE("int32");
     bus->Zero();
     bus->FromInterleaved(
         kTestVectorInt32, kTestVectorFrames, sizeof(*kTestVectorInt32));
-    VerifyBus(bus.get(), expected.get());
+    VerifyBusWithEpsilon(bus.get(), expected.get(), 1.0f / (kuint32max + 1.0f));
   }
 }
 
@@ -308,7 +331,7 @@ TEST_F(AudioBusTest, ToInterleaved) {
   scoped_ptr<AudioBus> bus = AudioBus::Create(
       kTestVectorChannels, kTestVectorFrames);
   // Fill the bus with our test vector.
-  for (int ch = 0; ch < kTestVectorChannels; ++ch) {
+  for (int ch = 0; ch < bus->channels(); ++ch) {
     memcpy(bus->channel(ch), kTestVectorResult[ch],
            kTestVectorFrames * sizeof(*bus->channel(ch)));
   }
@@ -317,21 +340,21 @@ TEST_F(AudioBusTest, ToInterleaved) {
     uint8 test_array[arraysize(kTestVectorUint8)];
     bus->ToInterleaved(bus->frames(), sizeof(*kTestVectorUint8), test_array);
     ASSERT_EQ(memcmp(
-        test_array, kTestVectorUint8, arraysize(kTestVectorUint8)), 0);
+        test_array, kTestVectorUint8, sizeof(kTestVectorUint8)), 0);
   }
   {
     SCOPED_TRACE("int16");
     int16 test_array[arraysize(kTestVectorInt16)];
     bus->ToInterleaved(bus->frames(), sizeof(*kTestVectorInt16), test_array);
     ASSERT_EQ(memcmp(
-        test_array, kTestVectorInt16, arraysize(kTestVectorInt16)), 0);
+        test_array, kTestVectorInt16, sizeof(kTestVectorInt16)), 0);
   }
   {
     SCOPED_TRACE("int32");
     int32 test_array[arraysize(kTestVectorInt32)];
     bus->ToInterleaved(bus->frames(), sizeof(*kTestVectorInt32), test_array);
     ASSERT_EQ(memcmp(
-        test_array, kTestVectorInt32, arraysize(kTestVectorInt32)), 0);
+        test_array, kTestVectorInt32, sizeof(kTestVectorInt32)), 0);
   }
 }