fix: address PR #4788 review comments on AVPixelFormat migration

- 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>
This commit is contained in:
Isaac Connor
2026-05-22 19:35:50 -04:00
parent a7c3fb7ce6
commit 1fe29202f5
3 changed files with 49 additions and 16 deletions

View File

@@ -64,9 +64,29 @@ Camera::Camera(
mLastAudioDTS(AV_NOPTS_VALUE),
bytes(0),
mIsPrimed(false) {
linesize = FFALIGN(av_image_get_linesize(pixelFormat, width, 0), 32);
// Guard against an unknown (colours, subpixelorder) pair that produced
// pixelFormat == AV_PIX_FMT_NONE, or any case where av_image_get_*
// returns a negative size — assigning those to unsigned would wrap to a
// huge value and break SHM sizing and downstream allocations.
pixels = width * height;
imagesize = av_image_get_buffer_size(pixelFormat, width, height, 32);
if (pixelFormat == AV_PIX_FMT_NONE) {
Error("Camera: unknown pixel format from colours=%u subpixelorder=%u; falling back to width*colours stride",
p_colours, p_subpixelorder);
linesize = width * colours;
imagesize = static_cast<unsigned long long>(height) * linesize;
} else {
int raw_linesize = av_image_get_linesize(pixelFormat, width, 0);
int raw_imagesize = av_image_get_buffer_size(pixelFormat, width, height, 32);
if (raw_linesize < 0 || raw_imagesize < 0) {
Error("Camera: av_image_get_* returned %d/%d for pixelFormat=%s; falling back",
raw_linesize, raw_imagesize, av_get_pix_fmt_name(pixelFormat));
linesize = width * colours;
imagesize = static_cast<unsigned long long>(height) * linesize;
} else {
linesize = FFALIGN(raw_linesize, 32);
imagesize = static_cast<unsigned long long>(raw_imagesize);
}
}
Debug(2, "New camera id: %d width: %d line size: %d height: %d colours: %d subpixelorder: %d capture: %d, size: %llu",
monitor->Id(), width, linesize, height, colours, subpixelorder, capture, imagesize);

View File

@@ -1003,15 +1003,19 @@ void Image::Assign(const Image &image) {
if ( image.buffer != buffer ) {
if (image.linesize > linesize) {
// Source has more per-row padding than the destination can hold. Copy
// only the destination's row capacity per line to avoid writing past
// the destination buffer on the last row. Source padding bytes beyond
// the common row width are discarded.
// This branch is only reached when dimensions and colours/subpixelorder
// match but linesize disagrees, which is an oddly-shaped Image. Copy the
// Y/primary plane line by line; planar chroma planes are not handled
// match but linesize disagrees, which is an oddly-shaped Image. Only
// the Y/primary plane is copied; planar chroma planes are not handled
// here. The common path is the flat copy below.
Debug(1, "Must copy line by line due to different line size %d != %d", image.linesize, linesize);
uint8_t *src_ptr = image.buffer;
uint8_t *dst_ptr = buffer;
for (unsigned int i=0; i< image.height; i++) {
(*fptr_imgbufcpy)(dst_ptr, src_ptr, image.linesize);
(*fptr_imgbufcpy)(dst_ptr, src_ptr, linesize);
src_ptr += image.linesize;
dst_ptr += linesize;
}

View File

@@ -1240,11 +1240,14 @@ bool Monitor::connect() {
Error("Shared data not initialised by capture daemon for monitor %s", name.c_str());
return false;
}
// No zms-side per-slot pixformat adoption needed: image_buffer was
// constructed above with the same hardcoded YUV420P that zmc uses, so
// both processes interpret the SHM bytes the same way without consulting
// the image_pixelformats[] array (which still records the actual format
// zmc wrote, kept for diagnostic visibility).
// image_buffer[] is constructed above with a hardcoded YUV420P placeholder
// format because we don't yet know which AVPixelFormat zmc has written into
// each SHM slot. Readers (zms, zma, etc.) MUST call ReadShmFrame() before
// interpreting a slot's bytes — it adopts the per-slot AVPixelFormat from
// image_pixelformats[] (written by zmc in WriteShmFrame) and updates the
// Image's imagePixFormat, colours, subpixelorder, size and linesize
// together. Direct image_buffer[i] access without ReadShmFrame() will
// mis-interpret any slot whose actual format differs from the placeholder.
// We set these here because otherwise the first fps calc is meaningless
last_fps_time = std::chrono::system_clock::now();
@@ -1787,7 +1790,9 @@ bool Monitor::CheckSignal(const Image *image) {
}
}
if (pix_fmt == AV_PIX_FMT_GRAY8 || zm_is_yuv420(pix_fmt)) {
if (zm_bytes_per_pixel(pix_fmt) == 1) {
// GRAY8 plus all planar YUV variants we transport (420P/J420P/422P/J422P).
// For planar formats the Y plane is the first byte at each pixel index.
if (*(buffer+index) != grayscale_val)
return true;
@@ -2860,12 +2865,16 @@ bool Monitor::setupConvertContext(const AVFrame *input_frame, const Image *image
void Monitor::WriteShmFrame(unsigned int index, Image *capture_image) {
// No conversion at the SHM-write side. zmc records the format the bytes
// were actually written in via image_pixelformats[index]; consumers in
// other processes (zms etc.) sync image_buffer[index]->AVPixFormat() from
// that array before reading via ReadShmFrame, so the SHM transports any
// format Image can represent without a sws_scale step. This is the
// central no-conversion promise of the AVPixelFormat migration.
// other processes (zms etc.) sync image_buffer[index] from that array
// before reading via ReadShmFrame, so the SHM transports any format
// Image can represent without a sws_scale step. This is the central
// no-conversion promise of the AVPixelFormat migration.
//
// Record imagePixFormat directly via PixFormat() — AVPixFormat() re-derives
// from the deprecated (colours, subpixelorder) pair and could propagate
// stale metadata.
image_buffer[index]->Assign(*capture_image);
image_pixelformats[index] = capture_image->AVPixFormat();
image_pixelformats[index] = capture_image->PixFormat();
}
Image *Monitor::ReadShmFrame(unsigned int index) {