- zm_libvnc_camera.cpp resize(): return FALSE and log if av_malloc fails,
rather than returning TRUE with a null frameBuffer (libVNC would then
write into it and crash). Matters more now that size_t sizing can
request large allocations for server-advertised dimensions.
- zm_libvnc_camera.cpp: compute the buffer sizes in the scale Debug() in
size_t with %zu, so the logged sizes can't overflow int and match the
values passed to SWScale::Convert.
- zm_libvlc_camera: widen LibvlcPrivateData::bufferSize to size_t and
compute it in size_t in PrimeCapture, so the allocation itself can't
overflow. Image::Assign now passes the same stored bufferSize used for
the allocation, so the read size can't exceed the buffer. Widen the
compare loop index to size_t to match.
Verified: both translation units compile with -Werror -fsyntax-only.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The frame buffer size calculations in the libVNC and libVLC camera
backends multiplied 16/32-bit operands and only widened the result to
size_t afterwards, so the multiplication itself could overflow before
the conversion. Flagged by CodeQL cpp/integer-multiplication-cast-to-long
(alerts 282, 145, 85). The VNC framebuffer dimensions are advertised by
the remote server (uint16_t), making the inputs externally influenced.
Cast the first operand to size_t so each product is computed in size_t:
- zm_libvnc_camera.cpp: SwScale::Convert src size (framebufferWidth *
framebufferHeight * 4) and dest size (width * height * colours)
- zm_libvnc_camera.cpp: resize() bufferSize, changed from int to size_t
(not converted to a larger type, so CodeQL did not flag it, but the
same overflow applied); Debug format updated to %zu
- zm_libvlc_camera.cpp: Image::Assign size (width * height * mBpp)
Verified: both translation units compile with -Werror -fsyntax-only.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Avoid sleeping up to MAX_SLEEP (500ms) at the end of runStream when
termination has been requested, so zms shuts down more promptly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
zm_terminate is written from signal handlers and read from daemon loops.
A plain bool is undefined to access from a signal handler and may be hoisted
out of a loop by the optimizer. std::atomic<bool> is lock-free on supported
platforms and matches the already-atomic per-object terminate_ flags. Update
the three Debug() vararg sites to call .load() since atomics can't pass
through ...
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The connection charset was "utf8", MySQL's alias for 3-byte utf8mb3, while
Monitors.Name and the DB default are utf8mb4. On a utf8mb3 connection any
4-byte UTF-8 character is read back as '?' and, when embedded in a log message
written to the Logs table, raises ERROR 1366 and truncates the value. Names
with such characters lost their multibyte portion in logs, keeping only the
ASCII tail.
Connect with utf8mb4, falling back to utf8 with a warning if the server
rejects it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- API (database.php.default): only set the PDO verify flag when SSL is
actually configured (ZM_DB_SSL_CA_CERT set), matching the web/Perl/C++
layers. Previously a fresh install's default (1) would set the flag on a
non-SSL connection, since the CakePHP datasource merges 'flags' uncondi-
tionally.
- Both PHP layers: cast to string and trim before parsing the value, and use
strict in_array, to avoid type-juggling and stray-whitespace edge cases.
- zm_db.cpp: use my_bool (not char) for the MYSQL_OPT_SSL_VERIFY_SERVER_CERT
fallback argument, the type libmysqlclient expects. That branch only
compiles on older clients without MYSQL_OPT_SSL_MODE, where my_bool exists.
refs #3816
Add a ZM_DB_SSL_VERIFY_SERVER_CERT setting so a database connection that uses
ZM_DB_SSL_CA_CERT can talk to a server with a self-signed or otherwise
non-matching certificate. When enabled, verification is by identity (the cert
must chain to the CA and its CN/SAN must match ZM_DB_HOST), consistent across
the C++ daemons, the PHP web interface, the CakePHP API and the Perl scripts.
This re-does the reverted #3817. That PR broke the build because it called
mysql_options(MYSQL_OPT_SSL_VERIFY_SERVER_CERT, ...), and that enum was removed
from the MySQL 8.0 C client in favour of MYSQL_OPT_SSL_MODE; it also passed a
c_str() where a my_bool* was expected, and referenced the PHP constant
unconditionally (fatal on PHP 8 for an upgraded install whose zm.conf predates
the option).
The option that controls server-cert verification differs by client library and
the symbols are enum values, not macros, so CMake feature-detects them by
compiling:
- HAVE_MYSQL_OPT_SSL_MODE (MySQL 5.7.11+/8.0, MariaDB Connector/C 3.1+)
- HAVE_MYSQL_OPT_SSL_VERIFY_SERVER_CERT (older MariaDB/MySQL)
zm_db.cpp uses SSL_MODE_VERIFY_IDENTITY / SSL_MODE_REQUIRED when the former is
available, else falls back to the latter with a proper my_bool.
Value handling is three-way in every layer: a truthy value verifies, a false-y
value (0/false/no/off) skips verification, and an empty/unset value leaves the
client default in place so existing installs are unchanged on upgrade. PHP, the
API datasource (via PDO flags) and the Perl DSN are all guarded with defined()
checks. Fresh installs default to 1.
Documents the full ZM_DB_* connection and SSL settings, including the hostname
verification gotcha when connecting by IP, in docs/userguide/configfiles.rst.
refs #3816
ONVIF PullMessages intermittently failed with ter:NotAuthorized (logged as
the misleading clock-drift error) every few thousand requests, then ZM tore
down a healthy subscription and re-subscribed. SOAP logs showed the trigger:
the failing request always had wsu:Timestamp/Created one second behind
UsernameToken/Created, while every successful request had them identical.
The cause is in set_credentials(): soap_wsse_add_Timestamp() and
soap_wsse_add_UsernameTokenDigest() each call time(NULL) on their own, so when
the two calls straddle a one-second boundary the two Created values diverge by
a second and Hikvision rejects the request. This is probabilistic, which is
why it hit roughly hourly per camera and constantly across a fleet.
Capture time(NULL) once, re-stamp the Timestamp Created/Expires from it, and
use soap_wsse_add_UsernameTokenDigest_at() so the token Created and its
password digest are pinned to the same instant. Both Created values are then
always identical.
Add tests/zm_onvif_wsse.cpp asserting the two Created values match.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
get_y_image() wraps the decoder Y plane zero-copy but recorded
FFALIGN(width,32) as the Image stride instead of the frame's real
linesize[0]. For widths that are not a multiple of 32 the decoder packs
the plane tighter than FFALIGN, so:
- Image::Rotate/Flip re-derived the source stride via
av_image_fill_arrays(...,32) and read past the end of the borrowed
plane (and skewed Y-channel motion analysis, which reads the same
buffer).
- Image::Flip sized its destination from this->size, which is tight for a
borrowed plane and smaller than the 32-aligned layout the planes are
written at, overrunning the destination.
Record the frame's real linesize in get_y_image(); use the Image's own
linesize for the source plane-0 stride in Rotate/Flip; size Flip's
destination from av_image_get_buffer_size(...,32). All are no-ops for
self-consistent ZM-allocated (32-aligned) images.
tests/zm_image_linesize.cpp: Rotate 90/180/270 and Flip H/V over a tight,
non-32-aligned GRAY8 source verifying correct output.
refs #4788
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
In passthrough mode the orientation was written as a display-matrix side
data plus the legacy rotate tag, but only rotations were handled; flips
(FLIP_HORI/FLIP_VERT) fell to an "Unsupported Orientation" warning. The
displaymatrix was av_malloc'd and only written in the rotation branches,
so a flipped passthrough monitor attached an uninitialized matrix as side
data — garbage rotation metadata in the MP4.
Initialise the matrix to identity (av_display_rotation_set(.,0)) so it is
always fully written, and express flips with av_display_matrix_flip so
rotation and flips are both carried losslessly in the display matrix with
no decode/re-encode. The legacy rotate tag only expresses rotation, so it
no longer warns for flips.
Note: the rotation component of the display matrix is honored by HTML5
<video>, but the reflection component may not be in all browsers; ZM's web
event player may need a CSS transform companion to render flips mirrored.
Desktop players (VLC/QuickTime) and the file metadata are correct.
refs #3399
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
SWScale::Convert chose av_image_fill_arrays alignment by heuristic
(width % 32 ? 1 : 32) for both buffers. Image buffers are always laid
out align-32, so any width not divisible by 32 made Convert read luma
rows 16 bytes short and chroma planes from packed offsets: diagonal
shear plus garbage chroma. Rotating a monitor is what produces such
widths (1280x720 ROTATE_270 -> 720 wide, 3840x2160 ROTATE_90 -> 2160
wide, both % 32 == 16), so every scaled view and every re-encode of a
rotated monitor was corrupted while unrotated monitors (1280/2688/3840
all % 32 == 0) were untouched. The rotate/flip segfault fix exposed
this: before it, rotated planar frames crashed zmc before reaching
Scale.
Alignment is a fact about how a buffer was laid out, not something
derivable from dimensions. Convert now takes explicit in/out alignment:
Image::Scale passes 32/32, the videostore encode path mirrors
get_out_frame's allocation choice, libvnc passes 1 (packed VNC
framebuffer) and 32 (Image WriteBuffer). Remove the unused
SetDefaults/ConvertDefaults API rather than threading alignments
through dead code.
Tests: new Scale regression case on a 720x1280 YUV420P image
(column-banded luma, uniform chroma) fails before the fix exactly as
observed live (sheared rows, V plane reading 0) and passes after.
Full suite 84/84.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
When monitor->ShmValid() returns false and the reconnect attempt fails,
the cleanup path erased the video_source/audio_source entries from their
maps without deleting the FifoSource objects, then called RemoveSession
on the MediaSession. Two problems:
1. ZoneMinderFifoSource is only stopped/joined in its destructor. erase()
on a raw pointer leaks the object and leaves its read_thread_ and
write_thread_ running.
2. The xop::H264Source / H265Source / AV1Source is owned by the
MediaSession and gets destroyed inside RemoveSession. The orphaned
FifoSource still holds a raw m_h264Source pointer set via
setH264Source(); the next SPS NAL parsed by the still-running
ReadRun thread calls H264Source::SetSPS on freed memory and crashes
in std::vector::assign.
Stack of the observed crash:
H264Source::SetSPS H264Source.h:34
H264_ZoneMinderFifoSource::splitFrames zm_rtsp_server_fifo_h264_source.cpp:53
ZoneMinderFifoSource::getNextFrame zm_rtsp_server_fifo_source.cpp:265
ZoneMinderFifoSource::ReadRun zm_rtsp_server_fifo_source.cpp:59
Mirror the correct order already used by the "monitor went away" path
above (lines 188-197) and by the shutdown path (lines 362-375): delete
the FifoSources first so their dtors set stop_ and join the threads,
then RemoveSession can safely destroy the H264Source.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The ONVIF event poller runs in its own thread (Run -> WaitForMessage ->
PullMessages). On shutdown ~ONVIF() joins that thread, but the join could
block past zmdc's 30s SIGTERM->SIGKILL window because two timeouts were
effectively unbounded:
- pull_timeout_seconds was only clamped to < 60s, so the camera could hold
the long-poll open that long.
- gSOAP connect/recv/send timeouts were 0 (infinite), so a hung camera
blocked recv() forever.
Either case let thread_.join() exceed 30s, causing zmdc to SIGKILL zmc.
Set the SOAP socket timeouts to 25s and clamp pull_timeout to 20s, keeping
the invariant pull_timeout (<=20) < socket_timeout (25) < zmdc window (30):
a quiet long-poll still returns normally, a hung connection aborts within
25s, and the worst-case join finishes with margin.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Image::Rotate and Image::Flip computed chroma plane dimensions with
AV_CEIL_RSHIFT(width, log2_chroma_w). The macro's runtime form is
-((-(a)) >> (b)), which relies on arithmetic right shift of a negative
value and is only valid for signed operands - FFmpeg always passes int.
Image::width/height are unsigned, so the negation wraps, the shift is
logical, and the result is 2^31 + ceil(w/2^b) instead of ceil(w/2^b):
for 1280x720 the chroma rotate received src_w=2147484288 and
src_h=2147484008 (captured in gdb), writing gigabytes out of bounds.
Effect: zmc's decoder thread segfaulted on the first decoded frame of
any monitor with a rotated or flipped orientation and a planar pixel
format - a monitor with decoding Always crash-loops.
Replace the macro at both sites with an explicit unsigned ceiling
shift helper and a comment documenting the trap.
Tests: new tests/zm_image.cpp covers Rotate 90/180/270 and Flip on
YUV420P with per-plane marker pixels, plus odd dimensions exercising
the chroma ceiling. The Rotate cases segfault before this fix (verified,
exit 139) and pass after. Full suite 85/85 via ctest. Live-verified on
a 1280x720 ROTATE_270 monitor with decoding Always: pre-fix crash
within seconds, post-fix 90s clean run under gdb.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The field was assigned in three branches of the constructor (always to
the same value as Camera::pixelFormat) and never read anywhere. Unlike
LocalCamera, FfmpegCamera doesn't run a sws_scale step at the Camera
layer — libavcodec hands frames to the pipeline directly — so there's
no separate capture-vs-image format distinction to track.
Drop the redundant assignments and the member declaration. The
canonical camera-side format is now exclusively Camera::pixelFormat
(via Camera::PixelFormat()).
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zm_get_pix_fmt_name() calls av_get_pix_fmt_name() which is declared in
<libavutil/pixdesc.h>. The header previously got away with relying on
transitive includes via zm_ffmpeg.h, but tests/zm_pixformat.cpp (and
any other consumer that includes only zm_pixformat.h) would fail to
compile. Add the explicit include so the header is self-contained.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Image::Colourise sizing-failure Error: av_image_get_buffer_size /
av_image_get_linesize return negative for AV_PIX_FMT_NONE and
unrecognised formats, so this branch fires exactly when
av_get_pix_fmt_name(p_req_pixfmt) can return nullptr. Switched to
zm_get_pix_fmt_name.
- Image::Deinterlace_4Field fallback Panic: same issue — fires when
imagePixFormat is outside our dispatch set, where av_get_pix_fmt_name
may return nullptr. Switched to zm_get_pix_fmt_name.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Image::Delta: pass delta8_bgr/rgb/argb/abgr/bgra/rgba/gray8 directly to
delta_row() instead of dereferencing with *. They're already function
pointers; the deref is redundant and would be undefined behaviour if
any pointer were null (Initialise() sets them, but the explicit *
obscures the intent and adds nothing).
- Switched the Delta fallback Panic from av_get_pix_fmt_name() to the
nullptr-safe zm_get_pix_fmt_name() wrapper. The Panic fires exactly
when imagePixFormat is unrecognised, where the raw function would
return nullptr.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The y += 2 loop ran while `y < height`, but copied even row y into odd
row y+1. For odd image heights, the final iteration had y == height-1
and wrote to row height — past the end of the image buffer in all three
colour branches (GRAY8/YUV-Y plane, RGB24, RGB32).
Stop one short of the last row (y < height - 1) so y+1 stays in
bounds; the orphan last row in odd-height images is left untouched
(consistent with the (even, odd) pairing this discard pass is doing).
Also added a guard for height < 2 to keep the unsigned `height - 1`
calculation from wrapping.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Monitor::GetImage and Monitor::getSnapshot bounds-check branches were
logging "Image Buffer has not been allocated", but by the time control
reaches them the empty-vector check has already passed — the buffer IS
allocated, the index is just out of range. Replaced with explicit
"index N out of range (image_buffer.size() = M)" so debugging starts
from the right place.
- Image::WriteBuffer unsupported-format error only printed `colours`,
but the mapping in zm_pixformat_from_colours depends on both `colours`
and `subpixelorder`, and planar formats legitimately have colours=1.
Print both fields so a bad (colours, subpixelorder) pair is
immediately identifiable.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Image(const AVFrame*) constructor only initialised blend_buffer_
and blend_buffer_size_ before calling AssignDirect(frame). AssignDirect
now calls DumpImgBuffer() on both the failure (invalidate) and success
paths to release any previously-owned buffer — but in the
freshly-constructed Image, buffer/buffertype/allocation are
uninitialised, so DumpImgBuffer would read garbage and potentially
free an invalid pointer.
Use constructor delegation so Image() runs first (zero-initialises
every member, sets buffertype = DONTFREE), making the subsequent
DumpImgBuffer call a no-op on a known-empty Image.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Image::Assign(const Image&): when the source linesize was *smaller*
than the destination's (e.g. src came from a held-buffer ctor with a
packed device stride while dst is FFALIGN'd to 32 from
av_image_get_buffer_size), the function fell through to the flat
memcpy of `size` bytes — over-reading the source by
(linesize - image.linesize) * height bytes. Undefined behaviour;
could crash or copy unrelated memory. Generalised the per-row branch
to handle both directions: trigger whenever linesize != image.linesize
and copy min(src, dst) bytes per row, so neither buffer is read or
written past its capacity. Planar formats still refuse explicitly
since per-row would lose chroma; the error message now reads "vs"
instead of ">" to match the broader condition.
- Added an explicit `#include <libavutil/pixdesc.h>` in zm_image.cpp.
The recent Rotate/Flip rewrites call av_pix_fmt_desc_get /
av_pix_fmt_count_planes / use AVPixFmtDescriptor, which previously
only compiled thanks to transitive includes from zm_ffmpeg.h →
libavutil/imgutils.h. Make the dependency explicit.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The raw-buffer Assign overload previously validated buffer_size against
av_image_get_buffer_size(..., align=32) and then memcpy'd `size` bytes
flat. But callers feed it packed source buffers — Camera::ImageSize() is
now derived with align=1 to describe device buffers (V4L2 mmap, raw RTP,
etc.) — so for non-32-aligned widths:
* the size check spuriously rejected valid packed source buffers, and
* if relaxed, the flat memcpy would have read past the source.
Validate the source size against the packed (align=1) layout and the
destination size/allocation against the aligned (align=32) layout, then
copy plane-by-plane via av_image_copy. av_image_fill_arrays gives us
both layouts' per-plane pointers and strides, and av_image_copy walks
each plane with its own src/dst linesize so per-row padding never
over-reads the source.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SystemTimePointToMysqlString appended ".%06d" microseconds to the string it
hands MySQL for the Events.StartDateTime datetime column. That column has no
fractional precision, and MySQL 8 ROUNDS a fractional value when storing it, so
a start_time of 23:59:59.5xx-.999999 local was promoted to 00:00:00 of the next
day. Event::SetPath() derives the on-disk day folder from to_time_t(start_time),
which truncates, so it landed on the previous day.
For continuous recording the event start is backdated to the preceding keyframe,
which for a section forced closed at local midnight falls just before midnight.
On MySQL 8 the DB row then recorded the next day while the files were written
under the previous day's folder, producing a permanent zmaudit path mismatch and
orphaned files when the event aged out (the purge path is built from
StartDateTime).
Format the value to whole seconds only so it matches to_time_t() used by
SetPath(), keeping the DB row and the disk folder on the same second regardless
of whether the DB engine rounds or truncates.
Add tests/zm_time.cpp covering the floor-not-round behaviour and consistency
with the to_time_t-derived path second.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GetImage() and getSnapshot() both ran the bounds check
`(size_t)index >= image_buffer.size()` before testing the sentinel
`index == image_buffer_count`. When no frames have been written yet,
shared_data->last_write_index equals image_buffer_count, the initial
fallback assigns that to index, and image_buffer.size() also equals
image_buffer_count (the vector is sized to image_buffer_count). So the
bounds check fired first, logging the misleading
"Image Buffer has not been allocated" and returning -1/nullptr instead
of reaching the intended "no images in buffer" branch (return 0 /
nullptr with a proper message). Reorder both functions to handle the
sentinel after confirming image_buffer is non-empty, then do the bounds
check, then the actual slot read.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The destination format for the same-format fast path was derived via the
deprecated AVPixFormat() getter, which re-derives from the legacy
(colours, subpixelorder) pair. If those fields ever drift out of sync
with imagePixFormat — or hit the GRAY8/YUV420P alias collision — the
fast path would compare against the wrong target and either fall
through to sws_scale unnecessarily or, worse, av_image_copy the wrong
layout. Use imagePixFormat directly and drop the manual
av_get_pix_fmt_name nullptr dance in the Debug log in favour of the
zm_get_pix_fmt_name wrapper.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Image::AssignDirect(const AVFrame*): both the invalidate() failure
path and the success path overwrote buffer/buffertype without freeing
any previously-owned buffer. If the Image currently owned a
ZM_BUFTYPE_ZM/MALLOC/NEW allocation, that memory was leaked on every
AssignDirect(frame) call. Added DumpImgBuffer() at the top of
invalidate() and immediately before the success-path buffer
reassignment; DumpBuffer is a no-op for DONTFREE buffers, so this is
safe whether the Image previously owned its memory or wrapped a
caller's buffer.
- Switched five remaining av_get_pix_fmt_name() calls flagged by review
to zm_get_pix_fmt_name():
* Monitor::GetAlarmImage warnings (zm_monitor.cpp:1432, 1438-1439)
* Monitor::ReadShmFrame warnings (zm_monitor.cpp:3042, 3049-3050)
* Camera ctor fallback Error (zm_camera.cpp:90)
All can be reached with AV_PIX_FMT_NONE or unrecognised formats where
av_get_pix_fmt_name returns nullptr, which is undefined when fed to %s.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Monitor::MonitorLink::score() previously returned 0 whenever the linked
monitor's state was not ALARM, even when its event was still open in
post-event countdown (ALERT). Combined with low AnalysisFPSLimit on the
source monitor (a common setup for HD/sub-stream pairs where the low-res
channel detects motion), state oscillates ALARM <-> ALERT around brief
motion pauses inside a single event. The link signal drops to 0 in those
gaps; the receiving monitor counts down its own PostEventCount and
closes the linked event prematurely — typically after a few seconds,
missing the remainder of a motion event that runs for minutes.
Return a sentinel score of 1 while the linked monitor is in ALERT so the
receiving event stays open. Do not return shared_data->last_frame_score
here: latching the score itself would re-apply a stale numeric to every
subsequent frame, inflating the accumulated score and biasing
snapshot.jpg selection. The sentinel keeps the open-event signal alive
without polluting the score channel.
Real-world evidence: monitor 28 (HD, links to monitor 36 sub-stream)
opened a 5.16-second linked event for a motion sequence that ran 300s
with two motion bursts 134s apart, missing the entire second burst.
With the latch, ALARM->ALERT transitions inside a single source event
are bridged. Long IDLE gaps (separate motion bursts) remain correctly
treated as separate triggers.
When no event is active, Monitor::Analyse() frees packet->image to save
RAM. Sensible while idle, but those packets remain in the PacketQueue as
the pre-event ring buffer. When motion later opens an event, the event
inherits the first N pre-event packets and Event::AddFrame writes
snapshot.jpg from packet->image. With image == nullptr, the snapshot is
never written.
Retain the image on keyframes (~one per GOP) while idle. Non-keyframes
are still freed, so the bulk of the RAM saving stays. When an event
opens, the pre-event window now contains at least one packet with a
valid image, giving Event::AddFrame something to snapshot from.
Cost is one retained Image per GOP per monitor while idle.
clear() was destroying ZMPackets while holding the queue mutex, unlike
queuePacket() and clearPackets() which defer destruction to a stack
vector that goes out of scope after the lock is released. ZMPacket
teardown frees Image, AVFrame, and AVPacket data and is expensive,
especially on PASSTHROUGH queues that may hold hundreds of MB. Doing it
under the mutex risked stalling capture and analysis threads during
shutdown or monitor reconfigure. Adopt the same deferred-destroy
pattern.
Also remove dead code identified in the audit:
- PacketQueue::unlock(ZMPacketLock*): defined but no callers; would
UAF if anyone passed a non-heap pointer.
- monitor_ field and setMonitor(): set never read, no callers.
- analysis_it field: declared, never used (Monitor::analysis_it is
the live one).
- get_stream_it(int) declaration: never defined.
- max_video_packet_count comment: clarify 0 means unlimited (the
setter clamps negatives to 0 and the limit check is > 0).
- Monitor::WriteShmFrame: image_pixelformats[index] was being recorded
even when image_buffer[index]->Assign(*capture_image) silently left
the slot untouched (Assign returns void; held-buffer undersize and
unknown src format both early-return without mutating the dest).
Readers would then adopt the newly-published format while the slot
still held the previous frame's bytes, producing garble. Check the
dest's post-Assign PixFormat against the source's and only publish on
match; warn and keep the previously-published format otherwise so
readers continue interpreting the slot bytes correctly.
- Monitor::WriteAlarmImage: same fix for alarm_image — verify Assign
adopted src.PixFormat() before publishing *alarm_image_pixelformat.
- Image::AVPixFormat(AVPixelFormat) setter: switched the four
format-name logs to the nullptr-safe zm_get_pix_fmt_name wrapper.
The first Error branch triggers exactly when new_pixelformat is
unrecognised, and av_get_pix_fmt_name returns nullptr for those.
- monitor.php: removed the manual translate fallback for
DeprecatedColoursSetting. web/includes/lang.php always merges
en_gb.php as a fallback when the active language differs, so
translate() never returns the bare key once en_gb.php defines it.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two Debug log sites in the hwaccel selection loop still called
av_get_pix_fmt_name directly. AV_PIX_FMT_NONE is a real possibility for
config->pix_fmt during the loop (the search may hit the sentinel) and
for hw_pix_fmt before find_fmt_by_hw_type returns a match. Route both
through zm_get_pix_fmt_name so a nullptr return can't be passed to %s.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Added zm_get_pix_fmt_name() in zm_pixformat.h: av_get_pix_fmt_name()
returns nullptr for unknown formats (including AV_PIX_FMT_NONE), and
passing nullptr to %s is undefined and can crash. Wraps with an
"unknown" fallback. Replaced the five raw uses flagged by review
(RemoteCameraRtsp/FfmpegCamera/VncCamera/LocalCamera constructors and
MonitorStream::sendFrame's Debug log) with the wrapper. Each can hit
AV_PIX_FMT_NONE before the first frame or via an unexpected
(colours, subpixelorder) pair.
- Monitor::GetImage and Monitor::getSnapshot now route the slot read
through ReadShmFrame so the per-slot AVPixelFormat zmc recorded is
adopted on image_buffer[index] before its bytes are interpreted.
Without this, the JPEG encode in GetImage and the ZMPacket returned
by getSnapshot would interpret SHM bytes using the placeholder format
set at attach time and produce garbled output when the slot's actual
format differs (the contract previously relied on callers calling
ReadShmFrame themselves). getSnapshot/GetTimestamp drop const since
ReadShmFrame mutates image_buffer[index]; callers in this codebase
already use a non-const Monitor pointer.
- Monitor::connect: updated the misleading "+64 bytes reserved" comment
to match the actual reservation of 63 + (alignof(AVPixelFormat) - 1).
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mem_size only reserved 64 bytes of slack for the 64-byte alignment of
shared_images, but the code subsequently rounds image_pixelformats up
to alignof(AVPixelFormat) too. In the worst case shared_images shifts
by 63 bytes and image_pixelformats shifts by alignof(AVPixelFormat)-1
more, which could push image_pixelformats / alarm_image_pixelformat
past the end of the mapped region. Reserve 63 + (alignof(AVPixelFormat)
- 1) bytes so both adjustments fit.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Image::Overlay (no-offset variant): rewrite all eight format branches
to walk row-by-row using each image's own linesize. The previous
linear walk with `buffer + size` as the end pointer drifted across
rows whenever this->linesize != image.linesize (which can happen for
images constructed via held-buffer ctors at a different stride) and
also walked into the destination's chroma planes for planar YUV
destinations whose `size` covers chroma. Inner loops bound by `width`
per row in every branch, so per-row padding is left untouched.
- Monitor::connect: align the image_pixelformats SHM base address up
to alignof(AVPixelFormat) before casting. shared_images +
2*image_buffer_count*image_size can be misaligned when image_size
isn't a multiple of alignof(AVPixelFormat) (e.g. GRAY8 with odd
width sourced from camera->ImageSize() before the SHM upper-bound
applies). An unaligned AVPixelFormat* is undefined behaviour on
strict-alignment ISAs and slow even on x86. The +64-byte padding
reserved in mem_size already covers the small shift.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The delta8_* and *_deinterlace_4field_* SIMD helpers process pixel runs
or 2D blocks without any concept of per-row stride. With the
FFALIGN(linesize, 32) layout, an Image's buffer may have per-row padding
(linesize > width*bpp), and feeding the helpers the raw buffer pointer
made them treat padding bytes as image data — wrong motion deltas and
visibly broken deinterlacing on non-32-aligned widths.
- Image::Delta: drive each delta8_* helper one row at a time, passing
`width` pixels per call with `buffer + y*src_linesize`,
`image.buffer + y*img_linesize`, and `pdiff + y*dst_linesize`. No
copies; just a stride-aware caller-side loop.
- Image::Deinterlace_4Field: the 4-field helpers internally use the
passed `width` as their row stride, so a per-row loop wouldn't
preserve their inter-row processing. Pack both inputs into
tightly-laid-out (linesize == width*bpp) temp buffers, run the
helper, then copy only the data bytes back into the padded source
buffer (padding stays intact). Fast path skips the copy when linesize
already equals width*bpp on both images (the 32-aligned-width case).
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Image::Assign(AVFrame*) fast path: PopulateFrame can fail
(av_buffer_create / av_image_fill_arrays errors); calling av_image_copy
on an unpopulated temp_frame is undefined. Check the return and bail
out cleanly on failure.
- Image::Scale: use the canonical imagePixFormat directly instead of
the deprecated AVPixFormat() getter (which re-derives via the legacy
(colours, subpixelorder) pair and would hit the GRAY8/YUV420P alias
collision if those drift).
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Image::DeColourise: updating only imagePixFormat=GRAY8 left size still
at the previous RGB sizing and linesize at the previous RGB stride.
Downstream ops that allocate via size (Flip/Rotate) under-allocate,
and row-stride loops that use linesize address the wrong rows. Now
recomputes size and linesize from the new GRAY8 layout via
av_image_get_buffer_size/av_image_get_linesize and calls
update_function_pointers().
- Image::AssignDirect(AVFrame*): never called update_function_pointers()
after the format change, so an Image that previously held a different
format kept stale fptr_delta/fptr_blend/fptr_convert bindings and
subsequent ops took the wrong optimized path. Added the call at the
end of the success path.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Camera::linesize/imagesize was changed by the migration to FFALIGN(.,32)
and av_image_get_buffer_size(...,32) to keep SIMD-friendly strides. But
Camera describes the *device-side* buffer that capture paths copy from
(V4L2 mmap buffers, raw RTP frames, etc.). Those external buffers are
tightly packed at the driver/source stride, not 32-byte aligned, so the
inflated imagesize causes:
- LocalCamera::PrimeCapture's `pSize != imagesize` check
(av_image_get_buffer_size(..., align=1) vs Camera::imagesize) to
Fatal for any width that isn't a multiple of 32.
- Other raw-socket capture paths to pass an oversized imagesize as the
source buffer size, risking out-of-bounds reads of driver-allocated
buffers.
Revert Camera::linesize/imagesize to align=1 (tightly packed). Image
internal buffers and SHM slots independently apply 32-byte alignment
where they need it (Monitor::connect already takes
max(camera->ImageSize(), av_image_get_buffer_size(RGBA, w, h, 32)) for
the SHM slot, so the SIMD-aligned slot capacity is preserved).
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Image::Rotate / Image::Flip: chroma plane dimensions were computed as
`width >> log2_chroma_w` / `height >> log2_chroma_h`, which floors.
For odd luma widths/heights the last chroma column/row was skipped,
leaving U/V samples unrotated/unflipped on odd-sized frames. Use
AV_CEIL_RSHIFT to match FFmpeg's plane dimension convention.
- Image::Assign(const Image&): the AV_PIX_FMT_NONE guard's error
message said "unexpected colours per pixel" and printed image.colours,
which is misleading — planar formats legitimately have colours==1.
Report the real cause (missing AVPixelFormat metadata) plus the
legacy (colours, subpixelorder) for context.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Image::HighlightEdges: the RGB24 and RGB32 output branches addressed
the destination buffer as `high_buff + ((y*linesize + lo_x) * 3|4)`,
where `linesize` is the *source* (GRAY8) stride. For non-32-aligned
widths the source GRAY8 stride and destination RGB24/RGB32 stride
differ in their padding, so multiplying the source stride by 3 or 4
doesn't yield the destination's row offset and writes spill past
high_buff on the last row. Read the destination's linesize from
high_image and use that for phigh. The neighbour-pixel lookups also
used `p ± width`; switched to `p ± src_linesize` so we follow the
actual source row stride. All three branches (GRAY8/RGB24/RGB32) now
pick neighbours via src_linesize.
- Image::MaskPrivacy: chroma plane dimensions were `width/2` and
`height/2` (truncating). For odd width/height the planar layout in
AVFrame uses ceil(W/2) / ceil(H/2), so the last column or row of
chroma was never neutralised, leaving a strip with the source's
original hue along the right/bottom edge — defeats the privacy mask.
Use (W+1)/2 and (H+1)/2; existing in-loop guards on the Y indices
already clamp the odd-edge lookup to the bitmap bounds.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
