Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
14297003:
Support concatenated spatialization data (Closed)
|
Created:
7 years, 8 months ago by Raymond Toy (Google) Modified:
7 years, 8 months ago CC:
chromium-reviews, darin-cc_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Visibility:
Public. |
DescriptionSupport concatenated spatialization data
BUG=227141
This is initial support for concatenated spatialization data to speed up loading of the HRTF database on Android. A
followup webkit patch (https://codereview.chromium.org/14304002/) will actually enable the concatenated data.
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=195537
Patch Set 1 : #
Total comments: 5
Patch Set 2 : #Messages
Total messages: 15 (0 generated)
Please review.
Switching over to the concatenated single resource seems like a good idea. Safari/iOS and GTK already use this. So this seems fine to me. We should do this for all operating systems and not just Android, since there's nothing Android-specific about the performance benefits we could gain. So please update the description of this CL and any comments to not mention Android in particular. Leaving to James for final review. https://codereview.chromium.org/14297003/diff/2001/webkit/glue/webkitplatform... File webkit/glue/webkitplatformsupport_impl.cc (right): https://codereview.chromium.org/14297003/diff/2001/webkit/glue/webkitplatform... webkit/glue/webkitplatformsupport_impl.cc:498: if (strcmp(name, "Composite") == 0) { use !strcmp() https://codereview.chromium.org/14297003/diff/2001/webkit/glue/webkitplatform... webkit/glue/webkitplatformsupport_impl.cc:506: #ifdef IDR_AUDIO_SPATIALIZATION_T000_P000 I assume that once you switch Blink over to using the single "composite" resource and delete the 240 individual resources from the .grd file that you'll remove the #ifdef IDR_AUDIO_SPATIALIZATION_T000_P000 case?? https://codereview.chromium.org/14297003/diff/2001/webkit/glue/webkitplatform... webkit/glue/webkitplatformsupport_impl.cc:674: StartsWithASCII(name, "Composite", true)) I assume once Blink switches to the single file that this can check just for "Composite"
owners lgtm for webkit/
https://codereview.chromium.org/14297003/diff/2001/webkit/glue/webkitplatform... File webkit/glue/webkitplatformsupport_impl.cc (right): https://codereview.chromium.org/14297003/diff/2001/webkit/glue/webkitplatform... webkit/glue/webkitplatformsupport_impl.cc:506: #ifdef IDR_AUDIO_SPATIALIZATION_T000_P000 On 2013/04/17 22:33:31, Chris Rogers wrote: > I assume that once you switch Blink over to using the single "composite" > resource and delete the 240 individual resources from the .grd file that you'll > remove the #ifdef IDR_AUDIO_SPATIALIZATION_T000_P000 case?? Yes, I will remove that, along with the 240 individual resources in the .grd file. But the actual 240 wav files will remain. https://codereview.chromium.org/14297003/diff/2001/webkit/glue/webkitplatform... webkit/glue/webkitplatformsupport_impl.cc:674: StartsWithASCII(name, "Composite", true)) On 2013/04/17 22:33:31, Chris Rogers wrote: > I assume once Blink switches to the single file that this can check just for > "Composite" Yes
lgtm
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rtoy@google.com/14297003/16001
Retried try job too often on win7_aura for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rtoy@google.com/14297003/16001
Sorry for I got bad news for ya. Compile failed with a clobber build on win7_aura. http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&... Your code is likely broken or HEAD is junk. Please ensure your code is not broken then alert the build sheriffs. Look at the try server FAQ for more details.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rtoy@google.com/14297003/16001
Retried try job too often on win7_aura for step(s) content_browsertests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rtoy@google.com/14297003/16001
Sorry for I got bad news for ya. Compile failed with a clobber build on win7_aura. http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&... Your code is likely broken or HEAD is junk. Please ensure your code is not broken then alert the build sheriffs. Look at the try server FAQ for more details.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rtoy@google.com/14297003/16001
Message was sent while issue was closed.
Change committed as 195537 |
