Closed
Bug 964376
Opened 10 years ago
Closed 10 years ago
AudioBufferSourceNode.playbackRate does not sound great anymore
Categories
(Core :: Web Audio, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla29
Tracking | Status | |
---|---|---|
firefox28 | --- | unaffected |
firefox29 | - | affected |
People
(Reporter: padenot, Assigned: karlt)
References
Details
(Keywords: regression)
Attachments
(3 files, 2 obsolete files)
1.36 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
1.88 KB,
patch
|
padenot
:
review+
|
Details | Diff | Splinter Review |
1.71 KB,
text/html
|
Details |
STR: - Open Firefox release - Run http://padenot.github.io/loopa/ - Click play - Change the playbackRate value - Open Firefox Nightly - Run http://padenot.github.io/loopa/ - Click play - Change the playbackRate value Expected: The sound quality is more or less equivalent Actual result: It sounds horrible in nightly
Assignee | ||
Updated•10 years ago
|
Keywords: regression,
regressionwindow-wanted
Comment 1•10 years ago
|
||
Regression window(m-c) Good: http://hg.mozilla.org/mozilla-central/rev/c9b60fc06e4c Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140107082035 Bad: http://hg.mozilla.org/mozilla-central/rev/8cdeb1bcdb02 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140107082333 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c9b60fc06e4c&tochange=8cdeb1bcdb02 Regression window(m-i) Good: http://hg.mozilla.org/integration/mozilla-inbound/rev/029a56c4ad4c Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140106192108 Bad: http://hg.mozilla.org/integration/mozilla-inbound/rev/90d56ff72f1a Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140106193422 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=029a56c4ad4c&tochange=90d56ff72f1a Suspected: Bug 943461,Bug 947796
status-firefox28:
--- → unaffected
status-firefox29:
--- → affected
tracking-firefox29:
--- → ?
Keywords: regressionwindow-wanted
OS: Linux → All
Version: unspecified → 29 Branch
Assignee | ||
Updated•10 years ago
|
Priority: -- → P1
Assignee | ||
Comment 2•10 years ago
|
||
Regressed in http://hg.mozilla.org/integration/mozilla-inbound/rev/0279107b81d3
No longer blocks: 947796
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → karlt
Assignee | ||
Comment 3•10 years ago
|
||
Some code was depending on this for correct operation, but then failed in non-debug builds. Instead, distort only mDuration to check that ProduceAudioBlock sets chunks. The following crashtests then detect this bug: 874915.html 874934.html 880342-1.html offline-buffer-source-ended-1.html
Attachment #8366988 -
Flags: review?(roc)
Assignee | ||
Comment 4•10 years ago
|
||
Attachment #8366992 -
Flags: review?(paul)
Attachment #8366988 -
Flags: review?(roc) → review+
Assignee | ||
Comment 5•10 years ago
|
||
(Previous was wrong patch.)
Attachment #8366992 -
Attachment is obsolete: true
Attachment #8366992 -
Flags: review?(paul)
Attachment #8366993 -
Flags: review?(paul)
Reporter | ||
Updated•10 years ago
|
Attachment #8366993 -
Flags: review?(paul) → review+
Assignee | ||
Comment 6•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3660c2bcacbb https://hg.mozilla.org/integration/mozilla-inbound/rev/d588d0299cb7 This particular bug would now show up as an assertion failure, but it's existence demonstrates that we don't have a test to check output from a looping buffer source after the playback rate changes from unit to non-unit.
Flags: in-testsuite?
Reporter | ||
Comment 7•10 years ago
|
||
Karl, I came to the same conclusion and wrote somewhat of a test for that. I'll clean it up, double check it, and upload it here.
Reporter | ||
Comment 8•10 years ago
|
||
Did you have something else in mind? This basically gives us the fundamental of the 440 sine wave, when slowed down by a factor of 50%. It works here, but because of audio glitches, we would need to apply some more filtering.
Attachment #8367275 -
Flags: feedback?(karlt)
Comment 9•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3660c2bcacbb https://hg.mozilla.org/mozilla-central/rev/d588d0299cb7
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Updated•10 years ago
|
Comment 10•10 years ago
|
||
I'm convinced that this if unfixed would have impacted a small segment of our users since it was fixed I don't think we need to track this at this time.
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8367275 [details]
test_audioBufferSourceNodePlaybackRate.html
I think this particular bug only happened when playback had started with playbackRate = 1.
I think we can test precise output in the time domain, if we bear in mind that the playbackRate change happens on a 128 sample boundary and ensure that the buffer loops after the change in playbackRate.
Attachment #8367275 -
Flags: feedback?(karlt)
Assignee | ||
Comment 12•10 years ago
|
||
I've asked rfw to look at making the changes in comment 11 and turning the test into a mochitest.
QA Contact: tony
Comment 13•10 years ago
|
||
I've unapplied the patch to see the behavior in the latest Nightly, but changing the playback rate results in an assertion failure, crashing Firefox: ###!!! ASSERTION: Invalid WebAudio chunk size: 'mLastChunks[i].GetDuration() == WEBAUDIO_BLOCK_SIZE', file /Users/Tony/mozilla-central/content/media/AudioNodeStream.cpp, line 445
Comment 14•10 years ago
|
||
Apologies, here's the one that actually crashed Firefox: Assertion failure: aDuration >= 0, at /Users/Tony/mozilla-central/media/webrtc/signaling/../../../content/media/MediaSegment.h:289
Comment 15•10 years ago
|
||
My bad, I didn't unapply both patches.
Comment 16•10 years ago
|
||
This mochitest test case for whether or not the sound is distorted by altering the playback rate after a given amount of samples (in this case 128). It creates the expected waveform and compares the two. With the patch unapplied, there is a massive difference between the two waveforms.
Attachment #8367275 -
Attachment is obsolete: true
Assignee | ||
Comment 17•10 years ago
|
||
Thanks, Tony. https://hg.mozilla.org/integration/mozilla-inbound/rev/a310ac56a47b
Flags: in-testsuite? → in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•