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

Request to allow promises for Hls.xhr.beforeRequest #359

Open
etang93 opened this issue Mar 14, 2018 · 16 comments
Open

Request to allow promises for Hls.xhr.beforeRequest #359

etang93 opened this issue Mar 14, 2018 · 16 comments

Comments

@etang93
Copy link

etang93 commented Mar 14, 2018

Description

I am requesting for beforeRequest to allow promises to be used within the function. The reason why I am requesting this functionality is because I am using service call to update the uri value.
I need to update the uri this way because I am using NGINX’s secure_link module along with WOWZA. The secure link module requires all paths to be hashed by the client for authorization to view the video. I am using a service to generate the hash which is where my issue stems from.
I have tried using fetch api to make my requests, but beforeRequest continues to update the uri with the Promised fetch request that has not yet been fulfilled. Making the fetch call synchronous via async/await also has the same issue.
The solution to resolve this issue was by making a synchronous XMLHttpRequest for my request, but this is a short term solution because XMLHttpRequest is being deprecated.
I would like to help with this effort if possible, but I am unsure of where to start.

Sources

Is a certain source or a certain segment affected? please provide a public (accesible over the internet) link to it below.

Steps to reproduce

Explain in detail the exact steps necessary to reproduce the issue.

Override the function Hls.xhr.beforeRequest. Set the options.uri equal to a value that is fetched from a service.
Example:

window[`videojs`].Hls.xhr.beforeRequest =  function (options) {        
                                  
                //set the uri to a value that is retrieved from a service
                //fetchRequest makes a service call via fetch that returns a full uri
                options.uri = fetchRequest(path);
                
                return options;
            }

Results

Options updates in videojs before the promise is fulfilled causing the uri to be null.

Expected

Please describe what you expected to happen that did not happen in the description.
Videojs should update the uri after the promise is fulfilled

Error output

If there are any errors in the console, from the player, or anywhere else please include them here:

Additional Information

Please include any additional information necessary here. Including the following:

videojs-contrib-hls version

what version of videojs-contrib-hls does this occur with?
videojs-contrib-hls 5.12.2

videojs version

what version of videojs does this occur with?
video.js 6.6.3

Browsers

what browsers are affected? please include browser and version for each
*Google Chrome: Version 64.0.3282.186

Platforms

what platforms are affected? please include operating system and version or device and version for each
*

Other Plugins

are any other videojs plugins being used on the page? If so, please list them with version below.
*

Other JavaScript

are you using any other javascript libraries or frameworks on the page? if so please list them below.
*fetch

@gidich
Copy link

gidich commented Mar 15, 2018

I completely agree that this is an issue.

While a callback is good for some things, having the ability to intercept the options object and modify it with a promise is a more modern and flexible approach.

Perhaps the callback method could be overloaded and either accept a callback or a promise.

I'd be glad to lend a hand to this effort, but in reviewing the code I don't see a straightforward way of accepting a promise as a means of altering the options object. If one of the maintainers could point to an approach to implement this feature which falls inline with the direction of library, I'd be glad to fork and take a stab at implementing this.

@srsub
Copy link

srsub commented Mar 26, 2018

I agree this is a pressing issue that needs a fix.

@VQuiles1
Copy link

The addition of this function would greatly help in the usage of this component.

@azamshaikh
Copy link

Looking forward for this fix as I needed on my project as well.

@forbesjo forbesjo transferred this issue from videojs/videojs-contrib-hls Jan 11, 2019
@stale
Copy link

stale bot commented Mar 12, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the outdated label Mar 12, 2019
@stale stale bot closed this as completed Mar 19, 2019
@thedavos
Copy link

anything on this?

@gkatsev gkatsev reopened this Apr 1, 2020
@stale stale bot removed the outdated label Apr 1, 2020
@stale
Copy link

stale bot commented May 31, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the outdated label May 31, 2020
@lior-alon
Copy link

It's a great idea. This will help replacing src through http get.

@stale stale bot removed the outdated label Jun 1, 2020
@stale
Copy link

stale bot commented Aug 1, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the outdated label Aug 1, 2020
@gkatsev
Copy link
Member

gkatsev commented Aug 5, 2020

It's something we're going to consider but it's also a bit complicated because the code where beforeRequest is used doesn't expect asynchronuos usage. Also, we're deciding whether we'll end up switching to fetch and that may change this API too.

@ithiru
Copy link

ithiru commented Jan 18, 2021

I switched to shaka with videojs and videojs-shaka, there was a specific request to enable for shaka and it has been addressed here

	<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.3/jquery.min.js"></script>
	<script src="https://cdnjs.cloudflare.com/ajax/libs/shaka-player/3.0.7/shaka-player.compiled.js"></script>
	<script src="videojs.min.js'"></script>
	<script src="videojs-shaka.min.js'"></script>
...

        var options = {
            controls: true,
            autoplay: false,
            preload: "auto",
            aspectRatio: "16:9",
            techOrder: ['shaka', 'chromecast', 'html5'],
            shaka: {
                debug: true,
                configuration: {
                    drm: {
                        servers: {
                            "org.w3.clearkey": "licenseurl"
                        }
                    }
                }
            }
        };

        async function getSignedUrl(url) {
            let signedUrl = url;

            try {
                result = await $.ajax({
                    url: "https://portal.test/api/sign-url",
                    type: "get", //send it through get method
                    data: {
                        url: url,
                    }
                });
                //console.log(result);
                signedUrl = result.url;
            } catch (error) {
                console.error(error);
            }
            return signedUrl;
        }

        var player = videojs("v_player", options, function onPlayerReady() {
			videojs.log("Your player is ready!");
			try {
            player.src([{
                               type: 'application/dash+xml',
				src: '/manifest.mpd'
			}]);
			} catch(error) {
				console.error(error);
			}

            this.tech().shaka_.getNetworkingEngine()
                .registerRequestFilter(async function(type, request) {
                    try {
                        request.uris[0] = await getSignedUrl(request.uris[0]);
                    } catch (e) {
                        console.error("requestFilter", e);
                    }
                });
        });

@daniiiboy
Copy link

Is there any workaround people have figured out? I have been struggling with this issue as well, as I need to fetch before the call

@LIOU2021
Copy link

LIOU2021 commented Nov 1, 2022

beforeRequest still no support promise

@coderisimo
Copy link

Which is really bad (((

@venomone
Copy link

I'm also urgently waiting for this to work

@venomone
Copy link

venomone commented Jan 7, 2024

I'm not sure if if this is already the whole picture to make it working:

const xhr = async function XhrFunction(options, callback) {
  // Add a default timeout
  options = merge$1({
    timeout: 45e3
  }, options); 

  const beforeRequest = XhrFunction.beforeRequest || videojs__default["default"].Vhs.xhr.beforeRequest; 
  const _requestCallbackSet = XhrFunction._requestCallbackSet || videojs__default["default"].Vhs.xhr._requestCallbackSet || new Set();
  const _responseCallbackSet = XhrFunction._responseCallbackSet || videojs__default["default"].Vhs.xhr._responseCallbackSet;

  if (beforeRequest && typeof beforeRequest === 'function') {
    videojs__default["default"].log.warn('beforeRequest is deprecated, use onRequest instead.');
    _requestCallbackSet.add(beforeRequest);
  } 

  const xhrMethod = videojs__default["default"].Vhs.xhr.original === true ? videojsXHR : videojs__default["default"].Vhs.xhr;

  // Async handling for beforeRequest hook
  let beforeRequestOptions = options;
  for (const hook of _requestCallbackSet) {
    beforeRequestOptions = await hook(beforeRequestOptions);
  }

  // Remove the beforeRequest function from the hooks set
  _requestCallbackSet.delete(beforeRequest);

  // xhrMethod will call XMLHttpRequest.open and XMLHttpRequest.send
  const request = xhrMethod(beforeRequestOptions || options, async (error, response) => {
    // Async handling for onResponse hooks
    for (const hook of _responseCallbackSet) {
      await hook(request, error, response);
    }

    // Handle the response with the original callback
    return await callbackWrapper(request, error, response, callback);
  });

  const originalAbort = request.abort;

  request.abort = function () {
    request.aborted = true;
    return originalAbort.apply(request, arguments);
  };

  request.uri = options.uri;
  request.requestTime = Date.now();
  return request;
};

xhr.original = true;
return xhr;

Can smb from the VideoJS team make an statement onto this ?

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