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

Failed to execute 'addSourceBuffer' on 'MediaSource': The MediaSource's readyState is not 'open'. #4903

Closed
martinstark opened this issue Jan 16, 2023 · 7 comments · Fixed by #5069
Labels
status: archived Archived and locked; will not be updated status: unable to reproduce Issue could not be reproduced by the team type: bug Something isn't working correctly
Milestone

Comments

@martinstark
Copy link
Contributor

Have you read the FAQ and checked for duplicate open issues?
Yes

What version of Shaka Player are you using?
4.2.6

Can you reproduce the issue with our latest release version?
Presumably, need huge volumes of plays to see issue, and cannot take 4.3.X to prod.

Can you reproduce the issue with the latest code from main?
n/a

Are you using the demo app or your own custom app?
custom

If custom app, can you reproduce the issue using our demo app?
n/a

What browser and OS are you using?
Chrome, Edge (notably not on Firefox)

What did you do?
Check our playback error metrics, and noticed unhandled errors thrown inside of Shaka. The errors are rare, in the magnitude of a handful per (tens of) thousands of plays.

Play a stream

What did you expect to happen?
Shaka plays the stream, or throws a Shaka Error.

What actually happened?

Shaka crashes with a native ReferenceError Failed to execute 'addSourceBuffer' on 'MediaSource': The MediaSource's readyState is not 'open'.

@martinstark martinstark added the type: bug Something isn't working correctly label Jan 16, 2023
@github-actions github-actions bot added this to the v4.4 milestone Jan 16, 2023
@joeyparrish
Copy link
Member

I don't doubt your data on this, but it's not enough for us to debug the issue. If you have a way to reliably reproduce the issue in the future, please let us know.

@joeyparrish joeyparrish added the status: unable to reproduce Issue could not be reproduced by the team label Jan 19, 2023
@martinstark
Copy link
Contributor Author

martinstark commented Jan 20, 2023

There is only a single place where .addSourceBuffer is called in the Shaka source code:

const sourceBuffer = this.mediaSource_.addSourceBuffer(type);

Seems like media source can close before init is called, or while the mediaSourceOpen_ promise at line 352 is being awaited. I believe that pushes the rest of the execution of the init function to the back of the event loop. With the async check allowing for state changes to occur elsewhere, I think it is reasonable to check if mediaSource.readyState === 'open' before proceeding with calling addSourceBuffer.

That would allow for throwing a Shaka error instead of crashing with a native error.

@joeyparrish
Copy link
Member

I would welcome a PR, even without solid repro.

It's hard for me to suggest the right kind of error to emit, though, because I don't understand the circumstances that would cause readyState to be closed right away after awaiting the promise to open it.

Did playback get cancelled for this to happen? If so, this.destroyer_.ensureNotDestroyed(); should handle it correctly AFAICT. If the root cause is something else, then we may need special handling of it.

@martinstark
Copy link
Contributor Author

martinstark commented Jan 21, 2023

I don't understand the circumstances that would cause readyState to be closed right away after awaiting the promise to open it

I'll need to look deeper at the code, but in theory that promise could have resolved long before init gets called, unless init is called synchronously after initiating the opening. It's not necessarily awaiting the opening, it's just checking if the promise has resolved, and the promise would still resolve even if the media source has been opened and then closed.

Other than that I'm also suspecting the player being destroyed while this or some other async action is being awaited.

As an aside, would it not be good practice to always check this.destroyer_.ensureNotDestroyed(); after awaiting any async action in the player, to avoid unnecessary code execution after a destroy call?

@avelad
Copy link
Collaborator

avelad commented Feb 3, 2023

@martinstark Do you think you will be able to send a PR for this?

@martinstark
Copy link
Contributor Author

martinstark commented Feb 3, 2023

@avelad a simple one to guard against the erroneous call, yes, but I don't think I'll find time to dig out the root cause in the near future. Is that good enough? If not I'm happy to close the ticket until I can spend more time on it.

@avelad
Copy link
Collaborator

avelad commented Feb 28, 2023

@martinstark I agree to include the guard and see if it doesn't happen in the future. Please send the PR. :)

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 status: unable to reproduce Issue could not be reproduced by the team type: bug Something isn't working correctly
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants