Skip to content

Feat: When hovering over an event's thumbs, play the event using HLS.#4839

Draft
IgorA100 wants to merge 18 commits into
ZoneMinder:masterfrom
IgorA100:patch-180448
Draft

Feat: When hovering over an event's thumbs, play the event using HLS.#4839
IgorA100 wants to merge 18 commits into
ZoneMinder:masterfrom
IgorA100:patch-180448

Conversation

@IgorA100
Copy link
Copy Markdown
Contributor

@IgorA100 IgorA100 commented May 16, 2026

  • If there is an index.m3u8 manifest file for the recorded event, we first attempt playback using HLS.
  • If playback using HLS does not start within 2 seconds, we attempt playback using MP4.
  • If the recorded event is longer than 1 minute and there's a problem with HLS, don't try to play it as MP4, but immediately switch to MJPEG playback.
  • Display the player type during playback (for both Live and recorded events) and the end of playback (for recorded events).
  • Fixed assigning the value of read cookies to stream.muted.
  • If errors occur, output them to the console.
  • Use video.autoplay = false instead of true, and then use video.play().then(_ => {. This will allow for more correct handling of promises and a repeat playback attempt if muted=false is an issue.

@IgorA100 IgorA100 changed the title When hovering over an event's thumbs, play the event using HLS. Feat: When hovering over an event's thumbs, play the event using HLS. May 20, 2026
@IgorA100 IgorA100 marked this pull request as ready for review May 20, 2026 20:34
@IgorA100
Copy link
Copy Markdown
Contributor Author

@connortechnology
Please check.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enhances thumbnail hover previews for recorded events by attempting HLS playback first (when an index.m3u8 manifest is available), with fallbacks to MP4 and then MJPEG, while also displaying the active player type/status during playback.

Changes:

  • Add HLS-based playback on event thumbnail hover with fallback logic (HLS → MP4 (short events) → MJPEG) and status display updates.
  • Extend event list AJAX responses to include HLS source URLs and event duration metadata.
  • Update Event::getStreamSrc() to route mode=mp4hls to a dedicated view_hls endpoint.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
web/skins/classic/js/skin.js Adds hover playback orchestration for HLS/MP4/MJPEG and injects player-status info into the overlay UI.
web/includes/Event.php Adjusts stream URL generation to support mp4hls via view_hls.
web/ajax/events.php Adds data-video-hls-src and duration attributes for thumbnails, enabling HLS-first hover playback decisions.
Comments suppressed due to low confidence (3)

web/skins/classic/js/skin.js:1506

  • getCookie('zmWatchMuted') is a string, so !!getCookie(...) will incorrectly mute when the cookie is 'false'. This breaks the user setting for RTSP2Web thumbnails; use an explicit string comparison instead of boolean coercion.
    const video = document.createElement('video');
    video.removeAttribute('controls');
    video.style.cssText = 'width: 100%; height: 100%;';
    video.autoplay = true;
    video.muted = !!getCookie('zmWatchMuted');
    video.playsInline = true;

web/skins/classic/js/skin.js:1608

  • The HLS hover player sets video.muted = !!getCookie('zmWatchMuted'), but the cookie value is 'true'/'false' (string). !!'false' is true, so hover playback will always be muted whenever the cookie exists. Use an explicit comparison to 'true' or preserve the prior !== 'false' behavior.
    video.style.cssText = 'width: 100%; height: 100%;';
    video.autoplay = false;
    video.muted = !!getCookie('zmWatchMuted');
    video.playsInline = true;
    container.appendChild(video);

web/skins/classic/js/skin.js:1692

  • The MP4 fallback player uses video.muted = !!getCookie('zmWatchMuted'), which treats the cookie string 'false' as truthy and forces muting. Use explicit string-to-boolean handling for this cookie value.
  const video = document.createElement('video');
  const previewRate = getPreviewRate();
  video.src = src;
  video.autoplay = true;
  video.muted = !!getCookie('zmWatchMuted');
  video.playsInline = true;
  video.playbackRate = previewRate;

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread web/skins/classic/js/skin.js Outdated
Comment thread web/skins/classic/js/skin.js Outdated
Comment thread web/skins/classic/js/skin.js
Comment thread web/includes/Event.php Outdated
Comment thread web/ajax/events.php Outdated
Comment on lines +348 to +350
if (file_exists($event->Path() . '/index.m3u8'))
$videoAttr .= ' data-video-hls-src="'.$event->getStreamSrc(array('mode'=>'mp4hls'), '&').'"';
$videoAttr .= ' data-video-duration-secs="'.$event->EndDateTimeSecs() - $event->StartDateTimeSecs().'"';
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e.g., rely on DefaultVideo() === 'index.m3u8'

This isn't the best option, as it only works for incomplete events. For completed events, DefaultVideo() will always contain the video file name, not the m3u8 manifest.

cache results

PHP automatically caches the file_exists() result. I'm not sure I need to create a new, custom caching system.

or carry an HLS-available flag from the DB/indexer).

I don't know how to do this. It seems to me that this information isn't stored anywhere in the DB/indexer.

Copy link
Copy Markdown
Contributor Author

@IgorA100 IgorA100 May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, we can always (without checking for the file's existence) provide a link to 'index.m3u8', but in this case, \views\view_hls.php will generate a 404 page and this error will be displayed in the browser console. I don't know how critical this is, but I don't like any errors, especially in large quantities.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe in \views\view_hls.php we should send a 204 status instead of a 404 if the 'index.m3u8' file is missing?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DefaultVideo exists precisely because of this hitting the fs is slow thing. I think the c++ side should be changed so that DefaultVideo is always the m3u8.
OR as copilot says, we add another db column to say whether the m3u8 exists. but the m3u8 should always exist so I think that is redundant.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but then every event will have it set to index.m3u8. Redundant.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but the m3u8 should always exist

I'm having trouble generating m3u8 files on two servers.
I haven't figured out why yet.

I think the c++ side should be changed so that DefaultVideo is always the m3u8.

Probably not always, but only if m3u8 actually exists.

I have a solution in mind:

  • \views\view_hls.php will return a 204 status code instead of 404. This will at least avoid errors in both the browser console and PHP logs.
    The absence of the m3u8 manifest file, I think, shouldn't generate a global 404 error.
    I think the [204 No Content] status would be more accurate.

  • web/ajax/events.php will always generate a link to "index.m3u8", which will avoid loading the file system.

  • In JS, I'll use XMLHttpRequest to check the response status. If it's not "200", then there's a problem with "index.m3u8" and we'll switch to the fallback.

I'll make a commit soon.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made new commits according to the algorithm described above.
No issues were found.

@IgorA100 IgorA100 marked this pull request as draft May 21, 2026 07:24
@IgorA100 IgorA100 marked this pull request as ready for review May 21, 2026 10:31
@connortechnology connortechnology requested a review from Copilot May 21, 2026 11:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

web/skins/classic/js/skin.js:1522

  • Hls.Events.ERROR callbacks receive (event, data), but this handler only declares a single parameter and logs it; that will log the event name and lose the useful data payload (type/details/fatal), making debugging harder. Update the handler signature to accept both parameters and log data (or at least data.details/type/fatal).
    if (Hls.isSupported()) {
      const hls = new Hls();
      hls.loadSource(hlsUrl);
      hls.attachMedia(video);
      hls.on(Hls.Events.ERROR, function(e) {
        console.error(e);
        hls.destroy();
        video.remove();
        fallbackToMjpeg();
      });

Comment thread web/skins/classic/js/skin.js
Comment thread web/skins/classic/js/skin.js
@IgorA100 IgorA100 marked this pull request as draft May 21, 2026 15:17
IgorA100 added 5 commits May 21, 2026 18:53
…valid, the return status code is 204 instead of 404. (view_hls.php)

This will avoid errors in the browser console and PHP logs.
….m3u8" manifest file, even if the physical file is missing. (events.php)

This will reduce the load on the file system.
…server response status and load the manifest only if the response status is 200. (skin.js)

This will avoid unnecessary errors.
…atusBar. (skin.js)

Also:
When executing createVideoElement(), autoplay is always false, and after the video is created, we now start playback via thumbnailVideoPlay().
@IgorA100 IgorA100 marked this pull request as ready for review May 21, 2026 16:59
@IgorA100
Copy link
Copy Markdown
Contributor Author

Ready for re-checking

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.

Comments suppressed due to low confidence (1)

web/skins/classic/js/skin.js:1523

  • hls.on(Hls.Events.ERROR, function(e) { ... }) only receives the event name string; the Hls.js ERROR callback is (event, data). As written, console.error(e) won’t print the actual error details, reducing the usefulness of the new logging. Accept both params and log data (and optionally data.type/details/fatal) before falling back.
    if (Hls.isSupported()) {
      const hls = new Hls();
      hls.loadSource(hlsUrl);
      hls.attachMedia(video);
      hls.on(Hls.Events.ERROR, function(e) {
        console.error(e);
        hls.destroy();
        video.remove();
        fallbackToMjpeg();
      });

Comment thread web/includes/Event.php
Comment thread web/ajax/events.php
Comment thread web/ajax/events.php
}

$videoAttr .= ' data-video-hls-src="'.$event->getStreamSrc(array('mode'=>'mp4hls'), '&').'"';
$videoAttr .= ' data-video-duration-secs="'.$event->EndDateTimeSecs() - $event->StartDateTimeSecs().'"';
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logically, Copilot is correct, but practically everything works without problems.
For some reason, EndDateTimeSecs() always returns the correct time. Apparently, for an unfinished event, the current time is returned. I couldn't quickly figure out why this was happening in \includes\Event.php.
@connortechnology Maybe you can say something?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It depends on the SQL used to populate the Event object. In some places is uses NOW(), in others it uses a subquery on Frames to determine length. I think the latter is the way to go generally. If we use now and the event in incomplete due to crash, it might be years old. I think Ideally we would use event->Duration() and have code in Duration() that figures it out.

Comment thread web/views/view_hls.php Outdated
Comment thread web/skins/classic/js/skin.js Outdated
Comment thread web/skins/classic/js/skin.js Outdated
Comment thread web/skins/classic/js/skin.js
Comment thread web/skins/classic/js/skin.js
@IgorA100 IgorA100 marked this pull request as draft May 22, 2026 06:49
IgorA100 added 2 commits May 22, 2026 10:27
…tting to the page body, save the message in the logs as Warning, since we're passing status code 204. (view_hls.php)

If the manifest is m3u8, then we don't write anything in the logs; we simply pass status code 204.
@IgorA100 IgorA100 marked this pull request as ready for review May 22, 2026 10:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

Comment thread web/skins/classic/js/skin.js
Comment thread web/skins/classic/js/skin.js Outdated
Comment thread web/skins/classic/js/skin.js
Comment on lines +1553 to +1565
video.play().then(() => {
if (infoStatusBar && currentMode) infoStatusBar.innerHTML = ' [' + currentMode + '] ';
console.debug(currentMode + " video player started playing");
if (eventStart && statusBar) updateTimeWallClock(video, eventStart, statusBar);
})
.catch((er) => {
if (er.name === 'NotAllowedError' && !video.muted) {
video.muted = true;
video.play().then(() => {
if (infoStatusBar && currentMode) infoStatusBar.innerHTML = ' [' + currentMode + '] ';
console.debug(currentMode + " video player started playing after muting");
if (eventStart && statusBar) updateTimeWallClock(video, eventStart, statusBar);
})
@IgorA100 IgorA100 marked this pull request as draft May 22, 2026 15:50
…e video. (skin.js)

- Pass the eventStart and statusBar parameters to createRtsp2webStream()
- Don't call playEventHLS() for Frames pages
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants