The alarmed/filtered pixel check methods handed Overlay() the raw GRAY8
scoring mask. Overlay keys on a non-zero source pixel and copies that byte, so
the highlight took whatever shape one byte has in the destination format: the
red channel on an RGB32 monitor, which is the whole reason alarms have always
come out red; all three channels on RGB24, giving white; luma alone on YUV420.
The zone's configured Alarm Colour was honoured only on the blob path, which
goes through HighlightEdges.
Generalise HighlightEdges into BuildHighlight, which takes an edges_only flag
and otherwise fills every marked pixel, and build the highlight for the pixel
methods the same way the blob path already builds its outline: in the
capture's own pixel format, carrying alarm_rgb, once scoring is finished with
the GRAY8 mask. HighlightEdges stays as a thin wrapper so the blob path and
its callers are unchanged.
This is a behaviour change: monitors left on the default red see no
difference, but a zone configured with any other Alarm Colour now paints that
colour instead of red or white.
Reverting the zone hunk fails the new test in all three of its sections.
Full suite: 148 cases, 12535 assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WBHBB95RBX7D9p8ge2WDZb
Monitor::connect() returns false with shared_data still null on every one of
its failure paths: the mmap file cannot be opened (wrong ownership, e.g.
after a package upgrade), fstat fails, ftruncate cannot grow it (/dev/shm out
of space -- a container with the default 64MB tmpfs hits this quickly, since
one 720x480 monitor with 10 buffers already asks for ~20MB and a 1080x720 one
asks for ~62MB), or mmap itself fails.
zmc's startup loop reacts by retrying:
while (!monitor->connect() and !zm_terminate) {
Warning("Couldn't connect to monitor %d", monitor->Id());
monitor->SetHeartbeatTime(std::chrono::system_clock::now());
sleep(1);
}
so the first thing it does after a failed connect is write through the null
pointer. zmc dies with SIGSEGV at address 0x80 instead of retrying, which
presents as a monitor that will not start and a capture daemon that keeps
crashing. The accessors on either side of these already guard with
`if (shared_data && shared_data->valid)`.
Reverting the guard fails the new test with SIGSEGV at zm_monitor_shm.cpp:66,
and restoring it passes. Same fix as release-1.38's a550545e4.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WBHBB95RBX7D9p8ge2WDZb
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
zm_update-1.39.31.sql gave monitors audio detection, but the level only ever
existed in shared memory, so it was gone the moment the frame passed and there
was nothing for the event view to plot. Add Frames.AudioLevel next to Score, on
the same 0-100 dBFS-derived scale the threshold uses.
What is stored is the peak since the previous row, not the level at the instant
the row was written. Frames rows are written well below the capture rate --
only alarm, bulk and score-increasing frames get one -- so sampling at write
time would drop exactly the short loud noises worth seeing on a timeline.
AudioDetector accumulates the peak as it decodes and Event::AddFrame takes it
where the row is built, which clears it so each row covers its own interval.
The Event constructor takes and discards it once, otherwise an event's first
row reports the loudest moment since the previous event ended.
This needs no shared memory change: zma is now an offline re-analysis tool and
the live analysis runs in a thread of zmc, alongside the capture thread that
runs the decoder, so the peak can stay in the AudioDetector. SharedData keeps
its documented 888-byte layout and its fixed offsets.
The frames ajax returns the column, and Score with it. elements in
web/ajax/status.php is a whitelist that never listed Score, which is why the
event view's cue strip has been reading an undefined Score off every frame.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpiSWBmtQkR5bcgpHWY4ME
Two problems on the exit paths out of the runStream loop, both raised in review.
paused_image and stopped_image were raw pointers freed only by a later iteration
observing that the state had changed. Any break leaves the function with one
still allocated: the stopped-state ttl break added by this branch, and the ttl
and frames_to_send breaks at the bottom of the loop that predate it. Hold both
in unique_ptr so the exit path stops mattering.
More seriously, those breaks leave zm_terminate false. StreamBase::checkCommandQueue
loops on !zm_terminate around a recvfrom with a one second SO_RCVTIMEO, so it
returns only once that flag is set, and runStream joins it unconditionally when
connkey is set. A stream that ended on its ttl or its frame count therefore sat
in join() rather than exiting, which is another way for a zms to outlive its
client. Set the flag on all three.
The two sendFrame failure breaks and the feof/ferror breaks already did this,
which is what the pattern should have been throughout.
Verified by reading: checkCommandQueue's only loop condition is the flag. Not
reproduced end to end, because no zmc is running on this machine and a stream
against a monitor with no capture takes the not-initialised path instead, which
already sets the flag correctly on its own timeout.
zms builds clean with no new warnings. C++ suite 133/133 on this branch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UkQwahn9pi1y4wJe9BTxjM
A zms in the stopped state could never exit. The branch handling it slept and
continued without writing anything, so the socket was never touched and a
client that had gone away was never discovered - no write, no EPIPE, no
SIGPIPE. The continue also skipped the ttl check at the bottom of the loop, so
even a deadline set by the caller did not apply. The only way out was an
explicit CMD_QUIT, and any stream that missed one stayed until the machine was
restarted.
That is what accumulates on the montage page: the tab is hidden, the browser
side sends CMD_STOP, and if the stream is later replaced rather than resumed,
nothing can address the old process again and it sleeps forever.
Hold the last captured frame on entering the stopped state and re-send it
every five seconds, the way the paused state already does, and apply ttl here
too. The frame is what makes a departed client detectable; it also keeps the
connection open for the resume that stopped is meant to allow, which is what
the state was documented to do. Where nothing has been captured yet there is
no frame to hold, so a text frame is sent instead - the write matters more
than what is in it.
setLastViewed is deliberately still not called: capture and decoding should not
be held active for a stream that is not playing.
refs #4706
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
macOS wraps machine context as a pointer to __darwin_mcontext64 with
registers under __ss, so the Linux uc_mcontext.pc / gregs access does
not compile. Add __APPLE__ branches for aarch64 and x86_64.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
InetSocket::bind creates its socket inside the getaddrinfo loop and
called ::bind without SO_REUSEADDR, so a connection lingering in
TIME_WAIT on the same port fails the bind with EADDRINUSE. Seen as
zm_comms test failures on macOS when the send/recv test runs before
the server bind tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ad70d5e80 added alarm_actions_fired to the Monitor constructor's
init list between audio_alarm_score and wallclock_timestamps, but
declared the member far later in the class, after linked_monitors.
Members are initialized in declaration order regardless of the order
written here, so gcc rejects it under -Werror=reorder:
src/zm_monitor.h:767:8: error: 'Monitor::alarm_actions_fired' will
be initialized after
src/zm_monitor.h:621:19: error: 'bool Monitor::wallclock_timestamps'
This is a plain bool with a constant initializer, so nothing observable
changes - only the order the entry is written in. Moved it to sit after
linked_monitors, matching where the member is declared.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B5KL9Xbi7K5aGsauLtd8tG
PacketQueue::get_event_start_packet_it() allocates a packetqueue_iterator and
registers it in PacketQueue::iterators. Every path out of Monitor::openEvent()
hands that pointer to the Event, which releases it in ~Event via free_it, except
the one that fails to lock the starting packet. That path returned nullptr and
left the iterator registered for the life of the process.
clearPackets() takes min_iterator_queue_index across all registered iterators and
stops removing at the first packet whose queue_index reaches it. An abandoned
iterator pins that to the front packet, so the trimmer removes nothing on every
invocation, and deletePacket() advances registered iterators onto the packet
after the one it removed, which drags the orphan onto each new front packet so it
can never age out. clear_packets_pending_ then latches true, downgrading the gate
so the futile scan re-runs on every video packet rather than only on keyframes.
The only removal path left is the emergency one-GOP eviction in queuePacket, so
the queue settles into refill a GOP, exceed max_video_packet_count, warn, evict a
GOP, repeat. It is pinned at its cap by construction, which is why the monitor
warns once per GOP at a constant rate for as long as zmc runs and no amount of
idle time or extra headroom recovers it.
Our trigger was an RTSP stream EOF mid-event during a reconnect on an ONVIF
camera. It is a race, so it leaks only when openEvent loses it, and each
occurrence leaks one more iterator.
Add tests covering both halves: that free_it takes an event start iterator back
out of the queue, and what an abandoned one does to trimming until it does.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Event::updateNotes only inserts into the note set and rewrites the Notes
column each time it gains a string. The audio note carried the live level, so
every distinct reading added another entry and another database write, and the
event ended up recording the whole climb:
Audio: level 10, level 11, level 12, ... level 78, ...
Measured on this install before the fix: monitor 1 reached 946 bytes of notes
and averaged 117, against 8 to 12 bytes on monitors without audio detection.
The note is now the constant AUDIO_CAUSE, matching the ONVIF and Amcrest notes
a few lines above. The level is still in the Debug line, which is where a
per-frame reading belongs.
Existing events keep their bloated notes. Rewriting recorded history to tidy
up would be worse than leaving it to age out with normal event deletion.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
Alarm turns a light on when motion starts; nothing turned it back off. The
existing EventEnd is not the answer: the alarm ends when the monitor leaves the
alert state, while the event carries on recording for the rest of its section,
which on a continuous-recording monitor is up to ten minutes later.
Analyse() leaves the alarm condition by three paths and all three fire it: the
normal ALERT to IDLE, a signal change, and the trigger being turned off. The
abnormal two matter most - a light switched on by an alarm must not stay on
because the camera lost signal.
That needs the firing to be paired, so alarm_actions_fired is set when Alarm
fires and cleared by EndAlarmActions. It guards both directions:
- the three call sites can run back to back, so without it one alarm could
fire AlarmEnd several times
- signal loss and trigger off also run on monitors that never alarmed, and an
unpaired AlarmEnd would switch a light off for an alarm that never happened
- the ALERT to ALARM re-trigger inside one incident deliberately does not
re-fire Alarm, so a single AlarmEnd still has to balance it
The migration MODIFYs the enum and appends the value, so stored rows keep their
meaning; appending to an enum does not renumber what is already there.
Tested: 8 assertions over the pairing rule, run standalone because the real
class needs a database and a camera - the ordinary pair, repeated calls firing
once, an unpaired call firing nothing, two alarms pairing independently, and
the re-trigger case. The action test now also requires every trigger name to
round trip through the TriggerOn enum, so an action saved by the editor cannot
load with a trigger the C++ does not recognise and fire at the wrong moment.
Build clean at 1.39.33, Perl 15 files / 249 assertions, ESLint 0 problems,
tests/js 154 assertions, php -l clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
A deprecation warning without a version attached is one people learn to
scroll past. Both notices now say deprecated as of 1.40, removed in 1.41.
Removal is 1.41 rather than 1.40 because 1.39.x is the development series.
Anyone running stable is on 1.38 and will never see a 1.39.x warning, so
removing in 1.40 would take the feature away in the same release that first
mentions it - no notice at all for the people it affects. Warning in 1.40 and
removing in 1.41 gives stable users one release to migrate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
ZoneMinder carries its own RTSP/RTP implementation - zm_rtsp.cpp,
zm_rtp_ctrl.cpp, zm_rtp_source.cpp, zm_rtp_data.cpp, zm_rtp.cpp and
zm_remote_camera_rtsp.cpp, about 66KB in total. It is hand-written parsing of
untrusted network input, which is the least rewarding kind of code to own, and
Ffmpeg already does the same job for more cameras and is maintained upstream.
This marks it deprecated. It does not remove anything and does not change how
any existing monitor captures.
The capture log now names the replacement instead of only saying the method is
going away:
Monitor 7 (Driveway): the Remote/RTSP capture method is deprecated and will
be removed. Change this monitor to Type 'Ffmpeg' with Source Path
rtsp://10.0.0.5:554/live
Monitor::RtspUrlFromRemote builds that URL from the Host, Port, Path, User and
Pass the monitor already stores. It is pure so it can be tested without a
camera, and it is the piece a conversion migration will need when the code is
finally deleted.
The URL is passed through remove_authentication() before being logged, the
same helper zm_ffmpeg_camera.cpp uses. A deprecation notice that wrote camera
passwords into /var/log/zm would be a bad trade.
Credentials are percent-encoded. A password containing @ or : otherwise splits
the authority in the wrong place and yields a URL pointing at a different host,
which fails in a way that is hard to read.
In the editor the protocol is now labelled "RTSP (deprecated)" and selecting it
explains what to change to. Only the label moved: htmlSelect still emits
value="rtsp", so updateMethods and the stored value are untouched.
No auto-converting migration. Deprecating and removing are separate steps, and
silently rewriting a working monitor to Ffmpeg could break a camera that Ffmpeg
handles worse - which is the reason to warn a release ahead of removing.
Tested: 16 assertions over the URL builder, run standalone because ctest still
cannot build against the Catch2 v2 installed here - empty credentials producing
no dangling userinfo, user without password, @ : / ? # and space in passwords,
a username needing escaping, missing leading slash, empty path, absent port,
query strings surviving, and that the masked form drops the password while
keeping the host. Full build clean, Perl 15 files / 249 assertions, ESLint 0
problems, tests/js 154 assertions, php -l clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
ZoneMinder has carried audio for years without ever listening to it: the
packets go into the event and nothing reads them. AudioDetector decodes
them in the capture thread and reports a 0-100 level, which Analyse()
turns into score alongside motion, ONVIF and Amcrest.
The level is dBFS-derived, not a raw amplitude ratio. Linear RMS is
unusable as a setting: ordinary speech sits at 1-3% of full scale, so
every sound worth catching would be crammed into the bottom two points of
the range and no operator could tune it. Mapping -60..0 dBFS onto 0..100
puts speech around 43 instead.
Three columns on Monitors: AudioDetection to enable it, AudioThreshold
for the level to alarm at, and AudioAlarmScore for what it contributes.
AudioAlarmScore defaults to 9, matching what an ONVIF or Amcrest alarm
already adds. A threshold of 0 means off, so enabling detection without
choosing a threshold cannot alarm on silence -- a plain level >= threshold
test would alarm on every packet in that state.
The level and the alarm flag go in SharedData's two spare bytes, renamed
from reserved1/reserved2. Offsets and sizes are unchanged, so the
888-byte cross-process layout and the Memory.pm and Monitor.php offset
tables all still agree; the readers are renamed in the same commit so the
level is available to the web UI.
The decoder is opened lazily on the first audio packet rather than at
camera setup, so a monitor with detection off never carries one and a
stream that gains audio on reconnect still gets picked up. Scoring reads
the capture thread's most recent level rather than scoring per audio
packet, because the score belongs to a video frame and audio packets do
not arrive in step with them.
Tested: 242 assertions over the pure helpers - the RMS of both sample
formats, the dB scale's monotonicity and endpoints, full-scale clamping
of decoder overshoot, and the threshold-0 case. The repo's Catch2 harness
needs v3 and only v2 is installed here, so tests/zm_audio_detector.cpp
was compiled against a v2 shim to check it, and an identical set of
assertions was run standalone.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
LoadActions() ran once during setup, so an action added or edited in the
monitor editor did nothing until that monitor's zmc was restarted. Zones
are already re-read by the Load() call in Reload(), and actions have the
same lifetime and the same editing pattern, so the two should behave the
same way.
LoadActions() clears the vector before filling it, so calling it again is
idempotent and the reload cannot accumulate duplicates.
Not covered by a test: the action tests exercise the pure mapping and
message-building functions, and there is no fixture for a database-backed
Monitor to assert the call from Reload(). Verified by inspection and a
syntax check.
A camera that detects motion should be able to sound a speaker, and the
speaker is rarely the camera: it is a separate device with its own address,
credentials, stream and Controls entry. Model it as a monitor and let a
monitor's alarm drive actions on other monitors.
Add Monitors.DeviceClass enum('Camera','Speaker'). This is what the device
is, as distinct from Type, which selects the capture backend - an IP speaker
still captures over Ffmpeg like any other RTSP device, so Type could not
carry the distinction.
Add the MonitorActions table: MonitorId is the monitor that triggers,
TargetMonitorId the device acted on, and the two are frequently different.
TriggerOn covers EventStart, EventEnd, Alarm and Manual.
Which actions a device is offered is decided by its Controls row - CanLight,
CanIndicatorLight, CanAudioPlay - so a device can only be asked to do what it
has been measured to do. The editor filters on this and the save path
re-checks it, because the request is not to be trusted.
Execution goes straight to the target's zmcontrol socket rather than forking
zmcontrol.pl per action: the daemon already accepts a line of JSON there, and
it is the same path the control panel uses. ActionCommandName maps the DB
enum onto a method name as a whitelist, so nothing out of the database
reaches the control daemon uninspected. Actions are fire-and-forget - a
speaker that is offline is logged and skipped, never allowed to hold up event
handling.
Alarm actions fire only on the genuine entry into alarm, not on the
ALERT->ALARM re-entry, which would re-sound a speaker within one incident.
EventEnd runs on the calling thread before the event is handed to the closing
thread, which does not capture `this`.
Manual actions appear as buttons on the watch page, and are the reason the
control panel is now shown for a monitor that has actions but no control of
its own. Firing one sends only the action id; the command and target are
rebuilt server side, and Control rights are required on the target device and
not merely on the monitor the action hangs off.
Also add --file to zmcontrol.pl, without which audioPlay could not be driven
from the command line. The web path was unaffected as it bypasses GetOptions.
Tests: tests/zm_monitor_action.cpp covers the command whitelist, the message
format (including that file id 0 is a real id and that a stale AudioFile is
never passed to a command that takes none), trigger names, and that every
value of the ActionType enum maps to a command.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTCzCbvGt8xKQRNCSA7o8
(cherry picked from commit b561341e988af69c3a46db854da410309f7cfa82)
Monitor::Pause() reset shared_data->last_write_index to image_buffer_count,
the "nothing written yet" sentinel. Once an OnDemand monitor went to sleep the
last captured image became unreachable, so zms mode=single gave up waiting and
returned "No image available." mode=jpeg worked only because runStream calls
setLastViewed() each iteration, waking capture for a fresh frame.
The zmc OnDemand loop also paused before capturing anything: on a fresh shm
last_viewed is 0, so the first iteration paused a primed camera and no initial
image was ever written for the console thumbnail. The GetLastWriteIndex()
guard that prevented this had been removed because Pause() clobbering the
index made it cycle Pause/Play.
Leave last_write_index alone in Pause() and restore the guard so capture
continues until one image has been written. The index then stays valid, so the
pause is stable and mode=single serves the last captured image.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A hardware encoder has a fixed number of sessions. When one event ends and
the next begins immediately, the outgoing event's encoder is torn down on
close_event_thread, so for a moment both hold a session and the new one is
refused. That is a race against a teardown already in flight, not a real
capacity limit, and it currently costs the recording its encoding.
VideoStore::open now waits for an in-flight close before giving up: if no
encoder would open and there was a close to wait for, it joins that close and
tries once more. It only serialises when it has to, and only when there was
something to wait for. If the retry also fails the passthrough fallback still
catches it.
Most of the diff in zm_videostore.cpp is indentation: the codec loop is
unchanged and simply moved into a lambda so it can be run twice. It was
already safe to re-run -- each iteration parses its own options dictionary,
because opening destroys it, and frees its context on failure.
The rest is the threading this needs. close_event_thread was reachable from
two threads with no lock of its own: closeEvent() on the analysis thread joins
it and assigns a new one, and Pause() on the capture thread joins it twice.
That is a data race today, before any of this. It now has
close_event_thread_mutex, and all four accesses take it, including the new
Monitor::WaitForEventClose.
No lock cycle: WaitForEventClose takes only that mutex, and the thread it
joins takes no Monitor lock at all -- Event's destructor reaches Monitor only
through Substitute and EventPrefix, neither of which locks, and its database
work goes through the queue's own mutex. So a caller holding the event lock
cannot deadlock against a caller holding this one. Pause() takes it after
closeEvent() returns rather than around the call, which would self-deadlock.
Suite unchanged at 12171 assertions in 133 cases, and clean under
ThreadSanitizer with no warnings -- though that is weak evidence for this
particular change, since nothing in the suite drives Monitor's event close
path. The retry itself is still unexercised: provoking it wants two events
back to back on a monitor whose encoder has a session limit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
When a monitor is set to ENCODE and every candidate encoder refuses to open,
VideoStore::open returned false and the event recorded no video at all. On a
hardware encoder that refusal is usually the card declining for want of
capacity, reported to ffmpeg as nothing more specific than a generic external
error, and it clears on its own once other events finish. Losing the
recording outright is a poor answer to a transient condition.
It now copies the input stream and writes its packets through unchanged, the
same as a monitor configured for PASSTHROUGH. The recording is not re-encoded,
which is what the operator asked for, but it exists.
Two things follow from that:
The copied stream needs the guard the PASSTHROUGH path already has against an
input reporting no dimensions. A 0x0 stream cannot be copied into the muxer,
which aborts writing the trailer rather than returning an error, so that case
still declines video and the event keeps its jpegs.
Callers have to ask the videostore whether it is encoding, not the monitor.
Event::AddPacket_ used the monitor's setting to decide a packet could start a
recording without being a keyframe, which is true while encoding and false for
copied packets. It now asks VideoStore::Encoding(), which reports the
monitor's setting unless open() fell back.
Ported from the ai_server branch, reduced. Left behind: its retry that waits
on a previous event's encoder teardown before giving up, which needs a mutex
around Monitor::close_event_thread and an audit of master's three existing
accesses to it -- a threading change to Monitor rather than an addition to
VideoStore, and worth its own commit. Also left behind the Quadra-specific
card selection inside the encoder-open path, which belongs with that backend.
NOT covered by a test. Reaching the fallback needs every encoder on the
machine to refuse a real open, and asserting the outcome needs a recorded
event to inspect; there is no scaffolding here for either. Verified by build
and by reading the resulting control flow. Full suite unchanged at 12171
assertions in 133 cases, no new warnings. Wants exercising against a monitor
whose configured encoder cannot open before it is relied on.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
zmDbDo, zmDbDoInsert and zmDbDoUpdate all decided what to do with a failed
query by testing for ER_LOCK_WAIT_TIMEOUT alone, which got both halves of
lock contention wrong:
- ER_LOCK_DEADLOCK was not retried at all. InnoDB resolves a deadlock by
rolling one side back and expects that side to re-run; instead the query
was logged and abandoned. Event creation goes through zmDbDoInsert, which
is where this actually bites.
- ER_LOCK_WAIT_TIMEOUT re-ran immediately and forever, with no delay and no
attempt limit, so two writers deadlocking against each other kept
colliding on the same schedule.
Both now go through one retry decision: five attempts with a jittered
50ms-doubling backoff, then give up and report. The jitter is what stops
two contending sessions waking together and repeating the deadlock.
The wait happens under db_mutex, which every other database user in the
process is blocked on, so the budget is deliberately small -- about 3.1s
across all five attempts. That is still far less than the unbounded
ER_LOCK_WAIT_TIMEOUT loop it replaces, where each round costs a full
innodb_lock_wait_timeout. The change in behaviour is that a query which
would eventually have won after many minutes is now abandoned; it is
reported at Error with the attempt count.
The error is now logged once, when giving up, rather than on every round.
mysql_errno is read next to mysql_error rather than after the logging
call, so it cannot be clobbered in between.
Two other things, both small and both in the same area:
zmDbEscapeString called mysql_real_escape_string unconditionally, and that
reads the character set off the connection, so a closed handle sends it
into freed state. It now falls back to escaping the injection-relevant
characters itself. That fallback is only correct because the connection is
utf8mb4, where no byte of a multi-byte sequence is ASCII and so no sequence
can absorb a trailing backslash; the comment says so, since it would be
wrong for a character set like GBK. It deliberately does not take
db_mutex to read the flag: the logger calls this from Error(), and
zmDbFetch reaches Error() while holding db_mutex, so locking here would
self-deadlock the process on any failed query.
zm_rtsp_server was the one daemon closing the database without stopping
the queue that writes through it. Fixed at the call site rather than
inside zmDbClose, which holds db_mutex while the queue thread needs that
same mutex to drain -- joining it from in there would deadlock.
Tests: tests/zm_db_contention.cpp covers the retry budget only, 2011
assertions. Verified it fails when the budget is removed. The retry loop
itself needs a database and two contending sessions and is NOT covered;
it wants verifying against a real server under contention before this is
relied on. Full suite 12171 assertions in 133 test cases. Builds clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
The parameter existed so the predicate could be called without the gSOAP
headers, but src/CMakeLists.txt:163 sets WITH_GSOAP as a PUBLIC compile
definition on the zm target, so it already reaches the tests target that
links it. The test could name SOAP_FAULT directly all along, and passing
it in only meant every caller repeated the same constant.
The declaration and definition move inside the WITH_GSOAP guard, where
SOAP_FAULT is in scope, and the test is guarded to match.
The predicate stays a free function rather than folding back into
WaitForMessage: it is six conditions over two nullable strings, and
covering it in place would mean constructing an ONVIF object and a live
soap context. The comment now says that, instead of claiming a header
dependency that was not real.
Tests: 16 assertions unchanged and passing. Full suite 10160 assertions in
132 test cases. Builds clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
WaitForMessage decided whether a failed PullMessages was an authentication
refusal by looking only for auth wording in the SOAP fault string. That
misses two shapes:
- A camera that rejects at the HTTP layer never produces a SOAP envelope.
gSOAP returns the status code as the result and there is no fault to
read, so a 401 fell through to the generic "Failed to get ONVIF
messages!" branch. That branch logs at Error on every poll, where the
auth branch logs once and then drops to Debug, so a camera in this state
filled the log at the polling rate for as long as it stayed unhappy.
- Several cameras leave the fault string generic and name the reason only
in the fault detail, which was not examined at all.
Both now route through the auth branch. Recovery is unchanged: either
branch increments retry_count, unsubscribes and marks the monitor
unhealthy, so this only affects which message is logged and how often.
The predicate moves to ONVIFIsAuthError, outside the WITH_GSOAP guard so
it can be tested without the gSOAP headers. It takes SOAP_FAULT as an
argument rather than including gsoap to reach the constant. 403 stays out
of it: it is a refusal of a request that did authenticate, so
re-authenticating will not fix it and the generic path is right.
Also fixes the two log sites in this file that pass a 64-bit value to a
%ld conversion. seconds_until_termination and seconds_overdue are
chrono::seconds::rep, and seconds_until_renewal was already being cast to
intmax_t while the format still said %ld. On a 32-bit target that
misaligns the varargs list and prints garbage. Same defect and same fix as
refs #4580. Every other %ld in this file is already explicitly cast to
long and is left alone.
Tests: tests/zm_onvif_auth_error.cpp, 16 assertions. Verified they fail
(2 of 16) against the previous predicate. Full suite: 10160 assertions in
132 test cases, all pass. Builds clean, no new warnings.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
last_status_time was never initialised, so it sat at the epoch and every
monitor's first UpdateFPS() saw a huge elapsed time and wrote its
Monitor_Status row immediately. The period is fixed, so from then on they
stayed in lockstep: on a site with 161 monitors the rows were only landing on
12 distinct seconds of the minute, peaking at 21 monitors writing within the
same second, against the ~2.7 you would get from an even spread.
Nothing re-randomises the offset once they are synchronised, so every restart
of the capture daemons - planned, or recovering from an incident - drops the
whole herd onto the smallest, hottest table in the schema at the moment the
system is least able to absorb it.
Phase the first write by monitor id. That spreads the writes over the update
interval and, unlike a random offset, gives the same monitor the same slot
across restarts instead of re-clustering. Name the interval while touching it;
it was a bare 10 at its only use.
Note this path is not ZM_STATS_UPDATE_INTERVAL, which only paces zmstats.pl -
Monitor::UpdateFPS() has always used its own hardcoded 10 second period.
Tested: c++ -std=c++17 -Wall -Wextra -fsyntax-only on zm_monitor.cpp, clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JkE45gkMjnkTiJiUySbe7V
A preclusive zone is the one zone type that ends a frame marked Alarmed()
while the frame score is zero. DetectMotion() checks preclusive zones first
and, when one alarms, deliberately clears both the alarm flag and the score
before skipping the active/inclusive/exclusive loops entirely, so it returns
0 via "return score ? score : alarm".
Since the analysis_image allocation became conditional on a non-zero motion
score (refs #4996), that combination leaves packet->analysis_image null while
the zone loop below still finds zone.Alarmed() true and a non-null
zone.AlarmImage() -- Zone::CheckAlarms allocates its mask buffer
unconditionally, and preclusive zones skip the type < PRECLUSIVE block that
would have replaced it. Overlay() then dereferences null on its first line.
Reproduced on a live camera: the analysis thread faults 147us after the
preclusive zone alarms, at fault address 0x40, which is the offset of
Image::width.
#0 Image::Overlay (this=0x0, image=...) at zm_image.cpp:2009
#1 Monitor::Analyse (this=0x...) at zm_monitor.cpp:2380
#2 AnalysisThread::Run () at zm_analysis_thread.cpp:35
Guarding the dereference keeps the intent of #4996: a zero-score frame is
not saved as an alarm frame, so there is nothing for the overlay to land on.
Allocating whenever any zone is alarmed would instead reintroduce the
per-frame analysis JPEG churn that #4996 removed.
The sibling call site in AnalyseFrame() already guards this way.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QshidF3mzoCWgpehm4iCJ3
Config items with mismatched types (e.g., boolean accessed as string)
previously called exit(-1), crashing the daemon. Now logs a warning
and returns a best-effort converted value from the raw string instead.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
(cherry picked from commit c6fda3d2373d1454411d1f7d24d8e8ee3ba72602)
Replace numeric ID-based config system with name-based lookup using
std::unordered_map. Config entries now have compiled-in default values,
and only rows where Value != DefaultValue are loaded from the database.
This eliminates the fragile dependency on sequential ID numbers that
required DB regeneration whenever a config entry was added.
The config generator (zmconfgen.pl) now produces three macros:
- ZM_CFG_DECLARE_LIST: declares Config struct members
- ZM_CFG_DEFAULTS_INIT: initializes members to compiled-in defaults
- ZM_CFG_MAP_INIT: registers name-to-member bindings for DB loading
Only daemon-relevant config entries (137 of 245) are included in the
C++ header; web-only settings (WEB_H_*, WEB_M_*, WEB_L_*, skin
defaults, etc.) are excluded.
Also fixes two pre-existing bugs exposed by removing numeric #defines:
- ZM_WATCH_MAX_DELAY was used as Seconds(139) instead of the actual
config value (the 139 was the config table row ID, not seconds)
- ZM_OPT_USE_AUTH evaluated as if(8) (always true) instead of
checking the actual auth setting
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
(cherry picked from commit 86d4a8e5f0)
The non increasing dts warning printed raw timestamps only, which are in
stream time_base units and hard to interpret. Add the backwards jump
converted to seconds via av_q2d(stream->time_base).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0113HRysoe2hwfEq4SdcCqFy
SIGHUP means reload. zmc responds by closing its events, disconnecting the
camera and reconnecting; the perl daemons respond by exiting so zmdc restarts
them. zmdc.pl logrot hupped every managed process, so the nightly logrotate
run cost about 8 seconds of capture on a default install.
Rotating a log file only needs the daemon to drop its file handle, so use a
separate signal for it. SIGWINCH is otherwise unused, is ignored by default and
exists on every supported platform.
- Logger (C++) installs a SIGWINCH handler beside its USR1/USR2 handler. The
handler only sets a flag; the next logPrint closes the file and the write
reopens it at the original path. This covers every C++ binary without
touching any daemon's main loop.
- Logger.pm registers WINCH alongside HUP in logSetSignal, which logInit
already calls, so the scripts that install their own HUP handler still
rotate.
- zmdc.pl logrot sends WINCH. The logrotate config is unchanged - it still
calls zmpkg.pl logrot.
Filter.pm and FilterTerm.php justified MAX_EVENT_DAYS by events not outliving
the nightly HUP, which is no longer what bounds them; cite SectionLength.
Tests: tests/zm_logger_rotate.cpp and tests/perl/test_log_rotate_signal.pl both
log, rename the file out from under the process, confirm writes still land in
the renamed file, signal WINCH and confirm the original path is written again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NDhTBPj9xEaT52pRmAufaP
