RESOLVED FIXED 76073
noteOn, noteGrainOn and noteOff idl should take doubles
https://bugs.webkit.org/show_bug.cgi?id=76073
Summary noteOn, noteGrainOn and noteOff idl should take doubles
Attachments
Patch (36.58 KB, patch)
2012-01-13 09:44 PST, Raymond Toy
no flags
Raymond Toy
Comment 1 2012-01-13 09:44:46 PST
Chris Rogers
Comment 2 2012-01-13 10:30:34 PST
This looks fine to me. I assume it fixes the off-by-one sample-accurate-scheduling issues you were seeing?
Raymond Toy
Comment 3 2012-01-13 10:37:58 PST
(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
Comment 4 2012-01-13 12:30:00 PST
Adam Barth, can you please provide final review? Looks good, but please check on why "mac" EWS bot is purple.
Chris Rogers
Comment 5 2012-01-13 12:31:34 PST
(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
Comment 6 2012-01-13 12:37:50 PST
(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
Comment 7 2012-01-13 12:39:54 PST
(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
Comment 8 2012-01-13 13:04:44 PST
Sorry, I missed that :) Looks good - you can ping abarth for final review
Adam Barth
Comment 9 2012-01-13 15:17:07 PST
Comment on attachment 122443 [details] Patch The purple Mac EWS is expected for non-committer.
WebKit Review Bot
Comment 10 2012-01-13 16:02:09 PST
Comment on attachment 122443 [details] Patch Clearing flags on attachment: 122443 Committed r105007: <http://trac.webkit.org/changeset/105007>
WebKit Review Bot
Comment 11 2012-01-13 16:02:14 PST
All reviewed patches have been landed. Closing bug.
James Robinson
Comment 12 2012-01-14 00:58:43 PST
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
Comment 13 2012-01-15 18:01:05 PST
(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
Comment 14 2012-01-15 22:11:17 PST
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
Comment 15 2012-01-16 10:08:15 PST
(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
Comment 16 2012-01-30 09:21:09 PST
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
Comment 17 2012-01-31 16:05:39 PST
75996 is not blocked on this anymore.
Raymond Toy
Comment 18 2012-02-13 11:26:25 PST
Closing. This issue fixed when bug 76659 landed.
Note You need to log in before you can comment on or make changes to this bug.