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

Setting the src is not synchronous #2326

Closed
dmlap opened this issue Jul 8, 2015 · 6 comments
Closed

Setting the src is not synchronous #2326

dmlap opened this issue Jul 8, 2015 · 6 comments
Milestone

Comments

@dmlap
Copy link
Member

dmlap commented Jul 8, 2015

Even when the tech is ready, setting the source doesn't take effect at the tech level for one tick because it's wrapped in a call to ready()

@dmlap dmlap added this to the v5.0.0 milestone Jul 8, 2015
@heff
Copy link
Member

heff commented Jul 9, 2015

Hmmm...that would appear be the result of #1667 and #2188.

We may want ready to be consistently async across techs, however we probably also still want the functionality that either calls the function synchronously or queues it for after the ready event.

Do we really need every function passed to ready(fn) to be triggered asynchronously, or do we just want the initial ready to happen async and then anything after that could be synchronous? If we could do the latter that would fix this issue.

@dmlap
Copy link
Member Author

dmlap commented Jul 10, 2015

Yeah, I think this introduces a lot of accidental asynchronicity that will make things much more complicated. Another possibility would be to make the "public" version asynchronous but convert all internal usage to a synchronous version.

@heff
Copy link
Member

heff commented Jul 10, 2015

Yeah, that could do it. Have a readySync() function, or a second arg, ready(fn, async=true). Not sure what the best API is but I like it. @gkatsev any thoughts?

heff added a commit to heff/video.js that referenced this issue Jul 22, 2015
For 5.0 we switched ready() to always execute listeners
asynchronously, but for most internal use cases we would
prefer the ready function to execute immediately if the
component is already ready.

Fixes videojs#2326
Related: videojs#2382, videojs#1667, videojs#2188
@dmlap
Copy link
Member Author

dmlap commented Jul 31, 2015

@heff it looks like #2392 didn't completely fix setting the source synchronously for the Flash tech because we're still setting it in an async ready handler on construction.

@heff
Copy link
Member

heff commented Aug 3, 2015

@dmlap that block sets sync=true (second arg) so it shouldn't be working any differently than 4.12 now. When that specific code runs the swf object is never ready. If we're trying to improve on the previous functionality then, we'll probably have to cache the source value and return that while the swf boots up.

@dmlap
Copy link
Member Author

dmlap commented Aug 4, 2015

Oops... I missed the true. All of the tests in contrib-hls now require an extra tick of the clock in player setup but it seems like that is because of the conversion to a source handler and this guy should have stayed closed.

@dmlap dmlap closed this as completed Aug 4, 2015
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants