Bug 76073 – noteOn, noteGrainOn and noteOff idl should take doubles

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Audio

[Raymond Toy]

See http://code.google.com/p/chromium/issues/detail?id=106147 and https://bugs.webkit.org/show_bug.cgi?id=75996.

[Raymond Toy]

Created attachment 122443 [details]
Patch

[Chris Rogers]

This looks fine to me.  I assume it fixes the off-by-one sample-accurate-scheduling issues you were seeing?

[Raymond Toy]

(In reply to comment #2)
> This looks fine to me.  I assume it fixes the off-by-one sample-accurate-scheduling issues you were seeing?

Yes, it does, (once the typo is fixed).  I also verified a couple of values in gain.wav that the signal now starts at the expected place instead of being off by one.   The audiobuffersource-playbackrate tests also tests noteOff (which I did verify for one case).  noteGrainOn is untested.

[Chris Rogers]

Adam Barth, can you please provide final review?

Looks good, but please check on why "mac" EWS bot is purple.

[Chris Rogers]

(In reply to comment #3)
> (In reply to comment #2)
> > This looks fine to me.  I assume it fixes the off-by-one sample-accurate-scheduling issues you were seeing?
> 
> Yes, it does, (once the typo is fixed).  I also verified a couple of values in gain.wav that the signal now starts at the expected place instead of being off by one. 

Does that mean you have to re-baseline "gain.wav"?  If so, then you should include that as part of this patch.

[Raymond Toy]

(In reply to comment #4)
> Adam Barth, can you please provide final review?
> 
> Looks good, but please check on why "mac" EWS bot is purple.

Because non-committers can't submit patches.

[Raymond Toy]

(In reply to comment #5)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > This looks fine to me.  I assume it fixes the off-by-one sample-accurate-scheduling issues you were seeing?
> > 
> > Yes, it does, (once the typo is fixed).  I also verified a couple of values in gain.wav that the signal now starts at the expected place instead of being off by one. 
> 
> Does that mean you have to re-baseline "gain.wav"?  If so, then you should include that as part of this patch.

The formatted diff shows a patch against gain-expected.wav.  Is that what you mean?  audiobuffersource-playbackrate-expected.wav was also updated.

[Chris Rogers]

Sorry, I missed that :)

Looks good - you can ping abarth for final review

[Adam Barth]

Comment on attachment 122443 [details]
Patch

The purple Mac EWS is expected for non-committer.

[WebKit Review Bot]

Comment on attachment 122443 [details]
Patch

Clearing flags on attachment: 122443

Committed r105007: <http://trac.webkit.org/changeset/105007>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[James Robinson]

webaudio/gain.html is failing on the chromium win / linux bots since this patch landed. I don't know how to "view" a diff or how to rebaseline it.

[Raymond Toy]

(In reply to comment #12)
> webaudio/gain.html is failing on the chromium win / linux bots since this patch landed. I don't know how to "view" a diff or how to rebaseline it.

Sorry for the trouble.  I cannot reproduce this on my linux or mac box.  If you can get the actual gain-actual.wav and send it to me, that would be very helpful.

[James Robinson]

It's failing on every windows and linux bot. The bots upload a .zip containing all the actual files, here's one with a failing gain.html:
http://build.chromium.org/f/chromium/layout_test_results/Webkit_Win__dbg__2_/117740/layout-test-results.zip

I don't know what to make of gain-actual.wav.

[Raymond Toy]

(In reply to comment #14)
> It's failing on every windows and linux bot. The bots upload a .zip containing all the actual files, here's one with a failing gain.html:
> http://build.chromium.org/f/chromium/layout_test_results/Webkit_Win__dbg__2_/117740/layout-test-results.zip
> 
> I don't know what to make of gain-actual.wav.

Thanks for the link.  One section of the actual wav file is one sample off compared to the expected wav file.  I will have to test this on a windows box later, but I think I know reason.  The reason is that windows still uses x87 instructions instead of sse2, so arithmetic is done with 80-bit precision.  (See http://code.google.com/p/chromium/issues/detail?id=106147#c6).  This messes up some comparisons because some have been rounded to double float precision, but some are still in 80-bit precision.

It will take a little bit to fix, but I won't be able to until I get to my windows box.

Thanks.

[Raymond Toy]

The test failures on windows should be fixed with bug 76659 (https://bugs.webkit.org/show_bug.cgi?id=76659), which has landed.

[Raymond Toy]

75996 is not blocked on this anymore.

[Raymond Toy]

Closing.  This issue fixed when bug 76659 landed.