MonitorStream.kill() unconditionally did `stream.onerror = null` and
`stream.onload = null` on whatever element the monitor was using. That was
written for the zms <img>, whose onerror/onload are inherited event-handler
accessors.
With go2rtc the element is <video-stream>, where onerror is a method on
VideoRTC.prototype (video-rtc.js) overridden by VideoStream (video-stream.js).
Assigning null there finds a writable data property on the prototype chain and
so creates an *own* property on the instance, shadowing the method for as long
as the element lives. replaceDOMElement() returns the same node when the tag
already matches, so select_go2rtc() handed the poisoned element back on the
next start, and the listener VideoRTC.onconnect() registers,
this.ws.addEventListener('error', (ev) => this.onerror(ev));
threw "TypeError: this.onerror is not a function" on the next websocket
failure. Any kill()-then-start() path reached it: switching monitors on watch,
the stop/play buttons, montage viewport handling. It also meant the restart
that VideoStream.onerror performs was silently dead after a kill().
Guard the assignments on the element actually being an IMG.
Add tests/js/monitorstream-kill.test.js, which evaluates the real
MonitorStream.js in a vm context and checks that kill() leaves a prototype
onerror callable on a <video-stream>, adds no own onerror/onload to it, and
still clears both on an <img>.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MXtwyhzssj24Xwjmx8EA2z
The page kept two copies of the same secret: auth_relay, the query fragment
every AJAX call is authenticated with, and auth_hash, the bare hash stamped
into stream <img> URLs. Different responses updated different copies, so
they could drift, and a drifted auth_hash produced stream URLs that zms
rejects.
ZMAuth stores only the relay and derives the hash from it, so the two cannot
disagree. Its helpers cover the four shapes the call sites used:
zmAuth.hash derived, '' under the plain/none relay forms
zmAuth.update(data) absorb the auth fields of any response
zmAuth.appendTo(url) authenticate a url, no-op when auth is off
zmAuth.applyTo(src) point a stream url at the current credential
appendTo also removes the `x ? '&'+x : ''` guard repeated at every call
site, some of which had omitted it and emitted a dangling '?'.
Migrates all call sites across web/js and the classic skin, and drops both
globals from skin.js.php.
Tests: tests/js/auth-helpers.test.js, 44 passing.
getStreamCmdResponse() responded to every ajax/stream.php failure the same way:
mint a fresh connkey and reload the img src. ajaxError() returns HTTP 200 with
result=Error, so these arrive in jQuery's done() rather than fail(), and all
twelve error paths in stream.php took that branch.
Only one of them means zms is gone. For the rest the process is still running
and streaming, and replacing the connkey makes it unaddressable: CMD_STOP,
CMD_QUIT and mode=single all then go to the new key, so nothing can reach the
old process and only SIGPIPE can stop it, which we know is unreliable. That is
why the reports of lingering zms after switching monitors were unaffected by
changes to what the stop path sends.
The timeout path made this routine rather than rare. On select() expiry
ajaxError is commented out, so the script carries on to socket_recvfrom() on a
now non-blocking socket. That returns false, and false == 0 under switch's loose
comparison, so a merely slow zms was reported as 'No data to read from socket'
and torn down.
stream.php now classifies each failure as no_socket, timeout, transient or
invalid, and sends it as 'reason'. The client restarts the stream only for
no_socket. A missing reason is still treated as fatal, so a php that predates
this keeps the old behaviour.
Before replacing the connkey the client now sends CMD_QUIT to the old one, so
the process we are about to lose track of is asked to exit. That is deliberately
not routed through streamCommand(): it must name its target explicitly, since
this.connKey is about to change, and its response must not feed back into
getStreamCmdResponse(), or a QUIT that also failed would re-enter the error path
and loop.
ajaxError() takes the classification as a third argument, named $reason because
$code is already the HTTP status, and only includes it when set, so the other
131 callers are unaffected.
Tests: tests/js covers the fatal/non-fatal decision including the no-reason
fallback, tests/php pins the classification mapping and the switch(false)
semantics the timeout branch depends on. Both verified to fail when the
behaviour is reverted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- If a video track is missing when it is required, we generate the event:
dispatchTracksReceived(videoFeedStream, {
status: 'aborted',
reason: 'playback-videoTrack-missing'
});
kill() cleared this.started before calling this.stop(), but stop() returns
early when !started. For the zms path that meant clearInterval() on
statusCmdTimer and streamCmdTimer never ran, activePlayer was never reset and
mediaStream/audioTrack/videoTrack were never released. Every kill() leaked a
pair of intervals, which adds up over a montage or watch page that cycles
monitors every few seconds.
Keep started set until stop() has done its work, and pass skipStreamCommand so
stop() doesn't follow CMD_QUIT with a CMD_STOP against a socket zms is already
tearing down. Clear connkey afterwards.
stop() already sets started=false and activePlayer='' at the end, so kill()
doesn't need to.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
$.ajaxQueue defers the $.ajax() call until earlier queued requests
finish, and jQuery serialises the data object at that later point.
streamCmdReq() passed its data object by reference, so callers that
hand it this.streamCmdParms directly had their command overwritten by
the streamCmdQuery timer setting command=CMD_QUERY before the request
was actually sent.
show_analyse_frames() is the only such caller, which is why the Show
Analysis button in the live view appeared to do nothing: zms received
CMD_QUERY instead of CMD_ANALYZE_ON and never switched to
FRAME_ANALYSIS.
Copy the params inside streamCmdReq() so every caller is covered.
streamCommand() and alarmCommand() already copied by hand.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Added getCountStreamErrors() and resetCountStreamErrors() methods.
- On the Montage page, run hls.js without parsing RTSP2WebType == "HLS" because we use automatic player switching, and even if we don't use the HLS type by default, the player may switch to HLS when using RTSP2Web.
- Added waitingWebrtcPlayback to video-stream.js to detect the start of RTC playback. If we receive a 'webrtc/candidate' candidate, the next step is to start playback. We wait 3 seconds, and if the video hasn't loaded, we log the error and switch to another player.
- Error logging in video-stream.js occurs via errorHandling() and allows us to display a text message in the video display block. - Before logging errors, we change monitorStream.player to a specific go2rtc_webrtc or go2rtc_mse player, since we don't have just "go2rtc" in the list of players to switch to.
- We'll simplify getting monitorStream: instead of looking for the parent liveStream and getting its ID, we'll directly request getMonitorStream(stringToNumber(this.id))
- Go2RTC errors now generate the onErrorGo2RTC event.
- Added the ability to display text information (typically used to inform about errors) in the block where the stream is displayed. The class for the block is ".info-text"
- We no longer need the changes made in https://github.com/ZoneMinder/zoneminder/commit/8b9546df3315c171d0fd8f0f6886a68119e7d951
- this.started = false; In this.stop() in MonitorStream.js we will move it higher in the code so that when stopping, the correct state of the stream is already set.
- 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.
Chrome decodes H.265 over neither WebRTC nor MSE, so an H.265 camera viewed
through the go2rtc player produced no picture and the no-video watchdog fell
straight through to ZMS MJPEG.
Register a lazily-transcoded "<id>_h264" go2rtc stream (ffmpeg:<id>#video=h264)
alongside the primary stream. go2rtc only spawns ffmpeg when a client actually
requests it, so H.264 cameras never pay for it. When the no-video watchdog fires
on a go2rtc player, request that transcode stream once before giving up and
falling to the next player.
The transcode is forced over MSE: go2rtc's on-the-fly H.264 lacks the periodic
parameter sets WebRTC needs (over WebRTC the browser receives packets but
assembles no frames), whereas MSE fragmented-MP4 decodes it. The transcode
attempt also gets a longer watchdog timeout to cover ffmpeg cold start.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When a go2rtc player connects but the source video codec cannot be decoded by
the browser (e.g. an H.265 camera viewed in Chrome, which supports HEVC over
neither WebRTC nor MSE), go2rtc negotiates the video track as inactive and
sends only audio. The video element "plays" audio with no picture and stays at
0x0, so the normal 'error' handler never fires and the player hangs on
"Loading..." indefinitely.
Add a watchdog armed when a go2rtc stream starts: if no video frame is decoded
within NO_VIDEO_TIMEOUT, register a playback error and advance through the
existing player-priority fallback (eventually ZMS MJPEG), which can render the
stream server-side. The watchdog is cleared on stop()/restart() so it never
fires against a subsequently selected player.
Verified in-browser on a 3840x2160 H.265 monitor: go2rtc_webrtc -> go2rtc_mse
-> zms, ending on a visible MJPEG image instead of an endless "Loading...".
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A live multipart (mode=jpeg) stream <img> whose baked auth hash expires
past AUTH_HASH_TTL is reconnected by the browser itself, reusing the same
src (same connkey, same dead hash). Every native reconnect returns 403, so
once the capture daemon drops the stream the client storms zms with auth
failures for hours. Observed in production: a single connkey retried 84
times over 2.5h, 880 failures on one monitor whose zmc was timing out,
while monitors with healthy zmc showed only baseline hash-rollover noise.
img_onerror only blanked the <img> src inside its async refresh callback,
which never ran once authRefreshAttempts reached the cap. On give-up the
stale src stayed live and the browser kept native-retrying it, which is the
storm. Blank src synchronously at the top of img_onerror so the browser's
retry loop stops immediately, and reconnect with a fresh connkey (the zms
process behind the old connkey has exited) after fetching a fresh hash.
Extract the src rewrite into a pure rebuildStreamSrc() helper in
auth-helpers.js with unit tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a console or stream tab is backgrounded or the machine sleeps past the
auth hash TTL, the baked-in auth= on nph-zms <img> URLs expires. The browser
keeps reconnecting with the stale hash, producing a burst of 403s from zms
while the session-backed page still renders.
Detect the tab becoming visible again (visibilitychange/pageshow) and probe
auth once against the navBar status endpoint before letting streams reconnect:
on success refresh the global auth_hash and repaint; on a dead session (401)
go straight to login instead of retrying. Route the navBar poll, console table
query, and per-stream error paths through a shared decision so 401/403 ends in
a single login redirect rather than a retry storm.
Put the auth functions (goToLogin, revalidateAuth, onAuthVisible) and the pure
authFailureAction/loginRedirectUrl helpers in web/js/auth-helpers.js as named
globals; skin.js only wires the visibility listeners. Node unit tests cover the
pure helpers (tests/js/auth-helpers.test.js).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
