455654 - Firefox does not start playback of an ogg/vorbis audio stream.

Status: VERIFIED FIXED

(Core :: Audio/Video, defect, P2)

Description

[Thomas B. Rücker [account mostly dormant]]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1a2) Gecko/20080829071937 Shiretoko/3.1a2
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1a2) Gecko/20080829071937 Shiretoko/3.1a2

The <audio> tag points to a continuous ogg/vorbis stream coming from an Icecast 2.3.2 streaming server. Firefox starts to pull the stream immediately but won't start playback.

Reproducible: Always

Steps to Reproduce:
1. Go to the example URL
2. If the stream is currently available you'll see a small bandwith burst and then continuous bandwith consumption around 30kbyte/s

Actual Results:  
Playback of the received stream does not start.

Expected Results:  
Playback should start right away.

- Icecast 2.3.2 does _not_ send an Content-Lenght header. (It's a continuous stream ffs, it doesn't have a content length!)
- By default newer Icecast versions send a Burst-on-connect to prefill the buffers of an listening client and thus speed up beginning of playback.
- Firefox will actually start playback, but only if it the TCP connection gets reset / EOF.
- It will only play the first chained ogg segment like described in Bug 455165
- This is a very common use-case for radio-station websites. Nowadays they use flash but I think many would gladly switch to a solution that does not have to cope with broken flash and its limitation to read mpeg1-audio-layer3 only.

Comment 1

[cajbir (:cajbir)]

This occurs due to 'autoplay' not kicking in until the element's readyState changes to CAN_PLAY_THROUGH. This never happens since it's a stream with no end, so no autoplay occurs. Calling 'play' on the object does start it fine.

The implementation needs to be fixed to implement:

"CAN_PLAY_THROUGH
Data for the immediate current playback position is available, as well as enough data for the user agent to advance the current playback position at least a little without immediately reverting to the DATA_UNAVAILABLE state, and, in addition, the user agent estimates that data is being fetched at a rate where the current playback position, if it were to advance at the rate given by the defaultPlaybackRate attribute, would not overtake the available data before playback reaches the effective end of the media resource on the last loop. "

Comment 2

[Thomas B. Rücker [account mostly dormant]]

Now it won't even start playback after the stream is killed on the server and ends with an EOF.
Comparing to http://www.ruecker-itk.de/icecast/firefox/vorbisstatictest.html it looks like a new bug. I'll open one after I narrow down when it was introduced.

doesn't work:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b2pre) Gecko/20081125 Minefield/3.1b2pre

does work (static - plays immediately, stream - after forcing EOF):
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b1pre) Gecko/20080928020443 Minefield/3.1b1pre

Comment 3

[cajbir (:cajbir)]

Just to be clear, the blocking 1.9.1 refers to fixing comment 1, not supporting chained oggs?

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We can't block FF3.1 on major liboggplay work that no-one is signed up to do. We probably won't be able to support chained Oggs in Firefox 3.1.

We should, however, block on autoplay working on streams (comment #1).

Comment 6

[Christopher Blizzard (:blizzard)]

Do we have enough rate information to be able to tell if we're able to keep up with the stream as its being downloaded and send the signal that we can autoplay?

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yes.

Comment 8

[IU]

I see this same issue on Windows.

Go to http://urn1350.net/listenlive then scroll down and click the "Ogg Vorbis" link.  A blank page opens and the stream does not play.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090123 Minefield/3.2a1pre ID:20090123033224

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I have a patch that basically works. It needs some cleanup, which I'll hopefully get done this week.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

OK, here's the patch! It ended up a bit big. Let me explain the parts, roughly in the order they occurin the patch.

-- Funnel all readyState changes to state HAVE_CURRENT_DATA and above through a new method in nsHTMLMediaElement::UpdateReadyStateForData. This method is where we decide what the new readyState will actually be.

-- nsHTMLMediaElement::ChangeReadyState is now responsible for firing events associated with readyState changes.

-- Statistics about download rate, progress, playback rate etc are now the responsibility of the decoder. A new method nsMediaDecoder::GetStatistics can be called by any thread to get a snapshot of the current state. We use this from the element in UpdateReadyStateForData and for dispatching progress events. Accordingly, existing statistics-getters in nsChannelReader and elsewhere are removed. The idea of centralizing this data is to make it easier to keep things in sync.

-- Statistics are updated via 4 new callbacks to the decoder object: NotifyDownloadSeeked, NotifyBytesDownloaded, NotifyDownloadEnded, NotifyBytesConsumed. These are sent by the stream or its associated objects (nsChannelToPipeListener). NotifyDownloadedEnded is added so that the decoder can decide whather to do ResourceLoaded, NetworkError or something else when OnStopRequest (or the equivalent) happens.

-- Channel rates, especially for downloading are computed with the help of a ChannelStatistics object which records how much data has flowed through a channel over certain time intervals while the channel was active. Later we may want to use smarter rate estimators that ignore conditions far in the past; this object will be the right place to implement those improvements.

-- nsOggDecoder tracks the current download position (where the download is in the stream), the current decoder position (where the decoder has read up to in the stream), the current progress position (what to report to the element; basically the download position but not updated during seeks and other reads that should be invisible), and the current playback position (the end of the data for the last frame we played). Tracking all these explicitly makes it relatively easy to see what's going on.

-- Streams need to notify the decoder about bytes being downloaded *before* a read from the stream can return that data, otherwise things get very confused.

-- The file stream strategy needs to defer some of its notifications via an asynchronous LoadedEvent, to ensure that notifications are sent on the main thread, and also ensure that NotifyDownloadEnded isn't sent synchronously in nsFileStreamStrategy::Open, which is difficult to deal with in decoders.

-- nsByteRangeEvent can use local variables instead of fields for some of its data.

-- The changes in nsHttpStreamStrategy::Seek are cosmetic.

-- nsOggDecoder's FrameData tracks the position in the stream where each frame's data ended. This is used to keep track of the current playback position.

-- The decoder state machine updates mPlaybackStatistics every time a frame is decoded into the queue. However, this playback rate estimator is only used when the content's duration or stream length are not known (see below).

-- I made a really big change to buffering. In nsOggDecoder, instead of guessing when the buffer gets low I just observe whether decoding a frame reads data that wasn't available when we started decoding the frame. If we have, I treat that as an underrun and start buffering. This makes buffering happen less often and avoids buffering depending on rate estimation. I made these buffering changes because the current system using estimates was fragile, and would often cause us to start buffering as soon as we started autoplaying, especially if we decided to start autoplaying right at the beginning of the stream (e.g. if it's a live stream).

-- nsOggDecoder::GetStatistics is fairly straighforward, except that when estimating playback rate, we use the duration divided by the stream size if we know both of those, otherwise falling back to an estimate based on current playback behaviour.

-- In nsOggDecoder::SetTotalBytes and other functions I'm increasing mTotalBytes if we read past the number of bytes the server told us about (i.e., if the server lied).

-- The changes to the Wave decoder are similar to the Ogg decoder. Again, buffering no longer depends on rate estimation; we just buffer when there's no more data to play. I also modified the Wave decoder so that it's more like the Ogg decoder in terms of firing ResourceLoaded; if the resource is fully loaded before we even fire MetadataLoaded, we delay telling the element until MetadataLoaded has fired.

-- In the tests, I replaced a bunch of uses of "ok" with "is" since that gives better messages.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

One thing I don't have here is tests. To test this properly we really need httpd.js to support bandwidth control.

Comment 13

[cajbir (:cajbir)]

Patch does not apply to trunk?

Hunk #1 succeeded at 961 (offset 38 lines).
Hunk #2 FAILED at 988.
Hunk #3 succeeded at 1171 (offset 44 lines).
1 out of 3 hunks FAILED -- saving rejects to file content/html/content/src/nsHTMLMediaElement.cpp.rej
patching file content/media/video/public/nsChannelReader.h
Hunk #1 FAILED at 66.
1 out of 1 hunk FAILED -- saving rejects to file content/media/video/public/nsChannelReader.h.rej
patching file content/media/video/public/nsChannelToPipeListener.h
patching file content/media/video/public/nsMediaDecoder.h
Hunk #3 FAILED at 187.
Hunk #4 succeeded at 250 (offset 14 lines).
1 out of 4 hunks FAILED -- saving rejects to file content/media/video/public/nsMediaDecoder.h.rej
patching file content/media/video/public/nsMediaStream.h
Hunk #1 FAILED at 83.
Hunk #2 FAILED at 168.
2 out of 2 hunks FAILED -- saving rejects to file content/media/video/public/nsMediaStream.h.rej
patching file content/media/video/public/nsOggDecoder.h
Hunk #2 FAILED at 353.
Hunk #3 succeeded at 412 (offset 10 lines).
Hunk #4 succeeded at 435 (offset 10 lines).
Hunk #5 succeeded at 492 (offset 10 lines).
Hunk #6 succeeded at 541 (offset 10 lines).
1 out of 6 hunks FAILED -- saving rejects to file content/media/video/public/nsOggDecoder.h.rej
patching file content/media/video/public/nsWaveDecoder.h
Hunk #1 FAILED at 186.
Hunk #2 succeeded at 239 (offset 10 lines).
Hunk #3 succeeded at 279 (offset 10 lines).
1 out of 3 hunks FAILED -- saving rejects to file content/media/video/public/nsWaveDecoder.h.rej
patching file content/media/video/src/nsChannelReader.cpp
Hunk #2 succeeded at 76 (offset 10 lines).
Hunk #3 succeeded at 131 (offset 10 lines).
patching file content/media/video/src/nsChannelToPipeListener.cpp
Hunk #1 succeeded at 44 with fuzz 2 (offset 2 lines).
Hunk #2 FAILED at 77.
Hunk #3 FAILED at 151.
2 out of 3 hunks FAILED -- saving rejects to file content/media/video/src/nsChannelToPipeListener.cpp.rej
patching file content/media/video/src/nsMediaStream.cpp
Hunk #1 FAILED at 64.
Hunk #2 succeeded at 195 (offset 28 lines).
Hunk #3 FAILED at 225.
Hunk #4 succeeded at 327 (offset 51 lines).
Hunk #5 succeeded at 372 (offset 51 lines).
Hunk #6 FAILED at 425.
Hunk #7 FAILED at 449.
Hunk #8 FAILED at 611.
Hunk #9 succeeded at 750 with fuzz 2 (offset 75 lines).
Hunk #10 succeeded at 791 (offset 75 lines).
Hunk #11 FAILED at 828.
Hunk #12 succeeded at 878 (offset 85 lines).
Hunk #13 succeeded at 913 (offset 85 lines).
6 out of 13 hunks FAILED -- saving rejects to file content/media/video/src/nsMediaStream.cpp.rej
patching file content/media/video/src/nsOggDecoder.cpp
Hunk #8 succeeded at 668 (offset 4 lines).
Hunk #9 succeeded at 849 (offset 4 lines).
Hunk #10 succeeded at 889 (offset 4 lines).
Hunk #11 succeeded at 910 (offset 4 lines).
Hunk #12 succeeded at 1005 (offset 4 lines).
Hunk #13 succeeded at 1054 (offset 4 lines).
Hunk #14 succeeded at 1110 (offset 4 lines).
Hunk #15 succeeded at 1251 (offset 4 lines).
Hunk #16 succeeded at 1277 (offset 4 lines).
Hunk #17 succeeded at 1301 (offset 4 lines).
Hunk #18 succeeded at 1333 (offset 4 lines).
Hunk #19 succeeded at 1441 (offset 4 lines).
Hunk #20 succeeded at 1563 (offset 4 lines).
Hunk #21 succeeded at 1643 (offset 4 lines).
Hunk #22 succeeded at 1780 (offset 4 lines).
Hunk #23 FAILED at 1926.
1 out of 23 hunks FAILED -- saving rejects to file content/media/video/src/nsOggDecoder.cpp.rej
patching file content/media/video/src/nsWaveDecoder.cpp
patching file content/media/video/test/Makefile.in
Hunk #1 FAILED at 62.
1 out of 1 hunk FAILED -- saving rejects to file content/media/video/test/Makefile.in.rej
patching file content/media/video/test/test_media_selection.html
patching file content/media/video/test/test_progress1.html
patching file content/media/video/test/test_progress2.html
patching file content/media/video/test/test_progress3.html
patching file content/media/video/test/test_progress4.html

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Guess I'll have to update it

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix v2

Okay, I've updated the patch. It was a fairly straightforward refresh except that the access-controls tests didn't work. There are two problems: first, the test is wrong, setting 'src' after the first test shouldn't launch a new load since the element is not empty. We ignore that, but then we hit a bug unrelated to my patch where Load() while there is a load in progress just cancels the existing load and then returns without doing the new load. Calling load() explicitly from the test fixes the first problem and works around the second problem.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Pushed http://hg.mozilla.org/mozilla-central/rev/ba595db2b681.

Tests needed here, but I don't have any yet.

Comment 17

[Boris Zbarsky [:bzbarsky]]

roc backed this out of m-c, so reopening.  I'm not sure whether he meant to do that or whether he was really trying to back out bug 445087.

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The bad news is that I backed this out by accident.

The good news is that it was in the tree for several test cycles on each platform and there were no video/audio test failures. Which means it didn't cause any test regressions and even better, test_progress1.html was reenabled and works now.

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Relanded http://hg.mozilla.org/mozilla-central/rev/08281f2915ef

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/4e7871e56495

Comment 21

[Tony Chung [:tchung]]

I cannot get any of the url tests from comment #2 and comment #8 to play. I'm trying this on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090521 Shiretoko/3.5pre and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090521 Minefield/3.6a1pre.

Can someone test the fix and verify if this is working now?

Comment 22

[Aaron Train [:aaronmt]]

Testing with the url in comment #2 and comment #8 in the same the same two build's and same OS in comment #21 both streams play on my laptop. Tony, can you double check?

Comment 23

[u279076]

I see no issues using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1pre) Gecko/20090521 Shiretoko/3.5pre, Windows Vista and Intel Mac 10.5 equivalent builds.  I think we can add the verified keyword here.

Comment 24

[Aaron Train [:aaronmt]]

Verified FIXED using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1pre) Gecko/20090528 Shiretoko/3.5pre, Ubuntu 9.04 (2.6.28-11-generic)