Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stability issues with seeking and buffering in Microsoft IE11 #251

Closed
torerikal opened this issue Dec 22, 2015 · 15 comments
Closed

Stability issues with seeking and buffering in Microsoft IE11 #251

torerikal opened this issue Dec 22, 2015 · 15 comments
Assignees
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly

Comments

@torerikal
Copy link

This problem can be reproduced with the "Sintel" 1080p MP4" manifest referred in the Shaka player app, or a custom example stream:

  1. Open Microsoft Edge or IE11 and the player test page, for instance http://shaka-player-demo.appspot.com/.
  2. Select the mentioned test manifest from the dropdown and press Load stream.
  3. Open F12 Developer tools, and select the Console tab.
  4. Do a random seek by clicking at a new position in the timeline control.
  5. Notice a red "undefined" error log entry in the console. Where could this originate from?
  6. Repeat the seek procedure a few more times. This multiplies the "undefined" log entries.
  7. Perhaps in 1 of 10 repetitions, an additional player error (custom event) shows up, with the description "Cannot fetch (audio/mp4): previous operation not complete". These might be followed by "Unexpected call to resync_()".
  8. More randomly, further repeated seeking makes the stream stall or freeze in a buffering state, and it is not possible to get the playback recovered from this. This appears to be more frequent on custom streams or in IE11. In this case, there is no traces of other types of errors than the ones mentioned in step 5 and 7.

With our specific streams, the unrecoverable buffer state is unfortunately occurring a bit too often, and we are looking for a solution to cover Edge/IE 11 with DASH.

At this time, it is hard to conclude that the unrecoverable state is related to the observations in the console. Still, it would be interesting if you could look into and sort out the mentioned errors. The "undefined" entries could be a bit hard to trace down, though...

@torerikal torerikal changed the title Stability issues when seeking and buffering in Microsoft IE11 and Edge Stability issues with seeking and buffering in Microsoft IE11 and Edge Dec 22, 2015
@joeyparrish joeyparrish self-assigned this Jan 5, 2016
@joeyparrish
Copy link
Member

"undefined" is caused by failing assertions which have no messages associated with them. In other browsers, this is not a huge issue because the JS console will still show the line number and file on which the assertion occurred. It would seem that for some reason, IE11 only gives you the message string and nothing else. (Because of this, we are now requiring messages for all assertions in Shaka v2.)

There's nothing I know of that can be done about "undefined" short of adding messages to all of them and re-running the test to see which assertions are failing.

I think I may have a handle of the seek problems, though. It may be an IE-specific race condition.

@joeyparrish joeyparrish added the type: bug Something isn't working correctly label Jan 12, 2016
@joeyparrish
Copy link
Member

Okay, not a race after all. Here's the setup:

  1. Shaka assumes only one buffered range because it only buffers in order. It has no logic to handle edge cases that could arise with multiple ranges.
  2. Seeking to an unbuffered position causes Shaka to clear the buffer (sbm.clear()) to maintain the above assumption.
  3. If multiple buffered ranges are detected, Shaka assumes something has gone horribly wrong and clears the buffer (sbm.clear()) to try to re-establish the above assumption.

sbm.clear() is not working correctly on IE11. It calls SourceBuffer.remove(0, Number.POSITIVE_INFINITY), and although IE fires an 'updateend' event to say it has done so, it hasn't. So, what we end with is:

  1. Seek to an unbuffered position causes Shaka to perform a clear() which is actually a no-op on IE11.
  2. onUpdate_ is called, but only one range (the old one) is still buffered. It does not notice that currentTime is outside of this range, so it grabs a new segment and buffers it.
  3. onUpdate_ is called again, notices that there are now multiple buffered ranges (old range, new segment) and tries to clear(), which is still a no-op on IE11.
  4. Go to 3.

We should be able to fix this. Here are the results of my preliminary testing:

  • remove(0, Number.POSITIVE_INFINITY) does nothing on IE11
  • remove(0, Number.MAX_VALUE) also does nothing on IE11
  • remove(0, mediaSource.duration) seems to work on IE11

@wolenetz
Copy link

That's quite ugly. Does the same situation occur with the Edge browser?

@joeyparrish
Copy link
Member

I quickly smoke-tested the app without my patch in Edge and had no issues.

@joeyparrish joeyparrish changed the title Stability issues with seeking and buffering in Microsoft IE11 and Edge Stability issues with seeking and buffering in Microsoft IE11 Jan 12, 2016
@joeyparrish
Copy link
Member

@torerikal: Please try the latest sources from master, or this hosted preview which includes the fix: http://ie11test.shaka-player-demo.appspot.com/

@joeyparrish
Copy link
Member

To document for future generations, on IE11:

  • remove(0, Math.pow(2, 39)) works.
  • remove(0, Math.pow(2, 40)) is a no-op.

@torerikal
Copy link
Author

Thanks a lot for your investigations and fixes.

Unfortunately, I still get similar errors both in Edge and IE11 (see screenshot), but less frequently than earlier. I have earlier mentioned the error message "Cannot fetch (audio/mp4): previous operation not complete". Now I only observe "Cannot clear (video/mp4): previous operation not complete". I don't know if this distinction is of interest.

With the fixes, I haven't been able to stall the playback in your test manifest or our custom streams, which do look like an improvement.

skjermbilde 2016-01-13 kl 13 45 09

joeyparrish added a commit that referenced this issue Jan 14, 2016
This makes sure that remove() is called in a way that is compatible
with IE11.  For more information, see #251.

Change-Id: Iabe752713ba74c7bf516b4e7b93bc83cfccdd481
@joeyparrish joeyparrish reopened this Jan 15, 2016
@joeyparrish
Copy link
Member

Okay, I'll take another pass at it. Perhaps I did not stress test it enough. I won't be able to get to this until the end of the month, though.

@joeyparrish
Copy link
Member

On IE11 by seeking often, I've been able to trigger the "undefined" assertion. Each one in my repo is paired with the error "Unable to get property 'resolve' of undefined or null reference". I've traced both to SourceBufferManager.onSourceBufferUpdateEnd_. The assertion is assert(this.operationPromise_) and the error is triggered by this.operationPromise_.resolve().

The only ways for this to occur would be a malfunction of EventManager in destroy() or for IE to send unexpected 'updateend' events. Since SourceBufferManager is not being destroyed in this scenario, it has to be unexpected 'updateend' events.

So far in my testing, this error has not been fatal, so it should suffice to ignore spurious or unexpected 'updateend' events.

I have not been able to reproduce any of the following with the latest sources:

  1. "Cannot fetch (audio/mp4): previous operation not complete"
  2. "Unexpected call to resync_()"
  3. stalled playback or other unrecoverable errors

@torerikal, are you still seeing any of these three problems on http://ie11test.shaka-player-demo.appspot.com/?asset=//storage.googleapis.com/widevine-demo-media/sintel-1080p/dash.mpd ?

shaka-bot pushed a commit that referenced this issue Jan 27, 2016
Issue #251

Change-Id: Ifd4375a40795adb7f514f0a89383a1825361dae0
@torerikal
Copy link
Author

Thank you for your effort in troubleshooting and pinpointing several problems. I am sorry to say that I still am able to reproduce the error, as described in my previous comment, fairly frequently in IE11, with your test link. I am not able to reproduce the problem in Edge anymore.

Edge is most important to us, so from our side, the most important part is solved.

If you want to look into it once more with regards to IE11, here are some more details:

In around twenty attempts, the problem is encountered seven times. Sometimes early after starting playback, on the third seek attempt, and sometimes after numerous seeking operations. In most cases, the playback stalls mid-seek when the errors show up in the console.

The problem appears both in IE11 on Windows 8.1 and Windows 10. My installations were running virtually, though.

I have tried to see if there is a special pattern in my "random" seeking that makes the error occur. Especially with regards to entering previously played ranges or ranges I assume not being buffered. Unfortunately, when repeating a pattern that once triggered the error, the error didn't show up on the second attempt, so it all looks fairly random.

A note about Edge: The "undefined" assertion log entries are still there, and they tend to show up more frequently after a while with playback and seeking.

shaka-bot pushed a commit that referenced this issue Feb 1, 2016
IE11 does not show stack traces on failed assertions in the console.
So if there is no message associated with a failed assertion, create
one based on the stack trace.  This should help debug errors on IE.

Note that in Shaka 2, we already require messages for all assertions,
so this is just a stop-gap measure to fix lingering bugs in Shaka 1.

Issue #251

Change-Id: Ib42c693278485fa2d3bad7e34f3c73d15d23be27
@joeyparrish
Copy link
Member

I've just committed a patch to add line numbers to those assertions without messages. With that, I'm able to see these failed assertions on IE11 with rapid random seeking:

  • stream.js, line 414: assert(!this.fetching_)
    • frequent, seems to be harmless
  • stream.js, line 443: assert(!this.fetching_)
    • less often, also seems harmless
  • stream.js, line 639: assert(!this.sbm_ || this.resyncing_)
    • less often, also seems harmless, always happens at the same time as line 443
  • source_buffer_manager.js, line 373: assert(!this.task_)
    • rare, but coincides with Player error and 'unexpected call to resync_()' message

Even with the Player error, the systems seems to recover just fine and continue playing.

These are just preliminary findings. I'll update this ticket as we make progress.

@joeyparrish
Copy link
Member

Promises are being resolved in an order other than what we expect. See https://github.com/google/shaka-player/blob/b47a03a/lib/util/task.js#L215

This is probably a combination of a poorly-solved race condition and some implementation detail of the Promise polyfill used on IE11.

In a nutshell, when we abort() a Task, we assume that the rejection of the Task's Promise is processed before the resolution of the abort() Promise. This assumption is not always met on IE, hence the failing assertions.

We seem to recover from it, but we should try to satisfy these assumptions on all platforms if possible. Shaka 2's StreamingEngine should be more robust by avoiding the need for these kinds of assumptions all-together.

@joeyparrish
Copy link
Member

There are several issues with Shaka 1's test suite running on IE11, but with a little clean-up, I am able to reproduce this out-of-order assertion in 7/120 runs of the test "Seek does not create unclosable gaps in the buffer".

@joeyparrish
Copy link
Member

Even better! If we change the Promise polyfill to use setTimeout(fn, 0) instead of setImmediate(fn), repro rate goes up to 100%.

shaka-bot pushed a commit that referenced this issue Feb 2, 2016
This fixes some of the (numerous) test failures on IE11:

- Move SBM assertion after spurious event check.
- Install polyfills before running all tests.
- Fix missing closing tag (caused SyntaxError in IE11's XML parser).
- Avoid Uint8Array.slice, not present on IE11.
- Remove 'done' argument from describe().
- Make interpretContentProtection use childNodes instead of children.
- Errors only show stack trace if thrown.
- Extend some timeouts to reduce flake.
- Match different error type for DataView errors.

Tests are still not clean on IE, and many make Chrome-specific
assumptions, but this is at least an improvement.  Shaka 2's tests are
clean on IE, so it may not be worth the effort to fix all tests on IE
in Shaka 1.

Working toward a fix for #251

Change-Id: I156024f9879fa35893886b3181a526483f0dd947
shaka-bot pushed a commit that referenced this issue Feb 2, 2016
- IE11 timer resolution seems to be 15ms minimum.  This stretches
  timing for Task tests to 15ms increments, which fixes Task test
  failures.
- Extend karma timeout to accommodate BrowserStack latency.
- Fix assertion caused by SBM abort() race.

Related to issue #251

Change-Id: Ie2bd1e629ecaaf3a96bc7deaad3c24976a0952cf
@joeyparrish
Copy link
Member

@torerikal, I believe we've fixed this now. Related tests are passing on IE11, and we have a new test to cover this bug. I'm also no longer able to reproduce manually. Please verify these latest changes and let us know if this fixes it for you:

http://ie11test.shaka-player-demo.appspot.com/?asset=//storage.googleapis.com/widevine-demo-media/sintel-1080p/dash.mpd

If so, we will roll up our recent IE improvements into v1.6.3. Thanks!

@shaka-project shaka-project locked and limited conversation to collaborators Mar 22, 2018
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

No branches or pull requests

5 participants