449307 - duration not supported for <video>

Status: VERIFIED FIXED

(Core :: Audio/Video, enhancement)

Description

[Mike Schroepfer]

Attachment: Testcase

duration, bufferedBytes, bufferingRate, and totalBytes don't currently seem to be implemented.  See attached testcase.

Comment 1

[Mike Schroepfer]

Chris - another one for you.  If you have a todo list somewhere and I'm causing you more admin overhead please tell me to buzz off.

Comment 2

[cajbir (:cajbir)]

No, this is great, keep them coming :)

Comment 3

[cajbir (:cajbir)]

The library we use for Ogg decoding doesn't provide a means to get duration. Raised in their bug tracking system:

http://trac.annodex.net/ticket/424

Comment 4

[Justin Dolske [:Dolske]]

Ugh. This is a big deal -- it'll be hard to promote <video> / Ogg if implementing a core piece of playback UI isn't possible. :-(

Is there anything that we can do to work around the issue? What's the likelyhood of a rapid liboggplay fix?

Comment 5

[cajbir (:cajbir)]

There seems to be a standard in the Ogg community to serve a header X-content-duration, giving the duration of the ogg file. I plan to detect if that exists and report the duration as that. This means duration will at least work with the sites that use standard Ogg serving software.

This will provide an interim solution until either liboggplay gets a fix or I find a workaround and then we can get duration from standard http servers.

Note that at the low level, getting duration from an ogg file requires reading to the end of the file to read the timestamp from the last packet. This means having to seek (so the http server would need to support byte range requests) or reading the entire file. This is probably why X-content-duration appeared in the first place - to have the seek occur on the server side to get the data for efficiency.

Comment 6

[Justin Dolske [:Dolske]]

Wow, having to do that just sounds full of fail. Also, Google has exactly one hit for "X-content-duration", so I'd have to assume most people know nothing about this.

Roc suggested that it might be possible to estimate the duration based on file size? If that's possible, I'm wondering if it would be better to have a imperfect solution that's almost always available (assuming Content-Length), instead of requiring a special hack.

Comment 7

[michael dale]

We really need duration to work for standard http servers...

Why can't we just open up some extra non-displayed oggplay threads one to
read/request the end of the file and one to read up to the latest bytes that
have been downloaded? 

Knowing how much is of the stream is available for local seeking and the stream
duration is critical.

Comment 8

[cajbir (:cajbir)]

I'm not entirely sure how having an estimated duration would work. If the
duration can change during playback (when the estimate is revised) then how
would javascript players find out about this? There's no event for duration
changed. 

At least one Javascript player I know of in development uses duration to
'chain' videos on the client side. A request from the ui to seek past the
duration seeks to the next video in the chain, etc. This would get difficult
with a duration that can change.

Hopefully the liboggplay trac ticket will get resolved fairly quickly. If not
and I get time I can add the functionality myself.

Comment 9

[cajbir (:cajbir)]

Regarding comment 7, the issue is liboggplay doesn't provide a way to get the information. I can't say to liboggplay 'seek to the last but 1 frame and tell me what it is'. Not that I know of anyway (the library is basically undocumented). Hopefully someone can comment on the trac ticket some workarounds.

One approach would be to initially get the duration using low level libogg functionality and manually seeking, etc. But if I'm doing that I might as well add the functionality to liboggplay.

Comment 10

[cajbir (:cajbir)]

And I should add that if the 'standard http server' doesn't support byte range requests there is no way to get the duration at all. I don't know how pervasive byte range request support is.

Comment 11

[cajbir (:cajbir)]

Comment 8 - oops I was wrong. The duration is allowed to change. So an estimate is a possibility:

http://www.whatwg.org/specs/web-apps/current-work/#dom-media-duration

durationchange is fired when it changes.

Comment 12

[michael dale]

in reference to  Comment 10, byte range requests are pretty widespread for example see this sudy done a while back:  http://www.mnot.net/papers/capabilities.html

Comment 13

[cajbir (:cajbir)]

totalBytes was implemented by bug 449159. bufferedBytes and bufferingRate (and maybe totalBytes) are being discussed for removal on the HTML5 W3C mailing list. duration still remains to be implemented.

Comment 14

[cajbir (:cajbir)]

Change bug title to reflect that duration is what remains to be implemented as per comment 13.

Comment 15

[cajbir (:cajbir)]

Attachment: Adds support for duration

Implements support for duration.

Comment 16

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

+    else if(!mSeeking && responseStatus == 200) {

if (

Would be good to have #defines for 200 and 206 so we don't have magic numbers in our code.

+    else if(!mSeeking && responseStatus == 200) {
+      // We weren't seeking and got a valid response status,
+      // set the length of the content. Since our initial
+      // request is actually a byte range request, this means
+      // the server does not support seeking.
+      PRInt32 cl = 0;
+      hc->GetContentLength(&cl);
+      mDecoder->SetTotalBytes(cl);
+      mDecoder->SetSeekable(PR_FALSE);
+    }
+    else if (!mSeeking && responseStatus == 206) {
+      // We weren't seeking but got a 206, this only
+      // happens when we sent our initial http request
+      // as a byte range request from offset 0 so we can
+      // detect if the server supports seeking.
+      mDecoder->SetSeekable(PR_TRUE);
       PRInt32 cl = 0;
       hc->GetContentLength(&cl);
       mDecoder->SetTotalBytes(cl);

You can factor this code into
  if (!mSeeking && (responseStatus == 200 || responseStatus == 206)) {
    ... set content length ...
    mDecoder->SetSeekable(responseStatus == 206);
  }

I'm a bit concerned about servers that might return 200 for a byte-range request starting at 0. But I guess this'll do for now. Might want to file a follow-up bug to be more careful about detecting byte-range requests.

+        PRInt64 d = oggplay_get_duration(mPlayer);
+        mon.Enter();
+        mDuration = d;
+      }
+    }
+
     // Inform the element that we've loaded the Ogg metadata
     nsCOMPtr<nsIRunnable> metadataLoadedEvent = 
       NS_NEW_RUNNABLE_METHOD(nsOggDecoder, mDecoder, MetadataLoaded); 
     
     NS_DispatchToMainThread(metadataLoadedEvent, NS_DISPATCH_NORMAL);

Check for SHUTDOWN state before we go off and fire an event.

Comment 17

[cajbir (:cajbir)]

Attachment: Address review comments

Comment 18

[cajbir (:cajbir)]

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

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Chris, is it fixed now?

Comment 20

[cajbir (:cajbir)]

Just waiting for the tree's to all go green before marking fixed in case I have to back out.

Comment 21

[cajbir (:cajbir)]

Backout http://hg.mozilla.org/mozilla-central/rev/6b7faeb31b48

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1225790360.1225795138.8939.gz

Comment 22

[Biju]

Attachment: video4.html modified

I saw "duration" on 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre)
Gecko/20081104 Minefield/3.1b2pre

But at the end of the play "duration" became NaN.
Is that a bug?

Comment 23

[cajbir (:cajbir)]

Is this a build you did yourself? The patch hasn't landed yet. I'll take a look, sounds like a bug.

Comment 24

[Biju]

(In reply to comment #23)
> Is this a build you did yourself? 

No, it is the nightly Gecko/20081104 Minefield/3.1b2pre 
and "duration" gives the value as 15.119999885559082 till the end of the play 
and then switches to NaN.

but on Gecko/20081105 Minefield/3.1b2pre it is again show duration = 0

Comment 25

[cajbir (:cajbir)]

A nightly build must have got the change before it was backed out. Current builds don't have this patch.

Comment 26

[cajbir (:cajbir)]

Attachment: Fix for memory corruption when requesting duration

Fix for memory corruption when requesting duration. Fix obtained from liboggplay r3774.

Comment 27

[cajbir (:cajbir)]

Attachment: Fixes problem mentioned in comment 22

Comment 28

[cajbir (:cajbir)]

http://hg.mozilla.org/mozilla-central/rev/fb44ae3d1182
http://hg.mozilla.org/mozilla-central/rev/f1d71b8ac3fe

Comment 29

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2) Gecko/20081201 Firefox/3.1b2 ID:20081201061100

Chris, the duration for the above testcase is given with 15.119999885559082. Is it a calculated value or why we have such a height precision? Do we really wanna have a resolution better than milliseconds?

Comment 30

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Tests are available here:
http://mxr.mozilla.org/mozilla-central/source/content/media/video/test/