A YUV420P chroma sample covers 2x2 luma pixels. Image::Crop started the
chroma planes at lo_x >> 1 and lo_y >> 1, so a crop starting on an odd
column or row had its chroma half a pixel off its luma. nph-zms zoom picks
any position, so a zoomed live view could show colour fringes.
For planar formats, move the crop window back to the chroma grid before
copying, keeping its size. Moving left or up can't leave the image.
Also use the existing ceil_rshift() for the chroma copy sizes instead of
writing the same calculation inline, moving it above Crop.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Image::Crop copied the cropped rows into a tightly packed
width * colours buffer. Since AssignDirect checks the size against
av_image_get_buffer_size(..., 32), it refused that buffer whenever a
cropped row was not a multiple of 32 bytes, and always for YUV420P,
whose chroma planes Crop did not copy at all. It logged "Attempt to
directly assign buffer from an undersized buffer", the image stayed
uncropped and the new buffer leaked. nph-zms crops every frame of a
zoomed live stream: zoom had no effect and the process grew by one
cropped frame per frame sent (about 39 MB in 10 s at 1280x720, 5 fps).
Allocate the result with av_image_get_buffer_size at 32-byte alignment
and copy each plane with its own offset and stride, as Image::Rotate and
Image::Flip do.
fixes#5265
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ReadJpeg() stored the JPEG header's width and height in the Image before
calling WriteBuffer(). WriteBuffer() reallocates only when the requested
size differs from the current one, so it saw no change and kept the
existing buffer and linesize, and the scanline loop then decoded every
row of a larger file past its end. A File monitor re-reads its source
into a monitor-sized image on every capture, so whoever can write that
file could overflow zmc's heap. Stored event JPEGs read by zms take the
same path.
Leave width and height to WriteBuffer(), as DecodeJpeg() already does.
FileCamera::Capture() also now refuses a file whose dimensions do not
match the monitor, since everything downstream is sized for the monitor.
Add a Catch2 case that reads a 256x192 JPEG into a 64x48 image. It
segfaulted before this change and passes after.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Since decoder-native passthrough, packet->image is typically YUV420P even
when the monitor's configured Colours is RGB, so the zone alarm highlight
(built from the configured colours) arrives at Image::Overlay() as
RGB24/RGB32 while the analysis image is planar YUV420. The dispatch keyed
on zm_bytes_per_pixel(target)==1, which conflates GRAY8 with planar YUV,
so the combination fell into the Colourise() branches: Colourise() warned
'Target image is already colourised, colours: 1' and returned, after which
the overlay loop walked the planar YUV buffer as packed RGB, corrupting
the saved analysis frames.
Gate the Colourise() branches to true GRAY8 targets and add a
YUV420-target branch that converts only the marked (non-black) highlight
pixels to YUV - luma plus the shared 2x2 chroma sample - honouring the
highlight's subpixel order.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HoNiNxwgyaHV29CbiueCUf
The "Target image is already colourised, colours: 1" warning fired from
Image::Colourise() during zone alarm overlay on YUV420P monitors, even with
ZM_COLOUR_JPEG_FILES off.
monitor->Colours() returns 1 for both GRAY8 and planar YUV420P (the GRAY8
alias), so zm_zone.cpp misclassified YUV420P monitors as grayscale and built
an RGB24 alarm highlight. Overlaying that RGB24 highlight onto the YUV420P
analysis_image hit the RGB-on-mono branch, which calls Colourise() - valid
only for GRAY8. Colourise() warned and bailed, after which the per-pixel loop
walked the 1.5x planar buffer as 3x packed RGB (out-of-bounds write).
Resolve the real capture format via zm_pixformat_from_colours() so true GRAY8
still upgrades to an RGB24 highlight while YUV420P keeps its format. Add a
YUV420P output branch to HighlightEdges (single alarm colour written to luma
and the shared chroma sample, transparent elsewhere) and a YUV420P-on-YUV420P
branch to Overlay (copies luma where non-zero, copies each chroma sample when
any covered luma pixel is marked). Colourise() is now only reached for GRAY8.
Add Catch2 coverage for the YUV420P Overlay masking and HighlightEdges output.
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>
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>
