Closed
Bug 762282
Opened 12 years ago
Closed 12 years ago
After having finished the download of an OGG video, networkState is NETWORK_LOADING.
Categories
(Core :: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: padenot, Assigned: padenot)
Details
Attachments
(1 file, 2 obsolete files)
This bug is intermittent, it takes me 3-4 times to reproduce. It seems easier to reproduce if the download speed is very high (but not too high like a local webserver). STR: - Load a OGG video (http://upload.wikimedia.org/wikipedia/commons/e/e6/Typing_example.ogv) ; - Start buffering, and wait for the download to complete (until all the file has buffered). Expected: - networkState is NETWORK_IDLE (1). Actual result: - networkState is NETWORK_LOADING (2). Additionnaly, a variable fraction of the buffering progress bar is not filled, despite having |buffered.end(0) == duration|. That seems to indicate that we don't fire a "progress" event at the end of the download.
Assignee | ||
Comment 1•12 years ago
|
||
This is probably because we download the end of the file to get the duration, when playing an OGG file, and this ensue : - ChannelMediaResource::OnStopRequest is called, because the underlying channel is closed, because we hit the end of the resource, so the channels is closed without us suspending it ; - mIgnoreClose is false, because it is never set to true ; - Hence, nsMediaCacheStream::NotifyDataEnded is called, with mDidNotifyDataEnded to false, since it is the first time we call that method ; - mDidNotifyDataEnded is flipped to true, which prevent subsequent call of this method to call ChannelMediaResource::CacheClientNotifyDataEnded, which prevent calling nsHTMLMediaElement::ResourceLoaded, and that does not change the networkState and does not fire the last progress, so the buffered bar does not update. This patch special-cases when seeking to the end to get the duration. This kind of breaks the nice abstractions we have, though.
Assignee | ||
Comment 2•12 years ago
|
||
Attachment #635491 -
Flags: review?
Assignee | ||
Updated•12 years ago
|
Attachment #635491 -
Flags: review? → review?(chris.double)
Assignee | ||
Updated•12 years ago
|
Attachment #630771 -
Attachment is obsolete: true
Attachment #630771 -
Flags: review?(chris.double)
Updated•12 years ago
|
Attachment #635491 -
Flags: review?(chris.double) → review+
Comment 3•12 years ago
|
||
Sorry for the drive-by, but I saw this while looking at a related bug. + // If we are seeking to get the duration, because we are playing an OGG file, + // ignore if the channel gets closed without us suspending it explicitly. We + // don't want to tell the element that the download has finished. + if (mSeekingForDuration) { + mIgnoreClose = true; + } else { + mIgnoreClose = false; + } This can just be: mIgnoreClose = mSeekingForDuration; Can you also rename all the "ForDuration" things to "ForMetadata" and remove the specific mention of Ogg (since this may be used for other situations)? That matches better with some other places we've handled this sort of thing (c.f. calls to SetReadMode(MODE_METADATA)).
Assignee | ||
Comment 4•12 years ago
|
||
Will do, thanks for the input!
Assignee | ||
Comment 5•12 years ago
|
||
If forgot to re-upload this one. This addresses comment 3 remarks. Carrying forward r+.
Assignee | ||
Updated•12 years ago
|
Attachment #635491 -
Attachment is obsolete: true
Assignee | ||
Updated•12 years ago
|
Attachment #652803 -
Flags: review+
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Comment 6•12 years ago
|
||
Made one drive-by whitespace fix here: >+void ChannelMediaResource::StartSeekingForMetadata() >+{ >+ mSeekingForMetadata= true; -----------------------^ and landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/d7d6e90e2580
Keywords: checkin-needed
Version: unspecified → Trunk
Comment 7•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d7d6e90e2580
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in
before you can comment on or make changes to this bug.
Description
•