ZM_Object's constructor treats an array as the object's row, and validInt()
and validCardinal() returned an array as they got it (preg_replace() does).
So wherever a request id reached `new ZM\X(...)`, raw or through those
validators, a crafted array built an object whose Id, UserId or MonitorId
the sender chose. The permission check then looked at those, and the delete
or update that followed used the Id. A user with Events=View deleted another
user's filter by posting Id[Id]=<filter>&Id[UserId]=<self> to the filter
delete action; the same shape reached events (MonitorId), users, reports,
snapshots and storage.
validInt() and validCardinal() now give '' for anything that isn't a single
value. The places that built an object from a raw $_REQUEST value (the
control and event ajax handlers, the event, filter, report, snapshot and
storage actions, view_hls, and the download and event detail modals) take
the id through validCardinal().
tests/php/test_request_ids_scalar.php covers the validators with arrays and
fails if web code builds an object straight from $_REQUEST, $_GET or $_POST.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The package workflows' publish steps ran easingthemes/ssh-deploy@main with
the zmrepo SSH key in its environment, and imported the package signing key
with crazy-max/ghaction-import-gpg@v7. A branch or tag is resolved when the
workflow runs, so whoever could move it upstream could run code with those
keys on our next push to master or release-1.38, and publish packages to the
apt and rpm repositories.
Pin each `uses:` to the commit its ref names today, with the ref as a comment
so dependabot keeps updating them. ssh-deploy is pinned to v6.0.1, which main
differs from only in release metadata. GitHub's own actions are pinned too.
utils/check-pinned-actions.py and the CI Pinned Actions workflow fail when a
workflow uses an action by anything but a full commit SHA.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
start() blocks SIGCHLD (and since #5199 TERM, INT and ABRT) across the fork.
The child unblocks the stop signals, but SIGCHLD stayed blocked through exec,
so every daemon zmdc started ran with it blocked (SigBlk 0x10000 in
/proc/<pid>/status for zmc, zmfilter, zmstats and zmwatch). Nothing depends
on it today, but a daemon that waits for SIGCHLD would never get it. Restore
the pre-fork mask just before exec.
The reaper took the signal from a wait status as $status & 0xfe, dropping its
low bit, so TERM was logged as signal 14, KILL as 8 and SEGV as 10, and the
core dump flag as $status & 0x01. Use 0x7f and 0x80.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix: give a monitor's new zmc a grace period in zmwatch before restarting it
zmwatch's startup grace counted from zmwatch's own start, so a zmc started
later (a monitor added or saved through the web UI or API, a zmdc restart)
had none. When a zmwatch pass landed in the seconds between that zmc being
forked and it mapping its shared memory, zmwatch logged "shared data not
valid" and asked zmdc to restart it, replacing a zmc that was about to come
up.
Count ZM_WATCH_MAX_DELAY per monitor from the pass that first found its
shared data missing or invalid, and forget it once the data is valid again
or after the restart. The zmwatch-start grace and the heartbeat checks are
unchanged, so a zmc that hangs after mapping its memory is restarted as
before.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix: don't lose a stop that zmdc sends to a daemon before it has exec'd
zmdc forks each daemon and, in the child, connects to the database, closes
every descriptor up to _SC_OPEN_MAX and only then resets TERM, INT and ABRT
to their defaults before exec. Until then the child runs zmdc's own
shutdown handler, which only sets $zm_terminate in the child, so a TERM sent
in that window is lost. The daemon then starts as if nothing happened, zmdc
waits KILL_DELAY (30 s) and KILLs it, and an event that zmc opened in the
meantime is left with no EndDateTime.
zmwatch's restart of a zmc that hasn't mapped its shared memory yet lands
in this window. Closing descriptors one by one takes about 1.7 s with a
1048576 open-file limit (Docker's default), which makes the window wide.
Block TERM, INT and ABRT across the fork along with SIGCHLD, and in the
child reset them to their defaults and unblock them first thing, so a stop
sent before exec ends the child and the reaper restarts or forgets it as
asked.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* perf: close only open descriptors in zmdc's child before exec
The child closed every descriptor number from 3 up to _SC_OPEN_MAX one by
one. With a 1048576 open-file limit (Docker's default) that is about 1.7 s
of close() calls before each daemon starts, and it is the window in which
zmwatch can find a just-started zmc without shared memory.
Close the descriptors /proc/self/fd lists instead, and keep the old loop
where /proc isn't available.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* fix: validate alarm packets in zmalarm-server.py
The alarm server accepted any packet from any host and mapped it to a
monitor by the Address field inside the packet, so a host on the network
could raise an alarm or a zmtrigger event on any configured camera.
Every malformed packet (short header, non-JSON body, unknown address)
raised an uncaught exception that ended the process, and a client that
connected without sending blocked the accept loop indefinitely.
- only accept an alarm whose Address matches the address it came from
and a monitor that uses that address
- read the header and body with a 5 s timeout, cap the body at 64 KiB,
log and drop bad packets instead of exiting
- resolve camera hostnames in monitor Paths and skip monitors without a
camera address instead of failing at startup
- pass the alarm description to the zmtrigger thread instead of reading
a global, and talk to zmtrigger over a plain socket (telnetlib was
removed in Python 3.13)
- honour the bare port number zmpkg.pl passes from ZM_OPT_ALS_PORT and
add --bind= to choose the listening address (default unchanged)
- move the pyzm imports into main() so the parser can be tested; add
tests/python/test_zmalarm_server.py
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix: accept alarms from cameras behind NAT in zmalarm-server.py
The source check required the packet's Address field to equal the address
the packet came from. A camera behind NAT reports its LAN address there
while its packets arrive from the NAT address, so its alarms were dropped.
The Address field is no credential, so the monitor is now chosen by the
sender: an alarm is accepted only from an address that belongs to a
monitor, the camera address in its Path or one listed for it under
[alarmserver] in secrets.ini, and it goes to one of that address's
monitors. The Address field only picks among several monitors on one
address, for cameras behind a shared NAT address. A host that belongs to no
monitor still cannot raise an alarm, and a camera cannot raise one on
another camera's monitor.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix: cancel the forced alarms zmalarm-server.py raised when it stops
An alarm packet armed the monitor, slept 60 s in a thread and disarmed it.
When zmdc stopped or restarted the server inside that window (SIGTERM, which
Python's default handler ends at once) the disarm never ran and the monitor
stayed in forced alarm, recording, until someone cancelled it by hand.
Keep the forced alarms the server raised in one place, one per monitor: a
new alarm for a monitor in forced alarm arms it again, as every packet did,
and moves its end instead of starting a second timer that would cut it
short. On the way out, SIGTERM included, the server cancels every alarm
still running and exits 0, so zmdc no longer logs the stop as abnormal. An
arm that fails is logged instead of ending the server.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
MonitorsController::edit() restarted zmc through the CakePHP model but
left the monitor's control server running. zmcontrol.pl reads
ControlAddress, ControlDevice and the Control when it starts, so after
an API edit PTZ kept using the old values: a camera password changed
through the API kept failing PTZ with 401 until zmcontrol.pl was
restarted by hand.
Restart the daemons the way the console's save does
(includes/actions/monitor.php), through ZM\Monitor: stop zmc and, for a
controllable monitor, zmcontrol.pl; start them again from the saved row
unless the monitor does not capture, is a WebSite or is deleted.
ZM\Monitor also sends the commands to the monitor's own server in a
multi-server setup, where the model's daemonControl() logged an error and
did nothing.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
actions/eventdetail.php checked only the global Events=Edit permission and
then updated the Name, Cause and Notes of every id in markEids, so a user
denied some monitors could rewrite those monitors' events. The event detail
modal likewise showed the name, cause and notes of any event id it was given.
Load each event and require $event->canEdit(), which adds the per-monitor
view check, as archiving and unarchiving in actions/events.php already do.
The action skips events that fail it; the modal leaves them out, and shows
nothing for a single event that fails.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
actions/control.php and actions/eventdetail.php redirected to
$_SERVER['HTTP_REFERER'] as is. That is the Referer of the POST itself, which
normally comes from a ZoneMinder page, and the CSRF token keeps other sites
from making the POST, so it isn't the open redirect the zone save had. But it
is the same pattern, and a missing Referer warned about an undefined index and
rendered the action's view instead of redirecting.
Use localRedirectUrl() as the zone save does, falling back to the monitor's
control view and to the events list.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
After saving a zone, actions/zone.php redirected to the REFERER the zone form
posted, which the form fills from the Referer its page was loaded with. A
zone editor who reached the zone page through a link on another site was sent
to that site after saving: an open redirect, usable to lead someone from a
trusted ZoneMinder page to a look-alike login. Forging the POST itself is
blocked by the CSRF token, so it needs an editor who followed such a link.
localRedirectUrl() returns the target only if it is a query-only URL or an
absolute http(s) URL on the host and port of the current request, and the
fallback otherwise: another host, //host, backslashes, user@host, other
schemes and control characters all give the fallback. The zone save falls
back to the zones list, as it already does with no Referer (#5207).
tests/php/test_local_redirect_url.php covers 21 cases.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The zone form posts its Referer back as REFERER, and posts it empty when
the form was opened from a typed URL or a bookmark. The action used it
whenever it was set, so $redirect was empty, index.php rendered the zone
view with the action's $zone (a database row, or array() for a new zone,
neither with Points) and ksort() at views/zone.php:105 failed with
'Argument #1 must be of type array, null given'. The zone was saved but
the browser got a 500.
Use REFERER only when it is not empty and fall back to the monitor's
zones list.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* fix: stop fixAmps() recursing forever on a navbar link with an entity
fixAmps() is declared with one parameter but called itself with a
position, which PHP drops, so every call searched from the first '&'
again. Once ZM_WEB_NAVBAR_LINKS held an entity (&, ', ...) or a
bare '&' followed later by a ';', rendering the left navbar recursed until
PHP ran out of memory, and every page answered 500 for users who use the
left navbar.
Escape the bare ampersands in one preg_replace() instead. Character
references are matched at the '&' itself, so a bare '&' is no longer
taken as valid because an entity appears later in the string, and
hexadecimal references (') are kept.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix: read the view of a navbar link only when its href has one
getAdditionalLinksHTML() parsed every left navbar link's href as if it
carried a query string with a view parameter. A link to another site
(https://host/docs) logged 'Undefined array key "query"', a parse_str()
deprecation and 'Undefined array key "view"' on every page view.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* test: cover lines logged to the database while a query runs
Live-database tests ([notCI][dblive]) for how the logger and zmDbDo
interact:
- a line logged on one thread while another thread is inside zmDbDo
reaches the Logs table
- zmDbDo's own debug lines stay out of Logs when the database level is
debug, so inserting a Logs row cannot log another one without end
- a dbQueue backlog over 40 rows does not deadlock threads that log
The first fails on master: zmDbDo sets the process-wide database log
level to NOLOG for the whole query, so the line is dropped.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix: stop dropping other threads' database log lines during queries
zmDbDo lowered the logger's database level to NOLOG for as long as its
query ran, so that its own lines (and, at a debug database level, the line
about every insert into Logs) could not queue more inserts into Logs. The
level is shared by the whole process, so every line any other thread
logged in that window never reached the Logs table or the web log view,
while the log file and syslog still had it. The save and restore of that
shared level from several threads is also a data race: a thread can
restore a stale NOLOG and leave database logging off.
Replace the toggle with Logger::DatabaseLogPause, a thread_local count
that keeps only the current thread's lines out of the database. zmDbDo
and the queue's backlog warning use it; mDatabaseLevel is no longer
written outside of initialise().
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix: warn about the database queue backlog outside the queue's lock
zmDbQueue::process logged its "db queue size has grown larger" warning
while holding the queue mutex. logPrint holds the logger's mutex while it
pushes a Logs row, which takes the queue mutex. With more than 40 rows
waiting, a thread logging to the database and the queue thread could each
hold one of the two locks and wait for the other, stopping every thread
that logs.
Take the backlog size under the lock and warn after releasing it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Gapless playback loads a new event page for each event, and each load moved
the video three times and ended in a page-wide wipe:
- video.js sets the player up on DOMContentLoaded without a size, while the
first changeScale() ran from initPage() on window load. The browser could
paint a collapsed video with the controls drawn up under it, and
changeScale() measured that collapsed layout (an empty inline-block still
leaves a line box), which made the video too small by about 17px.
- skin.js's ResizeObserver on the navbar statistics called endOfResize() for
the size it reports when it starts observing, and for every width change of
the figures, so changeScale() ran again 250ms later and corrected the size.
- With the stats panel turned off, initPage() hid it and widened the video's
column only after load.
- The page-leave roll-up in skin.js animated the content to nothing on every
reload, a wipe from the bottom up between events.
Size the video on DOMContentLoaded, after video.js has the player, and
remember the size per monitor (zmEventVideoSize<Id>) so event.php opens the
next page at it; changeScale() still corrects it if the window or layout
changed. Render the stats panel and the video's column width from the
zmEventStats cookie in event.php. Rescale for the navbar only when its height
changes. Move between events through goToEvent(), which sets
skipPageLeaveAnimation so the roll-up is skipped for that reload.
Measured in Chrome at 1920x915 on a 1280x720 event, each frame's video size
and controls position: before, every page went 1x0, 1144x644, 1174x660 with
the controls starting near the top of the video, and the content shrank from
851px to 363px before the reload. After, the page reached by gapless playback
paints at 1174x660 with the controls in place and does not move, and there is
no roll-up. The first event of a session still sizes once, as no size has
been remembered yet. Window resize and the stats button still rescale, for
both video.js and zms events.
tests/js/event-gapless-layout.test.js covers goToEvent() and the
DOMContentLoaded sizing; two of its three tests fail without this change.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
requestField() read Model[field] whenever the request's Model entry was an
array, empty or not. CakePHP's Model::set() treats the bare top-level fields
as the model's data when that entry is empty (_setAliasData), so
{"Monitor":[],"EventEndCommand":"x"} passed the System-edit check on the
monitor command fields, which found no value, while save() stored x. A
Monitors=Edit user without System=Edit could set EventStartCommand or
EventEndCommand, which zmc runs through /bin/sh.
Every other check built on requestField() had the same gap: the target
MonitorId when adding or editing an event, the EventId when adding a frame,
and the ParentId when editing a group.
Read Model[field] only when the Model entry is non-empty, as set() does, and
otherwise the bare field; a non-empty entry that is not an array yields
nothing, since set() skips it.
tests/php/test_api_request_field.php runs the real method against each
request shape.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The skin and css, the language and locale, the date, time and datetime
formats, the default bandwidth and the snapshots feature switch all
decide how the interface looks or what it offers, which is what that tab
is for, but they sat under System among the shutdown command, the shm
key and the audit intervals. System drops from 42 entries to 33 and
Display goes from 32 to 41.
The snapshots switch is read only by the skin: it gates a button in
montage and montage review, a menu item, and the snapshot views and
their permission checkboxes. Nothing in the daemons or the perl scripts
looks at it.
The category key is still 'web'. That is the tab now labelled Display;
the tab whose key is 'display' holds the per-session cookie settings and
stores no configuration at all, so a stored default does not belong
there. The hint setup for the moved entries -- the installed skin list,
the css list for the current skin, the bandwidth options, and the
language and locale lists -- moves to the same branch.
ZM_TIMEZONE stays under System. It reads like a presentation setting,
but Filter.pm uses it for date arithmetic when evaluating filters, so it
changes behaviour and not just what is displayed.
A migration, 1.39.40, is needed as well as the ConfigData change, because a
version upgrade does not rewrite Config: zmupdate only reloads it from
ConfigData under --freshen, which is mutually exclusive with an upgrade
run. Fresh installs take the new category from the generated
zm_create.sql.
Verified both paths converge on 33 system and 41 display entries with
identical membership: a fresh install of this branch, and a copy of a
real pre-change Config table with the migration applied. On the running
install the bandwidth, language and locale entries still render as their
pickers on the new tab, css as its list of choices, and the timezone
picker is still on System with its full list.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
The Display tab holds skin, css and navbar choices kept in a cookie for
the session looking at the page. They are not stored configuration and
do not affect anyone else, but the tab was named as though they were the
system's display settings. The settings that genuinely are the system's
display configuration -- the navbar, titles, thumbnails, event sorting,
animations, alarm popups -- sit on a tab labelled Web, which says where
they apply rather than what they do.
So the labels swap to match: the cookie tab becomes This Session, and
the Web tab becomes Display. A short note at the top of This Session
says the settings are kept in a cookie rather than saved on the server,
since a name alone will not carry that. The note goes inside the form
rather than beside it, because #optionsContainer is a flex row and two
siblings there split the width between them.
Labels only. The category keys are untouched, so ?tab=display and
?tab=web keep working, no configuration entry moves, and no migration is
needed. The one-off version/Versions case that was inline in the menu
builder becomes a small map beside the other two, applied at both places
that build the item list.
Only en_gb gains the new strings. Language files are loaded with en_gb
as the base and the chosen language layered over it, so the other 27
show the English until somebody translates them. Display and Web are
already translated in 22 of them, and those translations stay correct:
the word Display now labels the tab that actually holds display
settings.
Verified on a running install: the sidebar renders This Session for
tab=display and Display for tab=web, the note spans the full width above
the fields, all three tabs return 200 with their entry counts unchanged
and no php diagnostics.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
Two problems, both in exportsql.
The mysqldump command ended with a hardcoded database name, so --export
always read from a database called zm whatever ZM_DB_NAME said. It now
uses the configured name.
The primary keys were then blanked with a regex that replaced everything
between VALUES ( and the first quoted string:
s/VALUES \((.*?),'/VALUES \(NULL,'/ig
That holds only if the id is immediately followed by a quoted value. A
MonitorPresets row starts id,ModelId and both are unquoted, so the
pattern swallowed the pair into a single NULL and left the row one value
short of its columns. On the shipped data that corrupted all 75 preset
rows of a 126 row export; the 51 control rows came through intact, which
is presumably why it went unnoticed.
Replaced with the same parse the import now uses: split the values on
top level commas, find the id by column name, and blank just that one,
re-emitting the row in the form it arrived in. A row whose value count
does not match its table is left alone with a warning rather than
rewritten on a guess.
Verified against a real dump of the seed data: 126 rows, every one with
its id blanked, and not one value-count change -- against 75 corrupted
by the old pattern. Round-tripped the export back through --import into
an empty database: 51 controls and 74 presets, no broken names, no
unresolved control references, and every column identical to the source
apart from the one preset row whose name is a duplicate of another,
which the import has always collapsed because it keys on Name.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
The import read zm_create.sql line by line and pulled the Controls and
MonitorPresets rows out with a regex. Two things were wrong with that.
It found no controls at all. The seed rows moved to db/controls.sql,
which zm_create.sql only pulls in with a `source` directive, and the
loop did not follow it, so --import silently stopped importing ptz
controls entirely.
The regex also mis-parsed the rows it did find. It assumed every INSERT
was positional with the id first and the name straight after, then
replaced everything before the first ,' with NULL. Ten of the shipped
controls are written with an explicit column list, where the first value
is the name, so it overwrote the name with NULL and keyed the row on the
protocol instead:
IN : INSERT INTO `Controls` (`Name`,`Protocol`,`CanReset`)
VALUES ('Amcrest ASH42-B RPC','Dahua_RPC',1);
OUT: INSERT INTO `Controls` (`Name`,`Protocol`,`CanReset`)
VALUES (NULL,'Dahua_RPC',1);
It also happened to work for MonitorPresets only because the two columns
before the name are numeric and unquoted.
Replaces it with a parser that reads the column list when there is one
and asks the database for the column order when there is not, so nothing
depends on the name sitting at a fixed offset -- it is the second column
of Controls but the third of MonitorPresets. Values are split on top
level commas only, leaving quoted strings and parenthesised subqueries
intact, which matters now that a name may contain a comma and a preset
resolves its control with a SELECT. The id is blanked wherever it
appears so the row still gets a fresh one, and a row whose value count
does not match the table is skipped with a warning rather than inserted
crookedly.
`source` is followed, relative to the including file, so the controls
come back. A file that cannot be read warns instead of dying, since
zm_create.sql sources several that are optional.
Verified against the real seed data: 51 controls parsed where it
previously found none, 74 presets, no row imported with a null or empty
name, the ten explicit-column controls landing with their name and
protocol intact, every preset's control subquery resolving, and names
containing commas preserved. Exercised the splitter directly on a
positional row, an explicit-column row, a row with both a comma in the
name and a subquery, and a doubled-quote escape. Round-tripped a
mysqldump export back through the import. Generated script passes
perl -Tc.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
MonitorPresets.ControlId hardcoded the row numbers controls.sql happens
to produce, so the shipped presets only pointed at the right control as
long as nobody changed the number or order of rows in that file. Nothing
guaranteed that, and nothing detected when it broke. Removing the two
duplicate Pelco rows earlier in this branch shifted every id from 10 up
and silently re-pointed six presets at the wrong ptz driver.
The name is the identity, so look the control up by it:
ControlId = (SELECT `Id` FROM `Controls` WHERE `Name`='Foscam FI8620')
controls.sql is sourced before the preset inserts, so the rows exist by
then, and all 21 referenced names are unique in the table. ControlId is
a nullable varchar, so even a missing control degrades to an unset
control rather than failing the install.
This also corrects three mappings that were already wrong on master,
where a control had been inserted above the referenced row without the
presets being updated -- exactly the failure the ids invite:
Foscam FI9821W FFMPEG H.264 pointed at Foscam FI8908W
Loftek Sentinel PTZ pointed at Foscam FI9821W
SunEyes SP-P1802SWPTZ pointed at WanscamPT
Verified on a full fresh install: 51 controls, 75 presets, every one of
the 21 references resolving to the control named for it and none
unresolved. Re-ran it with three extra controls inserted ahead of the
shipped set, moving every id: the resolution is unchanged, which is the
property the row numbers never had.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
Dropping Controls.Type left Pelco-D and Pelco-P duplicated on existing
installs, each having existed twice as a Local and an Ffmpeg copy that
were identical in every other column. Fresh installs were already fixed
by removing the redundant rows from controls.sql; this does the same for
an upgrade, so the two converge.
Monitors pointing at the row being removed are moved to the survivor
before it is deleted, so no monitor loses its ptz.
Pelco-D and Pelco-P are not merged with each other. Their capability
flags are identical, but Protocol differs and selects the control
module, PelcoD.pm or PelcoP.pm -- different wire protocols that happen
to have the same feature set -- so one entry could not drive both.
Scoped to these two names rather than deduplicating the table in
general: a user's own same-named controls may differ in ways a query
over 105 columns cannot usefully check, and silently merging them would
discard their edits. A user's own 'My Pelco-D' on the same protocol is
left alone.
The DROP COLUMN is now guarded with the information_schema check the
other migrations use, scoped to DATABASE(). It was the one statement in
the file that failed rather than no-opped on a second run; the file is
applied three times in testing and is stable.
Verified on a scratch database seeded with a monitor on each doomed
twin, a monitor already on a survivor, a monitor on an unrelated
control, a monitor with no control, and a user-added control sharing
the protocol: all four twins collapse to two, every monitor keeps a
valid ControlId, no orphans, and the user's row is untouched. A fresh
install and an upgraded install now hold the same 51 control rows, byte
for byte.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
Controls.Type looks like the monitor type a control applies to, and has
been kept in step with the Monitors enum since 1.27, but nothing has
ever read it. The control dropdown on the monitor page lists every row
by Name, no query filters on it, and no code compares it to the
monitor's own Type. It was a field the edit form asked for and the
control list displayed back.
The column appears to be MonitorPresets.Type copied by analogy, where
the idea is real: that one decides what a preset creates. That also
explains why every migration altering one altered the other.
Removes the column from the schema, adds the 1.39.39 migration, the edit form row
and its type list, the control list column, the defaults in Control.php
and Control.pm, and the value from all 53 shipped rows in controls.sql.
This supersedes the 1.39.38 retyping of three libvlc controls to Ffmpeg:
the column they were being retyped into is gone.
zmcamtool needs no change -- its import regex rewrites the Id and
captures the Name, so it is not positional past that, and its export is
a mysqldump that follows the live schema.
Dropping Type left the two shipped Pelco rows byte-identical to their
twins, which existed only as a Local and an Ffmpeg copy of the same
control, so the redundant copies are removed: 53 shipped rows become
51, with both Pelco-D and Pelco-P still present once.
Existing installs are deliberately left with their duplicates. The same
pair collapses there too, but deleting rows a monitor's ControlId may
point at, to tidy a repeated name in a dropdown, is not a trade a
migration should make silently.
Verified by loading the old and rewritten controls.sql into scratch
databases and diffing every remaining column across all 53 rows: no
value shifted, which is the failure mode when a positional INSERT loses
a field. The file mixes positional and explicit-column INSERTs, so both
forms were handled. Fresh-install and migrated Controls schemas compare
equal, the shipped data loads into both, and Monitors and MonitorPresets
keep their Type. Build clean, 147 test cases pass, php -l and perl -c
clean on the edited files.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
Nothing links or dlopens libvlc any more, so the dev package is no
longer a build requirement anywhere: the debian, beowulf, ubuntu and
split-package control files, the podman Containerfile, the redhat and
opensuse specs, the centos CI image, and the five workflows that
installed libvlc-dev. The redhat spec also required vlc-core at
runtime, which was the dlopen target.
Documentation follows: the libVLC source tab section, the macos note
about libVLC not being found, and the WEB_FILTER_SOURCE source-type
list.
Two stale statements nearby are corrected while the files are open. The
Remote source tab still said its RTSP method "may be removed in the
future"; it was removed earlier on this branch. AGENTS.md still listed
RemoteCameraRTSP and LibVLCCamera among the camera classes, both of
which are now gone.
All workflow yaml parses, the three edited rst files render, and the
debian control field structure is unchanged apart from the removed
entries.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
Follows the daemons: no monitor can be type Libvlc any more, so the ui
should not offer it or carry text describing it.
Drops the source-type option and its deprecation notice, the Libvlc arm
of the Source Path and console source-column branches, the entry in the
control-capability type list, and the value from the api's Type
validation list. The OPTIONS_LIBVLC help block goes from all 25 language
files: monitor.php builds that key as OPTIONS_ plus the monitor type, so
it became unreachable the moment the type did. The Libvlc source-type
label goes from the 22 files that carried it.
SourceTypeDeprecated and Deprecated lose their last caller and are
removed. RemoteRtspDeprecated and Removed stay -- the rtsp protocol
option is still shown to monitors already set to it.
The WEB_FILTER_SOURCE help listed the affected source types; corrected
in ConfigData.pm.in and in the one translation that had localised it.
All 25 language files and the four edited php files parse clean under
php -l, and the config module parses under perl -c. No javascript
changed, so eslint has nothing to say here -- it cannot run in this
worktree anyway, which has no node_modules.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
Ffmpeg reaches the same network cameras libvlc did, so carrying a second
streaming library -- with its own threading, its own bugs and its own
build dependency -- buys nothing. The preceding migration moves existing
monitors to Ffmpeg first, so Monitor::Load cannot meet a type it no
longer recognises, which is Fatal and would stop zmc rather than skip
the monitor.
Drops LibvlcCamera, the LIBVLC camera type and its parse and
construction arms, LIBVLC_SRC and Camera::IsLibvlc, the ZM_NO_LIBVLC
option and the libvlc probe, and the HAVE_LIBVLC and HAVE_VLC_VLC_H
defines. Since the camera resolved libvlc through dlopen rather than
linking it, this removes a headers-only build dependency and a runtime
one, and nothing else needed relinking.
kOptionSeparators stays -- the Ffmpeg path still parses the Options
field with it -- but it is no longer a contract shared between two
readers, so its comment and the test that pins its behaviour are
reworded rather than removed.
Built clean from a fresh cmake configure, which no longer lists libVLC
among optional libraries; unit suite passes (147 cases, 12531
assertions); no libvlc strings remain in zmc.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
Libvlc is being removed rather than left deprecated, so existing
monitors have to move off it in the same release. Extends the migration
already added for NVSocket instead of adding a second
file, since this is one release and one upgrade step.
Monitors, Controls and MonitorPresets rows typed Libvlc all become
Ffmpeg, and the value is dropped from the three Type enums. The shipped
preset data is changed to match so a fresh install lands on the same
schema and the same rows: one MonitorPreset, which already carried an
rtsp url, and three PTZ controls, whose Type names the camera's control
protocol rather than its capture method, so retyping them keeps ptz
working on the migrated monitors.
A monitor's Options field is left as it is. The contents are libvlc
switches such as --rate=1 that ffmpeg will not understand, but
FfmpegCamera passes Options through av_dict_parse_string, which reports
unknown keys and carries on rather than refusing to open. Clearing the
field would destroy the only record of how the camera was tuned, to save
a log line.
Verified against a scratch database built from the live schema: Libvlc
and NVSocket monitors, Libvlc controls and presets, and existing Ffmpeg and Local
monitors. Fresh-install and migrated schemas compared equal.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
NVSocket spoke a bespoke socket protocol to one vendor's nvr. It has a
single known user, no way for anyone else to test it, and nothing it
reaches that ffmpeg cannot reach over rtsp or http, so it is removed
rather than carried further.
Drops RemoteCameraNVSocket, the NVSOCKET camera type and its parse and
construction arms, the source-type option and settings panel in the web
ui, and the stray ja_jp translation key. The preceding migration moves
any existing rows off the type first, so Monitor::Load cannot meet a
type it no longer recognises -- that path is Fatal and would stop zmc
rather than skip the monitor.
Built clean; unit suite passes (147 cases, 12531 assertions, run from
tests/ where the font fixtures' relative paths resolve). eslint clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
The table is indexed by Monitor::CameraType but had no entry for
LIBCURL, so every name from that point on belonged to the wrong type --
LIBCURL printed "NVSOCKET" and NVSOCKET printed "VNC" -- and VNC, the
last value, indexed one past the end of the array. The only reader is a
Debug(1) in Monitor::Load, so a VNC monitor read out of bounds whenever
debug logging was on.
Add a static_assert tying the table's length to the last enum value, so
adding or removing a camera type fails the build instead of silently
shifting the names again. Verified by removing an entry and confirming
the build fails on the assertion.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
NVSocket is removed in this release and Libvlc is deprecated. Both
reach cameras ffmpeg reaches, so existing monitors are converted rather
than left to fail at capture time. The migration is 1.39.38.
Libvlc only changes type; Path already holds the full url, and User
and Pass are merged by FfmpegCamera the same way.
NVSocket has no url to convert -- it spoke a bespoke socket protocol to
one vendor's nvr -- so those monitors are disabled rather than left with
a type the daemon no longer recognises, which is Fatal and would take
zmc down rather than skip the monitor. The rows are kept so their zones,
groups and history survive.
The value is then dropped from the Type enum in all three tables that
carry it, and from zm_create.sql.in so a fresh install matches.
Verified against a scratch database built with the live schema, covering
Libvlc, NVSocket, and existing Ffmpeg and Local monitors. Re-running the file is a no-op.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
Both reach cameras that ffmpeg reaches. Libvlc does it through a second
streaming library with its own build dependency, threading and bugs; NVSocket
speaks a bespoke socket protocol for one vendor's nvr that we have no way to
test. Neither earns the maintenance, and the same monitors work as Type Ffmpeg
with the camera's rtsp or http url.
Deprecated, not removed. A monitor using either still captures; it logs a
warning at startup naming what to change it to, and the form marks the type and
explains. That follows what Remote/RTSP did before it was removed, which is the
shape that gives people a release to move in.
The warnings were verified firing rather than assumed: switching a monitor to
each type in turn produces the expected message naming that monitor.
php -l and eslint clean; the three pre-existing duplicate lang keys are
unchanged. Suite 147 cases / 12531 assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
FfmpegCamera opened a decoder for the audio stream, stored it, exposed it
through Camera::getAudioCodecContext() and Monitor::GetAudioCodecContext(), and
freed it in Close(). Nothing decoded a single frame with it.
Both things that decode audio build their own from the stream's codecpar:
VideoStore::open() for transcoding to aac, and AudioDetector::Open() for
levels. The previous commit removed the last place the camera's context was
even passed, and it had been ignored there too.
So there is no audio decoder to move to the monitor, only one to stop opening.
The camera keeps finding the audio stream -- that is discovery, and packet
routing and timestamps need it -- and the secondary-input branch still adopts
mAudioStream from FFmpeg_Input, just not the context that came with it, which
FFmpeg_Input owns and uses itself.
Camera now holds no codec context of any kind. Between this and the video move,
a camera finds streams and produces packets, and whoever needs pixels or
samples opens their own decoder.
Verified end to end rather than by reading, on a camera that really does carry
audio: with RecordAudio on and the encoder transcoding, the recorded file has
both an hevc video track and an aac audio track over 848 frames. Suite 147
cases / 12531 assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
VideoStore took an AVCodecContext for the input audio and ignored it: the
member was initialised to nullptr and open() builds its own from
audio_in_stream's codecpar, with its own codec lookup, time_base and
avcodec_open2, and frees it in the destructor.
So the parameter described a relationship that did not exist, and following it
backwards -- through Monitor::GetAudioCodecContext() to the camera -- suggested
the camera's audio decoder was feeding recording. It was not.
Drop the parameter and say beside the member where the context actually comes
from.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
RTSPDescribe chose whether to take the media url from the DESCRIBE
response rather than the configured path. That was a quirk of
ZoneMinder's own rtsp implementation; ffmpeg decides it itself. Removing
that client left the setting with nothing behind it -- the monitor form
still drew the checkbox and the save path still wrote the column, but no
code read it, so the only thing it could do was mislead whoever ticked
it.
Drops the form row and the Remote-only block that existed to hold it,
the javascript that showed and hid it as the protocol changed, the field
in the php and perl monitor models, the default in the save action, the
column from the schema, and the label and help text from all the
language files carrying them.
The 1.39.37 migration drops the column behind an information_schema
guard scoped to DATABASE(), so the file stays re-runnable.
Verified by building a pre-upgrade database from upstream/master's own
schema and applying the migration: the column goes, and Monitors matches a fresh install of this
branch column for column. Migration applied three times with no error.
php -l clean on the four edited files and all 28 language files, perl -c
clean, build clean, 154 test cases pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
The hand-written rtsp client is gone, so a monitor still typed Remote
with Protocol rtsp has nothing left to capture with. Ffmpeg reaches every
camera that client did, so 1.39.37 converts those monitors rather than
leaving them to fail at capture time.
The monitor gains an equivalent url built from Host, Port and Path.
Credentials are deliberately left in the User and Pass columns instead of
being written into the url: FfmpegCamera merges them into the path itself
and percent-encodes them through the Url class, and only when the path
does not already carry credentials. Building the url here instead would
break any password containing @ : / or #. A Host that already embeds
user:pass@ keeps working too, because the resulting path then carries
credentials and the runtime merge leaves it alone.
Remote/http monitors are untouched. The statement was verified against a
scratch database covering rtsp with and without a port, relative, empty
and NULL paths, a Host with embedded credentials, and Remote/http.
Re-running the file is a no-op.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the rtsp client gone, Type=Remote with Protocol=rtsp cannot capture, so
the monitor form no longer offers it and the camera probes no longer suggest
it.
A monitor already saved as rtsp still sees the option, because dropping it
outright would mean opening such a monitor to read the warning silently
re-saved it as http -- swapping one broken configuration for another. For those
the option is labelled removed and the warning now says the monitor will not
capture until it is changed, rather than that it will stop working in a future
release.
The three probes that suggested Remote/rtsp -- ACTI in both probe
implementations, and Vivotek -- now suggest Type Ffmpeg with the equivalent
rtsp:// Source Path, which is the same stream by a maintained route.
Type=Remote keeps http. mjpeg and snapshot-jpeg cameras are what it is for.
php -l clean on all four files; eslint clean; the three pre-existing duplicate
lang keys are unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
rtsp_describe was passed to RemoteCameraRtsp and nowhere else, so with that
camera gone it was loaded from the database on every monitor read and then
ignored.
The SELECT column and its positional read come out together. They have to: the
row is walked by index, so removing one without the other silently shifts every
field after it rather than failing to build.
The Monitors.RTSPDescribe column stays. Dropping it is a migration and a web
change, which is a separate piece of work from no longer reading it.
Verified against the live database rather than by inspection, since a
misalignment here compiles perfectly: the monitor loads its name, 2560x1440
geometry, hevc codec and vaapi setting correctly, and captured 668 frames with
no errors.
Suite 147 cases / 12531 assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
RemoteCameraRtsp was the last camera that opened one, so Camera's
mVideoCodecContext and getVideoCodecContext() are now written by nobody and
read only by the fallback that existed for it.
Drop both. Monitor::GetVideoCodecContext() returns the monitor's context and
nothing else, OpenDecoder() no longer has to ask whether the camera brought its
own, and FfmpegCamera::Close() stops freeing a pointer it cannot have set. The
audio decoder stays with the camera; only video moved.
This is what the previous commits were working towards: one owner for the
decoder, with the hw device reference and the hw pix fmt storage beside it, and
no path where a camera can free a context the decoder thread is using.
Verified on hardware after the removal: vaapi negotiated, 646 frames into a
recorded event, no errors. Suite 147 cases / 12531 assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
ZoneMinder carried two rtsp implementations: ffmpeg, which every Type=Ffmpeg
monitor uses, and a hand-written one behind Type=Remote with Protocol=rtsp --
its own rtsp state machine, rtp transport, rtcp control and sdp parser, some
3200 lines. ffmpeg handles rtsp well enough now that a second implementation
earns nothing, and it was already marked deprecated in the interface.
A monitor still configured that way now logs an error naming the exact Source
Path to move to, built by RtspUrlFromRemote with the credentials stripped, so
the message is actionable rather than only a refusal. That helper and its tests
stay; they describe the migration, which still needs describing.
Type=Remote keeps its http protocol. mjpeg streams and snapshot-jpeg cameras
are what that capture method is for and they are unaffected.
Removed: zm_remote_camera_rtsp, zm_rtsp, zm_rtp, zm_rtp_ctrl, zm_rtp_data,
zm_rtp_source and zm_sdp. zm_rtsp_auth stays -- it is included by
zm_remote_camera.h and so is shared with the http camera. Everything named
zm_rtsp_server_* is the restreaming server and is unrelated.
Verified nothing outside the cluster referenced any of it: RtspThread,
RtpSource, RtpCtrlThread, RtpDataThread and SessionDescriptor had no callers
elsewhere in the tree, and RemoteCameraRtsp was named only by zm_monitor.
Build clean; suite 147 cases / 12531 assertions. An rtsp-over-ffmpeg monitor
still captures on hardware: vaapi negotiated, 646 frames into a recorded event,
no errors.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
Pause() releases the decoder, so Play() has to open another. It did not, and
an on-demand monitor would have resumed with a null context: Play() re-primes
the camera and restarts the decoder thread without going back through
PrimeCapture(), which is where the only OpenDecoder() call was.
This was introduced by moving ownership to the monitor. Before that the camera
rebuilt its own context inside camera->PrimeCapture(), which Play() does call,
so the gap did not exist.
Neither monitor on the machine this was developed against is on-demand, so the
earlier hardware runs exercised the startup path only and could not have shown
it. Found by following who calls what rather than by testing.
Verified by running an on-demand monitor through a full cycle: hardware
decoding is negotiated twice, once at prime and again at resume after the
pause, with the decoder and analysis threads restarting in between and no
errors. Without the fix the second open is absent.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
The comment above PrimeCapture() still described the decoder thread holding a
context it got from camera->getVideoCodecContext(), which the camera would free
and rebuild. The monitor owns that context now, so the first half no longer
described what happens. The second half still does: the decoder thread has to
be joined so it releases the in-flight packet locks in decoder_queue, or stale
entries survive the reconnect and the packetqueue fills and stays full. Kept
that and corrected the rest.
RemoteCameraRtsp declared get_VideoStream, get_AudioStream,
get_VideoCodecContext and get_AudioCodecContext -- four accessors duplicating
the base class's under a different naming convention, none of them called from
anywhere in the tree. Removed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
With the monitor opening the decoder, the camera's hwaccel_name,
hwaccel_device, hw_device_ctx and hw_pix_fmt are written and never read, and
the two hwaccel arguments to its constructor are passed only to be stored.
Remove them rather than leave fields that look like they still configure
something -- the monitor reads the same settings straight from its own members.
The re-arm comment in Close() described setting use_hwaccel back to true so a
transient hardware failure would not pin the monitor to software for the life
of the process. That flag is gone, but the behaviour is not: OpenDecoder()
builds a fresh decoder on every PrimeCapture(), so a reconnect retries hardware
decoding by construction. The reasoning moved there with it.
Close() keeps freeing mVideoCodecContext. It is a base-class member that an
Ffmpeg camera no longer sets, but RemoteCameraRtsp does, and the base is where
that belongs.
Verified on hardware again after the removal: vaapi still negotiates and 652
frames decoded into a recorded event with no errors. Suite 147 cases / 12531
assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
The camera's job is to produce packets; decoding belongs with the thread that
does it. FfmpegCamera opened the video decoder and handed the context out, so
Monitor::Decode() ran against a pointer whose lifetime the camera controlled.
PrimeCapture() already carried a comment about the consequence: the decoder
thread had to be stopped and joined before re-priming, because
camera->PrimeCapture() frees that context and builds another, and a decoder
running against the dying one is unsafe.
Monitor::OpenDecoder() now opens it, after camera->PrimeCapture() has produced
the stream to decode from, and Monitor::CloseDecoder() releases it where the
decoder thread has already been joined -- at the top of PrimeCapture and in
Pause(). The monitor holds the hw device reference and the hw pix fmt storage
alongside it, which is what the context's opaque pointer refers to, so the
lifetime is now one object's business rather than split across two.
Two cases deliberately do nothing:
Cameras that hand over images rather than packets -- local, vlc, vnc -- have
no video stream, so there is nothing to open.
RemoteCameraRtsp opens its own decoder and decodes through it.
GetVideoCodecContext() falls back to the camera's for that one rather than
running a second decoder over the same stream. That capture method is
deprecated and goes in 1.41, and this branch cannot be tested here, so it is
left as it is rather than changed blind.
Verified on hardware: an rtsp hevc monitor with DecoderHWAccelName=vaapi still
negotiates through the monitor-owned path ("Trying hwdevice vaapi", "Using
vaapi hardware decoding for hevc") and decoded 752 frames into a recorded
event with no errors.
Suite 147 cases / 12531 assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
The mismatch Debug compared the monitor's configured size against the decoder
context's. Both hold the same numbers -- the context is filled from the
stream's codecpar a few lines earlier -- but the stream is the thing actually
describing what the camera sends, and reading it here means the check no longer
needs a decoder to have been opened.
That matters for the next step: this was the camera's last use of the decoder
context for anything other than handing it out.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
Second piece of groundwork for letting the decoder thread own decoding. The
whole decoder setup -- codec selection, context allocation, thread count,
option defaults and overrides, hwaccel negotiation, avcodec_open2 and the
fallback -- sat as 184 lines inside FfmpegCamera::OpenFfmpeg(), reachable only
by a camera. Monitor cannot call it there, so the move would have meant
copying it.
Lift it to open_video_decoder() in zm_ffmpeg.cpp, leaving the camera a call
and the null check. No behaviour change: the body is the same code with camera
members replaced by parameters.
hw_pix_fmt and hw_device_ctx are taken by reference and keep their names, which
is not only convenience. The returned context stores &hw_pix_fmt in ctx->opaque
and get_hw_format dereferences it on every format negotiation, so the storage
has to outlive the context. A reference parameter says that at the signature
and makes a temporary impossible; the header says why.
Two things the extraction had to account for:
opts was declared once for the whole of OpenFfmpeg and shared with the input
open. It is freed unconditionally before the decoder block and nothing
repopulates it in between, so a fresh local is equivalent. The
AVDictionaryEntry it scanned with was shared the same way and is now local
too.
use_hwaccel was a camera member but only ever read at the gate inside this
block, cleared when a candidate's hwaccel failed so the next codec attempt
would not retry it. That is entirely internal to the open, so it is a local
now and the member is gone rather than left looking like it still gates
something.
Verified on hardware rather than by reading: an rtsp hevc monitor with
DecoderHWAccelName=vaapi still negotiates ("Decoder hevc supports type vaapi",
"Using vaapi hardware decoding for hevc", now logged from zm_ffmpeg.cpp) and
decoded 749 frames into a recorded event with no errors.
Build clean with no new warnings; suite 147 cases / 12531 assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
Groundwork for moving decoding out of the camera and into the decoder thread.
The parsing of DecoderHWAccelName sat inline in FfmpegCamera::OpenFfmpeg(),
where nothing could test it, and the ai_server branch has a second copy of the
same logic in its Monitor-side decoder open. Moving the decoder without pulling
this out first would carry the duplication across rather than end it.
No behaviour change: same iteration over the types this build offers, same
comma-separated priority list, same warn-and-skip for a name libavutil does not
know, same order.
Being a free function it can now be tested, which it could not be before. The
cases covered are the ones an operator's setting actually runs into -- a single
name, a priority list, spaces around the commas, a typo among good entries,
empty entries -- asserted against whatever device types the host reports rather
than against a fixed list, so the test travels. This host offers seven, so no
section skipped for want of data.
Confirmed non-vacuous by mutation: dropping TrimSpaces fails one case, and
accepting an unknown name instead of skipping it fails two.
Build clean with no warnings; suite 147 cases / 12531 assertions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
With ZM_LOG_DEBUG off, Logger::initialise() clamped every target level
(file, database, syslog, terminal) to INFO as well as the overall level.
SIGUSR1 raises the overall level, but the effective level is the lower of
that and the highest target level, so it stayed at INFO and the signal
did nothing.
Only cap the overall level. Targets keep their configured levels, so
SIGUSR1 behaves as if ZM_LOG_DEBUG_LEVEL had been incremented: targets
configured for debug start receiving it, targets at INFO stay at INFO.
Startup behaviour is unchanged since the effective level is still INFO.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
(cherry picked from commit af970628c3fe5fe76d163a30747cf534babbb8a6)
A blank line after the zoneminder package's Depends ended that stanza
early, so Recommends and Description formed a stanza of their own with
no Package field. dpkg rejects the whole file ("stanza lacks the
'Package' field"), so nothing could read its build depends either.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The ubuntu2004 control file (the one the deb package CI builds from),
the redhat spec and the opensuse spec already build-depend on
nlohmann-json, so the packages we ship are built with
ZM_HAS_NLOHMANN_JSON and carry AI detection results. distros/debian and
distros/beowulf did not, so a package built from either silently lost
them: CMake treats the library as optional and only warns.
It is header-only and in Debian since buster, which Beowulf is based on,
so it costs nothing at runtime and needs no Depends entry.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RemoteCameraHttp read the camera's Content-Length with atoi() and then asked
ReadData() for that many bytes, which passes the count straight to
Buffer::read_into(). Buffer::expand() worked in int: once the allocation passed
INT_MAX the spare space went negative, it grew again, and mAllocation + count
wrapped to a small value, so the following memcpy or read() wrote the camera's
data past a small heap block. Whoever answers the camera's HTTP request, often
on a plain-HTTP link, could crash zmc in a loop or corrupt its heap.
- RemoteCameraHttp::ParseContentLength() replaces the four atoi() calls. It
refuses a length that is not a number or is larger than twice the monitor's
raw frame size plus one network buffer (capped at 1 GiB), and GetResponse()
fails the response instead of buffering it.
- Buffer::expand() does its arithmetic in size_t, grows to exactly what is
needed, and refuses to hold more than INT_MAX bytes, since read_into()
returns the count as an int.
- Buffer::read_into() refuses a read that does not fit after expand() rather
than reading past the allocation.
tests/zm_buffer.cpp covers reads through a pipe, an oversized read, data kept
across a shift and a grow, and the Content-Length bounds.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
