video-dev / hls.js

HLS.js is a JavaScript library that plays HLS in browsers with support for MSE.
https://hlsjs.video-dev.org/demo
Other
14.77k stars 2.56k forks source link

Order of attachMedia and loadSource calls is inconsistent in docs and causes inconsistent results #4952

Closed jeffometer closed 1 year ago

jeffometer commented 2 years ago

What do you want to do with Hls.js?

The docs in https://github.com/video-dev/hls.js/blob/v1.2.3/docs/API.md#third-step-load-a-manifest show indicates a nesting of attachMedia and loadSource based on the MEDIA_ATTACHED event. But the version for 1.2.4 calls these methods synchronously.

At first I though this indicated a breaking change, but what I observed seemed to indicate some general confusing flakiness. Here is what I observed:

Under 1.2.3 nesting as in the docs as in the docs sometimes worked but sometimes did not reach the MEDIA_ATTACHED event. Under 1.2.4 calling synchronously as in the docs sometimes worked but also sometimes did not reach the MEDIA_ATTACHED event. Under 1.2.4 nesting the attachMedia under the MANIFEST_PARSED event seems to work every time (note this is different from the docs for both 1.2.3. and 1.24)

Should it matter how these get called? Is the flakiness an internal error or something weird in my setup? Thanks for helping clarify.

What have you tried so far?

No response

oliverswitzer commented 2 years ago

Just to provide code examples for the scenarios outlined by @jeffometer above...

When using 1.2.3, we tried using what was outlined in the docs for the 1.2.3 version (binding the manifest parsed listener from within the MEDIA_ATTACHED event and also calling hls.loadSource). This did not work (thus we chose to upgrade to 1.2.4):

 if (Hls.isSupported()) {
    var video = document.getElementById('video');
    var hls = new Hls();
    // bind them together
    hls.attachMedia(video);
    hls.on(Hls.Events.MEDIA_ATTACHED, function () {
      console.log('video and hls.js are now bound together !');
      hls.loadSource('http://my.streamURL.com/playlist.m3u8');
      hls.on(Hls.Events.MANIFEST_PARSED, function (event, data) {
        console.log(
          'manifest loaded, found ' + data.levels.length + ' quality level'
        );
      });
    });
  }

When using 1.2.4 we also tried what was outlined in the docs. This did not work:

if (Hls.isSupported()) {
    var video = document.getElementById('video');
    var hls = new Hls();
    hls.on(Hls.Events.MEDIA_ATTACHED, function () {
      console.log('video and hls.js are now bound together !');
    });
    hls.on(Hls.Events.MANIFEST_PARSED, function (event, data) {
      console.log(
        'manifest loaded, found ' + data.levels.length + ' quality level'
      );
    });
    hls.loadSource('http://my.streamURL.com/playlist.m3u8');
    // bind them together
    hls.attachMedia(video);
  }

This is the solution that worked every time with 1.2.4 (calling attachMedia from within event listener for manifest parsed event):

  var video = document.querySelector("video");
  videoSrc = video.dataset.url;

  if (window.Hls.isSupported()) {
    var hls = new window.Hls();
    hls.on(Hls.Events.MANIFEST_PARSED, function (event, data) {
      hls.attachMedia(video);
    });
    hls.loadSource(videoSrc)
  } else if (video.canPlayType("application/vnd.apple.mpegurl")) {
    video.src = videoSrc;
  }
robwalch commented 2 years ago

Can you clarify what is meant by not reaching media attached?

The nesting was removed from the docs because:

  1. It is unnecessary
  2. It results in errors when run a second time to load a new stream since detach and then attach will be called recursively in the attached event callback

The media element does not need to be attached for a stream to begin loading. Media should be detached however before loading a new stream in order to reset player state. The player does this automatically in loadSource. So, it is best practice to not call loadSource in a player event callback.

jeffometer commented 2 years ago

Hi Rob. I mean that the MEDIA_ATTACHED event never fires, the video source never gets set, and the readyState remains at 0. FWIIW, we do not get any error events, so it seems something failed to occur as opposed to something going wrong.

Your explanation makes perfect sense. However we still were only able to get consistent correct results by putting the attachMedia call in the MANIFEST_PARSED callback. Perhaps there is another issue causing a race condition? But we were unable to discover it.

Unfortunately we don't have an easily reproducible sandbox example, but you can see it in a public debugging room on our product at https://standups.staging.geometer.dev/geometer-io/public-debugging-room/recordings/2230. You'll have to log in via Google to see it though. Directly under the video element is a script tag with the following content:

<script>
              var loadVideo = () => {
                var video = document.querySelector("video");
                videoSrc = video.dataset.url;

                if (window.Hls.isSupported()) {
                  var hls = new window.Hls();
                  hls.on(Hls.Events.ERROR, function (event, data) {
                    console.error(event)
                  })
                  hls.on(Hls.Events.MANIFEST_PARSED, function (event, data) {
                    hls.attachMedia(video);
                  });
                  hls.loadSource(videoSrc)
                } else if (video.canPlayType("application/vnd.apple.mpegurl")) {
                  video.src = videoSrc;
                }
              }
              var tryLoadVideo = () => {
                if (window.Hls) {
                  setTimeout(() => loadVideo(), 0)
                } else {
                  requestAnimationFrame(tryLoadVideo)
                }
              }
              tryLoadVideo();
            </script>

We also have other js on that page (via alpine.js) which watches the video readyState, and the whole page is server rendered (via Elixir LiveView).

Thanks!

robwalch commented 1 year ago

I would need a sandbox example to reproduce. Thanks.

kdamken commented 1 year ago

Wanted to comment that I was seeing a similar issue. Here are some details.

robwalch commented 1 year ago

Check that the internal onMediaAttaching listeners are not throwing somewhere. MEDIA_ATTACHED is triggered on MediaSource 'sourceopen'. So if it never fires, then MSE failed somewhere here in buffer-controller:

  protected onMediaAttaching(
    event: Events.MEDIA_ATTACHING,
    data: MediaAttachingData
  ) {
    const media = (this.media = data.media);
    if (media && MediaSource) {
      const ms = (this.mediaSource = new MediaSource());
      // MediaSource listeners are arrow functions with a lexical scope, and do not need to be bound
      ms.addEventListener('sourceopen', this._onMediaSourceOpen);
      // link video and media Source
      media.src = self.URL.createObjectURL(ms);

// Setting the src should invoke this callback unless replaced:

  private _onMediaSourceOpen = () => {
    const { hls, media, mediaSource } = this;
    logger.log('[buffer-controller]: Media source opened');
    if (media) {
      this.updateMediaElementDuration();
      hls.trigger(Events.MEDIA_ATTACHED, { media });
    }

You probably want to make sure that something else isn't setting (or clearing) src on your media element after you call attach. That would likely prevent the 'sourceopen' callback from completing. If you are able to reproduce the issue, try adding 'emptied' and 'loadstart' event listeners to the media element you are attaching. The expected order of events here (when using a media element that already has something in "src") is:

[log] > attachMedia
> video event: "emptied" // src was removed or replayed and load() was called or invoked automatically 
> video event: "loadstart" // new src is loading
[log] > [buffer-controller]: Media source opened (MEDIA_ATTACHED)

So if you see another "emptied" after "loadstart" then you've got something else in your application interfering with the video element. So you might want to attach on "emptied" or MANIFEST_PARSED (which ever comes first).

robwalch commented 1 year ago

v1.3.3 includes changes in #5206 based on the concept above. If the media element dispatches "emptied" while attaching, and media.src does not match the MediaSource blob object URL being attached, then HLS.js will log an error that attach was interrupted. Check the console for this error to determine if something is setting the media element src outside of HLS.js while hls.attachMedia is setting up:

Media element src was set while attaching MediaSource (OBJECT_URL > NEW_SRC_VALUE)

This won't "fix" the issue, because the issue is not with HLS.j when something external modifies the src attribute on the media element. Add your own "emptied" listener or defer attaching until your app and its libs are done with the media element.