Last Comment Bug 670726 - Audio is scrambled when playing chunks of ogg in sequence
: Audio is scrambled when playing chunks of ogg in sequence
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla8
Assigned To: Chris Pearce (:cpearce)
Depends on:
Blocks: 670055
  Show dependency treegraph
Reported: 2011-07-11 12:37 PDT by Bobby Richter [:secretrobotron]
Modified: 2011-07-14 09:32 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1 (10.46 KB, patch)
2011-07-12 21:52 PDT, Chris Pearce (:cpearce)
kinetik: review+
Details | Diff | Review

Description Bobby Richter [:secretrobotron] 2011-07-11 12:37:01 PDT
I've been playing with hopping around audio elements, trying to get even slightly accurate playback by setting currentTime, and I found something which has crashed my browser once (8.0a1), and plays scrambled audio every time I run this test:

Clicking on these out of order produces the expected result, but clicking on "Tozan" and then "said" delivers some seriously scrambled audio, and then half of the clip I'm trying to hear.

There's no mozAudioWrite'ing happening or anything fishy. I'm simply using currentTime=start, and spinning until currentTime>end and returning for each clip.
Comment 1 Bobby Richter [:secretrobotron] 2011-07-11 12:38:03 PDT
Forgot to mention: Linux, 2.6.39-ARCH, probably ALSA.
Comment 2 Chris Pearce (:cpearce) 2011-07-11 15:02:35 PDT

This is a crash in the memcpy which I added in bug 670055. So a regression from bug 670055.
Comment 3 Bobby Richter [:secretrobotron] 2011-07-11 15:18:08 PDT
(In reply to comment #2)
> 7be032110711
> This is a crash in the memcpy which I added in bug 670055. So a regression
> from bug 670055.

Interesting, but not really what I'm seeing. I've only had a crash once out of dozens of tries. Related maybe?
Comment 4 Chris Pearce (:cpearce) 2011-07-11 15:23:36 PDT
I imagine it's related. I've only been able to make your testcase crash in a Windows opt build, not in a debug build.
Comment 5 Chris Pearce (:cpearce) 2011-07-11 16:00:21 PDT
The problem is in our usage of ns{Ogg,WebM}Reader::CanDecodeToTarget(). CanDecodeToTarget() does not take into account whether decoded audio data has been pushed to the hardware. So if CanDecodeToTarget() sees the seek target is close to the current playback position, it will claim that we can decode to the target, but if all audio from the current playback position up to after the target has been decoded and pushed to the audio hardware we can't actually decode to reach that target. The read cursor will be after the seek target's Clusters/pages, and the audio queue may even be full with SoundData from after the current playback position.

I bet this is also the cause of bug 634747.

I think we should just remove ns{Ogg,WebM}Reader::CanDecodeToTarget() until libcubeb lands and allows us to not need so much audio pushed to hardware.
Comment 6 Chris Pearce (:cpearce) 2011-07-12 21:52:11 PDT
Created attachment 545594 [details] [diff] [review]
Patch v1

* Remove ns{Ogg,WebM}Reader::CanDecodeToTarget() for the reasons I mentioned previously.
* Adjust assertion in nsOggReader::SeekBisection. It was being triggered in reftests causing orange. We were triggering this assertion during bisection when we backed off to the start of the seek interval while trying to sync to a page at the end of the interval on the first bisection.
* Remove some trailing references to "ms" in nsOggReader.
Comment 7 Matthew Gregan [:kinetik] 2011-07-12 21:59:00 PDT
Comment on attachment 545594 [details] [diff] [review]
Patch v1

Review of attachment 545594 [details] [diff] [review]:
Comment 8 Bobby Richter [:secretrobotron] 2011-07-13 10:04:21 PDT
@cpearce: tried your build. Seems to have fixed the problem entirely.
Comment 10 :Ehsan Akhgari (busy, don't ask for review please) 2011-07-14 09:32:58 PDT

Note You need to log in before you can comment on or make changes to this bug.