670726 - Audio is scrambled when playing chunks of ogg in sequence

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Bobby Richter [:secretrobotron]]

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:

http://robothaus.org/bugs/audio/test.html

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]]

Forgot to mention: Linux, 2.6.39-ARCH, probably ALSA.

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://crash-stats.mozilla.com/report/index/bp-66b5985d-c9d5-45f9-8fd8-7be032110711


This is a crash in the memcpy which I added in bug 670055. So a regression from bug 670055.

Comment 3

[Bobby Richter [:secretrobotron]]

(In reply to comment #2)
> https://crash-stats.mozilla.com/report/index/bp-66b5985d-c9d5-45f9-8fd8-
> 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 (Not reading bugmail)]]

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 (Not reading bugmail)]]

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 (Not reading bugmail)]]

Attachment: 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]]

Comment on attachment 545594 [details] [diff] [review]
Patch v1

Review of attachment 545594 [details] [diff] [review]:
-----------------------------------------------------------------

Comment 8

[Bobby Richter [:secretrobotron]]

@cpearce: tried your build. Seems to have fixed the problem entirely.

Comment 9

[Chris Pearce [:cpearce (Not reading bugmail)]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/cacb0abe58b9

Comment 10

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/cacb0abe58b9