760336 - When seeking in an unbuffered range on a live stream, playback stops and |played| reports bogus values.

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Paul Adenot (:padenot)]

STR:
- Load a video that reports a duration of |Infinity| ;
- Start playing, and after a few seconds, paste the following in the web console :
  
  var v = $$("video")[0];v.currentTime = v.buffered.end(0);

(that is, seek 10 seconds after the buffered range).

Expected result :
- The video restarts playback after having eventually paused for buffering. The
new position should be the closest position we can seek to, that is the last
time we have in a buffered range.
- |played| for this video reports now two distinct ranges.

Actual results :
- The video playback stops, and the throbber is shown indefinitely. The controls
show the "pause" button, as if the video was playing. The playback can be
restarted by clicking two times on the video (or on the play button).
- |played| stops updating. It reports the time we seeked from. If by playing, we
reach the point we wanted to seek to before having been clamped to the buffered
range, |played| to update again. On a debug build, we see that a warning is
firing :

  "Can't add a range if the end is older that the start."

Comment 1

[cajbir (:cajbir)]

I thought seeking in a live stream was not possible? That is "Expected Result" is not to seek at all?

Comment 2

[Timothy B. Terriberry (:derf)]

(In reply to Chris Double (:doublec) from comment #1)
> I thought seeking in a live stream was not possible? That is "Expected
> Result" is not to seek at all?

Seeking in buffered ranges in a live stream has always worked fine (for the most part). This is actually a big advantage of our implementation over Chrome's.

Comment 3

[Paul Adenot (:padenot)]

Attachment: Patch v0 : Use the currentTime from the decoder to compute the |played| ranges.

This fixes the |played| problem.

Comment 4

[cajbir (:cajbir)]

(In reply to Timothy B. Terriberry (:derf) from comment #2)
>
> Seeking in buffered ranges in a live stream has always worked fine (for the
> most part). This is actually a big advantage of our implementation over
> Chrome's.

Right, but the unbuffered areas are unseekable I thought. I think we disabled seeking in buffered areas in bug 756732?

Comment 5

[Timothy B. Terriberry (:derf)]

(In reply to Chris Double (:doublec) from comment #4)
> Right, but the unbuffered areas are unseekable I thought. I think we
> disabled seeking in buffered areas in bug 756732?

I don't think that's the bug you meant to link to. If it's truly disabled now, then I'll be sure not to upgrade my browser, because I personally find this feature very useful.

Comment 6

[cajbir (:cajbir)]

Oops, sorry. It was bug 756372. I agree the feature would be useful but that bug implied it didn't work. Ogg does work, it's only WebM that was disabled due to the lack of cues. I think we only support seeking in WebM files with cues.

Comment 7

[Timothy B. Terriberry (:derf)]

Ah, that makes much more sense. I hadn't realized bug 575140 never got fixed.

Comment 8

[Paul Adenot (:padenot)]

Attachment: Patch v0 : Restore units coherence when computing the buffering time.

This patch fixes the buffering problems, by using the proper unit (30 seconds instead of 30000 seconds). It also renames the constants to avoid subsequent confusion.

Comment 9

[cajbir (:cajbir)]

Paul, I'll need some sort of response to comment 1 before I can review these patches.

Comment 10

[Paul Adenot (:padenot)]

Chris, when we seek on live stream, in theory, it should works. As mentioned by derf, it is impossible (for now) to seek in a webm media if we don't have the cues, but works just fine using a ogg media (say, ogg/vorbis/theora, thanks to our bisection code). In the future, we can expect to be able to seek in webm.

To be more precise, per spec, if we seek outside of the |seekable| range (that is based on the |buffered| member) for non-seekable media stream like a live video), we should seek in the closest position in |buffered| to the current position. That is, if we seek in the future on a live stream, we should seek to the latest available position that is buffered.

The first patch fixes an obvious bug for the |played| member (which was not using a value inside the |seekable| range, by mistake), and the second one corrects a problem in the buffering logic code (milliseconds were used as seconds, thus resulting in an absurd value for the buffering wait time).

Comment 11

[cajbir (:cajbir)]

Comment on attachment 629053 [details] [diff] [review]
Patch v0 : Restore units coherence when computing the buffering time.

Review of attachment 629053 [details] [diff] [review]:
-----------------------------------------------------------------

Can you add a comment to mBufferingWait in nsBuiltinDecoderStateMachine.h stating what units it is (seconds). It's unfortunate that it's not currently commented.

Comment 12

[Paul Adenot (:padenot)]

Attachment: Adding r=chris.double + add comment.

Comment 13

[Paul Adenot (:padenot)]

Attachment: Adding r=chris.double

Comment 14

[cajbir (:cajbir)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/45dbfd1debfd
https://hg.mozilla.org/integration/mozilla-inbound/rev/5d04d59d85d0

Comment 15

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/45dbfd1debfd
https://hg.mozilla.org/mozilla-central/rev/5d04d59d85d0