Closed
Bug 888271
Opened 10 years ago
Closed 10 years ago
Don't offset twice by mOffset in AudioBufferSourceNode
Categories
(Core :: Web Audio, defect)
Core
Web Audio
Tracking
()
RESOLVED
FIXED
mozilla25
Tracking | Status | |
---|---|---|
firefox23 | --- | unaffected |
firefox24 | + | fixed |
firefox25 | + | fixed |
People
(Reporter: padenot, Assigned: padenot)
References
Details
(Whiteboard: [blocking-webaudio+][qa-])
Attachments
(2 files, 1 obsolete file)
1.10 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
3.68 KB,
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
We do that when we `buffer.loop == false`.
Attachment #768928 -
Flags: review?(ehsan)
Assignee | ||
Comment 1•10 years ago
|
||
With intended changes only, now. Test incoming.
Attachment #768930 -
Flags: review?(ehsan)
Assignee | ||
Updated•10 years ago
|
Attachment #768928 -
Attachment is obsolete: true
Attachment #768928 -
Flags: review?(ehsan)
Updated•10 years ago
|
Attachment #768930 -
Flags: review?(ehsan) → review+
Updated•10 years ago
|
status-firefox24:
--- → affected
status-firefox25:
--- → affected
tracking-firefox24:
--- → ?
tracking-firefox25:
--- → ?
Comment 2•10 years ago
|
||
Can you please explain the user impact here for us to make sure this is tracking worthy ? Not clear why we'd track, but we'd take a low risk uplift if needed for Aurora.
Assignee | ||
Comment 3•10 years ago
|
||
Well, this is a bug in our implementation. A developer willing to use the API would see weird behavior trying to start and AudioBufferSourceNode, the sound stopping at unexpected time. I've tested this manually, but as I said in comment 1, I'll write a mochitest.
Comment 4•10 years ago
|
||
Tracking, as we do not want to ship webaudio without this bug resolved.
Updated•10 years ago
|
Whiteboard: [blocking-webaudio+]
Comment 6•10 years ago
|
||
Comment on attachment 772551 [details] [diff] [review] Don't substract twice by mOffset in AudioBufferSourceNode. r= Review of attachment 772551 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/webaudio/test/test_audioBufferSourceNodeOffset.html @@ +30,5 @@ > + source.onended = function(e) { > + // The timing at which the audioprocess and ended listeners are called can > + // change, hence the fuzzy equal here. > + var errorRatio = samplesFromSource / (0.5 * context.sampleRate); > + ok(errorRatio > 0.97 && errorRatio < 1.03, "Correct number of samples received."); Please make this log the value of errorRatio as well, since this might fail intermittently I believe, and that would make it easier for us to adjust the value accordingly.
Attachment #772551 -
Flags: review?(ehsan) → review+
Assignee | ||
Comment 7•10 years ago
|
||
Pushed with some logging. https://hg.mozilla.org/integration/mozilla-inbound/rev/e4863e8d1f0a
Comment 8•10 years ago
|
||
Increase the errorRate allowance to avoid intermittent test failures. https://hg.mozilla.org/integration/mozilla-inbound/rev/242e0891cd29
Comment 9•10 years ago
|
||
(In reply to comment #8) > Increase the errorRate allowance to avoid intermittent test failures. > https://hg.mozilla.org/integration/mozilla-inbound/rev/242e0891cd29 Thanks!
Assignee | ||
Comment 10•10 years ago
|
||
I made the test even fuzzier because of failures. Note that it still tests the patch: without this, |samplesFromSource| would be 0. https://hg.mozilla.org/integration/mozilla-inbound/rev/13b49b25ebda
Comment 12•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e4863e8d1f0a https://hg.mozilla.org/mozilla-central/rev/242e0891cd29 https://hg.mozilla.org/mozilla-central/rev/13b49b25ebda
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 13•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/5e7172fc9aa4
Keywords: checkin-needed
Comment 14•10 years ago
|
||
Assuming this does not need QA verification due to in-testsuite coverage. Please remove [qa-] from the whiteboard and add the verifyme keyword if this needs to be manually verified.
Whiteboard: [blocking-webaudio+] → [blocking-webaudio+][qa-]
Updated•9 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•