75119 2011-12-22 13:27:53 -0800 MediaElementAudioSourceNode should support multi-channel > stereo 2012-05-21 15:23:28 -0700 1 1 1 Unclassified WebKit Web Audio 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 crogers james.wei crogers eric.carlson feature-media-reviews james.wei webkit.review.bot oldest_to_newest 526476 0 crogers 2011-12-22 13:27:53 -0800 Both up and down mixing paths for multi-channel > stereo need to be added into AudioBus::processWithGainFrom(), etc. And then the checks in AudioBufferSourceNode and MediaElementAudioSourceNode need to be removed. We should have layout tests (at least for AudioBufferSourceNode) 528049 1 james.wei 2011-12-29 17:45:49 -0800 (In reply to comment #0) > Both up and down mixing paths for multi-channel > stereo need to be added into AudioBus::processWithGainFrom(), etc. > And then the checks in AudioBufferSourceNode and MediaElementAudioSourceNode need to be removed. > > We should have layout tests (at least for AudioBufferSourceNode) roger, I'm interested in this feature implementation. If this task is not urgent and not so complicated for a newbie, can I have a try for it? thanks 528050 2 james.wei 2011-12-29 17:46:11 -0800 for 5.1 to stereo downmixing, there is no algorithm in https://dvcs.w3.org/hg/audio/raw-file/tip/webaudio/specification.html#UpMix-section, which algorithm should I use? thanks 529130 3 crogers 2012-01-03 16:05:09 -0800 Hi James, thanks for offering to do this. I think it's something you could probably tackle. But we'll have to first sort out the details of your other vsmul() optimizations in that same part of the code. If you see my latest comment there, I think we should maybe simplify the denormal handling: https://bugs.webkit.org/show_bug.cgi?id=75334 As far as the 5.1 to stereo case, the only question is the exact scaling factors (constant multipliers), so you can prepare a patch with close to the exact code (except the numeric values) and we can sort that out later. 529193 4 james.wei 2012-01-03 17:18:06 -0800 (In reply to comment #3) > Hi James, thanks for offering to do this. I think it's something you could probably tackle. But we'll have to first sort out the details of your other vsmul() optimizations in that same part of the code. If you see my latest comment there, I think we should maybe simplify the denormal handling: > > https://bugs.webkit.org/show_bug.cgi?id=75334 > > As far as the 5.1 to stereo case, the only question is the exact scaling factors (constant multipliers), so you can prepare a patch with close to the exact code (except the numeric values) and we can sort that out later. roger, thanks for the guidance. I will work on #75334 firstly. for up and down mixing, for source and destination, we both have 5 options: mono, stereo, quad, 5.0, 5.1. Current we only have 3 options: mono->mono, mono->stereo, stereo->stereo. do we need to support all channel up and down mixing(that is all 25 options)? 529208 5 crogers 2012-01-03 17:49:19 -0800 (In reply to comment #4) > (In reply to comment #3) > > Hi James, thanks for offering to do this. I think it's something you could probably tackle. But we'll have to first sort out the details of your other vsmul() optimizations in that same part of the code. If you see my latest comment there, I think we should maybe simplify the denormal handling: > > > > https://bugs.webkit.org/show_bug.cgi?id=75334 > > > > As far as the 5.1 to stereo case, the only question is the exact scaling factors (constant multipliers), so you can prepare a patch with close to the exact code (except the numeric values) and we can sort that out later. > > roger, thanks for the guidance. I will work on #75334 firstly. > > for up and down mixing, for source and destination, we both have 5 options: mono, stereo, quad, 5.0, 5.1. Current we only have 3 options: mono->mono, mono->stereo, stereo->stereo. > > do we need to support all channel up and down mixing(that is all 25 options)? We don't have to support all of them to start with. After all we only currently support a few of them, so anything is an improvement. Quad (4-channels) we can probably skip for now (and get to it later). Also, the most important are the up-mixing options (as opposed to down-mixing). For up-mixing to a number of channels which isn't known as having a specific channel layout such as 3 channels (which isn't 1,2,4,6 - for mono,stereo,quad,5.1) , the default up-mix should just be to copy/sum the channels into the first available channels, and then just ignore (or zero) the remaining channels. For example, if up-mixing stereo to 3 channels: channel0 -> channel0 channel1 -> channel1 ignore/silence -> channel2 This type of up-mix may also be interesting for 1,2,4,6, but we don't have to worry about that for now. 627219 6 142608 james.wei 2012-05-17 19:06:14 -0700 Created attachment 142608 Patch 627229 7 142608 crogers 2012-05-17 19:30:41 -0700 Comment on attachment 142608 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142608&action=review Thanks James! - looks good with nit > Source/WebCore/Modules/webaudio/MediaElementAudioSourceNode.cpp:71 > + if (!numberOfChannels || numberOfChannels > AudioContext::maxNumberOfChannels() \ style nit: Please remove continuation character '\' and leave everything on one long line 627260 8 142618 james.wei 2012-05-17 19:56:18 -0700 Created attachment 142618 Patch 627266 9 142608 james.wei 2012-05-17 19:59:18 -0700 Comment on attachment 142608 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142608&action=review >> Source/WebCore/Modules/webaudio/MediaElementAudioSourceNode.cpp:71 >> + if (!numberOfChannels || numberOfChannels > AudioContext::maxNumberOfChannels() \ > > style nit: Please remove continuation character '\' and leave everything on one long line fixed. thanks. Just back from chromium coding style. :) 627325 10 142618 webkit.review.bot 2012-05-17 21:56:23 -0700 Comment on attachment 142618 Patch Clearing flags on attachment: 142618 Committed r117541: <http://trac.webkit.org/changeset/117541> 627326 11 webkit.review.bot 2012-05-17 21:56:28 -0700 All reviewed patches have been landed. Closing bug. 629588 12 142608 eric 2012-05-21 15:23:28 -0700 Comment on attachment 142608 Patch Cleared review? from obsolete attachment 142608 so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again). 142608 2012-05-17 19:06:14 -0700 2012-05-21 15:23:28 -0700 Patch bug-75119-20120518100619.patch text/plain 2255 james.wei U3VidmVyc2lvbiBSZXZpc2lvbjogMTE3NTE1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMGUwMzQyNmRlZmQwZTQx ZTliY2RkOWE3ZGQ4OTc2YTBkNDVkNDE5YS4uMjVmOWYzMWVlNzk4M2NhYTFkYzRlZjI1NTlkMDJj MGE1ODJjYmQwZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDEyLTA1LTE3ICBXZWkg SmFtZXMgIDxqYW1lcy53ZWlAaW50ZWwuY29tPgorCisgICAgICAgIEF1ZGlvQnVmZmVyU291cmNl Tm9kZSBhbmQgTWVkaWFFbGVtZW50QXVkaW9Tb3VyY2VOb2RlIHNob3VsZCBzdXBwb3J0IG11bHRp LWNoYW5uZWwgPiBzdGVyZW8KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTc1MTE5CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgQXMgbXVsdGlwbGUgY2hhbm5lbCBzdXBwb3J0IGluIEF1ZGlvQnVzIGFuZCBvdGhl ciBwbGFjZXMgaXMgYWxyZWFkeQorICAgICAgICBpbXBsZW1lbnRlZCwgdGhlIGNoZWNrIGNhbiBi ZSByZW1vdmVkIGhlcmUuICAKKworICAgICAgICAqIE1vZHVsZXMvd2ViYXVkaW8vTWVkaWFFbGVt ZW50QXVkaW9Tb3VyY2VOb2RlLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6Ok1lZGlhRWxlbWVudEF1 ZGlvU291cmNlTm9kZTo6c2V0Rm9ybWF0KToKKwogMjAxMi0wNS0xNyAgU3VkYXJzYW5hIE5hZ2lu ZW5pICA8c3VkYXJzYW5hLm5hZ2luZW5pQGxpbnV4LmludGVsLmNvbT4KIAogICAgICAgICBNZW1v cnkgbGVhayBpbiBOZXR3b3JrSW5mb0NsaWVudApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUv TW9kdWxlcy93ZWJhdWRpby9NZWRpYUVsZW1lbnRBdWRpb1NvdXJjZU5vZGUuY3BwIGIvU291cmNl L1dlYkNvcmUvTW9kdWxlcy93ZWJhdWRpby9NZWRpYUVsZW1lbnRBdWRpb1NvdXJjZU5vZGUuY3Bw CmluZGV4IDhhZjQ0MjMwYjZlNGIyYWY1YjY4MDk1Nzc0ZTNmNTUzMGFlMWUxYjEuLjQxOWJkNTg1 Njk5NmJkMGYxMDYzMzBjNzFiNDU2ZTI5NjRkM2YxYjYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJD b3JlL01vZHVsZXMvd2ViYXVkaW8vTWVkaWFFbGVtZW50QXVkaW9Tb3VyY2VOb2RlLmNwcAorKysg Yi9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL3dlYmF1ZGlvL01lZGlhRWxlbWVudEF1ZGlvU291cmNl Tm9kZS5jcHAKQEAgLTY4LDkgKzY4LDggQEAgTWVkaWFFbGVtZW50QXVkaW9Tb3VyY2VOb2RlOjp+ TWVkaWFFbGVtZW50QXVkaW9Tb3VyY2VOb2RlKCkKIHZvaWQgTWVkaWFFbGVtZW50QXVkaW9Tb3Vy Y2VOb2RlOjpzZXRGb3JtYXQoc2l6ZV90IG51bWJlck9mQ2hhbm5lbHMsIGZsb2F0IHNvdXJjZVNh bXBsZVJhdGUpCiB7CiAgICAgaWYgKG51bWJlck9mQ2hhbm5lbHMgIT0gbV9zb3VyY2VOdW1iZXJP ZkNoYW5uZWxzIHx8IHNvdXJjZVNhbXBsZVJhdGUgIT0gbV9zb3VyY2VTYW1wbGVSYXRlKSB7Ci0g ICAgICAgIC8vIEZJWE1FOiBpbXBsZW1lbnQgbXVsdGktY2hhbm5lbCBncmVhdGVyIHRoYW4gc3Rl cmVvLgotICAgICAgICAvLyBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 NzUxMTkKLSAgICAgICAgaWYgKCFudW1iZXJPZkNoYW5uZWxzIHx8IG51bWJlck9mQ2hhbm5lbHMg PiAyIHx8IHNvdXJjZVNhbXBsZVJhdGUgPCBtaW5TYW1wbGVSYXRlIHx8IHNvdXJjZVNhbXBsZVJh dGUgPiBtYXhTYW1wbGVSYXRlKSB7CisgICAgICAgIGlmICghbnVtYmVyT2ZDaGFubmVscyB8fCBu dW1iZXJPZkNoYW5uZWxzID4gQXVkaW9Db250ZXh0OjptYXhOdW1iZXJPZkNoYW5uZWxzKCkgXAor ICAgICAgICAgICAgICAgIHx8IHNvdXJjZVNhbXBsZVJhdGUgPCBtaW5TYW1wbGVSYXRlIHx8IHNv dXJjZVNhbXBsZVJhdGUgPiBtYXhTYW1wbGVSYXRlKSB7CiAgICAgICAgICAgICAvLyBwcm9jZXNz KCkgd2lsbCBnZW5lcmF0ZSBzaWxlbmNlIGZvciB0aGVzZSB1bmluaXRpYWxpemVkIHZhbHVlcy4K ICAgICAgICAgICAgIExPRyhNZWRpYSwgIk1lZGlhRWxlbWVudEF1ZGlvU291cmNlTm9kZTo6c2V0 Rm9ybWF0KCV1LCAlZikgLSB1bmhhbmRsZWQgZm9ybWF0IGNoYW5nZSIsIHN0YXRpY19jYXN0PHVu c2lnbmVkPihudW1iZXJPZkNoYW5uZWxzKSwgc291cmNlU2FtcGxlUmF0ZSk7CiAgICAgICAgICAg ICBtX3NvdXJjZU51bWJlck9mQ2hhbm5lbHMgPSAwOwo= 142618 2012-05-17 19:56:18 -0700 2012-05-17 21:56:23 -0700 Patch bug-75119-20120518105622.patch text/plain 2236 james.wei U3VidmVyc2lvbiBSZXZpc2lvbjogMTE3NTE1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMGUwMzQyNmRlZmQwZTQx ZTliY2RkOWE3ZGQ4OTc2YTBkNDVkNDE5YS4uMjVmOWYzMWVlNzk4M2NhYTFkYzRlZjI1NTlkMDJj MGE1ODJjYmQwZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDEyLTA1LTE3ICBXZWkg SmFtZXMgIDxqYW1lcy53ZWlAaW50ZWwuY29tPgorCisgICAgICAgIEF1ZGlvQnVmZmVyU291cmNl Tm9kZSBhbmQgTWVkaWFFbGVtZW50QXVkaW9Tb3VyY2VOb2RlIHNob3VsZCBzdXBwb3J0IG11bHRp LWNoYW5uZWwgPiBzdGVyZW8KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTc1MTE5CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgQXMgbXVsdGlwbGUgY2hhbm5lbCBzdXBwb3J0IGluIEF1ZGlvQnVzIGFuZCBvdGhl ciBwbGFjZXMgaXMgYWxyZWFkeQorICAgICAgICBpbXBsZW1lbnRlZCwgdGhlIGNoZWNrIGNhbiBi ZSByZW1vdmVkIGhlcmUuICAKKworICAgICAgICAqIE1vZHVsZXMvd2ViYXVkaW8vTWVkaWFFbGVt ZW50QXVkaW9Tb3VyY2VOb2RlLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6Ok1lZGlhRWxlbWVudEF1 ZGlvU291cmNlTm9kZTo6c2V0Rm9ybWF0KToKKwogMjAxMi0wNS0xNyAgU3VkYXJzYW5hIE5hZ2lu ZW5pICA8c3VkYXJzYW5hLm5hZ2luZW5pQGxpbnV4LmludGVsLmNvbT4KIAogICAgICAgICBNZW1v cnkgbGVhayBpbiBOZXR3b3JrSW5mb0NsaWVudApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUv TW9kdWxlcy93ZWJhdWRpby9NZWRpYUVsZW1lbnRBdWRpb1NvdXJjZU5vZGUuY3BwIGIvU291cmNl L1dlYkNvcmUvTW9kdWxlcy93ZWJhdWRpby9NZWRpYUVsZW1lbnRBdWRpb1NvdXJjZU5vZGUuY3Bw CmluZGV4IDhhZjQ0MjMwYjZlNGIyYWY1YjY4MDk1Nzc0ZTNmNTUzMGFlMWUxYjEuLmRhZDA2N2Ez ZDgwOGFkODcwMTRmOTlkOGE2YTg3YWYzYWM4ZGYzNTQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJD b3JlL01vZHVsZXMvd2ViYXVkaW8vTWVkaWFFbGVtZW50QXVkaW9Tb3VyY2VOb2RlLmNwcAorKysg Yi9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL3dlYmF1ZGlvL01lZGlhRWxlbWVudEF1ZGlvU291cmNl Tm9kZS5jcHAKQEAgLTY4LDkgKzY4LDcgQEAgTWVkaWFFbGVtZW50QXVkaW9Tb3VyY2VOb2RlOjp+ TWVkaWFFbGVtZW50QXVkaW9Tb3VyY2VOb2RlKCkKIHZvaWQgTWVkaWFFbGVtZW50QXVkaW9Tb3Vy Y2VOb2RlOjpzZXRGb3JtYXQoc2l6ZV90IG51bWJlck9mQ2hhbm5lbHMsIGZsb2F0IHNvdXJjZVNh bXBsZVJhdGUpCiB7CiAgICAgaWYgKG51bWJlck9mQ2hhbm5lbHMgIT0gbV9zb3VyY2VOdW1iZXJP ZkNoYW5uZWxzIHx8IHNvdXJjZVNhbXBsZVJhdGUgIT0gbV9zb3VyY2VTYW1wbGVSYXRlKSB7Ci0g ICAgICAgIC8vIEZJWE1FOiBpbXBsZW1lbnQgbXVsdGktY2hhbm5lbCBncmVhdGVyIHRoYW4gc3Rl cmVvLgotICAgICAgICAvLyBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 NzUxMTkKLSAgICAgICAgaWYgKCFudW1iZXJPZkNoYW5uZWxzIHx8IG51bWJlck9mQ2hhbm5lbHMg PiAyIHx8IHNvdXJjZVNhbXBsZVJhdGUgPCBtaW5TYW1wbGVSYXRlIHx8IHNvdXJjZVNhbXBsZVJh dGUgPiBtYXhTYW1wbGVSYXRlKSB7CisgICAgICAgIGlmICghbnVtYmVyT2ZDaGFubmVscyB8fCBu dW1iZXJPZkNoYW5uZWxzID4gQXVkaW9Db250ZXh0OjptYXhOdW1iZXJPZkNoYW5uZWxzKCkgfHwg c291cmNlU2FtcGxlUmF0ZSA8IG1pblNhbXBsZVJhdGUgfHwgc291cmNlU2FtcGxlUmF0ZSA+IG1h eFNhbXBsZVJhdGUpIHsKICAgICAgICAgICAgIC8vIHByb2Nlc3MoKSB3aWxsIGdlbmVyYXRlIHNp bGVuY2UgZm9yIHRoZXNlIHVuaW5pdGlhbGl6ZWQgdmFsdWVzLgogICAgICAgICAgICAgTE9HKE1l ZGlhLCAiTWVkaWFFbGVtZW50QXVkaW9Tb3VyY2VOb2RlOjpzZXRGb3JtYXQoJXUsICVmKSAtIHVu aGFuZGxlZCBmb3JtYXQgY2hhbmdlIiwgc3RhdGljX2Nhc3Q8dW5zaWduZWQ+KG51bWJlck9mQ2hh bm5lbHMpLCBzb3VyY2VTYW1wbGVSYXRlKTsKICAgICAgICAgICAgIG1fc291cmNlTnVtYmVyT2ZD aGFubmVscyA9IDA7Cg==