silvermine / videojs-chromecast

MIT License
148 stars 75 forks source link

Error: preload method not defined. #30

Closed daveferrara1 closed 3 years ago

daveferrara1 commented 5 years ago

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

yokuze commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 4 years ago

@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 commented 4 years ago

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 https://github.com/silvermine/videojs-chromecast/pull/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 https://github.com/silvermine/videojs-chromecast/pull/31?

jthomerson commented 3 years ago

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