Intent to Implement and Ship: DynamicsCompressor.reduction is a float

[Raymond Toy]

Not sure this is needed; it's a bug in Chrome's implementation, but the API will change.

Contact emails

rt...@chromium.org, hong...@chromium.org

Spec

http://webaudio.github.io/web-audio-api/, 

http://webaudio.github.io/web-audio-api/#widl-DynamicsCompressorNode-reduction

Summary

The reduction attribute for the DynamicsCompressor node is currently implemented as an AudioParam. However, the WebAudio spec says it should be a readonly float.

Motivation

Aligns Chrome with the spec.

Interoperability and Compatibility Risk

Firefox already implements this as a float. Safari still follows Chrome. I did not check Edge, but suspect it also implements it as a float.

There is a compatibility risk because this change is not backward compatible. Previously, the developer would use node.reduction.value to get the value. Now it will be node.reduction.

Also, it doesn't really make sense for it to be an AudioParam, as explained in https://github.com/WebAudio/web-audio-api/issues/761#issuecomment-196934604

Ongoing technical constraints

None.

Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?

Yes

OWP launch tracking bug

https://crbug.com/595022

Link to entry on the feature dashboard

https://www.chromestatus.com/features/5086921298542592

Requesting approval to ship?

Yes.

[Chris Harrelson]

Hi,

It seems you feel that this is very unlikely to break sites. Is there any data to support that conclusion?

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

[Hongchan Choi]

Unfortunately the usage timeline does not keep track of individual AudioParams, so we don't have supporting data.

The (gain) reduction is the result of dynamics compression, so it is not what you can actually control. Thus I believe the breakage won't change the resulting sound from audio graph. The only risk I can think of is: if the app uses this value to visualize the current gain reduction of dynamics compression, it won't work because compressor.reduction.value does not exist. (Personally I've never seen that any demo or app does that so far.)

[Rick Byers]

The MDN docs say this is an AudioParam and even have sample code which (if I understand correctly) would now throw an exception (and so could badly break under this change):

// Create a compressor node
var compressor = audioCtx.createDynamicsCompressor();
...
compressor.reduction.value = -20;

Presumably the MDN docs matched the FF implementation at one time, right?  Any idea when they changed and what compat testing they did to convince themselves changing was web compatible?  If there's not already good data on the web compat impact, we should at least do an httparchive search to try to find existing usage of this property (or worst case add an UMA counter and wait to collect stats from at least beta channel).

Rick

[Rick Byers]

I did an httparchive search (using Feb 1st desktop data) for ".reduction.value" and found 5 sites (of the top 470k sites) with hits:

http://www.takemetotomorrowland.com/

http://www.seagate.com/

http://www.chrismckenzie.com/

http://www.sonorans-valley.com/

http://www.googulator.com/

Spot-checking the first it looks like it would be broken (unhandled exceptions) by this change:

                            this._processor.onaudioprocess = function() {

                                var t = n._source.reduction.value;

                                n._gain.gain.value = 0 == t ? 1 : Math.pow(10, t / 20)

                            }

Can you investigate these 5 sites and see if / how badly broken they are in practice?  Presumably some are already broken on Firefox, right?  5/470k (0.001%) isn't that bad but it seems clear that the breakage is at least non-zero.

[Raymond Toy]

Thanks for looking up these examples. I'll take a look soon.

For the record, the spec issue is https://github.com/WebAudio/web-audio-api/issues/243, in Sep 2013, and the actual change happened in https://github.com/WebAudio/web-audio-api/pull/404 in Nov 2014.

It appears as if Mozilla made the change in Mar, 2015 (https://hg.mozilla.org/mozilla-central/annotate/0f645ecf15a2/dom/webidl/DynamicsCompressorNode.webidl)

[Rick Byers]

Thanks.  It looks like there wasn't a ton of consideration given to the web compat implications in the Mozilla change:

Let's hope that existing Web content doesn't rely on this.  Do you know if Blink and WebKit are going to update their implementations accordingly?

This spec fix came from the Blink folks, so I'm sure they update their implementation (if not already, I don't have a canary build here). I don't think existing applications use this attribute much, its sole purpose is for monitoring, and I haven't seen a lot of compressor usage, let alone any kind of monitoring of the compressor itself, in the wild. This is just anecdotal, though.

From the httparchive search it seems likely their intuition is correct there (there definitely doesn't appear to be a LOT of usage).  Still we owe developers the due diligence to show we're doing everything reasonable to avoid breaking their code.

Rick

[Raymond Toy]

I'm perfectly happy to change this to an Intent to Deprecate and  then make the change to a readonly float after, say, 6 mo (or more?)

That's around M52 or so.

[Rick Byers]

That's definitely and option, but it's tricky in itself.  Eg. what's the guidance we'd give for avoiding getting the deprecation message?  Just don't use the API at all until we fix it?  Presumably it's not possible to feature-detect this in a way that wouldn't trigger the deprecation message, right?  We really want to avoid sending warnings when developers are following our guidance (dramatically weakens the strength of our warnings in general).  Also if we delay, usage may just go up making the problem worse.

I think the key is understanding how breaking this is in practice and if there's anything we can do to mitigate the break.  Eg. if 3 of the 5 httparchive results are using the same shared open source library on GitHub then we should submit a pull-request to that library.  Or maybe we'll get lucky and none of the sites will have a noticeable behavior change (eg. they sound slightly worse but the unhandled exception doesn't cause any more problems than that).  In which case I'd say it's probably best to just make the change and watch for any major fallout in beta.  Regardless we should make sure this change is included in the beta blog post, and that the chromestatus entry includes a link to a sample which correctly works with both APIs (eg. using typeof to make the usage conditional).

Sound reasonable?

   Rick

[Hongchan Choi]

Rick, this is very interesting database and thanks for your insight on this. 

Can you let us know how to use httparchive and custom query? We could use the data for future intent.

[Hongchan Choi]

I gave a quick look on those 5 sites:

http://www.takemetotomorrowland.com/

http://www.seagate.com/

These two sites are using something called 'Klang.js' and the code from spot-checking came from this library. (http://klangclients.plan8.se/)

Also fetching the reduction value happens in ScriptProcessor's onaudioprocess callback, which is also being deprecated.

http://chrismckenzie.com/

http://www.sonorans-valley.com/

These web sites are using '.reduction.value' for a different class, not web audio. (It uses something called WAGNER, for shader)

http://www.googulator.com/

This web site uses a polyfill for Web Audio API. (https://github.com/g200kg/WAAPISim)

I'll try to contact authors of these websites if necessary.

[Raymond Toy]

On Thu, Mar 17, 2016 at 9:25 AM, Rick Byers <rby...@chromium.org> wrote:

That's definitely and option, but it's tricky in itself.  Eg. what's the guidance we'd give for avoiding getting the deprecation message?  Just don't use the API at all until we fix it?  Presumably it's not possible to feature-detect this in a way that wouldn't trigger the deprecation message, right?  We really want to avoid sending warnings when developers are following our guidance (dramatically weakens the strength of our warnings in general).  Also if we delay, usage may just go up making the problem worse.

I thought maybe we could support both, but I think that's pretty gross. I'm not even sure if we could and it would be very weird. 

I think the key is understanding how breaking this is in practice and if there's anything we can do to mitigate the break.  Eg. if 3 of the 5 httparchive results are using the same shared open source library on GitHub then we should submit a pull-request to that library.  Or maybe we'll get lucky and none of the sites will have a noticeable behavior change (eg. they sound slightly worse but the unhandled exception doesn't cause any more problems than that).  In which case I'd say it's probably best to just make the change and watch for any major fallout in beta.  Regardless we should make sure this change is included in the beta blog post, and that the chromestatus entry includes a link to a sample which correctly works with both APIs (eg. using typeof to make the usage conditional).

Sound reasonable?

Yes, sounds like a plan.  FWIW, I looked through Bugzila for issues.  I couldn't find any related to reduction not working for Firefox.  I'll take that as a good sign that no one is actually using it.

Curiously, Edge says it uses an AudioParam:  https://msdn.microsoft.com/en-us/library/dn954869(v=vs.85).aspx  I'm rather surprised since the change went in to the spec 2 years ago.

[Rick Byers]

On Thu, Mar 17, 2016 at 5:27 PM, Raymond Toy <rt...@google.com> wrote:

On Thu, Mar 17, 2016 at 9:25 AM, Rick Byers <rby...@chromium.org> wrote:

That's definitely and option, but it's tricky in itself.  Eg. what's the guidance we'd give for avoiding getting the deprecation message?  Just don't use the API at all until we fix it?  Presumably it's not possible to feature-detect this in a way that wouldn't trigger the deprecation message, right?  We really want to avoid sending warnings when developers are following our guidance (dramatically weakens the strength of our warnings in general).  Also if we delay, usage may just go up making the problem worse.

I thought maybe we could support both, but I think that's pretty gross. I'm not even sure if we could and it would be very weird. 

I think the key is understanding how breaking this is in practice and if there's anything we can do to mitigate the break.  Eg. if 3 of the 5 httparchive results are using the same shared open source library on GitHub then we should submit a pull-request to that library.  Or maybe we'll get lucky and none of the sites will have a noticeable behavior change (eg. they sound slightly worse but the unhandled exception doesn't cause any more problems than that).  In which case I'd say it's probably best to just make the change and watch for any major fallout in beta.  Regardless we should make sure this change is included in the beta blog post, and that the chromestatus entry includes a link to a sample which correctly works with both APIs (eg. using typeof to make the usage conditional).

Sound reasonable?

Yes, sounds like a plan.  FWIW, I looked through Bugzila for issues.  I couldn't find any related to reduction not working for Firefox.  I'll take that as a good sign that no one is actually using it.

Curiously, Edge says it uses an AudioParam:  https://msdn.microsoft.com/en-us/library/dn954869(v=vs.85).aspx  I'm rather surprised since the change went in to the spec 2 years ago.

Edge often copies Chrome's bugs unfortunately.  This may be a sign that they hit a real-world site that was affected...  Any WebAudio contacts there you could ask?

[Raymond Toy]

On Thu, Mar 17, 2016 at 2:33 PM, Rick Byers <rby...@chromium.org> wrote:

On Thu, Mar 17, 2016 at 5:27 PM, Raymond Toy <rt...@google.com> wrote:

On Thu, Mar 17, 2016 at 9:25 AM, Rick Byers <rby...@chromium.org> wrote:

That's definitely and option, but it's tricky in itself.  Eg. what's the guidance we'd give for avoiding getting the deprecation message?  Just don't use the API at all until we fix it?  Presumably it's not possible to feature-detect this in a way that wouldn't trigger the deprecation message, right?  We really want to avoid sending warnings when developers are following our guidance (dramatically weakens the strength of our warnings in general).  Also if we delay, usage may just go up making the problem worse.

I thought maybe we could support both, but I think that's pretty gross. I'm not even sure if we could and it would be very weird. 

I think the key is understanding how breaking this is in practice and if there's anything we can do to mitigate the break.  Eg. if 3 of the 5 httparchive results are using the same shared open source library on GitHub then we should submit a pull-request to that library.  Or maybe we'll get lucky and none of the sites will have a noticeable behavior change (eg. they sound slightly worse but the unhandled exception doesn't cause any more problems than that).  In which case I'd say it's probably best to just make the change and watch for any major fallout in beta.  Regardless we should make sure this change is included in the beta blog post, and that the chromestatus entry includes a link to a sample which correctly works with both APIs (eg. using typeof to make the usage conditional).

Sound reasonable?

Yes, sounds like a plan.  FWIW, I looked through Bugzila for issues.  I couldn't find any related to reduction not working for Firefox.  I'll take that as a good sign that no one is actually using it.

Curiously, Edge says it uses an AudioParam:  https://msdn.microsoft.com/en-us/library/dn954869(v=vs.85).aspx  I'm rather surprised since the change went in to the spec 2 years ago.

Edge often copies Chrome's bugs unfortunately.  This may be a sign that they hit a real-world site that was affected...  Any WebAudio contacts there you could ask?

Yeah, I can ask someone.  I really need to  get a Win 10 machine so I can just try it out.  Maybe it's like Firefox where the implementation does this but the docs say that. :-)

[Raymond Toy]

I asked Microsoft's WebAudio representative.  They also implement reduction as an AudioParam and not a float.  He also said making it a float makes sense and is something then could do.

[Rick Byers]

On Thu, Mar 17, 2016 at 12:38 PM, Hongchan Choi <hong...@chromium.org> wrote:

Rick, this is very interesting database and thanks for your insight on this. 

Can you let us know how to use httparchive and custom query? We could use the data for future intent.

For sure.  Here's some notes with sample queries.

[Raymond Toy]

On Fri, Mar 18, 2016 at 9:35 AM, Raymond Toy <rt...@google.com> wrote:

I asked Microsoft's WebAudio representative.  They also implement reduction as an AudioParam and not a float.  He also said making it a float makes sense and is something then could do.

He also said they'd probably pick up this change for the next Edge update coming sometime soon.

(No, I don't have a firm date on this.) 

[Raymond Toy]

What's the conclusion here?  Firefox has already made the change (at the last F2F in either Sapporo or Boston), Edge will probably make the change soon.

AFAICT, there have been no bug reports for Firefox or Chrome about the reduction attribute not working.

While it is a breaking change, it seems relatively safe.

[Raymond Toy]

A friendly ping.  Should we or should we not change the reduction parameter from an AudioParam to a plain float.

(I still need to add a code snippet/polyfill to support using both the AudioParam and float value.)

[Rick Byers]

So sorry for the delay!  

So the conclusion from the analysis of the httparchive results was that there were really only 2 sites (of top 500k) that would be broken by this change (both due to Klang.js).  Has anyone reached out to the maintainers of klang.js (presumably in...@plan8.se) to let them know about this change?

I agree with the conclusion that, while a breaking change, all indications are that the total breakage will be very small, easy to understand/correct, and the alternative (adding a new API under a new name, trying to deprecate the old one, etc.) is probably not worth the cost (platform complexity, developer confusion, etc.).  So LGTM1 to ship assuming someone at least reaches out to klang.js maintainers.

[Raymond Toy]

On Tue, Apr 19, 2016 at 10:51 AM, Rick Byers <rby...@chromium.org> wrote:

So sorry for the delay!  

So the conclusion from the analysis of the httparchive results was that there were really only 2 sites (of top 500k) that would be broken by this change (both due to Klang.js).  Has anyone reached out to the maintainers of klang.js (presumably in...@plan8.se) to let them know about this change?

I agree with the conclusion that, while a breaking change, all indications are that the total breakage will be very small, easy to understand/correct, and the alternative (adding a new API under a new name, trying to deprecate the old one, etc.) is probably not worth the cost (platform complexity, developer confusion, etc.).  So LGTM1 to ship assuming someone at least reaches out to klang.js maintainers.

hongchan@ sent an email to the klang.js maintainers, and I've updated the chrome features entry to include a link to a polyfill for this.

[Rick Byers]

On Tue, Apr 19, 2016 at 4:33 PM, Raymond Toy <rt...@google.com> wrote:

On Tue, Apr 19, 2016 at 10:51 AM, Rick Byers <rby...@chromium.org> wrote:

So sorry for the delay!  

So the conclusion from the analysis of the httparchive results was that there were really only 2 sites (of top 500k) that would be broken by this change (both due to Klang.js).  Has anyone reached out to the maintainers of klang.js (presumably in...@plan8.se) to let them know about this change?

I agree with the conclusion that, while a breaking change, all indications are that the total breakage will be very small, easy to understand/correct, and the alternative (adding a new API under a new name, trying to deprecate the old one, etc.) is probably not worth the cost (platform complexity, developer confusion, etc.).  So LGTM1 to ship assuming someone at least reaches out to klang.js maintainers.

hongchan@ sent an email to the klang.js maintainers, and I've updated the chrome features entry to include a link to a polyfill for this.

Looks great, thank you for the extra developer outreach here!  

Chris, WDYT - does this discussion address your question/concern about the compat risk?

[Philip Jägenstedt]

On Tue, Apr 19, 2016 at 10:51 PM, Rick Byers <rby...@chromium.org> wrote:

On Tue, Apr 19, 2016 at 4:33 PM, Raymond Toy <rt...@google.com> wrote:

On Tue, Apr 19, 2016 at 10:51 AM, Rick Byers <rby...@chromium.org> wrote:

So sorry for the delay!  

So the conclusion from the analysis of the httparchive results was that there were really only 2 sites (of top 500k) that would be broken by this change (both due to Klang.js).  Has anyone reached out to the maintainers of klang.js (presumably in...@plan8.se) to let them know about this change?

I agree with the conclusion that, while a breaking change, all indications are that the total breakage will be very small, easy to understand/correct, and the alternative (adding a new API under a new name, trying to deprecate the old one, etc.) is probably not worth the cost (platform complexity, developer confusion, etc.).  So LGTM1 to ship assuming someone at least reaches out to klang.js maintainers.

hongchan@ sent an email to the klang.js maintainers, and I've updated the chrome features entry to include a link to a polyfill for this.

Looks great, thank you for the extra developer outreach here!  

Chris, WDYT - does this discussion address your question/concern about the compat risk?

For my own part, LGTM2, hopefully the klang.js maintainers will have enough time to adapt to the change.

Philip

[Dimitri Glazkov]

LGTM3

:DG<

[Hongchan Choi]

Just to keep you updated: I've got the response from the author of Klang.js (t...@plan8.se) and he said he would fix the issue as soon as possible.