Remove static const from AudioBus interleave methods and optimize!

media/base/audio_bus.cc

 // 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.
 
 #include "media/base/audio_bus.h"
 
 #include <limits>
 
 #include "base/logging.h"
 #include "media/audio/audio_parameters.h"
 #include "media/base/limits.h"
 
 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) &
           (AudioBus::kChannelAlignment - 1)) == 0U;
 }
 
 // Calculates the required size for an AudioBus with the given params, sets
 // |aligned_frames| to the actual frame length of each channel array.
 static int CalculateMemorySizeInternal(int channels, int frames,
                                        int* out_aligned_frames) {
   // Choose a size such that each channel will be aligned by
   // kChannelAlignment when stored in a contiguous block.
   int aligned_frames =
       ((frames * sizeof(float) + AudioBus::kChannelAlignment - 1) &
        ~(AudioBus::kChannelAlignment - 1)) / sizeof(float);
 
   if (out_aligned_frames)
     *out_aligned_frames = aligned_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>
 static void FromInterleavedInternal(const void* src, int start_frame,
-                                    int frames, AudioBus* dest) {
+                                    int frames, AudioBus* dest,
+                                    Format bias, float min, float max) {
   const Format* source = static_cast<const Format*>(src);
-
-  static const Fixed kBias = std::numeric_limits<Format>::is_signed ? 0 :
-      std::numeric_limits<Format>::max() / 2 + 1;
-  static const float kMaxScale = 1.0f / (kBias ? kBias - 1 :
-      std::numeric_limits<Format>::max());
-  static const float kMinScale = 1.0f / (kBias ? kBias :
-      -static_cast<Fixed>(std::numeric_limits<Format>::min()));
-
-  int channels = dest->channels();
+  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) {
-      Fixed v = static_cast<Fixed>(source[offset]) - kBias;
-      channel_data[i] = v * (v < 0 ? kMinScale : kMaxScale);
+      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>
 static void ToInterleavedInternal(const AudioBus* source, int start_frame,
-                                  int frames, void* dst) {
+                                  int frames, void* dst,
+                                  Format bias, Fixed min, Fixed max) {
   Format* dest = static_cast<Format*>(dst);
-
-  static const Format kBias = std::numeric_limits<Format>::is_signed ? 0 :
-      std::numeric_limits<Format>::max() / 2 + 1;
-  static const Fixed kMaxValue = kBias ? kBias - 1 :
-      std::numeric_limits<Format>::max();
-  static const Fixed kMinValue = kBias ? -kBias :
-      std::numeric_limits<Format>::min();
-
-  int channels = source->channels();
+  const int channels = source->channels();
   for (int ch = 0; ch < channels; ++ch) {
     const float* channel_data = source->channel(ch);
     for (int i = start_frame, offset = ch; i < start_frame + frames;
          ++i, offset += channels) {
-      float v = channel_data[i];
-      Fixed sample = v * (v < 0 ? -kMinValue : kMaxValue);
+      const float v = channel_data[i];
 
-      if (sample > kMaxValue)
-        sample = kMaxValue;
-      else if (sample < kMinValue)
-        sample = kMinValue;
+      Fixed sample;
+      if (v < 0)
+        sample = static_cast<Fixed>(v * -min) & min;

[DaleCurtis, 2013/04/08 21:00:30]

Sadly, this is actually just plain wrong. I'm not sure the conditionals can be
avoided here. I'll take another look and get a rereview once fixed.

[DaleCurtis, 2013/04/09 00:40:34]

Fixed. Using the ternary here compiles into a cmov which the original if block
does not. Smaller gains, but correct ones this time :)

+      else
+        sample = static_cast<Fixed>(v * max) & max;

[DaleCurtis, 2013/04/08 21:00:30]

Ditto.

 
-      dest[offset] = static_cast<Format>(sample) + kBias;
+      dest[offset] = static_cast<Format>(sample) + bias;
     }
   }
 }
 
 static void ValidateConfig(size_t channels, int frames) {
   CHECK_GT(frames, 0);
   CHECK_LE(channels, static_cast<size_t>(limits::kMaxChannels));
 }
 
 static void CheckOverflow(int start_frame, int frames, int total_frames) {
@@ skipping 139 matching lines @@
   for (int i = 0; i < channels; ++i)
     channel_data_.push_back(data + i * aligned_frames);
 }
 
 // TODO(dalecurtis): See if intrinsic optimizations help any here.
 void AudioBus::FromInterleavedPartial(const void* source, int start_frame,
                                       int frames, int bytes_per_sample) {
   CheckOverflow(start_frame, frames, frames_);
   switch (bytes_per_sample) {
     case 1:
-      FromInterleavedInternal<uint8, int16>(source, start_frame, frames, this);
+      FromInterleavedInternal<uint8, int16>(
+          source, start_frame, frames, this,
+          kUint8Bias, 1.0f / kUint8Min, 1.0f / kUint8Max);
       break;
     case 2:
-      FromInterleavedInternal<int16, int32>(source, start_frame, frames, this);
+      FromInterleavedInternal<int16, int32>(
+          source, start_frame, frames, this,
+          0, 1.0f / kint16min, 1.0f / kint16max);
       break;
     case 4:
-      FromInterleavedInternal<int32, int64>(source, start_frame, frames, this);
+      FromInterleavedInternal<int32, int64>(
+          source, start_frame, frames, this,
+          0, 1.0f / kint32min, 1.0f / kint32max);
       break;
     default:
       NOTREACHED() << "Unsupported bytes per sample encountered.";
       ZeroFramesPartial(start_frame, frames);
       return;
   }
 
   // Don't clear remaining frames if this is a partial deinterleave.
   if (!start_frame) {
     // Zero any remaining frames.
@@ skipping 10 matching lines @@
                              void* dest) const {
   ToInterleavedPartial(0, frames, bytes_per_sample, dest);
 }
 
 // TODO(dalecurtis): See if intrinsic optimizations help any here.
 void AudioBus::ToInterleavedPartial(int start_frame, int frames,
                                     int bytes_per_sample, void* dest) const {
   CheckOverflow(start_frame, frames, frames_);
   switch (bytes_per_sample) {
     case 1:
-      ToInterleavedInternal<uint8, int16>(this, start_frame, frames, dest);
+      ToInterleavedInternal<uint8, int16>(
+          this, start_frame, frames, dest,
+          kUint8Bias, kUint8Min, kUint8Max);
       break;
     case 2:
-      ToInterleavedInternal<int16, int32>(this, start_frame, frames, dest);
+      ToInterleavedInternal<int16, int32>(
+          this, start_frame, frames, dest, 0, kint16min, kint16max);
       break;
     case 4:
-      ToInterleavedInternal<int32, int64>(this, start_frame, frames, dest);
+      ToInterleavedInternal<int32, int64>(
+          this, start_frame, frames, dest, 0, kint32min, kint32max);
       break;
     default:
       NOTREACHED() << "Unsupported bytes per sample encountered.";
       memset(dest, 0, frames * bytes_per_sample);
       return;
   }
 }
 
 void AudioBus::CopyTo(AudioBus* dest) const {
   CHECK_EQ(channels(), dest->channels());
   CHECK_EQ(frames(), dest->frames());
 
   // Since we don't know if the other AudioBus is wrapped or not (and we don't
   // want to care), just copy using the public channel() accessors.
   for (int i = 0; i < channels(); ++i)
     memcpy(dest->channel(i), channel(i), sizeof(*channel(i)) * frames());
 }
 
 }  // namespace media