fix: corrupted scaled/converted output for rotated monitors (SWScale buffer alignment)#4909
Merged
connortechnology merged 1 commit intoJun 13, 2026
Conversation
… guessed buffer alignment from width 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>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes corrupted scale/convert output produced by SWScale::Convert when processing rotated monitors (common widths like 720/2160 that are not multiples of 32). The core change is to stop guessing buffer row alignment from dimensions and instead require callers to pass the actual alignment used to lay out each buffer, matching how ZoneMinder allocates image buffers and how certain raw sources (e.g., VNC) are packed.
Changes:
- Update
SWScale::Convert/GetBufferSizeAPIs to accept explicit input/output buffer alignments and remove the unused defaults API. - Plumb correct alignment values through
Image::Scale, the VideoStore encode conversion path, and the libVNC capture path. - Add a regression test that reproduces the rotated-monitor scaling corruption for YUV420P at width 720.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/zm_swscale.h |
Updates SWScale API to require explicit buffer alignments; removes defaults API. |
src/zm_swscale.cpp |
Implements alignment-aware buffer sizing/fill and conversion behavior. |
src/zm_image.cpp |
Ensures Image::Scale passes align-32 for both in/out Image buffers. |
src/zm_videostore.cpp |
Passes output alignment matching ZMPacket::get_out_frame allocation choice. |
src/zm_libvnc_camera.cpp |
Passes in/out alignments for packed VNC input and align-32 Image output. |
tests/zm_image.cpp |
Adds regression coverage for scaling YUV420P at non-32-multiple widths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mRfb->si.framebufferWidth * mRfb->si.framebufferHeight * 4, | ||
| //SWScale::GetBufferSize(AV_PIX_FMT_RGBA, mRfb->si.framebufferWidth, mRfb->si.framebufferHeight), | ||
| directbuffer, | ||
| width * height * colours, |
Comment on lines
+129
to
+133
| /* Check the buffer sizes against the layout each alignment implies */ | ||
| size_t needed_insize = GetBufferSize(in_pf, width, height, in_alignment); | ||
| if (needed_insize > in_buffer_size) { | ||
| Warning( | ||
| "The input buffer size does not match the expected size for the input format. Required: %zu for %dx%d %d Available: %zu", | ||
| "The input buffer size does not match the expected size for the input format. Required: %zu for %dx%d %d align %d Available: %zu", |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SWScale::Convertchoseav_image_fill_arraysalignment by heuristic —(width % 32 ? 1 : 32)— for both the source and destination buffers. ZoneMinder image buffers are always laid out align-32, so any width not divisible by 32 madeConvertread luma rows 16 bytes short and chroma planes from packed offsets: diagonal shear plus garbage chroma.Rotating a monitor is exactly what produces such widths:
1280x720 ROTATE_270→ 720 wide (720 % 32 == 16)3840x2160 ROTATE_90→ 2160 wide (2160 % 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 recently merged rotate/flip segfault fix (#4900) is what exposed this — before it, rotated planar frames crashed
zmcbefore ever reachingScale.Fix
Alignment is a fact about how a buffer was laid out, not something derivable from its dimensions.
Convertnow takes explicit in/out alignment from each caller:Image::Scalepasses 32/32get_out_frame's allocation choicelibvncpasses 1 (packed VNC framebuffer) for input and 32 (ImageWriteBuffer) for outputThe unused
SetDefaults/ConvertDefaultsAPI is removed rather than threading alignments through dead code.Testing
New
Scaleregression 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.Files:
src/zm_image.cpp,src/zm_swscale.cpp/.h,src/zm_videostore.cpp,src/zm_libvnc_camera.cpp,tests/zm_image.cpp.🤖 Generated with Claude Code