Is intent to implement and ship needed for all user-visible changes?

[Raymond Toy]

It was recently discovered that WebAudio's lowpass and highpass filter cannot represent all lowpass and highpass biquad filters.  After some discussion, we decided to fix this by changing the formulas for the filters.  This change is not backward compatible; there will be differences in the output.  However, we did try to minimize the differences.  (You can play around with a simulation of the new filter design at http://rtoy.github.io/webaudio-hacks/more/biquad/biquad-lowpass-q.html?usedB=true)

Someone did propose that we support both the new and old filter equations, which would keep backward compatibility, but that was deemed to be too confusing.

Since this is a user-visible change, is an intent to implement and ship needed?  If not, would a chrome feature entry be appropriate at least to inform developers that a change is being made?

--

Ray

[Jeremy Roman]

"Trivial" changes (where "trivial" is left somewhat to the discretion of reviews in the area) don't need to go through the launch process.

http://www.chromium.org/blink#trivial-changes

For example, we don't use an Intent to Implement and Ship when Skia slightly changes how some effect is rasterized. Nor do we usually go through the process when we're fixing non-compliance with the spec, or fixing an obvious bug.

API owners can weigh in on how trivial they think this is, if you're unsure.

[Philip Jägenstedt]

That is a fantastic demo and visualization, Ray! The legend says that yellow and blue should be Orig Mag and Phase, but those don't show up in the graph. What I was trying to see is how big the difference would be. Do you know what the very worst case in terms of audible change to existing usage of BiquadFilterNode?

As for what merits going through the intents process and not, I think that if this is sufficiently unlikely to be noticed by any user or require any change to code in the wild, then it's in the "trivial" bucket. You're in the best position to make that judgment, though.

Needless to say, it's important that there's some agreement between implementors in https://github.com/WebAudio/web-audio-api/issues/771 before going ahead so that we don't end up with both the old and the new models in various browsers for a long time. If the difference is ever large enough so that a developer would want to code against both models, then some means for feature detection would be needed too.

--
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.

[Rick Byers]

Agreed.  Obviously a lot of bug fixes can be "user visible" and occasionally even "require change to code in the wild" to some extent, but it would be crazy to add the intent process to most bug fixes.  So it's a little grey.

Another good rule of thumb is "would we want to update the MDN docs to reflect this change" (or otherwise communicate it to developers proactively).  If so it's probably worth having the intent discussion (or at least starting a thread on blink-dev like this one to discuss the specifics).  

But ultimately, yes, we trust the judgement of the engineers and OWNERS for an area to evaluate the potential for compat/interop risk and decide on a case-by-case basis if something should be considered a "trivial" web exposed change.  Also since there's a judgement call here, if we're not occasionally making mistakes on both sides of this line (realizing in retrospect we probably should have applied more diligence, or that we wasted peoples time by applying too much) then we're probably being too conservative or aggressive.

[Alex Komoroske]

One thing to note is that the intents and the entries on chromestatus.com are how our developer relations team keeps track of things to document and publicize for developers. If you think a given change could be important enough to need one of: 1) getting noted as at least a bullet in the beta blog posts (example), 2) require some kind of how-to-use documentation (example), or 3) require MDN docs to be created or updated, then it's best to err on the side of sending an intent or filing a chromestatus.com entry.

[Raymond Toy]

On Wed, Apr 27, 2016 at 7:41 AM, Philip Jägenstedt <phi...@opera.com> wrote:

That is a fantastic demo and visualization, Ray! The legend says that yellow and blue should be Orig Mag and Phase, but those don't show up in the graph. What I was trying to see is how big the difference would be. Do you know what the very worst case in terms of audible change to existing usage of BiquadFilterNode?

Oh.  Try setting Q to a small value like 0.  You'll see four curves.  For higher values of Q, the magnitude and phase curves for the two filters overlap.  For these small values, the differences between the filters are easily audible, even for my poor ears.

I don't have a good feel for what the change will be existing usage.  I'm guessing that most usages of lowpass and highpass filters either just use the default or Q was "randomly" changed until it sounded right.

 

As for what merits going through the intents process and not, I think that if this is sufficiently unlikely to be noticed by any user or require any change to code in the wild, then it's in the "trivial" bucket. You're in the best position to make that judgment, though.

Yeah, Each day I change my mind on whether this is just an obvious fix or something needs more exposure.  That's why I asked. 

Needless to say, it's important that there's some agreement between implementors in https://github.com/WebAudio/web-audio-api/issues/771 before going ahead so that we don't end up with both the old and the new models in various browsers for a long time. If the difference is ever large enough so that a developer would want to code against both models, then some means for feature detection would be needed too.

This was discussed at the F2F a few weeks ago and there was general agreeement among Mozilla, Microsoft and us that it needs to be fixed and this was the right fix.  I assume Mozilla will fix this soon. Presumably Microsoft will too in time for their next update.

Feature detection is a pain.  I think the only way is to create an offline context filter a known signal and compare the output with the expected result.  Or maybe set Q to two different negative values and compare the filtered result.  The old filter internally clipped Q to 0.  This doesn't happen in the new filter.

[Raymond Toy]

On Wed, Apr 27, 2016 at 7:50 AM, Rick Byers <rby...@chromium.org> wrote:

Agreed.  Obviously a lot of bug fixes can be "user visible" and occasionally even "require change to code in the wild" to some extent, but it would be crazy to add the intent process to most bug fixes.  So it's a little grey.

Yeah, for obvious bug fixes even if they change behavior, I just normally make the change.  I couldn't decide what to do about this particular issue.

 

Another good rule of thumb is "would we want to update the MDN docs to reflect this change" (or otherwise communicate it to developers proactively).  If so it's probably worth having the intent discussion (or at least starting a thread on blink-dev like this one to discuss the specifics).

The spec has changed (and clarified some issues), so the MDN docs do need to be updated. 

[Jeremy Roman]

On Wed, Apr 27, 2016 at 11:38 AM, Raymond Toy <rt...@google.com> wrote:

On Wed, Apr 27, 2016 at 7:41 AM, Philip Jägenstedt <phi...@opera.com> wrote:

That is a fantastic demo and visualization, Ray! The legend says that yellow and blue should be Orig Mag and Phase, but those don't show up in the graph. What I was trying to see is how big the difference would be. Do you know what the very worst case in terms of audible change to existing usage of BiquadFilterNode?

Oh.  Try setting Q to a small value like 0.  You'll see four curves.  For higher values of Q, the magnitude and phase curves for the two filters overlap.  For these small values, the differences between the filters are easily audible, even for my poor ears.

I don't have a good feel for what the change will be existing usage.  I'm guessing that most usages of lowpass and highpass filters either just use the default or Q was "randomly" changed until it sounded right.

If you are worried (I have no idea whether you ought to be), you could add a histogram for the values of Q that are used in practice, and (assuming the change to do so isn't too intrusive) request a cherry-pick to beta.

[Philip Jägenstedt]

On Wed, Apr 27, 2016 at 5:38 PM, Raymond Toy <rt...@google.com> wrote:

On Wed, Apr 27, 2016 at 7:41 AM, Philip Jägenstedt <phi...@opera.com> wrote:

That is a fantastic demo and visualization, Ray! The legend says that yellow and blue should be Orig Mag and Phase, but those don't show up in the graph. What I was trying to see is how big the difference would be. Do you know what the very worst case in terms of audible change to existing usage of BiquadFilterNode?

Oh.  Try setting Q to a small value like 0.  You'll see four curves.  For higher values of Q, the magnitude and phase curves for the two filters overlap.  For these small values, the differences between the filters are easily audible, even for my poor ears.

I don't have a good feel for what the change will be existing usage.  I'm guessing that most usages of lowpass and highpass filters either just use the default or Q was "randomly" changed until it sounded right.

Ah, well that explains it, for Q>4 or so, the curves are so close that one can't see the yellow and blue curves below. With tinnitus and bad headphones, I can tell the difference at Q=3 for lowpass and highpass, but at Q=4 the difference is so small that I might not notice in a blind test. For anything other than noise and pure tones, I think it'll be much harder still to tell the difference.

I suppose there are two ways to learn more. One is to do an httparchive search for "BiquadFilterNode" and just see what the Q value is on the sites you find. The other is a histogram as Jeremy suggested. If Q<3 is only a small proportion of the usage, then just making the change seems reasonable.

Philip 

[Raymond Toy]

On Wed, Apr 27, 2016 at 8:46 AM, Jeremy Roman <jbr...@chromium.org> wrote:

On Wed, Apr 27, 2016 at 11:38 AM, Raymond Toy <rt...@google.com> wrote:

On Wed, Apr 27, 2016 at 7:41 AM, Philip Jägenstedt <phi...@opera.com> wrote:

That is a fantastic demo and visualization, Ray! The legend says that yellow and blue should be Orig Mag and Phase, but those don't show up in the graph. What I was trying to see is how big the difference would be. Do you know what the very worst case in terms of audible change to existing usage of BiquadFilterNode?

Oh.  Try setting Q to a small value like 0.  You'll see four curves.  For higher values of Q, the magnitude and phase curves for the two filters overlap.  For these small values, the differences between the filters are easily audible, even for my poor ears.

I don't have a good feel for what the change will be existing usage.  I'm guessing that most usages of lowpass and highpass filters either just use the default or Q was "randomly" changed until it sounded right.

If you are worried (I have no idea whether you ought to be), you could add a histogram for the values of Q that are used in practice, and (assuming the change to do so isn't too intrusive) request a cherry-pick to beta.

An interesting idea! Do you have a pointer to how to do this (or a simple CL)?  Delaying the change makes the pain greater, but we should measure this.

[Raymond Toy]

The default value is 1  (because that was the old default), and I suspect most users either don't know or don't care what Q is and are more concerned with the cutoff frequency.  So, this will certainly change the output, but it's really hard to tell if it matters without listening to every single example. :-(

Hmm. We could change the default Q of the new filter to something that matches the response of the old filter.  That might make the change even less noticeable.

 

Philip 

[Raymond Toy]

For the record:  https://www.chromestatus.com/features/5687523284090880 is the feature entry.

I'ved added sample links to code to detect which implementation you have and a small conversion function to convert a Q value from the old filter to an equivalent Q for the new filter.

[Jeremy Roman]

On Thu, Apr 28, 2016 at 12:33 PM, Raymond Toy <rt...@google.com> wrote:

On Wed, Apr 27, 2016 at 8:46 AM, Jeremy Roman <jbr...@chromium.org> wrote:

On Wed, Apr 27, 2016 at 11:38 AM, Raymond Toy <rt...@google.com> wrote:

On Wed, Apr 27, 2016 at 7:41 AM, Philip Jägenstedt <phi...@opera.com> wrote:

That is a fantastic demo and visualization, Ray! The legend says that yellow and blue should be Orig Mag and Phase, but those don't show up in the graph. What I was trying to see is how big the difference would be. Do you know what the very worst case in terms of audible change to existing usage of BiquadFilterNode?

Oh.  Try setting Q to a small value like 0.  You'll see four curves.  For higher values of Q, the magnitude and phase curves for the two filters overlap.  For these small values, the differences between the filters are easily audible, even for my poor ears.

I don't have a good feel for what the change will be existing usage.  I'm guessing that most usages of lowpass and highpass filters either just use the default or Q was "randomly" changed until it sounded right.

If you are worried (I have no idea whether you ought to be), you could add a histogram for the values of Q that are used in practice, and (assuming the change to do so isn't too intrusive) request a cherry-pick to beta.

An interesting idea! Do you have a pointer to how to do this (or a simple CL)?  Delaying the change makes the pain greater, but we should measure this.

(I'm sure there's a better guide to this somewhere, but for the life of me I can't find it right now.)

I don't know the audio code well enough to know where the right place to record a sample is, but have a look at base/metrics/histogram.h (but if the code in question is in Blink, use Source/platform/Histogram.h instead). Roughly, you wind up putting in code like this (giving a Blink example here):

// sample is an int32_t (if your sample is a float, you may have to scale it so that the interesting range of values fits)

DEFINE_STATIC_LOCAL(CustomCountHistogram, qHistogram, ("WebAudio.HighPassFilter.Q", minValue, maxValue, numberOfBuckets));

qHistogram.count(sample);

You then add a corresponding entry to tools/metrics/histograms/histograms.xml, and send those changes together as one CL.

[Rick Byers]

On Thu, Apr 28, 2016 at 4:00 PM, Jeremy Roman <jbr...@chromium.org> wrote:

On Thu, Apr 28, 2016 at 12:33 PM, Raymond Toy <rt...@google.com> wrote:

On Wed, Apr 27, 2016 at 8:46 AM, Jeremy Roman <jbr...@chromium.org> wrote:

On Wed, Apr 27, 2016 at 11:38 AM, Raymond Toy <rt...@google.com> wrote:

On Wed, Apr 27, 2016 at 7:41 AM, Philip Jägenstedt <phi...@opera.com> wrote:

That is a fantastic demo and visualization, Ray! The legend says that yellow and blue should be Orig Mag and Phase, but those don't show up in the graph. What I was trying to see is how big the difference would be. Do you know what the very worst case in terms of audible change to existing usage of BiquadFilterNode?

Oh.  Try setting Q to a small value like 0.  You'll see four curves.  For higher values of Q, the magnitude and phase curves for the two filters overlap.  For these small values, the differences between the filters are easily audible, even for my poor ears.

I don't have a good feel for what the change will be existing usage.  I'm guessing that most usages of lowpass and highpass filters either just use the default or Q was "randomly" changed until it sounded right.

If you are worried (I have no idea whether you ought to be), you could add a histogram for the values of Q that are used in practice, and (assuming the change to do so isn't too intrusive) request a cherry-pick to beta.

An interesting idea! Do you have a pointer to how to do this (or a simple CL)?  Delaying the change makes the pain greater, but we should measure this.

(I'm sure there's a better guide to this somewhere, but for the life of me I can't find it right now.)

I don't know the audio code well enough to know where the right place to record a sample is, but have a look at base/metrics/histogram.h (but if the code in question is in Blink, use Source/platform/Histogram.h instead). Roughly, you wind up putting in code like this (giving a Blink example here):

// sample is an int32_t (if your sample is a float, you may have to scale it so that the interesting range of values fits)

DEFINE_STATIC_LOCAL(CustomCountHistogram, qHistogram, ("WebAudio.HighPassFilter.Q", minValue, maxValue, numberOfBuckets));

qHistogram.count(sample);

You then add a corresponding entry to tools/metrics/histograms/histograms.xml, and send those changes together as one CL.

Is there already a UseCounter for this API?  If not you should add one too - that way we can tell both how common the API usage is, and of that what fraction would be impacted.

[Raymond Toy]

On Thu, Apr 28, 2016 at 1:04 PM, Rick Byers <rby...@chromium.org> wrote:

On Thu, Apr 28, 2016 at 4:00 PM, Jeremy Roman <jbr...@chromium.org> wrote:

On Thu, Apr 28, 2016 at 12:33 PM, Raymond Toy <rt...@google.com> wrote:

On Wed, Apr 27, 2016 at 8:46 AM, Jeremy Roman <jbr...@chromium.org> wrote:

On Wed, Apr 27, 2016 at 11:38 AM, Raymond Toy <rt...@google.com> wrote:

On Wed, Apr 27, 2016 at 7:41 AM, Philip Jägenstedt <phi...@opera.com> wrote:

That is a fantastic demo and visualization, Ray! The legend says that yellow and blue should be Orig Mag and Phase, but those don't show up in the graph. What I was trying to see is how big the difference would be. Do you know what the very worst case in terms of audible change to existing usage of BiquadFilterNode?

Oh.  Try setting Q to a small value like 0.  You'll see four curves.  For higher values of Q, the magnitude and phase curves for the two filters overlap.  For these small values, the differences between the filters are easily audible, even for my poor ears.

I don't have a good feel for what the change will be existing usage.  I'm guessing that most usages of lowpass and highpass filters either just use the default or Q was "randomly" changed until it sounded right.

If you are worried (I have no idea whether you ought to be), you could add a histogram for the values of Q that are used in practice, and (assuming the change to do so isn't too intrusive) request a cherry-pick to beta.

An interesting idea! Do you have a pointer to how to do this (or a simple CL)?  Delaying the change makes the pain greater, but we should measure this.

(I'm sure there's a better guide to this somewhere, but for the life of me I can't find it right now.)

I don't know the audio code well enough to know where the right place to record a sample is, but have a look at base/metrics/histogram.h (but if the code in question is in Blink, use Source/platform/Histogram.h instead). Roughly, you wind up putting in code like this (giving a Blink example here):

// sample is an int32_t (if your sample is a float, you may have to scale it so that the interesting range of values fits)

DEFINE_STATIC_LOCAL(CustomCountHistogram, qHistogram, ("WebAudio.HighPassFilter.Q", minValue, maxValue, numberOfBuckets));

qHistogram.count(sample);

You then add a corresponding entry to tools/metrics/histograms/histograms.xml, and send those changes together as one CL.

Is there already a UseCounter for this API?  If not you should add one too - that way we can tell both how common the API usage is, and of that what fraction would be impacted.

We have a UseCounter for createBiquadFilter, but not for any of the attributes (is that even possible?).  I don't know what happened between Mar 2015 and Jul 2015 with the sudden increase in usage and the sudden fall back to a more "normal" level.

[Raymond Toy]

On Thu, Apr 28, 2016 at 1:00 PM, Jeremy Roman <jbr...@chromium.org> wrote:

On Thu, Apr 28, 2016 at 12:33 PM, Raymond Toy <rt...@google.com> wrote:

On Wed, Apr 27, 2016 at 8:46 AM, Jeremy Roman <jbr...@chromium.org> wrote:

On Wed, Apr 27, 2016 at 11:38 AM, Raymond Toy <rt...@google.com> wrote:

On Wed, Apr 27, 2016 at 7:41 AM, Philip Jägenstedt <phi...@opera.com> wrote:

That is a fantastic demo and visualization, Ray! The legend says that yellow and blue should be Orig Mag and Phase, but those don't show up in the graph. What I was trying to see is how big the difference would be. Do you know what the very worst case in terms of audible change to existing usage of BiquadFilterNode?

Oh.  Try setting Q to a small value like 0.  You'll see four curves.  For higher values of Q, the magnitude and phase curves for the two filters overlap.  For these small values, the differences between the filters are easily audible, even for my poor ears.

I don't have a good feel for what the change will be existing usage.  I'm guessing that most usages of lowpass and highpass filters either just use the default or Q was "randomly" changed until it sounded right.

If you are worried (I have no idea whether you ought to be), you could add a histogram for the values of Q that are used in practice, and (assuming the change to do so isn't too intrusive) request a cherry-pick to beta.

An interesting idea! Do you have a pointer to how to do this (or a simple CL)?  Delaying the change makes the pain greater, but we should measure this.

(I'm sure there's a better guide to this somewhere, but for the life of me I can't find it right now.)

I don't know the audio code well enough to know where the right place to record a sample is, but have a look at base/metrics/histogram.h (but if the code in question is in Blink, use Source/platform/Histogram.h instead). Roughly, you wind up putting in code like this (giving a Blink example here):

// sample is an int32_t (if your sample is a float, you may have to scale it so that the interesting range of values fits)

DEFINE_STATIC_LOCAL(CustomCountHistogram, qHistogram, ("WebAudio.HighPassFilter.Q", minValue, maxValue, numberOfBuckets));

qHistogram.count(sample);

You then add a corresponding entry to tools/metrics/histograms/histograms.xml, and send those changes together as one CL.

Thanks!  The value is a float, but we'll come up with an appropriate scaling.  And we just decided we should also measure the order used for the IIRFIlterNode, so I'll probably start there since it's simpler.

[Raymond Toy]

For the record, UMA stats for the Q value has landed.  (No results yet).  We won't try to get this into the beta channel since M52 branch is coming soon. I'll report back after we get some more data.

[Raymond Toy]

Results are in and as somewhat expected Q=1 is the most common (83%) Q value for lowpass filters. The next most common value is Q=0 (6%). (Resolution of Q values is 0.25 dB).

Based on the demo at https://rtoy.github.io/webaudio-hacks/more/biquad/biquad-lowpass-q.html?usedB=true, if you plug in 1 in the Q new field, there is still a small but noticeable difference between the old and new filters.

For highpass filters, Q=1 is most common, but only 53%.  The second most common is Q=3 (20%).  The audible difference in this case is much less noticeable.  For Q=3, the difference is even less apparent (as the frequency response graph shows).

So unless the Q distributions drastically change, I would like to land the fix/update after M52.

[Raymond Toy]

FWIW, I'm going to land this change in today or tomorrow.  FF is going to land these changes very soon (or already) too.