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)

x86_64
Linux
defect
Not set
normal

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.
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: nobody → paul
Status: NEW → ASSIGNED
Attachment #630771 - Flags: review?(chris.double)
Attachment #635491 - Flags: review? → review?(chris.double)
Attachment #630771 - Attachment is obsolete: true
Attachment #630771 - Flags: review?(chris.double)
Attachment #635491 - Flags: review?(chris.double) → review+
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)).
Will do, thanks for the input!
If forgot to re-upload this one. This addresses comment 3 remarks. Carrying
forward r+.
Attachment #635491 - Attachment is obsolete: true
Attachment #652803 - Flags: review+
Keywords: checkin-needed
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
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.