Closed Bug 670726 Opened 10 years ago Closed 10 years ago

Audio is scrambled when playing chunks of ogg in sequence


(Core :: Audio/Video, defect)

Not set





(Reporter: bobby, Assigned: cpearce)




(1 file)

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.
Forgot to mention: Linux, 2.6.39-ARCH, probably ALSA.
Assignee: nobody → chris
OS: Linux → All
Hardware: x86_64 → All
(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?
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.
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.
Attached patch Patch v1Splinter Review
* 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.
Attachment #545594 - Flags: review?(kinetik)
Comment on attachment 545594 [details] [diff] [review]
Patch v1

Review of attachment 545594 [details] [diff] [review]:
Attachment #545594 - Flags: review?(kinetik) → review+
@cpearce: tried your build. Seems to have fixed the problem entirely.
Whiteboard: [inbound]
Target Milestone: --- → mozilla8
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
You need to log in before you can comment on or make changes to this bug.