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
15.01k stars 2.59k forks source link

removeAllListeners prevents destroy from functioning properly #6761

Open matkokvesic opened 1 month ago

matkokvesic commented 1 month ago

What version of Hls.js are you using?

1.5.15

What browser (including version) are you using?

128.0.6613.138 (Official Build) (arm64)

What OS (including version) are you using?

macOS 15

Test stream

https://test-streams.mux.dev/x36xhzz/x36xhzz.m3u8

Configuration

{}

Additional player setup steps

Calling hls.destroy() doesn't detach previously attached html video element.

In case where same

Checklist

Steps to reproduce

Example code to reproduce issue:

HTML:

<script src="https://cdn.jsdelivr.net/npm/hls.js@latest"></script>
<video id="video" controls></video>

JS:

let video = document.getElementById('video');
let hls = new Hls();
hls.loadSource('https://test-streams.mux.dev/x36xhzz/x36xhzz.m3u8');
hls.attachMedia(video);

hls.on(Hls.Events.ERROR, function (event, data) {
  console.log(event, data)
});

setTimeout(() => {
  hls.removeAllListeners();
  // hls.detachMedia(); without this line we will see getAppendedFrag (and other) errors
  hls.destroy();

  console.log('attaching video element with new hls instance')

  let hls2 = new Hls();
  hls2.loadSource('https://test-streams.mux.dev/x36xhzz/x36xhzz.m3u8');
  hls2.attachMedia(video); 
}, 3000)
  1. Just hit play button after video reloads to get console errors

Expected behaviour

No errors

What actually happened?

Errors in console produced by "hanging" destroyed hls.js instance.

Console output

hls.js@latest:1 Uncaught TypeError: Cannot read properties of null (reading 'getAppendedFrag')
    at r.getAppendedFrag (hls.js@latest:1:137654)
    at r.checkFragmentChanged (hls.js@latest:1:400901)
    at r.onTickEnd (hls.js@latest:1:385486)
    at r.doTick (hls.js@latest:1:385393)
    at e.tick (hls.js@latest:1:104570)
    at r.onMediaPlaying (hls.js@latest:1:389727)

Chrome media internals output

No response

robwalch commented 1 month ago

destroy calls removeAllListeners after detachMedia because internal components listen for DESTROYING to clean up. The TypeError above is caused by calling removeAllListeners before destroy externally.

robwalch commented 1 month ago

Do not call removeAllListeners before destroy.

robwalch commented 1 month ago

This issue can be categorized as a bug if you change the title to “removeAllListeners prevents destroy from functioning properly”. It is categorized as “works as expected” because destroy always calls detachMedia.

It’s a good catch and something worth looking into improving. I don't believe we have a documented method for removing only external listeners. That is not a supporter feature.

matkokvesic commented 1 month ago

Thanks! Title changed to identify a bug in this issue.