The previous comment block claimed event_update_trigger fires BEFORE UPDATE
and that the lock order was buckets -> Event_Summaries -> Events. Neither
matches the code: triggers.sql defines event_update_trigger as AFTER UPDATE,
and InnoDB X-locks the matched Events row during WHERE evaluation before
either BEFORE or AFTER trigger bodies fire — so the canonical chain is
Events[Id] -> buckets[EventId] -> Event_Summaries[MonitorId]
which is what triggers.sql already documents. Comment-only change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Records the InnoDB X-lock order that all writers (zma trigger path, zmstats
prune+resync, zmfilter/Event::delete) must follow to avoid the deadlock cycles
fixed in the surrounding commits. Comment only; no behavior change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous resync code in zmstats and zmaudit used multi-table
UPDATEs against Event_Summaries that joined the bucket tables:
UPDATE Event_Summaries es
LEFT JOIN (SELECT ... FROM Events_Hour ...) h ON ...
LEFT JOIN (SELECT ... FROM Events_Day ...) d ON ...
... SET es.HourEvents = h.c, ...
zmaudit additionally used scalar correlated subqueries against Events
for the Total/Archived columns and against Events for Storage.DiskSpace.
MariaDB takes S-locks on the joined and sub-queried rows for the
duration of any multi-table UPDATE statement, regardless of isolation
level. event_update_trigger and event_delete_trigger hold X-locks on
those same bucket rows while they walk the trigger body, so the resync
deadlocks against active event lifecycle traffic. Captured a textbook
example in SHOW ENGINE INNODB STATUS:
TX(1) zma: HOLDS X Events_Hour[42229643]
WAITS X Event_Summaries[28]
TX(2) zmstats: HOLDS X Event_Summaries[2,4,5,...,28,...,73]
WAITS S Events_Hour[42229643]
Replace the JOIN/subquery pattern in both scripts with a snapshot phase
followed by per-monitor UPDATEs:
1. SELECT MonitorId, COUNT(*), SUM(DiskSpace) FROM each bucket
(and the equivalent Total/Archived aggregate from Events).
Plain SELECTs do consistent reads and take no row locks.
2. SELECT MonitorId FROM Event_Summaries to widen the universe so
monitors with empty buckets still get zeroed out.
3. For each monitor, UPDATE Event_Summaries SET ... WHERE MonitorId=?.
Each UPDATE only X-locks one ES row and reads no other table.
zmaudit's five separate UPDATEs collapse to one snapshot phase plus
one UPDATE per monitor. Storage DiskSpace gets the same treatment.
zmstats keeps the same outer transaction (BEGIN ... COMMIT, RC
isolation, retry on 1213) so the bucket DELETEs and the resync stay
atomic, but the resync no longer reads the bucket tables under lock.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three issues in the existing Stats/Event_Data/Frames/Events delete
sequence:
- On any zmDbDo error inside the transaction the code called
dbh->commit() instead of dbh->rollback(). The server-side
transaction was already rolled back when InnoDB picked us as the
deadlock victim, so the commit() was effectively against a fresh
auto-started TX, but the bug pattern leaked through as confusing
state and prevented any retry.
- There was no retry. errno 1213 is expected under contention with
zmstats and zma touching the same Event_Summaries[MonitorId] row,
and the loser is supposed to re-run.
- At REPEATABLE READ, two concurrent filter workers deleting events
with adjacent EventIds take next-key/gap locks on each other's
rows in the bucket tables.
Rewrite the delete block as a retry loop: SET TRANSACTION ISOLATION
LEVEL READ COMMITTED, begin_work, run the four DELETEs, commit on
success. On any error rollback (was: commit). On errno 1213 retry up
to 5 times with backoff. Skip both the isolation switch and the
rollback-then-retry when the caller is managing their own transaction
(in_transaction); they would be the wrong scope to act in.
Falls through to the storage DiskSpace adjustment only on commit, so
a deadlocked delete leaves the event for the next filter pass instead
of orphaning the row with stale storage accounting.
Note: do NOT pre-lock Event_Summaries[MonitorId] FOR UPDATE here, even
though the trigger touches it last. Pre-locking puts ES before
buckets[Id] in the lock acquisition order, which inverts against zma's
event_update_trigger path (Events[A] -> buckets[A] -> ES[N]) and
re-introduces the cycle the rest of this work is removing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Deadlock detection (errno 1213) is part of normal InnoDB operation
under contention; the engine rolls back the loser and expects the
caller to re-run the statement on a fresh transaction. Most callers
into zmDbDo go through autocommit, where there's no caller-managed
transaction state for a retry to disturb.
When AutoCommit is on, retry the statement up to 5 times with
exponential backoff (~100ms -> ~1.6s, jittered). When AutoCommit is
off, the caller owns the transaction and a unilateral retry would
silently succeed against a TX that no longer reflects the work the
caller staged before this statement; preserve the existing behavior
of logging and returning undef so the caller can rebuild the TX
itself.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a CreatedBy column to the Reports table and a canEdit() method on
the Report class so $report->canEdit() (already called from
web/ajax/reports.php) resolves to a real check. canEdit() permits the
report owner (CreatedBy == user) or any user/role with System=Edit.
Wire actions/report.php to stamp CreatedBy on first save and refuse
save/delete on existing reports the current user cannot edit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds paths-ignore for db/, docs/, distros/, misc/, onvif/, scripts/
and *.md/*.sql/*.in files. CodeQL analyses cpp and javascript only,
so changes confined to these paths produce no new findings and don't
need to spend Actions minutes or generate ~610 MB of cache.
Verified the ignored directories contain no .c/.cpp/.h/.js files.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Runs daily at 03:00 UTC and via workflow_dispatch. Groups caches by
key prefix (stripping trailing run/sha suffixes) and keeps the N
newest per prefix, deleting the rest. Defaults to keeping 2.
Without this, CodeQL builds left ~10 GB of per-commit caches behind,
exhausting the org Actions storage quota.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When AUTH_HASH_IPS is enabled and ZoneMinder is behind a reverse proxy
(e.g. Nginx in front of Apache), the hash is generated using
HTTP_X_FORWARDED_FOR (the real client IP) but was validated using only
REMOTE_ADDR (the proxy's IP), causing all authentication to fail.
Fix by consistently using HTTP_X_FORWARDED_FOR (first IP only, to guard
against spoofed multi-value headers) with REMOTE_ADDR as fallback in
all three places:
- web/includes/session.php: where remoteAddr is stored for hash generation
- web/includes/auth.php: getAuthUser() validation (PHP, also used by zms CGI)
- src/zm_user.cpp: zmLoadAuthUser() validation (C++ zms binary)
refs #4758
Agent-Logs-Url: https://github.com/ZoneMinder/zoneminder/sessions/959dfe9d-edea-4de5-a3a0-f90b758e5628
Co-authored-by: connortechnology <925519+connortechnology@users.noreply.github.com>
Replaces the uninformative "Any ONVIF options required. This is an
optional field." with a full reference covering:
- key=value,key=value format with an example
- pull_timeout, subscription_timeout, max_retries,
timestamp_validity, soap_log, renewal_enabled, expire_alarms,
closes_event — including defaults, valid ranges, and when to use each
Also fixes pre-existing typo: "alam" → "alarm".
Agent-Logs-Url: https://github.com/ZoneMinder/zoneminder/sessions/149f7873-ca12-424f-85d4-4bdc179d2488
Co-authored-by: connortechnology <925519+connortechnology@users.noreply.github.com>
Eight Copilot comments from the second review pass:
1. monitor.php (#3): "Deprecated - will be auto-detected..." note next
to TargetColorspace bypassed translate(). Added DeprecatedColoursSetting
key to en_gb and routed the deprecation note through translate() so it
localises with the rest of the form.
2. tests/zm_pixformat.cpp (#4): zm_colours_from_pixformat / round-trip
tests didn't cover the new YUV422P/YUVJ422P entries (added by
02e6be6b4). Added explicit assertions in both test cases — bumps
pixformat coverage from 105 to 115 assertions.
3. zm_image.cpp WriteBuffer (#5): linesize and size were derived from
p_width * p_colours, which undercounts planar YUV* (where p_colours=1
via the GRAY8 alias collision but actual buffer needs ~1.5x/2x for
chroma). Use av_image_get_buffer_size and av_image_get_linesize for
the AVPixelFormat instead, with bail-out on either failing.
4. zm_image.cpp AssignDirect (#6, #7): av_image_get_buffer_size returns
int and can be negative; assigning that into unsigned size/allocation
wrapped to a huge value. Check the return first, treat negative as the
same "unsupported format" failure as zm_colours_from_pixformat
returning false, and reset size/allocation/linesize/pixels to 0
(alongside imagePixFormat=NONE/colours=0/subpixelorder=0) so the
Image is left in a single coherent invalid state instead of partially
stale.
5. zm_image.cpp Assign (#8): av_get_pix_fmt_name(format) can return
nullptr (e.g. AV_PIX_FMT_NONE / unknown); passing that into
Debug(..., "%s", ...) would segfault. Capture once with a fallback
string before logging.
6. zm_monitor.cpp Capture path (#9): same nullptr issue with two Debug
calls — capture native_fmt_name once with fallback.
7. zm_monitor.cpp can_passthrough comment (#10): comment claimed
YUVJ422P would be converted to YUV420P because Image drops chroma,
but can_passthrough now allows YUV422P/YUVJ422P passthrough since
02e6be6b4 added 4:2:2 support. Updated the comment to describe the
current behavior (full 4:2:0 + 4:2:2 planar passthrough plus GRAY8
and RGB24/32) so the code and the rationale agree.
Tests: 76 cases, 788 assertions.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Image::Overlay() warned when (colours == image.colours &&
subpixelorder != image.subpixelorder), which made sense when
(colours, subpixelorder) was the canonical format identifier.
Now that imagePixFormat is canonical, the check produces false
positives in a normal code path: zm_monitor.cpp's analysis pass calls
analysis_image->Overlay(*(zone.AlarmImage())) where the destination is
YUV420P (colours=1 via the GRAY8/YUV420P=1 alias collision in
zm_rgb.h, subpixelorder=ZM_SUBPIX_ORDER_YUV420P=11) and the source is
the zone's GRAY8 alarm mask (colours=1, subpixelorder=NONE=2). The
overlay dispatch below already handles this correctly via
zm_bytes_per_pixel(imagePixFormat) == 1 on both sides — only the Y
plane of the dest is touched, leaving chroma untouched, which is
exactly the intent. The warning was just noise.
Reframe the check around imagePixFormat: warn only when the
AVPixelFormat actually matches but the ZM (colours, subpixelorder)
metadata diverges, which would indicate a real format-tracking bug.
The new message also names the AVPixelFormat for context, instead of
two opaque integers.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two issues flagged by Copilot review on the AVPixelFormat-migration PR,
plus one build fix that was needed to land them:
1. zm_local_camera.cpp set subpixelorder to BGR for V4L2_PIX_FMT_RGB24
captures. V4L2_PIX_FMT_RGB24 is byte-order R,G,B in memory and is
mapped to AV_PIX_FMT_RGB24 by getFfPixFormatFromV4lPalette earlier
in the same file, so the matching ZM subpixel order is RGB. Setting
BGR meant red and blue were swapped in the captured image whenever
a V4L2 camera was configured with the RGB24 palette. Long-standing
bug — preserved unchanged through the AVPixelFormat migration —
now fixed to ZM_SUBPIX_ORDER_RGB.
2. Image::AssignDirect(const AVFrame*) called zm_colours_from_pixformat
without checking the return value, leaving colours/subpixelorder at
their previous values for any unsupported AVPixelFormat. Wrap the
call and on failure put the Image into an explicit invalid state
(AV_PIX_FMT_NONE plus zeroed colours/subpixelorder) so the
inconsistency surfaces immediately instead of producing wrong-format
reads downstream.
3. Drop the u_buffer = ... / v_buffer = ... assignments inside
Image::Assign()'s identity-copy path. Those members exist on the
ai_server lineage but not on master, so the PR branch did not
compile against master as-is. av_image_copy reads the planes
directly out of temp_frame->data, so the assignments were not
load-bearing — they look like leftover state-tracking that didn't
survive the upstreaming. Comment notes why the lines were removed.
Tests pass: 76 cases, 778 assertions.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After entering a tag name and pressing "Enter," focus will be moved to "#tagInput." This allows you to enter multiple tags without manually positioning the cursor.
When the cursor is in "#tagInput" and the "Ctrl" + "Left" or "Ctrl" + "Right" key combination is pressed, a new tag (or an existing one attached to an event) entered in "#tagInput" will be added and the previous or next event will be navigated to. This eliminates unnecessary mouse movements for pressing the "tagPrevBtn" and "tagNextBtn" buttons.
