- 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>
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::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>
- 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>
- 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>
Six format-dispatch fallbacks were still printing only the legacy
`colours` value, but the dispatch in each spot is now keyed off
pixelFormat/imagePixFormat. With the GRAY8/YUV420P alias collision
(both colours=1) the legacy value frequently doesn't identify the true
format, so a "Unexpected colours: 1" log on a misroute is useless.
Updated each Panic to include the AVPixelFormat enum value and its
human-readable name (via av_get_pix_fmt_name), plus the legacy
(colours, subpixelorder) for context:
- RemoteCameraRtsp constructor
- FfmpegCamera constructor
- VncCamera constructor
- LocalCamera conversion-selection branch
- VideoStream::SetupCodec mpeg helper
- Image::Delta unknown-format fallback
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Image::Overlay(image, lo_x, lo_y): rebase source and destination
pointers per row using each image's own linesize. The previous
sequential psrc++ drifted across rows whenever linesize >
width*colours (i.e. non-32-aligned widths).
- Image::Rotate / Image::Flip: rewrite as plane-aware via
av_image_fill_arrays. Each plane is rotated/flipped independently
using its own dimensions and stride, so planar YUV (YUV420P/J420P
and YUV422P/J422P for 180/hflip/vflip; YUV420P/J420P additionally
for 90/270) no longer loses chroma. YUV422P 90/270 is explicitly
refused — the chroma subsampling would transpose to vertical-only,
which isn't the same AVPixelFormat. Packed RGB24/RGB32/GRAY8 go
through the same helper with bpp set per format. Inner helpers
use linesize for stride, so non-32-aligned widths no longer drift
into per-row padding.
- Image::AssignDirect(AVFrame*): derive the wrapped size from the
AVFrame's own linesize[] via av_image_fill_pointers rather than
av_image_get_buffer_size(..., 32) — decoder-allocated frames may
have a different alignment, so the 32-aligned size could overstate
the actual buffer. Also sanity-check that data[1]/data[2]/data[3]
are contiguous from data[0] (a non-contiguous AVFrame can't be
wrapped with a single Image::buffer pointer). Both failure paths
fully invalidate the Image (the existing helper).
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Continuation of the FFALIGN(linesize, 32) consumer fixes. Alignment is
kept for SIMD performance; only the addressing in consumers is fixed.
- Image::Outline: both branches (dx>=dy and dx<dy, all three colour
paths) now compute pixel addresses as buffer + y*linesize + x*colours
instead of width-based offsets, so lines aren't drawn into per-row
padding for non-32-aligned widths.
- Image::Fill(Rgb, density, Polygon): the scan-line polygon filler
computes each scanline base as buffer + scan_line*linesize + lo_x*bpp
in all three colour branches.
- Image::Deinterlace_Linear / Deinterlace_Blend /
Deinterlace_Blend_CustomRatio: all three colour branches address rows
by y*linesize (instead of y*width, y*width*3, y*width<<2), so they
blend the correct rows for non-32-aligned widths. Inner loops still
process exactly `width` pixels per row; per-row padding is untouched.
- Image::MaskPrivacy: after masking the Y plane, also neutralise the
chroma (U/V) samples covering masked pixels for planar YUV formats
(YUV420P/J420P/YUV422P/J422P). Previously, masking only the Y plane
left chroma carrying the original hue, so source colour bled through
the privacy mask — a real privacy leak when the capture format is
planar YUV. Conservative rule: if any covered Y bit is masked, set
the chroma sample to 128 (neutral). Plane pointers/strides come from
av_image_fill_arrays so the math works for any aligned plane layout.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When av_image_get_buffer_size or zm_colours_from_pixformat rejects the
AVFrame's format, AssignDirect(AVFrame) previously cleared
size/linesize/colours but left width/height assigned and buffer pointing
into frame->data[0]. A caller that ignored the error log could still
walk width*height pixels through that pointer — and once the AVFrame is
freed the buffer dangles. Clear width/height/pixels/buffer too so the
Image is unambiguously empty after a failed assign. Reorder validation
to run before any state mutation so a successful assign also leaves
every field consistent in a single step.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The FFALIGN(linesize, 32) introduced by the AVPixelFormat migration adds
per-row padding for non-32-aligned widths. Functions that walked pixel
buffers using width*bytes_per_pixel as the row stride wrote into padding,
shifted rows sideways, or under-allocated output buffers for those
widths. Convert each affected pixel op to use the Image's linesize as
the per-row stride. Alignment is kept for SIMD/perf; only the consumer
addressing is fixed.
- Image::MaskPrivacy: reset the row pointer to `buffer + y*linesize`
each iteration instead of monotonically incrementing across rows.
- Image::Annotate (GRAY8/RGB24/RGB32 branches): use linesize for the
per-row advance and char-block reset. The RGB32 branch additionally
uses `linesize / sizeof(Rgb)` as the Rgb-typed stride.
- Image::Fill and Image::Fill(colour, density, ...): compute each row's
base address as `buffer + y*linesize + lo_x*colours` instead of
`colours*(y*width + lo_x)`.
- Image::Colourise: allocate the output buffer with
av_image_get_buffer_size(target_fmt, w, h, 32) and copy row by row
using src_linesize / dst_linesize. The previous tightly-packed
allocation undersized the buffer (so AssignDirect would reject it)
and smeared pixels across row boundaries for non-aligned widths.
- Image::Deinterlace_Discard (all three colour branches): index source
and destination rows by y*linesize instead of (y*width)*bytes.
- Monitor::CheckSignal: convert the random linear index to (x, y) and
address as y*LineSize() + x*bytes_per_pixel, so samples don't read
per-row padding or land on the wrong row.
- Monitor::Capture signal-loss path: rewrote the publish-order comment
so it matches the actual code (WriteShmFrame, then timestamp, then
last_write_time, then last_write_index as the commit step).
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Image::Assign(raw buffer overload): derive new_size from
av_image_get_buffer_size(pix_fmt, w, h, 32) instead of
width*height*colours, which undercounts planar formats (YUV420P/J420P/
YUV422P/J422P) and would silently truncate U/V planes. Update linesize
in the same branch so LineSize() consumers (JPEG encode, overlay) stay
in sync with the new format. Reject AV_PIX_FMT_NONE and negative
av_image_get_buffer_size returns up-front.
- Image::AVPixFormat(AVPixelFormat) setter: validate the requested
format via zm_colours_from_pixformat() and check both av_image_*
return values before mutating any state. On failure, leave
imagePixFormat/size/linesize untouched so callers can recover instead
of inheriting wrapped-unsigned junk.
- Image::Assign(const Image&) stride-mismatch branch: refuse planar
formats (YUV420P/J420P/YUV422P/J422P) explicitly. The per-row copy
only touches the Y plane; doing it silently on planar input leaves
U/V uninitialised in the destination and produces solid-green output
downstream. Caller must reconvert or reallocate.
- Monitor::connect: size SHM slots to an upper bound across every
supported AVPixelFormat (RGBA at w*h, align=32) instead of the
monitor's configured camera->ImageSize(). Prevents "Held buffer is
undersized" failures when the no-conversion pipeline transports a
format larger than the legacy DB Colours selection (e.g. YUV422P or
RGBA into a GRAY8-configured monitor). Store the chosen capacity on
the Monitor as shm_slot_size.
- Monitor::ReadShmFrame: validate image_pixelformats[index] both as a
recognised enum AND that its av_image_get_buffer_size fits
shm_slot_size before calling AVPixFormat(fmt). Treats a corrupted or
torn SHM value as ignorable instead of letting it overrun the held
slot. Single zm_colours_from_pixformat() call (no longer duplicated).
- web/.../monitor.php: inline fallback for
translate('DeprecatedColoursSetting') so non-en_gb locales render an
English string instead of the literal key name.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Image::Overlay: in the 1-byte-per-pixel branch, bound the loop by
std::min(height*linesize, image.height*image.linesize) instead of
`buffer + size`. For planar YUV destinations `size` includes chroma
planes, while GRAY8 sources have only Y bytes; the old loop read past
image.buffer and clobbered the destination's chroma. Restricting to the
smaller of the two Y-plane spans keeps the overlay in the luma plane
and never over-reads the source.
- Monitor::ReadShmFrame: image_pixelformats[] lives in SHM and is written
by another process, so its value must be treated as untrusted. Validate
via zm_colours_from_pixformat() before passing into AVPixFormat(fmt) —
an unsupported enum value would make av_image_get_buffer_size return
negative and wrap into the Image's unsigned size/linesize members.
Compare against PixFormat() (canonical imagePixFormat) instead of the
deprecated AVPixFormat() getter.
- Monitor::Capture signal-loss path: reorder SHM publishing so the slot
bytes and per-slot shared_timestamps[index] are written first, then
last_write_time is set from packet->timestamp (the fresh value, not
the stale tv_sec the slot held from its previous occupant), and
last_write_index is assigned last as the commit step. Readers gate on
last_write_index, so all per-slot state must be visible before that
final assignment.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Image::Assign: when source linesize > destination linesize, copy only
the destination's row capacity per line; previously copied
image.linesize bytes into rows of smaller linesize, overflowing the
destination buffer on the last row.
- Monitor::CheckSignal: dispatch the 1-byte-per-pixel branch via
zm_bytes_per_pixel(pix_fmt) == 1 so YUV422P/J422P are sampled on the
Y plane like GRAY8/YUV420P, instead of falling through and reporting
"no signal".
- Monitor::WriteShmFrame: record image_pixelformats[index] via
capture_image->PixFormat() (canonical imagePixFormat) instead of
AVPixFormat(), which re-derives from the deprecated
(colours, subpixelorder) pair and could propagate stale metadata.
- Monitor::connect: rewrite stale comment that claimed readers don't
need per-slot pixformat adoption; readers MUST call ReadShmFrame()
to adopt the actual format zmc wrote.
- Camera::Camera: guard linesize/imagesize derivation against
AV_PIX_FMT_NONE and negative returns from av_image_get_linesize /
av_image_get_buffer_size, falling back to width * colours stride so
unsigned wrap-around can't break SHM sizing.
refs #4788
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Several Image methods were silently producing wrong output for planar
YUV formats and for any format-width pair where the natural linesize
isn't a multiple of 32. Each surfaced as a different visual artefact
once the SHM started carrying YUV420P/RGB32/etc. through to the
JPEG encoders without an upfront convert-to-RGB32 step:
- Image::Assign(const Image &): sized new_size from
image.height * image.linesize, which counts only the Y plane for
planar layouts. Use image.size, which is populated from
av_image_get_buffer_size and includes chroma. Without this an
Assign'd YUV420P image left Cb/Cr at zero — solid green output via
YCbCr-to-RGB later.
- Image::AssignDirect(width,height,colours,...): computed
new_buffer_size as W*H*p_colours and linesize as W*p_colours, both
wrong for planar (1.5x undersize) and for non-32-aligned RGB widths
(the actual buffer stride after sws_scale + av_image_fill_arrays
with align=32 is FFALIGN(W*bpp, 32), not W*bpp). Use
av_image_get_buffer_size and FFALIGN(av_image_get_linesize(...), 32)
so the recorded size/linesize match the real buffer layout.
Mismatched linesize produced the diagonal-shift artefact in
RGB32 streams at scaled widths like 1094.
- Image::WriteBuffer: same FFALIGN linesize fix; it was using the
unaligned natural linesize too.
- Image::Scale(new_width, new_height): scale_buffer was sized
(new_W+1)*(new_H+1)*colours which undercounts planar formats.
SWScale::Convert correctly checks the buffer against
av_image_get_buffer_size and returns an error, but the caller
ignored the return value and AssignDirect'd the empty/uninit
buffer anyway. Size with av_image_get_buffer_size, check the
Convert return, free and bail on failure.
- Image::Scale(factor): the hand-rolled pixel-doubling/decimation
loop treated the buffer as packed `colours`-bytes-per-pixel —
scaled only the Y plane and dropped chroma. Drop the loop and
delegate to Scale(new_width, new_height), which now uses sws_scale
for all formats.
- Image::EncodeJpeg + Image::WriteJpeg: format dispatch and
scanline writer had no planar-YUV support. WriteJpeg's existing
branch unpacked the buffer as packed YUYV 4:2:2 with offset
scanline*W*2, which read W*H/2 bytes past the end of any YUV420P
buffer — a latent crash that became reachable once image_buffer
could carry YUV420P data, segfaulting from the Event thread on
every event-frame write. Add a planar branch that uses
av_image_fill_arrays for plane pointers and per-plane linesizes
and feeds JCS_YCbCr scanlines built from the Y/U/V planes —
works for both 4:2:0 (YUV420P/YUVJ420P) and 4:2:2 (YUV422P/YUVJ422P).
- Image::Overlay: the warning fired on a benign GRAY8-on-YUV420P
case (both report colours=1 due to the GRAY8/YUV420P alias
collision in zm_rgb.h, but their imagePixFormat differs). Reframe
the check so it only warns when imagePixFormat actually matches
but the ZM (colours, subpixelorder) metadata diverges — i.e. a
real format-tracking bug.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.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>
Use av_image_copy instead of sws_scale when source and dest format +
dimensions match. Eliminates unnecessary per-frame pixel processing
for passthrough formats (e.g. YUVJ422P from MJPEG cameras).
refs #4735
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The migration from colours==ZM_COLOUR_GRAY8 to imagePixFormat==GRAY8
broke YUV420P handling: the old check matched both GRAY8 and YUV420P
(both had colours==1) but the new check only matched GRAY8.
For pixel-rendering operations (Annotate, Fill, Outline, DrawLine,
Rotate, Flip, Delta, MaskPrivacy, Deinterlace, Overlay) that work on
the Y-plane of any 1-byte-per-pixel format, replace the GRAY8-only
check with zm_bytes_per_pixel(imagePixFormat)==1. This covers GRAY8,
YUV420P, YUVJ420P, YUV422P, and YUVJ422P.
Format-identification sites (JPEG encoding, Colourise, DeColourise)
that genuinely distinguish grayscale from YUV are left unchanged.
refs #4735
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
After migrating format dispatch from colours to imagePixFormat,
several methods that update colours/subpixelorder were not also
updating imagePixFormat, leaving it stale. This caused format
misidentification downstream — e.g. DecodeJpeg falling back to
RGB24 while imagePixFormat still claimed RGBA, producing vertical
lines and washed-out colors in the live stream.
Fix WriteBuffer, Assign(buffer), Assign(Image), AssignDirect(buffer),
and AssignDirect(AVFrame) to keep imagePixFormat in sync.
refs #4735
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ZM_COLOUR_GRAY8, ZM_COLOUR_YUV420P, and ZM_COLOUR_YUVJ420P were all
defined to 1, making format identification via colours ambiguous.
LocalCamera misidentified YUV420P as GRAY8, causing V4L2 MJPEG cameras
to decode to grayscale via expensive sws_scale conversion.
Replace the legacy ZM_COLOUR_*/ZM_SUBPIX_ORDER_* integer pair with
AVPixelFormat as the single source of truth for pixel format dispatch:
- Add src/zm_pixformat.h with central format helpers:
zm_pixformat_from_colours, zm_colours_from_pixformat,
zm_bytes_per_pixel, zm_db_colours_to_pixformat, zm_is_rgb32,
zm_is_rgb24, zm_is_yuv420
- Add AVPixelFormat pixelFormat member + PixelFormat() accessor to Camera
- Add PixFormat() accessor to Image, delegate AVPixFormat methods
to shared helpers
- Migrate all ~100 format dispatch comparisons in zm_image.cpp,
zm_local_camera.cpp, zm_ffmpeg_camera.cpp, zm_remote_camera_rtsp.cpp,
zm_libvlc_camera.cpp, zm_libvnc_camera.cpp, zm_monitor.cpp,
zm_mpeg.cpp from colours/subpixelorder checks to imagePixFormat/
AVPixelFormat checks
- Deprecate GetFFMPEGPixelFormat, delegate to zm_pixformat_from_colours
- Fix DeColourise bug: imagePixFormat was not updated to GRAY8
- Deprecate ZM_COLOUR_* and ZM_SUBPIX_ORDER_* constants in zm_rgb.h
- Add deprecation notice on Monitor.Colours web UI dropdown
- Add 13 Catch2 test cases (105 assertions) for format mapping helpers
refs #4735
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Image::Fill(Polygon) implements scan-line polygon fill but iterated
through active edges one at a time instead of in pairs. For convex
polygons (always exactly 2 active edges per scan line) this happened
to work, but for non-convex polygons it would fill the gaps between
concave sections.
A banana-shaped zone, for example, would have its inner concave area
incorrectly marked as inside the zone, causing motion detection to
trigger on the area the user explicitly drew the zone to avoid.
Fix by stepping the iterator by 2 to fill between pairs of edges
following the standard parity rule for scan-line polygon fill.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add www-data to video and dialout groups in Debian/Ubuntu postinst
scripts so zmc can access /dev/video* devices on fresh installs.
RedHat packaging already handled this via gpasswd in %post.
Add compile-time ZM_FONTDIR to zm_config_data.h.in and use it as a
fallback in zm_image.cpp when the configured font_file_location is not
found, fixing font loading failures caused by stale DB config values.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
colours and subpixelorder were set to GRAY8/NONE before the
format detection logic, so RGB32 SSSE3/fast paths were never
reached and subpixel order switches always hit the default case.
Save originals before overwriting and set output values after
conversion.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace per-frame AllocBuffer/free cycle with a persistent
blend_buffer_ member that is allocated once and reused across
calls. For non-holdbuffer images, swap buffer pointers instead
of freeing and reallocating. Eliminates ~8MB alloc+free per
frame for 1080p RGBA.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Merge() reset pdest and psrc to buffer base on every pixel iteration,
so only buffer[0] was ever written. Move pdest outside the loop and
use direct array indexing for source buffers.
Highlight() had the same pointer reset bug plus iterated over size
(total bytes) instead of pixels, and the unsigned diff calculation
wrapped around instead of computing absolute difference. Fix pointer
management, loop bound, and diff comparison.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Previously, when av_buffer_create() failed and returned nullptr, the code
only issued a Warning and continued, assigning the null pointer to the
frame buffer and returning success. Now properly returns -1 on failure.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
