When a monitor's zms exits, the status poll gets reason=no_socket and the
stream is reloaded under a new connkey. A no_socket reply takes over a second
(stream.php waits for the socket file to appear) while the poll runs every
second, and every request on the page shares one ajaxQueue, so a second poll
for the dead connkey is usually already queued when the first reply arrives.
Its reply then reloaded the stream again: it sent CMD_QUIT to the zms the
first reload had just started and replaced the connkey a second time.
Each restart of an unreachable monitor therefore left an orphaned zms behind,
and a poll queued for that orphan between the two reloads could find it
shutting down. stream.php then waited the full MSG_TIMEOUT and logged
"Timed out waiting for msg ... after waiting 5000 milliseconds", while every
other monitor's poll on the page waited behind it in the shared queue.
Bind each reply to the connkey its request was sent with and drop replies
for a connkey we no longer hold, as EventStream already does. alarmCommand()
passes its connkey explicitly, since binding getStreamCmdResponse directly
would hand it jQuery's textStatus in that argument.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- When calling `monitorStream.manageAvailablePlayers()`, now pass the candidate player as an argument instead of determining the player from stored cookies inside `manageAvailablePlayers()`. This is necessary because cookies could belong to either the Watch page or the Zone page.
- On the Zone page, load `hls.min.js` if Go2RTC or RTSP2Web is being used for the monitor.
- `monitorStream.setPlayer()` is now executed within `monitorStream.manageAvailablePlayers()`. Previously, `manageAvailablePlayers()` only handled the assignment of `monitorStream.player`.
- Added a new function, `this.setRTSP2WebType()`, to avoid code duplication. This ensures that `RTSP2WebType` is checked and set even if `setPlayer()` is not called on the page.
- Minor optimization regarding variable usage.
- If `hls.min.js` is not loaded, attempting to play via HLS will now trigger a "The HLS library is not loaded" warning in the browser console and initiate a stream restart. Previously, this would result in an error.
- The `janus` variable is no longer global; it now belongs to `MonitorStream`.
- In the `Janus.init` callback, do not perform any actions if the monitor is in a stopping state or is already stopped.
- Check for the existence of the `destroy` method before calling `janus.destroy()`.
- Allow the Janus player to be selected in `this.selectPlayer` regardless of the `streamChannel` on the Montage page. This will not cause visual confusion, as the Montage page does not allow selecting a `streamChannel` (it uses the default one).
However, if the default `streamChannel` is a secondary one and the default player is set to Janus, the Janus player would never be selected on the Montage page. I believe this is incorrect, as we should allow for more flexibility.
With its player left on Auto, a monitor with go2rtc enabled had
selectNextPlayer() pick go2rtc_webrtc first, and with ZM_GO2RTC_PATH
empty select_go2rtc() then alerted "ZM_GO2RTC_PATH is empty", on the
montage for every such monitor, although nobody had chosen go2rtc.
The fallback chain already skipped go2rtc for a monitor without it
enabled; skip it when ZM_GO2RTC_PATH is empty too. A monitor or browser
that chose go2rtc reaches select_go2rtc() directly and still gets the
alert.
A node test drives selectPlayer() through the restart chain for Auto
without a path, Auto with one, and go2rtc chosen explicitly.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ZoneMinder's RTSP server only starts when ZM_MIN_RTSP_PORT is set, and it
defaults to empty. The go2rtc and Janus managers decided to use the
restream from the monitor's RTSP Server flag (Janus: its restream flag)
alone, so with the port unset they built rtsp://127.0.0.1:0/<name>.
go2rtc was given that as the monitor's primary stream and as
<id>_ZoneMinderPrimary, neither of which can play; Janus pointed its
mountpoint at it.
Use the restream only when the port is set as well, and warn when a
monitor asks for it without one, so the camera's own stream is used.
MonitorStream applies the same test before choosing the
_ZoneMinderPrimary stream, which is no longer registered in that case.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
go2rtc serves its API under /api, and its websocket at /api/ws. zmc
appends /streams to ZM_GO2RTC_PATH and the browser appends /ws, so the
setting only worked if it ended in /api. The natural value to enter is
the server address, http://host:1984, and with that zmc reached
/streams, which is go2rtc's RTSPtoWeb compatibility endpoint, so no
stream was ever registered, and the browser's /ws was a 404. Where
another player was available the failure went unnoticed.
Normalise the setting where URLs are built from it: a trailing slash is
dropped and /api is added unless the path already ends in it, which also
covers go2rtc behind a reverse proxy at a sub-path.
- Go2RTCApiEndpoint() in zm_utils, used by zmc's Go2RTCManager.
- go2rtcApiUrl() in skin.js, used by MonitorStream and the console
thumbnail stream. It assigns the path once, since stripping the slash
from a bare host's "/" and appending gave "//api", as the old
pathname += '/ws' gave "//ws".
- The option's help says the suffix is optional.
Catch2 and node tests cover bare hosts, trailing slashes, an existing
/api, a proxy sub-path and credentials in the URL.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
EventStream receives the stream into a DOM <img> and copies it to the
canvas from a rAF loop. Gecko hands canvas drawImage() the frame the
img held when it was first decoded and never refreshes that snapshot,
so a multipart/x-mixed-replace img drawn to a canvas repaints one frame
forever while the stream keeps arriving. Montage review showed a single
frozen frame in Firefox while the timeline advanced; Chrome, whose
drawImage() tracks the element, played the same streams normally.
createImageBitmap() reads the frame the element is showing now. It is
async, so a tick is skipped while a request is outstanding rather than
queueing requests up behind the stream rate, and the pending flag is
cleared on teardown along with the img.
The img is left offscreen: measured against a reference
multipart/x-mixed-replace server, an img at left:-9999px, 1x1,
visibility:hidden still yields a fresh frame on every
createImageBitmap(), so Gecko is not culling the decode.
Verified against montage review on Firefox 154 and Chrome 154, driving
both from a script that hashes each monitor's canvas 40 times at 250ms:
before, Firefox saw 1 distinct frame per canvas and Chrome 40; after,
both see 40.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KLYswrguKc6bBYL8JZbpmZ
icons is the toolbar icon map that thirteen view scripts, and logpanel.js,
pass to bootstrapTable(). It was defined in skin.js, which xhtmlFooter()
loads last of all, after every view's own script. A client that loads the
whole page is fine, because skin.js still runs before DOMContentLoaded,
but one that stops short -- a crawler that caps resource loading, an
aborted load -- runs the view's ready handler against an undefined icons
and the table never initialises:
Uncaught ReferenceError: icons is not defined
at HTMLDocument.initPage (.../views_js_console-....js:458:12)
Move it to web/js/table-helpers.js, which already exists to hold what the
bootstrap-table views share and is loaded well before them, and export it
alongside the other helpers for the node tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reverts the previous commit's always-on measurement. Decoding audio for every
monitor that has it spends CPU on a number almost nothing reads, which is the
wrong trade even though it did solve the chicken and egg of picking a
threshold without ever seeing a level.
Measure when something is actually going to use the reading instead:
- AudioDetection is on, as before, so nothing changes for a monitor that
scores on audio; or
- somebody asked. SharedData gains audio_level_until, a wall clock second
the capture thread keeps measuring up to. The monitor editor's new level
meter pushes it forward while it is on screen and the measurement lapses a
few seconds after the page is left, so nothing has to send a stop and a
crashed browser cannot leave a monitor decoding forever.
When the reading stops being wanted the decoder is released and the published
level and peak are cleared, so a stale number is not left looking current and
an old peak does not land on the next frame row written.
audio_level_until is carved out of analysis_pad rather than appended, so
SharedData stays 888 bytes and no existing offset moves; the static_asserts,
Memory.pm and Monitor.php are updated together and all three now agree the
field is at +880.
The meter itself is on the audio settings, shown whether or not
AudioDetection is checked, because the level is what you need in order to
choose a threshold. It draws the threshold currently in the input as a mark on
the bar so a reading can be judged against it before saving, and a monitor
whose zmc is not running reads "no reading" rather than a confident 0, which
would be indistinguishable from silence.
Frames.AudioLevel is therefore 0 again on monitors that do not score on audio.
That is what the graph already treats as "no audio data", so it draws no line
rather than a flat one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
Gating the measurement on AudioDetection made the graph useless for the job
it is most wanted for. AudioThreshold is a per-device number -- the floor on
one camera's mic is nothing like another's -- so it has to be measured before
it can be set, but nothing was measured until it was already set. Enabling
detection with a guessed threshold to find out what the real one should be is
backwards.
The level is now read for every monitor with decodable audio. AudioDetection
governs only whether crossing the threshold contributes a score, which is
what the setting is named for. shared_data->audio_alarm stays 0 when it is
off, so nothing downstream changes for a monitor that does not want audio
alarms.
Nothing here depends on Analysing either. The measurement is in
Monitor::Capture, which runs on whatever Analysing is set to, and frame rows
come from Event::AddFrame, which a continuously recording monitor reaches
through the RECORDING_ALWAYS path with motion detection off. So a monitor
that only records continuously still gets levels on its rows.
Since Monitor::Capture retries Open on every audio packet until it succeeds,
and that now happens for every monitor with audio rather than the handful with
detection on, AudioDetector remembers a codec it has already failed to find a
decoder for. Without it a stream ZoneMinder cannot decode logs a warning at
the audio packet rate for as long as the monitor runs. A reconnect bringing a
different codec is still tried.
The cost is one audio decode per monitor with audio, where before it was one
per monitor with detection enabled. That is small next to the video path, but
it is not nothing on a box with many cameras.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
The cue strip under the video showed one flat red band per alarm period. It
tried to encode the score as a bar height, 'height: '+frame.Score+'px', but
never could: the frames ajax did not return Score, so every bar got
'height: undefinedpx' and fell back to the stylesheet's height: 100%. So the
motion level it looks like it is drawing has never actually been drawn.
Replace it with a line graph of both series over the length of the event. The
alarm periods stay, as a pale wash behind the lines, so nothing that was
readable before is lost.
The two series do not share a vertical scale. Audio level is 0-100 by
construction, but a motion score is a sum over zones with no upper bound, so
pinning both to 0-100 would flatten the audio line against the floor on any
event scoring above 100. Each is scaled to its own maximum, and hovering reads
out the exact values, which is what the numbers are wanted for. An event whose
rows are all zero -- recorded before the column existed, or by a monitor with
AudioDetection off -- draws no audio line at all, rather than a flat line
claiming silence was measured.
The readout goes into the existing #indicator, which already tracks the mouse
across the whole progress bar, instead of a second tooltip competing for the
same pixels.
The geometry lives in web/js/LevelGraph.js so it can be tested without a DOM,
and so the polyline and the hover readout cannot disagree about where a given
second sits. The bar grows from 1.25em to fit the graph, which needed two
consequential CSS changes: #indicator now spans the taller bar, and
.progressBox drops from 0.66 opacity to 0.25, because at full strength the
played part of the graph is unreadable behind it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
This can happen, for example, on the Console page when hovering over an image thumbnail, because the `video-stream` element created for go2rtc in `createGo2rtcStream()` is not assigned an ID.
The staleness window was unsound. calculateAuthHash() keys the hash to the
clock hour it was minted in and getAuthUser() accepts the last
ZM_AUTH_HASH_TTL hourly buckets, so a hash dies at the top of an hour rather
than at some age. generateAuthHash() then serves the cached one until it is
half a TTL old, so what arrives can already be nearly spent: on the defaults a
hash minted at 10:59 is still handed out at 11:58 and is refused at 12:00. A
client stamping that arrival as fresh for an hour skips the probe until 12:58
and restarts its streams on a dead hash - the exact failure this was written to
prevent. No fixed window is safe, because the remaining life of a hash we hold
can be anything down to zero, and AUTH_STALE_MS also ignored the configured
ZM_AUTH_HASH_TTL.
So drop AUTH_STALE_MS, authIsStale() and authFreshAt, and have whenAuthFresh()
revalidate. The one case that can still skip the probe is having no hash at all
- authentication off, or a relay form that does not use one - where there is
nothing that can expire and nothing a probe would report. revalidateAuth()
already shares one request between concurrent callers, so a resume that wakes
several of these still costs a single probe, and that is what the montage code
did unconditionally before any of this.
refreshTablesPendingVisibility() now returns as soon as it finds nothing was
deferred. It is bound on every classic page including the unauthenticated ones,
and the version before this ran the whole auth path on an empty queue, so
merely becoming visible could fire a probe with no work behind it.
Tests: two authIsStale cases removed with the function, two whenAuthFresh cases
added - a probe is sent and the callback held until it answers, and no probe is
sent when there is no hash. Reintroducing a fast path fails the first. Full JS
suite green, ESLint clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UkQwahn9pi1y4wJe9BTxjM
The probe went out as zmAuth.appendTo(...), carrying the very hash it existed to
replace. zm_authenticate_request() resolves a request against exactly one source:
a non-empty auth= in the URL enters the ZM_AUTH_HASH_LOGINS branch (on by
default), and when getAuthUser() rejects it the chain has already been taken, so
the userFromSession() arm below it never runs. A live session cookie then
authenticates as nobody.
Past ZM_AUTH_HASH_TTL - a tab hidden longer than two hours on the defaults, which
is exactly the case this change is for - the probe was therefore the one request
guaranteed to fail, and its failure is read as 'login', so the user was bounced
to the login page with a perfectly good session. That is worse than the 403s in
the log this set out to remove.
Send the probe bare. The session cookie is what answers, which is the question
being asked: who am I, and what is my current hash?
That also makes the failure handling mean what its comment claimed. A rejection
now really is a dead session rather than a dead hash, so redirecting to login on
it is right - and both 401 and 403 reach it, which the comment now says.
Also correct the AUTH_STALE_MS comment: authIsStale() is a strict comparison, so
a credential confirmed exactly AUTH_STALE_MS ago is still fresh, as the test
asserts.
Tests: tests/js/auth-helpers.test.js, 51 passed (4 new for revalidateAuth,
covering the bare probe, shared in-flight request, callbacks surviving a
transient failure, and login on a rejected session). Reverting the probe to the
credentialed form fails two of them.
refs #5093
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nr76CednxtDt2nPuq6WrbL
whenAuthFresh() gated on how long the tab had been hidden, which only covers
one of the ways the page stops hearing about the credential. montage's idle
timeout is another: hitting ZM_WEB_VIEWING_TIMEOUT stops every monitor, and
their status polls with them, while the tab stays visible the whole time. The
Are You Still Watching modal can then sit there for hours, so the auth hash
baked into the monitor srcs is just as dead as after an overnight sleep, and
a hidden-time gate would wave it straight through.
Track the age of the credential itself instead. ZMAuth.update() stamps it
whenever a reply carries auth_relay or auth - even when the value is
unchanged, since the server has still just confirmed it - and revalidateAuth()
stamps it too, so authentication being off doesn't leave every caller
revalidating once an hour forever. authHiddenTooLong(hiddenAt, now) becomes
authIsStale(freshAt, now); onAuthVisible() no longer keeps a timestamp.
That subsumes montage's refreshAuthAndStartMonitors(), which duplicated
revalidateAuth()'s navBar probe and fired a second one on every refocus.
Both call sites are now whenAuthFresh(startVisibleMonitors), which shares
the in-flight probe rather than racing it.
Tested: node tests/js/auth-helpers.test.js (47 passed), node
tests/js/table-helpers.test.js (8 passed), npx eslint on the changed files.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkE45gkMjnkTiJiUySbe7V
Hidden tabs have their timers throttled, and frozen/slept ones stopped
outright, so nothing refreshes the credential while the tab is in the
background. The server rotates the auth hash at half of AUTH_HASH_TTL
(generateAuthHash()), so the hash baked into a stream <img> src or a
deferred table url is usually dead by the time the tab is refocused. Every
restarted stream and table poll then fires a request that 403s and logs an
auth error before anything gets around to revalidating.
Record when the tab goes hidden and treat the credential as stale once more
than an hour has passed. Add whenAuthFresh(cb), which runs cb straight away
after a short alt-tab but queues it behind a revalidation after a long
sleep, so nothing makes an authenticated request on the expired hash.
revalidateAuth() now queues callbacks rather than dropping them when a probe
is already in flight, clears the hidden timestamp only once a reply actually
arrives, and still runs the queued callbacks on a transient failure - a
network blip is no reason to leave the page's streams stopped. A 401 still
redirects to login and drops the queue.
Gate the two visibility-driven callers on it: watch.js startPage() and
table-helpers.js refreshTablesPendingVisibility(). montage.js already
refreshes auth before restarting its monitors.
Tested: node tests/js/auth-helpers.test.js (48 passed), node
tests/js/table-helpers.test.js (8 passed), npx eslint on the changed files.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkE45gkMjnkTiJiUySbe7V
select_zms() has three ways out, and two of them send a stream command before
the tail of the function sets started. streamCommand() drops anything sent while
that is false, so such a branch reports success having sent nothing and the
picture sits on its last keepalive frame.
The resume branch was fixed on this branch already. The branch below it, for a
page rendered with mode=paused, has the same shape and was missed. With auth on
it needed a still-valid hash to reach, so it was intermittent; with auth off,
where there is no hash that can go stale, the srcAuthCurrent change on this
branch makes it the path every initial load takes. Set started there too.
Add tests/js/monitorstream-resume.test.js, which asserts what reaches the wire
rather than which branch ran: the resume and initial-paused paths each send
exactly one CMD_PLAY on the connkey they are supposed to address, resuming
leaves src and connkey alone, and a stale auth hash still rebuilds and quits the
process the old connkey addressed. Removing the one-line fix fails the
initial-paused case and leaves the other three passing.
Full JS suite passes, ESLint clean on both files.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UkQwahn9pi1y4wJe9BTxjM
select_zms() has a branch that resumes an existing zms with CMD_PLAY rather
than rebuilding the stream, but three things stopped it ever working. Every one
of them showed up only when the page was driven for real.
stop() ends by clearing activePlayer, which is the only thing that branch
tests, so after any stop it was unreachable and start() fell through to "new
src, new connkey" - a second zms, the first left running and no longer
addressable. That is the montage page's accumulating processes: hide the tab,
the handler stops each stream, and returning replaces rather than resumes. The
same happens when a monitor is scrolled out of view and back. stop() now
remembers what it shut down, and select_zms() resumes on that as well as on
activePlayer, provided we still hold the connkey to address it.
srcAuthCurrent required a non-empty zmAuth.hash, which is '' whenever auth is
off or the relay carries no hash. There is nothing that can go stale in that
case, so the src is as current as it will ever be; requiring a hash sent every
such install down the rebuild path for no reason.
streamCommand() drops anything sent while !started, and started is not set
until the end of select_zms(), so the resume issued its CMD_PLAY into nothing
and the stream stayed stopped. Resuming after a pause worked only because
pause() leaves started set. It is now set before the command goes out.
Finally, the rebuild path clears the "Loading..." info block from img_onload,
which cannot fire on a resume because src never changes. Without clearing it,
a stream that had in fact resumed sat behind that block and its still image and
looked frozen - the fault that made this look unfixable at first.
restart() is excluded from all of it: it is the error path, whatever failed may
be that very zms, and a broken img is not repaired by CMD_PLAY.
Verified on a live montage page with two monitors: across two hide/show cycles
the same two zms processes are kept - no orphans, no respawn - and both
pictures are live afterwards, with the zms status reporting stopped=0 and
~15 fps.
refs #4706
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
select_zms() mints a fresh connkey whenever it has to rebuild the stream src,
without telling the process the old connkey belonged to that it is finished.
Once the key is replaced nothing can reach that process again: no CMD_QUIT can
be delivered, and a stopped one will not notice on its own.
getStreamCmdResponse() already learned this - its reload path calls
quitConnKey() first, with a comment saying why - but the path every ordinary
start() takes did not. quitConnKey had exactly two references in the file: its
own definition and that one use.
refs #4706
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Montage review created 86 stream connkeys in five minutes while only two of
them were ever polled, and each stream painted a single frame before being
replaced about 1.3s later.
The status poll for a connkey whose zms had exited returned result=Error,
recover() restarted the stream under a new connkey, and the reply already
queued for the old one arrived afterwards and restarted the new stream too.
Every restart painted its first frame, which reset consecutiveErrors and
recoveryDelay in img.onload, so the backoff never grew and the attempt limit
was never reached.
- Tag each ajax/stream.php exchange with the connkey it was issued for and
ignore a reply, success or failure, once that connkey has been replaced.
stream.php does not echo the connkey back, so the client tracks it.
- Restart only on reason=no_socket, the one class that means zms is gone,
matching streamErrorIsFatal() in MonitorStream.js. The helper is duplicated
as EventStream.errorIsFatal() because montagereview.php does not load
MonitorStream.js.
- Start the poll timer next to the src that created the connkey instead of in
img.onload, which is not a dependable per-restart signal for a
multipart/x-mixed-replace img, and let the first query wait a full interval
so a stream that is still starting is not read as a missing socket.
- Reset the recovery counters only on a successful status reply, so the
exponential backoff and the attempt limit both work.
Also stop appending a second '?' to UrlToZMS, which already carries
'?monitor=N', so the logs no longer show monitor=24?source=event.
tests/js/eventstream-connkey.test.js covers the stale-reply rules, the fatal
classification and the URL separator.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019YEe1M3PWVMqYWihUeAa5N
-- Check isActive first.
-- Run error registration
- When checking for errors in nextCycleView(), check monitorStream.player instead of (monitorStream.selectedPlayer || monitorStream.activePlayer)
- When executing MonitorStream.restart(), do not clear the player's error count. This will allow us to analyze playback issues. We do this clearing elsewhere.
- Instead of Math.random(), we'll use Date.now().
This will make the code cleaner and more understandable.
- We also now use refreshStreamSrc() when stopping stream playback.
- When closing a socket, check the session before clearing the WebSocket, not after.
- Execute resetCountStreamErrors() when playback starts successfully.
- Recheck the session inside videoEl.onplay after the 500ms timeout.
- Exit this.img_onload if the monitor is already stopped.
- Assign the 'zm:tracksReceived' listener only after stream playback has started, not during start().
- Additional processing of MonitorStream.playbackSessionId
- Calling stream.load() inside stop() for the rtsp2web type MSE is redundant and can lead to unnecessary warnings. We do everything necessary in stopMse().
- When restarting a stream, call updatePlayerControls() instead of streamCmdStop() to avoid duplicating the stop() command (relevant for the Watch page).
- Now, to switch to the next player, we won't call MonitorStream.selectNextPlayer() in different places in the code. Instead of selectNextPlayer(), we will always call restart(), which will select the next player. This will allow us to more accurately clear all unnecessary objects. To achieve this, we have slightly modified restart(). - For go2rtc, execute playbackSessionId = generateUUID() when changing the SRC, not when selecting a player, since go2rtc itself can change src.
- Avoid looping when executing selectNextPlayer() during errors in the ZMS player.
- Added the "fatal" argument to streamErrorRegistration(), which prevents unnecessary relaunches of the same player that was previously playing in the event of a fatal playback error.
- Added a setter for src to the VideoStream class for go2rtc.
- Added support for monitorStream.isActive for the Watch page.
- Added the updatePlayerControls() function for the Watch page to manage the state of the player buttons. In the future, we need to create a single function for managing the state of the player buttons. It's a bit of a mess right now. We'll do this in the next PR to avoid breaking anything.
- To optimize performance, part of the code from getTracksFromStream() has been moved directly to the listener in MonitorStream.
Applying a named filter and clicking LIST MATCHES landed on Events showing
"No matching records found" until a manual refresh. Two independent defects
produce that, and both are fixed here.
Stored filter selections overriding the applied filter
------------------------------------------------------
The zmFilter_* cookies remember what was last selected in console, montage and
montagereview. They are a convenience for the default, filter-less page, but
they were also applied on top of a filter the request had specified, silently
widening or narrowing it: the applied filter was ANDed with a date range left
over from an earlier visit to another view, so it matched nothing. Refreshing
appeared to fix it because the bar's reset control blanks those inputs.
The cause was a layering one. Filter::simple_widget() refilled any empty term
value from that term's cookie as it rendered the input, so it overrode the value
addTerm() had been given, from below the layer that knows what the request asked
for. An earlier attempt to fix this in events.php could not hold for that
reason: it blanked the values and they were refilled during rendering, and
ajaxRequest() sends the live inputs rather than the URL.
Filter now resolves nothing. It renders the value it was handed, and the six
cookie fallbacks are gone; terms keep their cookie name so edits still persist
client-side. The callers that build those terms decide instead, next to
getFilterSelection(), which already resolved request-before-cookie this way.
Each resolves its value into a local before the addTerm() calls, testing
$use_stored in the same statement that reads the cookie, so the rule is visible
at the point it applies.
montagereview needs a window to draw whatever happens, so with a filter present
it derives one from the filter's own terms and otherwise defaults to the last
hour, rather than reaching for the stored window; its Notes term no longer seeds
from a cookie inside the branch that already has an explicit filter.
Tables never retrying a request skipped while hidden
----------------------------------------------------
The table views skip their ajax request while the page is hidden so a background
tab does not poll. Bootstrap-table calls that function on init as well as on
refresh, though, and a skipped request was never re-issued: the table rendered
"No matching records found" over a result it never asked for, with nothing to
bring it back. A page can be hidden for the whole of its load - opened in a
background tab, restored, or behind another window - and the same guard is in
seven views: events, console, log, frames, reports, snapshots and watch.
deferTableRequestWhileHidden() skips the request as before and records the table,
so it is refreshed the moment the page becomes visible. The queue is drained
before refreshing, because refresh() calls the ajax function synchronously and
would otherwise re-add a table that is still hidden.
Tests
-----
tests/audit-filter-cookies.php checks both halves of the first rule: that Filter
resolves nothing, and that every stored-selection read in a caller tests
$use_stored. It works a statement at a time, joining continuation lines, since a
value and its guard often span a line break. Anything wider is too coarse: the
enclosing block holds other guarded reads and would mask one that lost its own.
tests/js/table-helpers.test.js covers the deferral queue, including a table
re-deferred during its own refresh.
Verified against a live instance with the stale cookies still set: a "last hour"
named filter queried 0 of 4 matching events before and 12 of 12 after; the
filter-less page still restores the stored date range; and a table deferred while
hidden repaints on becoming visible.
applyTo appended the relay to an empty src, yielding a bare '?auth=...'.
Callers that treat a blank src as "nothing to load" would then set it on an
<img>, where it resolves against the current page and loads the surrounding
HTML as an image. montagereview's loadImage2Monitor is one such caller.
Introduced with ZMAuth; the code it replaced returned the src untouched.
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
