495159 - Video playback stucks forever for HTTP resources when pressing End key (Fn+Right on Mac)

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

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

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090527 Shiretoko/3.5pre ID:20090527031214

When having a ogg video open (playing or paused) and you press the End key (Fn+Right on Mac) the video playback stucks and you are not able to continue playing. The page and the whole video content has to be reloaded.

This combination is used to seek to the end of the stream. It would make sense that something break while we still loading the data but even when we have buffered all data the shortcut will break the video playback.

This bug could be similar to bug 494316 but we consume only 30% of cpu load in this state. And it happens on all platforms outside of a VM.

Comment 1

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

Steps:

1. Open a page like http://tinyvid.tv/show/36mvdjoflfskb
2. Wait until the video has been loaded (optional)
3. Press play
4. Press Fn+Right or End
5. Click Pause/Play

After step 4 you will see the throbber and with step 5 you will not be able to pause or play the video.

Comment 2

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

Updating summary on findings with roc on IRC.

Comment 3

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

Another infinite loop in oggplay_step_decoding.

725	  num_records = oggplay_callback_info_prepare (me, &info);
Value returned is $1 = 1
(gdb) p num_records
$2 = 1
(gdb) n
726	  if (info != NULL) {
(gdb) p num_records
$3 = 1
(gdb) p info
$4 = (OggPlayCallbackInfo **) 0x0
(gdb) n
730	    r = 0;
(gdb) n
736	  for (i = 0; i < me->num_tracks; i++) {
(gdb) n
737	    oggplay_data_clean_list (me->decode_data[i]);
(gdb) n
736	  for (i = 0; i < me->num_tracks; i++) {
(gdb) n
740	  if (me->shutdown) {
(gdb) n
744	  if (info == NULL) {
(gdb) n
638	    if (me->active_tracks == 0) {
(gdb) n
655	    need_data = 0;
(gdb) n
656	    for (i = 0; i < me->num_tracks; i++) {
(gdb) n
657	      if (me->decode_data[i]->active == 0)
(gdb) n
659	      if (me->decode_data[i]->content_type == OGGZ_CONTENT_CMML)
(gdb) n
661	      if (me->decode_data[i]->content_type == OGGZ_CONTENT_KATE)
(gdb) n
665	        me->decode_data[i]->current_loc
(gdb) n
664	      (
(gdb) n
656	    for (i = 0; i < me->num_tracks; i++) {
(gdb) n
675	    if (!need_data) {
(gdb) n
725	  num_records = oggplay_callback_info_prepare (me, &info);

Comment 4

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

(gdb) p me->num_tracks
$5 = 1
(gdb) p me->active_tracks
$6 = 1
(gdb) p *me->decode_data[0]
$12 = {
  serialno = 1032923656, 
  content_type = 0, 
  content_type_name = 0x3178d86 "Theora", 
  decoded_type = OGGPLAY_YUV_VIDEO, 
  granuleperiod = 143164149, 
  last_granulepos = 1, 
  offset = 0, 
  current_loc = 143164149, 
  active = 1, 
  final_granulepos = -1, 
  player = 0xa7e67f0, 
  data_list = 0x0, 
  end_of_data_list = 0x0, 
  untimed_data_list = 0x0, 
  stream_info = OGGPLAY_STREAM_JUST_SEEKED, 
  preroll = 0
}
(gdb) p me->target
$13 = 143164148

I don't think I understand liboggplay well enough to make more progress right now.

Comment 5

[Mike Shaver (:shaver emeritus)]

When did this start?

Comment 6

[Mike Shaver (:shaver emeritus)]

Also, can we reproduce it with mouse clicking?  If not, I think we should consider disabling home/end bindings for 3.5.

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

I would suspect that this regressed with the landing for bug 486899. Anyone wanna confirm for me?

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

(In reply to comment #6)
> Also, can we reproduce it with mouse clicking?  If not, I think we should
> consider disabling home/end bindings for 3.5.

Whimboo notes that in a VM (but not a native system) he sees similar problems in bug 494316, but I think that's coincidence and perhaps similar, but not actually the same thing.

I'd be fine with disabling the home/end keybindings, will need to make sure to notify the user doc people if that happens by adding user-doc-needed.

Comment 9

[David Bolter [:davidb] (NeedInfo me for attention)]

Heh yeah I mid-air collided wanting to ask "Do we get similar problems attempting to drag all the way right via mouse?"

I agree with disabling end (and home if that makes things less confusing), while we fix.

Comment 10

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

(In reply to comment #6)
> Also, can we reproduce it with mouse clicking?  If not, I think we should
> consider disabling home/end bindings for 3.5.

Dragging the scrubber to the end has the same effect. So its not only the shortcut.

Comment 11

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

But that only happens while the video is loading. Afterward it is fine. Rob doesn't you already have a patch for the mouse part?

Comment 12

[Tony Chung [:tchung]]

(In reply to comment #11)
> But that only happens while the video is loading. Afterward it is fine. Rob
> doesn't you already have a patch for the mouse part?

I confirm the same behavior on trunk and 191 nightly.   This is while the video is still downloading:
- dragging to end via mouse, will display throbber, but eventually recovers and allows playback when download is completed
- Fn-Right/Left keys will display throbber, but will NOT recover and disallow playback when download is completed.

So the culprit remains with the home/end keybindings.

Comment 13

[David Bolter [:davidb] (NeedInfo me for attention)]

Just to be clear, it is the end keybinding right? Has anyone encountered stuckness with home?

Comment 14

[Aaron Train [:aaronmt]]

(In reply to comment #13)
> Just to be clear, it is the end keybinding right? Has anyone encountered
> stuckness with home?

On Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090528 Shiretoko/3.5pre testing on video http://tinyvid.tv/show/31wwwhrm2tb3i 

1. Click play (download still processing)
2. FN-Left Arrow (Home)
3. Video scrubber placed at 0:00 mark
4. Video continues to play fine

On the other hand, FN-Right Arrow causes the loop and throbber

Comment 15

[Mike Beltzner [:beltzner, not reading bugmail]]

Let's just kill the home and end keybindings, please. Assigning to Dolske, but if someone else wants to take it, go ahead.

Comment 16

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: Minimalistic band-aid fix.

This might break some tests, I'm just posting this so we have it ready if necessary.

Comment 17

[David Bolter [:davidb] (NeedInfo me for attention)]

If Rob has a patch for the mouse part (see comment #11); we should confirm whether it fixes this bug.

Dolske, FYI I uploaded to Try to see if I'm breaking any tests on m-c.

Comment 18

[Joe Drew (not getting mail)]

I was able to get this to hang when scrubbing with the thumb. Here's the regression window:

http://hg.mozilla.org/releases/mozilla-1.9.1/pushloghtml?fromchange=8a3631b43eeb&tochange=490e8e218fe6

Which includes a monster commit by roc.

Comment 19

[Joe Drew (not getting mail)]

It doesn't seem to hang in today's 1.9.1 build while using only the mouse, though.

Comment 20

[Justin Dolske [:Dolske]]

Comment on attachment 380209 [details] [diff] [review]
Minimalistic band-aid fix.

So, we could disable the keyboard shortcuts, though as others have noted the problem still happens when moving the scrubber with the mouse. The key commands are quite basic, so the root bug here is that the video backend shouldn't break when seeking to the beginning/end. Disabling home/end would be an ineffective bandaid. [I'm ok with doing so, if someone wants to argue for that, just saying based on the data at hand it's not going to help.]

Comment 21

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

I have a scripted testcase that triggers the bug easily. If people write their own players, as they in fact are doing, they'll hit this bug themselves.

Comment 22

[Justin Dolske [:Dolske]]

(Bouncing to roc, since this looks to be a backend problem)

Comment 23

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

Attachment: testcase

Move this test file test_bug495159.html into content/media/video/test, add it to Makefile.in. Then if you run mochtests and load this test, it will trigger the bug. (It's not a proper mochitest yet since it doesn't ever stop.)

Comment 24

[Chris Pearce [:cpearce (Not reading bugmail)]]

Ok, I've looked into this, and I think I understand it. The problem here is caused by bug 487612 + bug 463358 + liboggplay bug. When we seek, we clamp the seek time to be in the range [0...duration]. In nsOggDecodeStateMachine::Seek() we add the time of the first frame, so that seeks work in videos which don't start at time 0 (but our UI reports them as starting at time 0). This video starts at time 0, but the time on the first frame is being reported as 66ms. So we seek to the end+66ms. The seek bails out, without actually seeking. Then our "decode forward from the last key frame to the current time" loop will decode forever as the loop condition will never be met, hence the infinite loop.

Chris Double is looking into incorrect first frame times as part of bug 487519, but the loop needs to be fixed so that it can't run forever.

Comment 25

[David Bolter [:davidb] (NeedInfo me for attention)]

Aside: to follow up on the bandaid patch try, it had a clean run on windows, a
star-able run on linux, but not 100% sure about the progress test fails on this
OSX run:
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1243539175.1243546608.11204.gz

Comment 26

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch v1

Patch v1. Fix seek so that if seek fails because the target is outside the range of the media, we don't try to decode forward to the target time's frame. We'll just fail the seek, and not move the currentTime. In the Tinyvid testcase that Henrik provides, when we press End while playing, we'll skip a few frames, then resume playback. But we won't hang! Once we fix the first frame time issue in bug 487519, seeking-past-end will be clamped correctly, and we'll seek to the end.

It doesn't really make sense to include the testcase Rob provided, as we don't seek to the end when we seek past it; we need to wait until we fix the first frame time issue in bug 487519 before seeking-past-end is clamped correctly.

Comment 27

[Joe Drew (not getting mail)]

http://hg.mozilla.org/mozilla-central/rev/75f6b6d9dc52

Comment 28

[Joe Drew (not getting mail)]

This patch doesn't apply cleanly to 1.9.1; can you provide an updated patch?

Comment 29

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

With this patch the End key doesn't work anymore. Pressing the key will pause the video for a second and continues playing at the same position but doesn't jump to the end. We haven't checked in the patch for disabling the End key, right?

Comment 30

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to comment #28)
> This patch doesn't apply cleanly to 1.9.1; can you provide an updated patch?

I am about to leave for the airport, I'm going away for the weekend, I will try to supply a new patch tonight.

(In reply to comment #29)
> With this patch the End key doesn't work anymore. 

Yeah, that's correct. There's a bug in liboggplay which is causing this. Once bug 487519 lands, it should work.

> We haven't checked in the patch for disabling the End key,
> right?

Yup.

Comment 31

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch v1 (1.9.1 branch)

Patch v1, for 191.

Comment 32

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

Verified fixed on trunk with builds on all platforms (except Linux where video playback doesn't work in my VM) like Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090529 Minefield/3.6a1pre ID:20090529031523

Comment 33

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

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b5db8afbd67a

Comment 34

[Tony Chung [:tchung]]

Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090602 (BT-canvas) Shiretoko/3.5pre Ubiquity/0.1.5

Comment 35

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

Works fine for 1.9.1 with todays builds on Windows and Linux too.