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
std::quick_exit does not exist on OpenBSD, which is why the openbsd
branch replaced these calls with std::abort(). abort() raises SIGABRT,
so the child dumps core and the parent's wait status reports a signal
death for what is just a failed exec.
Both call sites are in a forked child whose execl/execlp failed. _exit
is POSIX, exists on OpenBSD, and is what a child in that state should
call: no atexit handlers, no flushing of stdio buffers inherited from
the parent.
Build: cmake --build . --target zmc -> clean, no warnings.
loadInitialEventData() selected on unix_timestamp(`EndDateTime`) > t. An event
still being written has no EndDateTime, unix_timestamp(NULL) is NULL, and
NULL > t is never true, so the event currently recording could never be found.
zms.cpp:374 prefers the monitor_id form of setStreamStart whenever both
monitor and time are given, which is what montage review always sends, so its
`event` parameter never got a chance to help. Reviewing the present moment
returned the no-image placeholder, and every such request logged at ERR and
inserted a row into Logs.
Falls back to StartDateTime + Length, which zmc flushes every few seconds. An
event with neither ends where it starts, so a crash-orphaned event cannot match
every query and be picked ahead of the real one by the ORDER BY. Same
expression the API's Event model uses for EndTimeSecs.
Finding no event for a time is also no longer ERR. A client may ask for a time
the monitor was not recording, and it does so once per request.
Verified against a live instance: for a montage review request into the
recording event, the installed zms returned 2935 bytes (the placeholder) and
logged the failure, the rebuilt one returned a 134949 byte frame and logged
nothing.
The 9-byte auth_prefix buffer tripped -Wformat-truncation because auth is
64 bytes. Use %.8s in the Warning format instead, which truncates at print
time and drops the buffer. Logged output is unchanged: first 8 chars of the
hash plus "..." when longer.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Options field was a single line text input, so a list of options had to
be typed as one comma separated run. Make it a textarea, sized to the number
of entries it already holds, so each option can go on its own line.
Nothing needs to parse this: both consumers already take a set of separator
characters, they were just never given the newlines.
av_dict_parse_string() (Ffmpeg) and Split() (Libvlc) both skip empty
entries, so a blank line, crlf, or a trailing newline all work, and comma
separated values keep working unchanged.
The separator set is a named constant rather than a literal at each call
site so the tests exercise the value the cameras actually pass - with a
literal they would keep passing if a call site were reverted. Verified by
setting it back to "," which fails both test cases.
The SharedData static_assert failed on 32-bit builds, reporting 880 where
888 was expected. The struct is not packed, and the i386 SysV ABI aligns
double and uint64_t to 4 bytes where x86-64 aligns them to 8. That drops
the two interior pads x86-64 inserts implicitly, before capture_fps and
before the startup_time union, shifting every subsequent member by 4 and
then 8 bytes.
The union-with-uint64_t idiom already in the struct forces time_t to a
fixed 8-byte size, but it cannot force alignment, so the stated goal of
an identical layout on both word sizes was not actually being met.
Declare both pads explicitly as epadding1/epadding2. On x86-64 they
occupy space the compiler was already padding, so that layout is
unchanged byte for byte and existing shared memory segments are
unaffected. On i386 they restore the missing alignment, giving 888 bytes
with offsets identical to x86-64.
This was a real ABI bug, not a stale assertion. Memory.pm hardcodes
align 8 for double, uint64 and time_t64 regardless of architecture, and
Monitor.php hardcodes the x86-64 offsets outright, so both readers
already assumed the 888-byte layout on every platform. A 32-bit build
therefore disagreed with its own Perl and PHP readers.
Verified by compiling the struct at -m32 and -m64: all offsets now match
across both, the x86-64 layout is unchanged from before, and the
computed Memory.pm offsets and hardcoded Monitor.php offsets agree with
the C struct on all cross-checkable fields.
Also assert the offsets of the two alignment-sensitive members plus
control_state, so a future divergence names the member rather than only
reporting a size delta. The /* +N */ comments were previously documented
as a packed-layout ideal that did not reflect reality; with both pads
explicit they are now the real offsets, and have been corrected. The
header comment claiming 472 bytes was stale by several releases.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
strncpy() into a fixed 9-byte buffer trips -Wstringop-truncation at -O2:
error: 'char* strncpy(char*, const char*, size_t)' output may be
truncated copying 8 bytes from a string of length 63
The truncation is deliberate (log only the first 8 chars of the auth
hash) and the copy was already safe, since auth_prefix is zero
initialised so the final byte stays NUL. snprintf() expresses the same
intent, always NUL-terminates, and does not warn.
Only reproducible in an optimised build; -O0 Debug trees never see it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both functions initialised p_pixfmt before the setjmp() that installs the
libjpeg error recovery point, and only read it afterwards. GCC warns that
a longjmp() out of zm_jpeg_error_exit could restore a stale register copy:
warning: variable 'p_pixfmt' might be clobbered by 'longjmp' or 'vfork'
[-Wclobbered]
Move the initialisation below the setjmp() block instead of marking the
variable volatile. Nothing between the old and new positions uses the
value and the error paths do not reference it, so behaviour is unchanged
and no live value crosses the jump.
This unblocks builds using ENABLE_WERROR.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A monitor whose source signals full range (Dahua h264) decodes to
AV_PIX_FMT_YUVJ420P, and Monitor passes that format through to shared memory
unchanged. When zms scales such a frame for montage or console thumbnails,
SWScale::Convert mapped only the input format through fix_deprecated_pix_fmt()
and handed the still-deprecated YUVJ format to swscale as the destination, so
libswscale logged
deprecated pixel format used, make sure you did set range correctly
for every context it built. Watch view at 100% scale never hit it because
Image::Scale returns early when the dimensions already match.
Map deprecated formats on both sides in SWScale::Convert (both the buffer and
the AVFrame overload), in Image::Assign(AVFrame*), in
Monitor::setupConvertContext and in the LocalCamera conversion context.
Mapping the destination YUVJ format to its non-J equivalent makes swscale
default that side to limited range, which would compress full-range output.
Replace zm_sws_set_input_range() with zm_sws_set_ranges(), which takes the
original pre-fix formats for both sides and sets srcRange/dstRange accordingly.
Image::Assign(AVFrame*) now compares the mapped source and destination formats,
so a YUVJ420P frame into a YUVJ420P image takes the av_image_copy fast path
instead of running through swscale.
Image::Scale built a fresh SWScale, and therefore a fresh sws context, on every
call. That is a full sws_init_context per scaled frame per stream, and it is
what turned the warning into a per-frame flood rather than a one-off. Reuse a
thread_local SWScale; sws_getCachedContext re-inits itself when the geometry
changes.
Tests: zm_swscale_range.cpp installs an av_log callback and asserts that
SWScale::Convert with a YUVJ destination and Image::Scale on a YUVJ420P image
emit no deprecated-format message, plus a luma check that the range handling
still survives the mapping.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NDhTBPj9xEaT52pRmAufaP
openComms() logs an error but carries on to bind() when it cannot open or
flock() the .lock file, so a zms can be serving without holding the lock. In
that state the unlink in closeComms() could remove a socket belonging to the
zms that does hold the lock, leaving it unreachable.
Guard the unlink on lock_fd >= 0. Leaking the socket file when we never held
the lock is no worse than the behaviour before the unlink was added.
Expand the comment to spell out that this path is the command socket rather
than the .lock file, that our successor unlinks it itself on the way to bind(),
and why the unlink has to precede releasing the lock.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lock_fd was initialised to 0, but every other use in the class treats a
negative value as "no lock held": openComms() sets it to -1 when open() or
flock() fails, and closeComms() guards on `lock_fd >= 0` before closing.
The constructor's 0 passes that guard, so a StreamBase destructed without
openComms() having succeeded calls close(0) and closes stdin. Reaching it only
takes connkey > 0 plus a runStream() that returns before openComms(), and
MonitorStream::runStream() has two such returns: the STREAM_SINGLE branch and
the !monitor branch. mode=single URLs still carry a connkey, so both are
reachable in normal operation.
zms exits shortly afterwards and does little in between, so the practical
impact today is small. It is still closing a descriptor the class does not own,
and once fd 0 is free the next open() in the process silently lands on it.
Add a regression test covering destruction with and without a connkey. It saves
and restores fd 0 so a failure can't cascade into the rest of the suite.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
closeComms() left /var/lib/zoneminder/sock/zms-NNNNNNs.sock behind, and the
only thing that ever removed it was the unlink() before bind() in a later zms
that happened to draw the same connkey. Socket files accumulated indefinitely.
web/ajax/stream.php uses file_exists() on that path to decide whether zms is
listening, and waits up to a second for it to appear before giving up. A file
left by an exited zms defeats that wait: the check passes immediately, the
sendto() gets ECONNREFUSED, and the command is reported as failed even though
the new zms was about to bind. genConnKey() draws from six digits, so a page
cycling monitors every five seconds reuses a key well within an hour.
Unlink while we still hold the flock. A second zms with the same connkey blocks
on flock(LOCK_EX) in openComms() before it unlinks and binds, so it cannot have
created its own socket yet and we can't delete a file belonging to it. The lock
file itself is still left alone, since another zms may be waiting on it.
This is best effort: zms killed by a signal still leaves the socket behind. In
that case the process is usually still running, so the file being there is not
wrong.
Also reset lock_fd after closing it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SessionDescriptor::generateFormatContext() copied the camera-controlled
SDP sprop-parameter-sets value into a fixed char pvalue[1024] stack
buffer with an unbounded strcpy(), so a malicious or MitM'd RTSP camera
returning an oversized sprop-parameter-sets in its DESCRIBE response
could overflow the stack (saved frame pointer / return address) for RCE
as the capture process user.
A real H.264 SPS/PPS base64 blob is small; refuse anything that would
not fit the buffer instead of copying it. The existing inner base64
parse loop was already bounded, so only this outer copy was unsafe.
Refs GHSA-wg5h-vcgv-74pv.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Isaac Connor