videojs / http-streaming

HLS, DASH, and future HTTP streaming protocols library for video.js
https://videojs-http-streaming.netlify.app/
Other
2.48k stars 421 forks source link

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

Open etang93 opened 6 years ago

etang93 commented 6 years ago

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

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

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

VQuiles1 commented 6 years ago

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

azamshaikh commented 6 years ago

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

stale[bot] commented 5 years ago

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.

thedavos commented 4 years ago

anything on this?

stale[bot] commented 4 years ago

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.

lior-alon commented 4 years ago

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

stale[bot] commented 4 years ago

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.

gkatsev commented 4 years ago

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

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

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 commented 1 year ago

beforeRequest still no support promise

coderisimo commented 1 year ago

Which is really bad (((

ghost commented 8 months ago

I'm also urgently waiting for this to work

ghost commented 8 months ago

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 ?

qixiaojian310 commented 1 month ago

I have the same issue, I want to use indexeddb as a cache to store the stream file in to local. but onRequest can't accept an async function.