Support concatenated spatialization data

Support concatenated spatialization data

BUG=227141
This is a followup to https://codereview.chromium.org/14297003 that
enables support for concatenated spatialization data.  All platforms will now use the concatenated data set.

On Android, this
speeds up loading of the HRTF database from 10+ sec to less than 2
sec.

This cannot be landed until https://codereview.chromium.org/14297003
and https://codereview.chromium.org/14311015/ have landed.

R=crogers@google.com, jamesr@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=149361

[Raymond Toy (Google), 2013-04-17 19:55:05]

Please review.

[Chris Rogers, 2013-04-17 22:36:56]

https://codereview.chromium.org/14304002/diff/1/Source/WebCore/platform/audio...
File Source/WebCore/platform/audio/HRTFElevation.cpp (right):

https://codereview.chromium.org/14304002/diff/1/Source/WebCore/platform/audio...
Source/WebCore/platform/audio/HRTFElevation.cpp:66: // concatenated response
file to speed it up.
Comment adds little value, please remove.

https://codereview.chromium.org/14304002/diff/1/Source/WebCore/platform/audio...
Source/WebCore/platform/audio/HRTFElevation.cpp:67: #if USE(WEBAUDIO_GSTREAMER)
|| OS(ANDROID)
I don't think this should be Android-specific since all operating systems can
benefit.

https://codereview.chromium.org/14304002/diff/1/Source/WebKit/chromium/WebKit...
File Source/WebKit/chromium/WebKit.grd (right):

https://codereview.chromium.org/14304002/diff/1/Source/WebKit/chromium/WebKit...
Source/WebKit/chromium/WebKit.grd:252: <include
name="IDR_AUDIO_SPATIALIZATION_COMPOSITE"
file="..\..\WebCore\platform\audio\resources\Composite.wav" type="BINDATA"/>
Since we're switching to a single resource, the other 240 individual ones should
be deleted

[Chris Rogers, 2013-04-17 22:38:55]

Also, please write up a specific bug for the resource loading issue on Android
(decodeAudioData(), etc.) since this doesn't solve the general performance issue
- this should be a P0 bug

[Raymond Toy (Google), 2013-04-18 16:26:18]

On 2013/04/17 22:38:55, Chris Rogers wrote:
> Also, please write up a specific bug for the resource loading issue on Android
> (decodeAudioData(), etc.) since this doesn't solve the general performance
issue
> - this should be a P0 bug

crbug.com/232973

[Raymond Toy (Google), 2013-04-23 17:28:46]

Please review.  I've addressed all of the comments as suggested and made all
platforms now use the concatenated data set.

[Chris Rogers, 2013-04-23 19:51:44]

Please add the conditional compilation back in.  We'll have it turned on by
default, but need to original code when we iterate on new impulse response
databases, now being discussed on public-audio

#ifdef USE_CONCATENATED_IMPULSE_RESPONSES

To do this properly we may need to add a gyp define

https://codereview.chromium.org/14304002/diff/12001/Source/core/platform/audi...
File Source/core/platform/audio/HRTFElevation.cpp (right):

https://codereview.chromium.org/14304002/diff/12001/Source/core/platform/audi...
Source/core/platform/audio/HRTFElevation.cpp:143: // It's passed in as an
internal ASCII identifier and is an implementation detail.
This comment used to be just before line 140 and you've left out part of the
comment.  The full comment is:

    // Construct the resource name from the subject name, azimuth, and
elevation, for example:
    // "IRC_Composite_C_R0195_T015_P000"
    // Note: the passed in subjectName is not a string passed in via JavaScript
or the web.
    // It's passed in as an internal ASCII identifier and is an implementation
detail.

[Raymond Toy (Google), 2013-04-23 22:15:36]

Please review.  I've restored the conditional code and added the necessary gyp
variable to control whether we use the concatenated data or not.

I do not know why the trybots are red, especially the win_layout_rel bot which
can't apply my patch but the other bots can.

+tony:  features.gypi

[Chris Rogers, 2013-04-23 22:32:17]

lgtm as far as HRTFElevation.cpp changes

question for Tony below:

https://codereview.chromium.org/14304002/diff/26001/Source/WebKit/chromium/We...
File Source/WebKit/chromium/WebKit.grd (right):

https://codereview.chromium.org/14304002/diff/26001/Source/WebKit/chromium/We...
Source/WebKit/chromium/WebKit.grd:12: <include
name="IDR_AUDIO_SPATIALIZATION_COMPOSITE"
file="../../core/platform/audio/resources/Composite.wav" type="BINDATA"/>
Question for Tony, is there any way to use the .gyp variable
"WTF_USE_CONCATENATED_IMPULSE_RESPONSES" in this .grd file?

[Raymond Toy (Google), 2013-04-24 20:38:49]

https://codereview.chromium.org/14304002/diff/26001/Source/WebKit/chromium/We...
File Source/WebKit/chromium/WebKit.grd (right):

https://codereview.chromium.org/14304002/diff/26001/Source/WebKit/chromium/We...
Source/WebKit/chromium/WebKit.grd:12: <include
name="IDR_AUDIO_SPATIALIZATION_COMPOSITE"
file="../../core/platform/audio/resources/Composite.wav" type="BINDATA"/>
On 2013/04/23 22:32:17, Chris Rogers wrote:
> Question for Tony, is there any way to use the .gyp variable
> "WTF_USE_CONCATENATED_IMPULSE_RESPONSES" in this .grd file?

Yes, it is possible. I'm working on it now.  I think, however, that it will
require a chromium-side change first to set up appropriate variable to control
all of this.

[Raymond Toy (Google), 2013-04-24 21:34:13]

tony: Please review the gyp changes.  You may also want to take a look at
https://codereview.chromium.org/14311015/ which defines
use_concatenated_impulse_responses which controls whether the concatenates
responses are used.

The grd file has been updated so that either the concatenated file or the
unconcatenated files are included, depending on
use_concatenated_impulse_responses.

[Dirk Pranke, 2013-04-25 20:50:41]

lgtm.

[Raymond Toy (Google), 2013-04-26 23:22:06]

Chris, please review HRTFElevation.cpp again.  The current version added changes
to use RefPtr and PassRefPtr in various places, so the concatenated case had to
be updated to match.

[Chris Rogers, 2013-04-28 20:01:37]

Still lgtm

[commit-bot: I haz the power, 2013-04-29 15:10:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rtoy@google.com/14304002/43001

[commit-bot: I haz the power, 2013-04-29 15:42:41]

Retried try job too often on linux_layout_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_layo...

[commit-bot: I haz the power, 2013-04-29 18:16:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rtoy@google.com/14304002/43001

[commit-bot: I haz the power, 2013-04-29 18:54:53]

Retried try job too often on linux_layout_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_layo...

[jamesr, 2013-04-29 20:20:31]

rubber-stamp lgtm for the grd

[Raymond Toy (Google), 2013-04-29 20:36:17]

Committed patchset #7 manually as r149361 (presubmit successful).