Add SSE optimizations to AudioRendererMixer.

Optimizes the FMAC operation with SSE.  Performance is on par with
FFmpeg's vector_fmac function, without the hassle of 32-byte align
and over allocation.

VectorFMAC_SSE requires 16-byte alignment of the source and dest
vectors.  Size does not need to be a multiple of 4 though.

Performance results from AudioRendererMixerTest.VectorFMACBenchmark:

Benchmarking 200000 iterations:
VectorFMAC_C took 2030.73ms.
VectorFMAC_SSE (unaligned size) took 598.33ms; which is 3.39x faster than VectorFMAC_C.
VectorFMAC_SSE (aligned size) took 597.71ms; which is 3.40x faster than VectorFMAC_C and 1.00x faster than VectorFMAC_SSE (unaligned size).

BUG=133637
TEST=media_unittests + AudioRendererMixer/* tests.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=149581

[DaleCurtis, 2012-07-18 03:42:32]

PTAL. You seemed most interested in this discussion at lunch, so you get to
review :)

Still needs an AlignedMemory<> setup, which I will pipe in another CL before
landing this.

[DaleCurtis, 2012-07-18 03:44:08]

On 2012/07/18 03:42:32, DaleCurtis wrote:
> PTAL. You seemed most interested in this discussion at lunch, so you get to
> review :)
> 
> Still needs an AlignedMemory<> setup, which I will pipe in another CL before
> landing this.

All the performance issues I reported this morning were due to two things:
1. Forgetting I had profiling turned on.
2. DCHECK nonsense on Release vs Official builds that we discussed.

[Ami GONE FROM CHROMIUM, 2012-07-18 04:42:27]

Pretty much the same comments I had on the other intrinsics CL.

But yay to intrinsics working so well!

[DaleCurtis, 2012-07-27 01:29:13]

Made all the same changes as with SincResampler. PTAL.

Waiting on resolution for AlignedAlloc/AlignedFree on Windows:
https://groups.google.com/a/chromium.org/forum/?fromgroups#!topic/chromium-de...

[Ami GONE FROM CHROMIUM, 2012-07-27 17:33:11]

http://codereview.chromium.org/10802005/diff/5001/content/renderer/media/audi...
File content/renderer/media/audio_device_thread.cc (right):

http://codereview.chromium.org/10802005/diff/5001/content/renderer/media/audi...
content/renderer/media/audio_device_thread.cc:9: #include
"base/memory/aligned_memory.h"
CL description says "without the hassle of 32-byte align" - I think you could be
clearer about the requirements/non-requirements.

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
File media/base/audio_renderer_mixer.cc (right):

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer.cc:175: // Ensure |src| and |dest| are aligned
properly.
Note an alternative is to handle a leading unaligned segment like you do the
trailing unaligned segment.  I don't know that that's worth doing.

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
File media/base/audio_renderer_mixer.h (right):

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer.h:52: // Multiply each element of |src| (up to
|len|) by |scale| and add to |dest|.
nit:
float* dest
is usually read in chromium style as "output param receiving a float".  But here
you're using it to mean "output array of floats".  Does it work to say
float[] dest
instead, for clarity?

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer.h:55: static void VectorFMAC_SSE(const float*
src, float scale, int len,
Doco that you require src/dest to be SSE-aligned.

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
File media/base/audio_renderer_mixer_unittest.cc (right):

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_unittest.cc:23: namespace switches {
why?

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_unittest.cc:44: // Ensure various optimized
VectorFMAC() methods return the right value.
s/right/same/

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_unittest.cc:69: memset(output_vector.get(), 0,
sizeof(float) * kHighLatencyBufferSize);
By init'd output with 0 you're avoiding testing the "accumulate" part of FMAC ;)
Replace memset of 0 with std::fill of 3?

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_unittest.cc:77: std::fill(input_vector.get(),
input_vector.get() + kHighLatencyBufferSize,
Why is this necessary?

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_unittest.cc:79: memset(output_vector.get(), 0,
sizeof(float) * kHighLatencyBufferSize);
ditto

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_unittest.cc:118: memset(input_vector.get(), 1,
sizeof(float) * kHighLatencyBufferSize);
Are you intentionally testing inputs of inputs that aren't 1?
(this is setting bytes, not floats)

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_unittest.cc:130: // Benchmark VectorFMAC_SSE()
with unaligned size; I.e., size % 4 != 0.
Comment here that the API insists the head *is* aligned?

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_unittest.cc:132: memset(input_vector.get(), 1,
sizeof(float) * kHighLatencyBufferSize);
ditto

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_unittest.cc:147: memset(input_vector.get(), 1,
sizeof(float) * kHighLatencyBufferSize);
ditto

[DaleCurtis, 2012-07-27 21:23:42]

http://codereview.chromium.org/10802005/diff/5001/content/renderer/media/audi...
File content/renderer/media/audio_device_thread.cc (right):

http://codereview.chromium.org/10802005/diff/5001/content/renderer/media/audi...
content/renderer/media/audio_device_thread.cc:9: #include
"base/memory/aligned_memory.h"
On 2012/07/27 17:33:11, Ami Fischman wrote:
> CL description says "without the hassle of 32-byte align" - I think you could
be
> clearer about the requirements/non-requirements.

There I'm specifically referring to versus FFMpeg's vector_fmac.  I'll add some
info about requiring a 16-byte alignment.

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
File media/base/audio_renderer_mixer.cc (right):

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer.cc:175: // Ensure |src| and |dest| are aligned
properly.
On 2012/07/27 17:33:11, Ami Fischman wrote:
> Note an alternative is to handle a leading unaligned segment like you do the
> trailing unaligned segment.  I don't know that that's worth doing.

Certainly not after all the trouble I've gone to get aligned allocations in! :)

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
File media/base/audio_renderer_mixer.h (right):

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer.h:52: // Multiply each element of |src| (up to
|len|) by |scale| and add to |dest|.
On 2012/07/27 17:33:11, Ami Fischman wrote:
> nit:
> float* dest
> is usually read in chromium style as "output param receiving a float".  But
here
> you're using it to mean "output array of floats".  Does it work to say
> float[] dest
> instead, for clarity?

Done.

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer.h:55: static void VectorFMAC_SSE(const float*
src, float scale, int len,
On 2012/07/27 17:33:11, Ami Fischman wrote:
> Doco that you require src/dest to be SSE-aligned.

Done.

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
File media/base/audio_renderer_mixer_unittest.cc (right):

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_unittest.cc:23: namespace switches {
On 2012/07/27 17:33:11, Ami Fischman wrote:
> why?

Saw it done this way elsewhere. Changed.

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_unittest.cc:44: // Ensure various optimized
VectorFMAC() methods return the right value.
On 2012/07/27 17:33:11, Ami Fischman wrote:
> s/right/same/

Done.

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_unittest.cc:69: memset(output_vector.get(), 0,
sizeof(float) * kHighLatencyBufferSize);
On 2012/07/27 17:33:11, Ami Fischman wrote:
> By init'd output with 0 you're avoiding testing the "accumulate" part of FMAC
;)
> Replace memset of 0 with std::fill of 3?

Done.

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_unittest.cc:77: std::fill(input_vector.get(),
input_vector.get() + kHighLatencyBufferSize,
Whoops, don't need to reset input vector, but output vector needs to be reset,
otherwise it'll accumulate the previous VectorFMAC_C result.

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_unittest.cc:79: memset(output_vector.get(), 0,
sizeof(float) * kHighLatencyBufferSize);
On 2012/07/27 17:33:11, Ami Fischman wrote:
> ditto

Done.

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_unittest.cc:118: memset(input_vector.get(), 1,
sizeof(float) * kHighLatencyBufferSize);
On 2012/07/27 17:33:11, Ami Fischman wrote:
> Are you intentionally testing inputs of inputs that aren't 1?
> (this is setting bytes, not floats)

Yeah, just needed to initialize the array with something. Switched to
std::fill(3) for clarity.

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_unittest.cc:130: // Benchmark VectorFMAC_SSE()
with unaligned size; I.e., size % 4 != 0.
On 2012/07/27 17:33:11, Ami Fischman wrote:
> Comment here that the API insists the head *is* aligned?

Seems redundant at this point, it's already in the header and we've got aligned
allocations above. It's clearer to me to focus on what's different here.

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_unittest.cc:132: memset(input_vector.get(), 1,
sizeof(float) * kHighLatencyBufferSize);
On 2012/07/27 17:33:11, Ami Fischman wrote:
> ditto

Done.

http://codereview.chromium.org/10802005/diff/5001/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_unittest.cc:147: memset(input_vector.get(), 1,
sizeof(float) * kHighLatencyBufferSize);
On 2012/07/27 17:33:11, Ami Fischman wrote:
> ditto

Done.

[Ami GONE FROM CHROMIUM, 2012-07-27 21:47:21]

LGTM

http://codereview.chromium.org/10802005/diff/11002/media/base/audio_renderer_...
File media/base/audio_renderer_mixer.h (right):

http://codereview.chromium.org/10802005/diff/11002/media/base/audio_renderer_...
media/base/audio_renderer_mixer.h:54: static void VectorFMAC_C(const float& src,
float scale, int len,
s/&/*/ here and above.
(const* is different from *)

If you like, fine to pass as const[], too.

[DaleCurtis, 2012-07-27 22:54:54]

Thanks for review. Will land once AlignedMemory stuff resolves.

http://codereview.chromium.org/10802005/diff/11002/media/base/audio_renderer_...
File media/base/audio_renderer_mixer.h (right):

http://codereview.chromium.org/10802005/diff/11002/media/base/audio_renderer_...
media/base/audio_renderer_mixer.h:54: static void VectorFMAC_C(const float& src,
float scale, int len,
On 2012/07/27 21:47:21, Ami Fischman wrote:
> s/&/*/ here and above.
> (const* is different from *)
> 
> If you like, fine to pass as const[], too.

Done.

[commit-bot: I haz the power, 2012-08-02 00:57:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/10802005/7005

[commit-bot: I haz the power, 2012-08-02 03:27:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/10802005/18005

[commit-bot: I haz the power, 2012-08-02 05:58:49]

Change committed as 149581