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

Error: preload method not defined. #30

Closed
daveferrara1 opened this issue Nov 6, 2018 · 9 comments
Closed

Error: preload method not defined. #30

daveferrara1 opened this issue Nov 6, 2018 · 9 comments
Assignees

Comments

@daveferrara1
Copy link

daveferrara1 commented Nov 6, 2018

Using MUX we found that calls to the tech method preload were not satisfied one we begin casting.

daveferrara1 pushed a commit to daveferrara1/videojs-chromecast that referenced this issue Nov 6, 2018
@yokuze
Copy link
Contributor

yokuze commented Nov 7, 2018

@daveferrara1 Thank you for the report. We'd like to get a little more information about the issue, if we could. As part of our code review process, we try to make sure that we understand the issue as best we can so that we can verify that a PR fully addresses it. For example:

  1. What version of Video.js is used?
  2. What exactly is the "MUX" you refer to? Is it this and if so, what version and how are you using it?
  3. What are the steps to reproduce the issue?

etc.

The best way to answer these questions and to clarify an issue is to provide a minimal, complete, verifiable example. Would you be able to create one for this issue? Here's a codepen to get you started (note that it uses VideoJS 7.3.0 and videojs-chromecast 1.1.0). This also gives us a good start toward testing a fix.

Side note: I can see from this comment on the PR you submitted that it's likely that something is calling the preload function on the player, which delegates to the active Tech, and our ChromecastTech does not currently have a preload function so the error is thrown. However, it would still be a good idea to have an MCVE that shows this and that we can test against.

Thanks!

@daveferrara1
Copy link
Author

Had a look at your code pen. That has a console error. I noticed the chromecast button is missing but still in the dom but it has vjs-hidden class so likely its missing because its problematic in that sandboxed iframe.

So that pen does not cast. Maybe it used to, or your just casting the browser window (that works), and not the player in the MCVE. Maybe its a paid feature?

You got it right on -- mux is looking for the preload method.

Would have to assume they think its common.

We are using:
MUX:
https://mux.com/
DOCS:
https://docs.mux.com/docs/web-integration-guide
NPM:
https://www.npmjs.com/package/videojs-mux
UNPKG:
https://unpkg.com/videojs-mux@2.4.1/dist/videojs-mux.js

@yokuze
Copy link
Contributor

yokuze commented Nov 8, 2018

@daveferrara1 Sorry, I should have specified: when using the Codepen, you have to cast from the "debug" view (https://s.codepen.io/mluedke/debug/MzKmKq/xJAjOqwNbEnk) because the preview in the editor view is embedded in a sandboxed iframe, which causes the error you noted. The "debug" view is not enclosed in an iframe. I tend to do all of my testing in the "debug" view.

Please give that a try and let me know if you have any other issues creating the MCVE.

@yokuze
Copy link
Contributor

yokuze commented Nov 8, 2018

A note for our reference:

The preload function isn't in the list of required methods to implement when implementing a Tech. However, the Video.js Player class delegates some of its function calls to the underlying Tech, one of which is the call to preload. So, if videojsPlayerInstance.preload() is ever called while the Chromecast Tech is active, it'll throw an error.

We should prevent throwing an error, either by implementing the preload function in a way that makes sense for Chromecast, or by adding a no-op function. The Youtube Tech that Video.js created uses a no-op function.

Of course, doing that doesn't guarantee that it fixes the issue with MUX if MUX is relying on preload in some critical way, but at least it gives clients the opportunity to handle what our preload function returns, without running into an error when calling it.

@AndrewStobie
Copy link

This is only semi related but I can't get the chromecast tech to work in a simple example with preload

pen here

I have a full on react app that is no longer seeming to load anything from the cast_sender script I'm wondering if that is happening here.

@jthomerson
Copy link
Member

@AndrewStobie it's not clear at all why you think the problem with your CodePen is related to this issue. If it is related to this issue, please clarify your reasoning. Also, please open your console and look at the errors that it's reporting. The first thing I saw when trying to open your CodePen was that there's a 404 because you have an invalid URL to the ChromeCast plugin JS file. There may be other issues - I'm not sure - but looking at the console should get you on the right track.

@jthomerson
Copy link
Member

@yokuze did this issue get resolved? I see #41 is closed, but it seems to have been closed by #43, which only added seekable and not preload. So, I'm not sure if #41 and this issue (#30) were really completed?

@jmattiace
Copy link

Hi folks! I wanted to resurface this issue here as I have run into the same problem. Essentially, if any plugin attempts to call preload() (such as Mux) with the chromecast tech active, the console will display errors:

VIDEOJS: Video.js: preload method not defined for Chromecast playback technology. TypeError: this.tech_[method] is not a function

@daveferrara1 has created a PR #31 that uses @yokuze's suggestion of a noop and I have confirmed that this in fact prevents the errors from being spewed in the console. I have also confirmed that in this case, the Mux plugin is handling the errors gracefully, but videojs is still logging them to the console. So there aren't any major issues there.

Wondering if we can please merge in #31?

@jthomerson
Copy link
Member

Sorry this took so long. Thanks @daveferrara1! I just published v1.2.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants