Closed Bug 964376 Opened 7 years ago Closed 7 years ago

AudioBufferSourceNode.playbackRate does not sound great anymore


(Core :: Web Audio, defect, P1)

29 Branch



Tracking Status
firefox28 --- unaffected
firefox29 - affected


(Reporter: padenot, Assigned: karlt)



(Keywords: regression)


(3 files, 2 obsolete files)

- Open Firefox release
- Run
- Click play
- Change the playbackRate value
- Open Firefox Nightly
- Run
- Click play
- Change the playbackRate value

The sound quality is more or less equivalent

Actual result:
It sounds horrible in nightly
Regression window(m-c)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140107082035
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140107082333

Regression window(m-i)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140106192108
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140106193422

Suspected: Bug 943461,Bug 947796
Blocks: 943461, 947796
OS: Linux → All
Version: unspecified → 29 Branch
Priority: -- → P1
Assignee: nobody → karlt
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:
Attachment #8366988 - Flags: review?(roc)
(Previous was wrong patch.)
Attachment #8366992 - Attachment is obsolete: true
Attachment #8366992 - Flags: review?(paul)
Attachment #8366993 - Flags: review?(paul)
Attachment #8366993 - Flags: review?(paul) → review+

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?
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.
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)
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
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.
Comment on attachment 8367275 [details]

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)
I've asked rfw to look at making the changes in comment 11 and turning the test into a mochitest.
QA Contact: tony
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
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
My bad, I didn't unapply both patches.
Attached file test_bug964376.html
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
Thanks, Tony.
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.