Add vector_math::FMUL. Replace audio_util::AdjustVolume.

Add vector_math::FMUL.  Replace audio_util::AdjustVolume.

Removes the integer based volume adjustment code from the
melting pot that is audio_util in favor of an
AudioBus::AdjustVolume() method which works on float.

The driver behind the method is a new vector_math::FMUL
method which is SSE optimized.  Benchmarks put it in line
with the vector_math::FMAC() method.

Benchmarking 200000 iterations:
FMUL_C took 1962.52ms.
FMUL_SSE (unaligned size) took 493.03ms; which is 3.98x faster than FMUL_C.
FMUL_SSE (aligned size) took 491.79ms; which is 3.99x faster than FMUL_C and 1.00x faster than FMUL_SSE (unaligned size).

BUG=120319, 171540, 226447
TEST=new media_unittests.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=192905

[Chris Rogers, 2013-04-05 21:31:35]

lgtm - Dale, thanks so much for doing this!

https://codereview.chromium.org/13726011/diff/6001/media/audio/audio_util.h
File media/audio/audio_util.h (right):

https://codereview.chromium.org/13726011/diff/6001/media/audio/audio_util.h#n...
media/audio/audio_util.h:10: #include "build/build_config.h"
just curious why these new header files?

https://codereview.chromium.org/13726011/diff/6001/media/base/audio_bus.cc
File media/base/audio_bus.cc (right):

https://codereview.chromium.org/13726011/diff/6001/media/base/audio_bus.cc#ne...
media/base/audio_bus.cc:312: if (volume > 0 && volume < 1) {
I would suggest generalizing it by calling is Scale() and removing the < 1
restriction (you can still optimize for the==1 case.  It's valid to want to
scale >1 and even <0 (for phase inversion).

I know right now we're using it in  a specific use case for <audio> (and
similar) where volume must be 0<=volume<=1 but the general case is also useful.

Perhaps we can keep the AdjustVolume() method with these limits and implement it
in terms of another AudioBus method called Scale() which does not have these
limits - WDYT?

https://codereview.chromium.org/13726011/diff/6001/media/base/audio_bus.h
File media/base/audio_bus.h (right):

https://codereview.chromium.org/13726011/diff/6001/media/base/audio_bus.h#new...
media/base/audio_bus.h:95: // Scale internal channel values by 0 <= |volume| <=
1.  If an invalid value
please see comment in .cc about supporting other ranges

[Chris Rogers, 2013-04-05 21:33:01]

Oh, one last thing, can you also please add this in for the AUHAL back-end too?

[DaleCurtis, 2013-04-05 23:52:30]

https://codereview.chromium.org/13726011/diff/6001/media/audio/audio_util.h
File media/audio/audio_util.h (right):

https://codereview.chromium.org/13726011/diff/6001/media/audio/audio_util.h#n...
media/audio/audio_util.h:10: #include "build/build_config.h"
On 2013/04/05 21:31:35, Chris Rogers wrote:
> just curious why these new header files?

The defined(OS_WIN) should have build_config.h available. time.h was a mistake.

https://codereview.chromium.org/13726011/diff/6001/media/base/audio_bus.cc
File media/base/audio_bus.cc (right):

https://codereview.chromium.org/13726011/diff/6001/media/base/audio_bus.cc#ne...
media/base/audio_bus.cc:312: if (volume > 0 && volume < 1) {
On 2013/04/05 21:31:35, Chris Rogers wrote:
> I would suggest generalizing it by calling is Scale() and removing the < 1
> restriction (you can still optimize for the==1 case.  It's valid to want to
> scale >1 and even <0 (for phase inversion).
> 
> I know right now we're using it in  a specific use case for <audio> (and
> similar) where volume must be 0<=volume<=1 but the general case is also
useful.
> 
> Perhaps we can keep the AdjustVolume() method with these limits and implement
it
> in terms of another AudioBus method called Scale() which does not have these
> limits - WDYT?
> 

Lets see if we can get away with just the Scale() method for now. 
AudioRendererHost::OnSetVolume doesn't except volume ranges outside of [0, 1],
so we should be fine.

https://codereview.chromium.org/13726011/diff/6001/media/base/audio_bus.h
File media/base/audio_bus.h (right):

https://codereview.chromium.org/13726011/diff/6001/media/base/audio_bus.h#new...
media/base/audio_bus.h:95: // Scale internal channel values by 0 <= |volume| <=
1.  If an invalid value
On 2013/04/05 21:31:35, Chris Rogers wrote:
> please see comment in .cc about supporting other ranges

Done.

[henrika (OOO until Aug 14), 2013-04-08 18:41:00]

Great work Dale; highly appreciated!

LGTM w/ nits.

Don't know the SSE parts well enough to provide valid comments. Same with your
gyp, gypi changes.

[henrika (OOO until Aug 14), 2013-04-08 18:41:09]

https://codereview.chromium.org/13726011/diff/12001/media/base/audio_bus.cc
File media/base/audio_bus.cc (right):

https://codereview.chromium.org/13726011/diff/12001/media/base/audio_bus.cc#n...
media/base/audio_bus.cc:312: if (volume > 0 && volume < 1) {
Add special case for volume = 1.o where we do nothing.

https://codereview.chromium.org/13726011/diff/12001/media/base/vector_math.h
File media/base/vector_math.h (right):

https://codereview.chromium.org/13726011/diff/12001/media/base/vector_math.h#...
media/base/vector_math.h:21: // |dest| must be aligned by kRequiredAlignment.
What happens if they are not properly aligned?

[DaleCurtis, 2013-04-08 19:11:40]

https://codereview.chromium.org/13726011/diff/12001/media/base/audio_bus.cc
File media/base/audio_bus.cc (right):

https://codereview.chromium.org/13726011/diff/12001/media/base/audio_bus.cc#n...
media/base/audio_bus.cc:312: if (volume > 0 && volume < 1) {
On 2013/04/08 18:41:09, henrika wrote:
> Add special case for volume = 1.o where we do nothing.

Whoops, done!

https://codereview.chromium.org/13726011/diff/12001/media/base/vector_math.h
File media/base/vector_math.h (right):

https://codereview.chromium.org/13726011/diff/12001/media/base/vector_math.h#...
media/base/vector_math.h:21: // |dest| must be aligned by kRequiredAlignment.
On 2013/04/08 18:41:09, henrika wrote:
> What happens if they are not properly aligned?

When using mm_load instead of mm_loadu (unaligned) an unaligned load will
generate a fault. AudioBus takes care of ensuring our allocations are aligned
correctly.

[commit-bot: I haz the power, 2013-04-08 19:11:52]

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

[commit-bot: I haz the power, 2013-04-08 21:30:38]

Change committed as 192905