WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Raymond Toy
Reported
2012-01-11 10:12:15 PST
See
http://code.google.com/p/chromium/issues/detail?id=106147
and
https://bugs.webkit.org/show_bug.cgi?id=75996
.
Attachments
Patch
(36.58 KB, patch)
2012-01-13 09:44 PST
,
Raymond Toy
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Raymond Toy
Comment 1
2012-01-13 09:44:46 PST
Created
attachment 122443
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug