533822 - Audio files can't be played more than once

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[Ryan Doherty (:rdoherty)]

I have a page with a bunch off <audio> tags here: http://ryandoherty.net/clouserwsoundboard/

and they can't be played more than once. Clicking the play button again results in the loading animation displaying and Firefox taking up 100% of one of my CPU cores.

This works in 3.5, but not Minefield.

Comment 1

[Matthew Gregan [:kinetik]]

The decode thread is entering an infinite loop in oggplay_step_decoding because it expects more data but has no active tracks, so never tries to consume more data.

There's a bug filed upstream (https://trac.annodex.net/ticket/496), but we should fix this locally.

Comment 2

[Matthew Gregan [:kinetik]]

In this particular case, what happens is:

When attempting to replay, we seek back to the start of the media (actually to 0.014s), mark the media tracks as active, then call oggplay_step_decoding via DecodeToFrame.  After this, the first packet passed to oggplay_callback_audio is:

$30 = {
  packet = 0x15c6c050 "\v", 
  bytes = 0, 
  b_o_s = 0, 
  e_o_s = 512, 
  granulepos = -1, 
  packetno = 94
}


Which is pretty bogus--as far as oggz-dump is concerned, there is no packet 94, and no 0 byte packets in the stream.  When the callback receives this packet, the fact that the eos flag is set causes the track to be disabled.

Comment 3

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

Aside from any bugs here, we should probably only bother to call DecodeToFrame() if we've got an active video track.

Comment 4

[Matthew Gregan [:kinetik]]

That makes sense, but I'm not sure it's worth special casing the call to DecodeToFrame...  thoughts?

Anyway, there is a trailing packet with a granulepos of -1.  It doesn't show up in oggz-dump because of the same bug that bites us here--it's queued in the packet_buffer list for later processing (when the next valid GP is seen), but is never actually processed.  It turns out that this file would've triggered the assertion in oggz_close that I fixed in bug 498380.  It seems like clearing the packet_buffer list on close isn't sufficient for correct behaviour--it also needs to be cleared in other situations.  I've got a patch that clears the packet_buffer in oggz_read when EOF is hit, but it probably needs to be handled in a few other places too.

Comment 5

[Matthew Gregan [:kinetik]]

I think doing a |oggz_dlist_deliter(oggz->packet_buffer, oggz_read_free_pbuffers);| in oggz_reset_streams (or somewhere else in the seek reset code) is the correct fix.  I'll post a patch shortly.

Comment 6

[Matthew Gregan [:kinetik]]

Attachment: patch v0

Conrad might want to weigh in on this as well.

Comment 7

[Matthew Gregan [:kinetik]]

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

Requesting for 1.9.2 since this is a regression, and also fixes an infinite loop in the media playback code.

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

Worth taking, not blocking. If this patch has baked and is ready, please nominate for approval1.9.2.2?

Comment 9

[Matthew Gregan [:kinetik]]

Comment on attachment 420683 [details] [diff] [review]
patch v0

This has baked for a while now.  Small fairly safe change that fixes a regression from 3.5 when playing some files.

Comment 10

[LegNeato]

Comment on attachment 420683 [details] [diff] [review]
patch v0

a=LegNeato for 1.9.2.5. Please ONLY land this on mozilla-1.9.2 default, as we
are still working on 1.9.2.4 on the relbranch

Comment 11

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/dacbfe855969

Comment 12

[Tony Chung [:tchung]]

Ryan, what an awesome site!  go clouserw.

Verified fix in Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US;
rv:1.9.2.7pre) Gecko/20100630 Namoroka/3.6.7pre
and 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.7pre) Gecko/20100630
Namoroka/3.6.7pre

also trunk:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:2.0b2pre)
Gecko/20100630 Minefield/4.0b2pre