Last Comment Bug 760336 - When seeking in an unbuffered range on a live stream, playback stops and |played| reports bogus values.
: When seeking in an unbuffered range on a live stream, playback stops and |pla...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla16
Assigned To: Paul Adenot (:padenot)
:
Mentors:
Depends on: 768583 791344
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-31 17:44 PDT by Paul Adenot (:padenot)
Modified: 2012-09-17 00:15 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v0 : Use the currentTime from the decoder to compute the |played| ranges. (1.13 KB, patch)
2012-05-31 19:36 PDT, Paul Adenot (:padenot)
cajbir.bugzilla: review+
Details | Diff | Splinter Review
Patch v0 : Restore units coherence when computing the buffering time. (2.13 KB, patch)
2012-05-31 22:03 PDT, Paul Adenot (:padenot)
cajbir.bugzilla: review+
Details | Diff | Splinter Review
Adding r=chris.double + add comment. (3.29 KB, patch)
2012-06-05 18:43 PDT, Paul Adenot (:padenot)
no flags Details | Diff | Splinter Review
Adding r=chris.double (1.36 KB, patch)
2012-06-05 18:43 PDT, Paul Adenot (:padenot)
no flags Details | Diff | Splinter Review

Description Paul Adenot (:padenot) 2012-05-31 17:44:43 PDT
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) 2012-05-31 17:57:43 PDT
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) 2012-05-31 18:00:20 PDT
(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) 2012-05-31 19:36:32 PDT
Created attachment 629028 [details] [diff] [review]
Patch v0 : Use the currentTime from the decoder to compute the |played| ranges.

This fixes the |played| problem.
Comment 4 cajbir (:cajbir) 2012-05-31 20:40:49 PDT
(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) 2012-05-31 20:45:24 PDT
(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) 2012-05-31 20:49:15 PDT
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) 2012-05-31 20:53:02 PDT
Ah, that makes much more sense. I hadn't realized bug 575140 never got fixed.
Comment 8 Paul Adenot (:padenot) 2012-05-31 22:03:09 PDT
Created attachment 629053 [details] [diff] [review]
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) 2012-06-02 18:53:27 PDT
Paul, I'll need some sort of response to comment 1 before I can review these patches.
Comment 10 Paul Adenot (:padenot) 2012-06-04 10:22:26 PDT
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) 2012-06-05 14:53:42 PDT
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) 2012-06-05 18:43:11 PDT
Created attachment 630404 [details] [diff] [review]
Adding r=chris.double + add comment.
Comment 13 Paul Adenot (:padenot) 2012-06-05 18:43:41 PDT
Created attachment 630406 [details] [diff] [review]
Adding r=chris.double

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