Improved RTSP2Web processing in HLS mode. - #4968
Merged
Merged
Conversation
- Assign video._hls when Hls() is executed, not just when an error occurs. This will allow Hls to be destroyed more correctly even if no error occurs. - Destroy Hls using the hlsDestroy() function, in which we remove all listeners. This will prevent memory leaks and unintended and unpredictable Hls code execution in the future. - When hovering over an image thumbnail and starting RTSP2Web_Hls, let's clear video._fallbackTimer when Media ATTACHED. RTSP2Web can take a long time to load, even more than 5 seconds. But it will most likely load eventually. If an error occurs later (for example, a manifest error or something else), the "Hls.Events.ERROR" listener will be triggered, and we will switch to the fallbackToMjpeg() method. It's probably better to wait a bit and play using HLS rather than immediately switch to ZMS.
…b in HLS mode, stop playback and restart. Yes, for now, it will restart in an infinite loop if a specific player is selected in the browser. However, this will be changed in the next PR ZoneMinder#4960. - Execute this.updateStreamInfo('', '') only after playback starts, not after MEDIA_ATTACHED, since that will cause a delay. There may be more errors later (for example, an unsupported audio codec, an m3u8 manifest error, or other errors.) - Execute this.updateStreamInfo('', 'Error') and log errors only if the error is fatal. - Moved this.streamErrorRegistration() lower in the code.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refines RTSP2Web HLS playback handling in the ZoneMinder web UI by improving teardown behavior, error/fallback timing, and when stream status is updated, with the goal of reducing leaks and making player switching more accurate.
Changes:
- Assigns and clears HLS state more consistently (e.g.,
video._hls) and attempts to centralize HLS teardown via a helper. - Adjusts RTSP2Web HLS event handling (e.g., fatal-only error handling, update stream info on playback start).
- Tweaks error counting behavior to stop cycling once the current player is identified.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| web/skins/classic/js/skin.js | Adds HLS instance tracking, changes fallback/error teardown to use a new hlsDestroy() helper, and tweaks fallback timing behavior. |
| web/js/MonitorStream.js | Switches HLS teardown to hlsDestroy(), adds additional HLS event handlers, and adjusts when stream info/error counters are updated. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1644
to
+1655
| const hlsDestroy = function(hls) { | ||
| if (hls && Hls) { | ||
| Object.keys(Hls.Events).forEach(function(eventName) { | ||
| hls.off(Hls.Events[eventName], function(event, data) { | ||
| }); | ||
| }); | ||
| hls.destroy(); | ||
| hls = null; | ||
| } else { | ||
| console.warn("Hls cannot be destroyed because it is not loaded."); | ||
| } | ||
| }; |
Comment on lines
1631
to
1634
| video._fallbackTimer = setTimeout(function() { | ||
| if (video.readyState < 2) { | ||
| if (video._hls) video._hls.destroy(); | ||
| if (video._hls) hlsDestroy(video._hls); | ||
| video.remove(); |
| video._hls.destroy(); | ||
| video._hls = null; | ||
| } | ||
| if (video._hls) hlsDestroy(video._hls); |
Comment on lines
+1813
to
+1817
| this.updateStreamInfo('', `Error. AAC codec "${data.audio.codec}" is not supported.`); //HLS | ||
| this.streamErrorRegistration(); | ||
| hlsDestroy(this.hls); | ||
| this.restart(this.currentChannelStream); | ||
| } |
Comment on lines
+1824
to
1828
| if (!data || !data.fatal) return; | ||
| this.updateStreamInfo('', 'Error'); //HLS | ||
| this.streamErrorRegistration(); | ||
| if (!data || !data.fatal) return; | ||
| this.hls.destroy(); | ||
| hlsDestroy(this.hls); | ||
| this.restart(this.currentChannelStream); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It's probably better to wait a bit and play using HLS rather than immediately switch to ZMS.