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

player.tech() is not available when "xhr-hooks-ready" is called #1499

Open
ianjaku opened this issue Mar 20, 2024 · 8 comments
Open

player.tech() is not available when "xhr-hooks-ready" is called #1499

ianjaku opened this issue Mar 20, 2024 · 8 comments

Comments

@ianjaku
Copy link
Contributor

ianjaku commented Mar 20, 2024

Description

player.tech() is not yet available when "xhr-hooks-ready" is called. The code described as an example in the readme thus does not work out of the box.

Example: https://jsbin.com/kulewerepe/1/edit?html,console,output

Sources

Any source

Steps to reproduce

  1. Follow the getting started on https://videojs.com/getting-started
  2. Attach a callback to the "xhr-hooks-ready" hook
  3. Try to access player.tech on that hook

Results

Expected

player.tech().vhx.xhr to be accessible

Error output

player.tech() is undefined

Additional Information

This has been a recurring issue I've faced in a few video.js implementations. I do hope that I'm not misunderstanding the intended usage of player.on('xhr-hooks-ready', ...) and when it should be called. From my understanding of the video.js & http-streaming docs player.tech() should be available when "xhr-hooks-ready" finishes.

videojs-http-streaming version

Tested and confirmed on video.js v8.10.0 (videojs-http-streaming v3.10.0)
and on video.js v8.11.8 (videojs-http-streaming v3.12.0)
videojs-http-streaming x.y.z

videojs version

what version of videojs does this occur with?
video.js x.y.z

Browsers

Verified on Firefox 123.0.1 & Chrome 116.0.5845.96
*

Platforms

Ubuntu 22.04
*

Other Plugins

Yes, but none of them are needed to reproduce the bug

Other JavaScript

Yes, but none of them are needed to reproduce the bug

Copy link

welcome bot commented Mar 20, 2024

👋 Thanks for opening your first issue here! 👋

If you're reporting a 🐞 bug, please make sure you include steps to reproduce it. We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.
To help make it easier for us to investigate your issue, please follow the contributing guidelines.

@adrums86
Copy link
Contributor

adrums86 commented May 6, 2024

Hi @ianjaku I think the docs need an update to cover some cases like this where the player may not be ready when the xhr-hooks-ready event is fired. I suspect the reason is because the VhsSourceHandler is ready for global hooks before the tech is ready, so the hooks ready event fires first. A quick fix is adding a player.ready callback inside the xhr-hooks-ready listener. For example:

const player = videojs("video", {});
    player.on("xhr-hooks-ready", () => {
      player.ready(() => {
        console.log("Player tech:", player.tech({ IWillNotUseThisInPlugins: true }));
      });
    });

@ianjaku
Copy link
Contributor Author

ianjaku commented May 6, 2024

Hey @adrums86,

Thank you for your response.

Sadly player.ready seems to fire too late as it fires after the request for the master playlist's content has already been fired.

Take for example the following code: https://jsbin.com/hakuxebaka/1/edit?html
I'm trying to add an authorization token to the end of the manifest file, and all other calls made from the player. ("?some=test" in the example).

So it seems "xhr-hooks-ready" fires before tech() is created, and player.ready() fires after the master playlist has already been fetched.

Ideally there would be a way to attach an onRequest hook that runs before any requests have been fired without having to do so globally as this would clash with other videos on the page.

@adrums86
Copy link
Contributor

adrums86 commented May 6, 2024

@ianjaku no problem, yeah this is a tricky one. I was having the same issue when working on the hooks implementation last year and thus the event was created to try and split that timing issue. Apparently it doesn't work in all cases though. ☹️

Unfortunately most of the core videojs team is quite busy right now, so it might be a bit before we can give this some further attention. I'll add a note to take another look at this when I'm able, if you're feeling up to it we always welcome PRs, your contribution would be greatly appreciated!

@lllllsmallgirl
Copy link

lllllsmallgirl commented May 10, 2024

@adrums86 waiting for fixing this bug!!!
or is there any other solutions to call tech() before the master playlist fetched.

@adrums86
Copy link
Contributor

The core development team has been extremely busy recently with other priorities, when we are able to work on fixing this bug we will update the issue. That said, as an open source organization, we highly encourage community contribution. The 'xhr-hooks-ready' event is fired here. Feel free to do some debugging and see if you can find where the timing is off here. Even if you're not able to fix it, any information would be helpful. Thanks!

@ionTea
Copy link

ionTea commented Jul 1, 2024

I'm also running into this issue, it's blocking me from updating from v7 to v8 unfortunately

@adrums86
Copy link
Contributor

adrums86 commented Jul 1, 2024

There has been some improvement to the xhr library that likely renders the xhr hooks in VHS obsolete. I would encourage everyone to try using the new interceptorStorage interface in the latest version of the xhr library, this is included in video.js v8.15.0+ https://github.com/videojs/video.js/releases/tag/v8.15.0. Documentation on the usage of this interface can be found in the readme for the xhr library. https://github.com/videojs/xhr?tab=readme-ov-file#xhrrequestinterceptorsstorage-and-xhrresponseinterceptorsstorage

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

No branches or pull requests

4 participants