Last Comment Bug 752661 - Media element reloads opus file multiple times when play is disabled.
: Media element reloads opus file multiple times when play is disabled.
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla15
Assigned To: Timothy B. Terriberry (:derf)
:
: Maire Reavy [:mreavy]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-07 13:29 PDT by Ralph Giles (:rillian) needinfo me
Modified: 2012-06-02 12:30 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Make ReadMetadata() fail when there are no active Ogg streams (1.76 KB, patch)
2012-06-01 12:52 PDT, Timothy B. Terriberry (:derf)
kinetik: review+
Details | Diff | Splinter Review
Make ReadMetadata() fail when there are no active Ogg streams v2 (1.76 KB, patch)
2012-06-01 15:53 PDT, Timothy B. Terriberry (:derf)
tterribe: review+
Details | Diff | Splinter Review

Description Ralph Giles (:rillian) needinfo me 2012-05-07 13:29:19 PDT
Visiting https://people.xiph.org/~giles/2012/opus/ with today's nightly, and the media.opus.enabled pref set to false, The first <audio> element detects that it can't play the (short) opus file and disabled itself.

The second element spends a good half a minute making range requests for the longer sample. It should instead fail promptly.
Comment 1 Ralph Giles (:rillian) needinfo me 2012-05-07 13:31:34 PDT
Web console log:

[13:27:34.480] Media resource https://people.xiph.org/~giles/2012/opus/detodos.opus could not be decoded. @ https://people.xiph.org/~giles/2012/opus/
--
[13:28:04.420] Media resource https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus could not be decoded. @ https://people.xiph.org/~giles/2012/opus/
--
[13:29:26.606] GET https://people.xiph.org/~giles/2012/opus/ [HTTP/1.1 200 OK 267ms]
[13:29:26.890] GET https://people.xiph.org/~giles/2012/opus/background.png [HTTP/1.1 304 Not Modified 92ms]
[13:29:26.896] GET https://people.xiph.org/~giles/2012/opus/logo-wordmark-nightly.png [HTTP/1.1 304 Not Modified 304ms]
[13:29:26.900] GET http://i.creativecommons.org/l/by-nc-sa/1.0/80x15.png [HTTP/1.1 304 Not Modified 211ms]
[13:29:26.904] GET https://people.xiph.org/~giles/2012/opus/detodos.opus [HTTP/1.1 304 Not Modified 270ms]
[13:29:26.907] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 336ms]
[13:29:27.179] Media resource https://people.xiph.org/~giles/2012/opus/detodos.opus could not be decoded. @ https://people.xiph.org/~giles/2012/opus/
[13:29:27.227] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 315ms]
[13:29:27.543] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 214ms]
[13:29:27.758] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 360ms]
[13:29:28.118] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 579ms]
[13:29:28.698] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 585ms]
[13:29:29.284] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 587ms]
[13:29:29.869] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 585ms]
[13:29:30.454] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 570ms]
[13:29:31.024] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 587ms]
[13:29:31.612] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 565ms]
[13:29:32.176] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 584ms]
[13:29:32.760] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 587ms]
[13:29:33.347] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 585ms]
[13:29:33.933] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 595ms]
[13:29:34.528] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 583ms]
[13:29:35.112] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 588ms]
[13:29:35.700] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 574ms]
[13:29:36.275] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 585ms]
[13:29:36.860] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 587ms]
[13:29:37.448] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 605ms]
[13:29:38.052] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 589ms]
[13:29:38.642] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 570ms]
[13:29:39.212] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 590ms]
[13:29:39.802] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 587ms]
[13:29:40.389] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 589ms]
[13:29:40.979] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 592ms]
[13:29:41.571] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 581ms]
[13:29:42.152] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 585ms]
[13:29:42.738] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 590ms]
[13:29:43.328] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 581ms]
[13:29:43.909] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 587ms]
[13:29:44.496] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 584ms]
[13:29:45.080] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 586ms]
[13:29:45.666] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 589ms]
[13:29:46.255] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 590ms]
[13:29:46.845] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 587ms]
[13:29:47.432] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 590ms]
[13:29:48.022] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 593ms]
[13:29:48.615] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 585ms]
[13:29:49.200] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 586ms]
[13:29:49.786] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 668ms]
[13:29:50.455] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 591ms]
[13:29:51.046] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 592ms]
[13:29:51.638] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 572ms]
[13:29:52.212] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 572ms]
[13:29:52.782] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 591ms]
[13:29:53.374] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 586ms]
[13:29:53.960] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 596ms]
[13:29:54.556] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 587ms]
[13:29:55.143] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 589ms]
[13:29:55.733] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 585ms]
[13:29:56.318] GET https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus [HTTP/1.1 206 Partial Content 585ms]
[13:29:56.902] Media resource https://people.xiph.org/~giles/2012/opus/ehren-paper_lights-64.opus could not be decoded. @ https://people.xiph.org/~giles/2012/opus/
Comment 2 Timothy B. Terriberry (:derf) 2012-05-31 20:17:12 PDT
This still happens even when using a browser old enough not to have any Opus support compiled in at all (e.g., Fx 7).
Comment 3 Matthew Gregan [:kinetik] 2012-05-31 20:35:49 PDT
The call to RangeEndTime in nsOggReader::ReadMetadata is causing the stream to seek backwards from the end of the stream searching for the end time to calculate the duration.  I guess we shouldn't try to do that if there are no streams active in the container.
Comment 4 Timothy B. Terriberry (:derf) 2012-06-01 12:52:55 PDT
Created attachment 629302 [details] [diff] [review]
Make ReadMetadata() fail when there are no active Ogg streams

(In reply to Matthew Gregan [:kinetik] from comment #3)
> The call to RangeEndTime in nsOggReader::ReadMetadata is causing the stream
> to seek backwards from the end of the stream searching for the end time to
> calculate the duration.  I guess we shouldn't try to do that if there are no
> streams active in the container.

Here's a simple patch that stops trying to do that.
Comment 5 Matthew Gregan [:kinetik] 2012-06-01 15:48:49 PDT
Comment on attachment 629302 [details] [diff] [review]
Make ReadMetadata() fail when there are no active Ogg streams

+  } else

Use braces around the return, please.
Comment 6 Timothy B. Terriberry (:derf) 2012-06-01 15:53:05 PDT
Created attachment 629379 [details] [diff] [review]
Make ReadMetadata() fail when there are no active Ogg streams v2

(In reply to Matthew Gregan [:kinetik] from comment #5)
> Use braces around the return, please.

Prevailing style in the media code appeared to be to drop them for error returns, but I personally prefer them.

Updated.
Comment 7 Timothy B. Terriberry (:derf) 2012-06-02 06:29:18 PDT
That is a pretty green try push: https://tbpl.mozilla.org/?tree=Try&rev=31313031a5dd
Comment 8 Timothy B. Terriberry (:derf) 2012-06-02 06:53:09 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/6bf12a1bc5f0

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