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

removeAllListeners prevents destroy from functioning properly #6761

Open
5 tasks done
matkokvesic opened this issue Oct 9, 2024 · 4 comments
Open
5 tasks done

removeAllListeners prevents destroy from functioning properly #6761

matkokvesic opened this issue Oct 9, 2024 · 4 comments
Labels
Bug Revisit-at-later-release-cycle Will revisit during release cycle indicated by the Milestone Suggested-Workaround Works as expected

Comments

@matkokvesic
Copy link

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

@matkokvesic matkokvesic added Bug Needs Triage If there is a suspected stream issue, apply this label to triage if it is something we should fix. labels Oct 9, 2024
@robwalch
Copy link
Collaborator

robwalch commented Oct 9, 2024

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 robwalch added Works as expected and removed Bug Needs Triage If there is a suspected stream issue, apply this label to triage if it is something we should fix. labels Oct 9, 2024
@robwalch
Copy link
Collaborator

robwalch commented Oct 9, 2024

Do not call removeAllListeners before destroy.

@robwalch
Copy link
Collaborator

robwalch commented Oct 9, 2024

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 matkokvesic changed the title destroy() method should call detachMedia() removeAllListeners prevents destroy from functioning properly Oct 9, 2024
@matkokvesic
Copy link
Author

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

@robwalch robwalch added Bug Revisit-at-later-release-cycle Will revisit during release cycle indicated by the Milestone labels Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Revisit-at-later-release-cycle Will revisit during release cycle indicated by the Milestone Suggested-Workaround Works as expected
Projects
None yet
Development

No branches or pull requests

2 participants