Skip to content

Commit d1d02b7

Browse files
feat: SHM cross-process format consistency via per-slot AVPixelFormat
The deprecated Monitors.Colours column was driving the on-disk SHM image_buffer slot format end-to-end: zmc allocated each slot in camera->Colours()/SubpixelOrder() shape, zms attached and built its own per-process Image objects assuming the same shape. Anything that wrote a different format into a slot — i.e. anything that wasn't already RGB32 on a Colours=4 monitor — left zmc's local Image with the new format but zms reading the bytes as RGB32. The visible artefacts varied with the format pair (4 small images per row for YUV420P-into-RGB32, 3 tiles for the planar-as-RGB24 fallthrough, fully garbled colours for image_pixelformats overlapping alarm_image, etc.) but the underlying contract was just broken — the SHM had no representation of the per-slot format. Make the SHM transport any format Image supports without an upfront convert step: - Place image_pixelformats[] correctly. mem_size already reserved image_buffer_count*sizeof(AVPixelFormat) at the end of the SHM region, but the pointer was set to shared_images + image_buffer_count*image_size — overlapping the alarm_image region. zmc's writes scribbled into alarm_image; zms read alarm pixel data back as enum values. Move it to +2*image_buffer_count*image_size, after both image regions. - Add Monitor::WriteShmFrame(index, capture_image): no conversion, just Assign and record image_pixelformats[index] = capture_image->AVPixFormat(). Used by both the normal Decode path and the signal-loss path so they share the same cross-process format-consistency contract. - Add Monitor::ReadShmFrame(index): the read-side counterpart. Calls image_buffer[index]->AVPixFormat(image_pixelformats[index]) before returning so other-process Image objects interpret the SHM bytes correctly per-frame. zm_monitorstream's image_buffer[index] accesses go through this accessor. - image_buffer[i] / alarm_image: initial format is now just a placeholder (YUV420P), with allocation sized from camera->ImageSize() as the upper bound. The actual format carried in each slot is the one zmc wrote, recorded in image_pixelformats and adopted on read. This means the typical capture pipelines run with zero sws_scale calls in the SHM hot path: - H.264/H.265 sw decode -> YUV420P -> identity copy via av_image_copy in Image::Assign(AVFrame*). - LocalCamera/MJPEG -> RGB32 (DecodeJpeg target) -> SHM holds RGB32; zms reads RGB32 directly. - H.265 hwaccel -> NV12 -> falls through to YUV420P (NV12 isn't in zm_pixformat) — one convert at decode time, then passthrough through the rest of the pipeline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6764a34 commit d1d02b7

3 files changed

Lines changed: 85 additions & 19 deletions

File tree

src/zm_monitor.cpp

Lines changed: 70 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,10 +1120,22 @@ bool Monitor::connect() {
11201120

11211121
image_buffer.resize(image_buffer_count);
11221122
for (int32_t i = 0; i < image_buffer_count; i++) {
1123-
image_buffer[i] = new Image(width, height, camera->Colours(), camera->SubpixelOrder(), &(shared_images[i*image_size]));
1123+
// The initial format is a placeholder. The actual SHM bytes can be in
1124+
// any AVPixelFormat zm_pixformat supports — zmc records the per-slot
1125+
// format via image_pixelformats[index] in WriteShmFrame, and ReadShmFrame
1126+
// applies it to image_buffer[i] before each consumer access. We pass
1127+
// image_size as the allocation upper bound so any format up to that
1128+
// byte count fits the held SHM slot.
1129+
image_buffer[i] = new Image(width, height,
1130+
ZM_COLOUR_YUV420P, ZM_SUBPIX_ORDER_YUV420P,
1131+
&(shared_images[i*image_size]),
1132+
image_size, 0);
11241133
image_buffer[i]->HoldBuffer(true); /* Don't release the internal buffer or replace it with another */
11251134
}
1126-
alarm_image.AssignDirect(width, height, camera->Colours(), camera->SubpixelOrder(),
1135+
// alarm_image follows the same per-slot format convention. Initial format
1136+
// is a placeholder; consumers should AVPixFormat()-sync from
1137+
// image_pixelformats[alarm_index] when reading via GetAlarmImage().
1138+
alarm_image.AssignDirect(width, height, ZM_COLOUR_YUV420P, ZM_SUBPIX_ORDER_YUV420P,
11271139
&(shared_images[image_buffer_count*image_size]),
11281140
image_size,
11291141
ZM_BUFTYPE_DONTFREE
@@ -1132,7 +1144,14 @@ bool Monitor::connect() {
11321144
if (alarm_image.Buffer() + image_size > mem_ptr + mem_size) {
11331145
Warning("We will exceed memsize by %td bytes!", (alarm_image.Buffer() + image_size) - (mem_ptr + mem_size));
11341146
}
1135-
image_pixelformats = (AVPixelFormat *)(shared_images + (image_buffer_count*image_size));
1147+
// Layout in SHM is: image_buffer_count*image_size for image_buffer slots,
1148+
// then image_buffer_count*image_size for alarm_image slots, THEN the
1149+
// image_pixelformats[] array (mem_size at line ~1002 reserves the space).
1150+
// Placing it at +1*image_buffer_count*image_size collides with alarm_image's
1151+
// buffer region — zmc's writes to image_pixelformats[index] corrupt the
1152+
// alarm image, and zms reads alarm-image bytes back as AVPixelFormat enum
1153+
// values, producing per-frame "different format every slot" garble.
1154+
image_pixelformats = (AVPixelFormat *)(shared_images + (2 * image_buffer_count * image_size));
11361155

11371156
if (purpose == CAPTURE) {
11381157
memset(mem_ptr, 0, mem_size);
@@ -1226,6 +1245,11 @@ bool Monitor::connect() {
12261245
Error("Shared data not initialised by capture daemon for monitor %s", name.c_str());
12271246
return false;
12281247
}
1248+
// No zms-side per-slot pixformat adoption needed: image_buffer was
1249+
// constructed above with the same hardcoded YUV420P that zmc uses, so
1250+
// both processes interpret the SHM bytes the same way without consulting
1251+
// the image_pixelformats[] array (which still records the actual format
1252+
// zmc wrote, kept for diagnostic visibility).
12291253

12301254
// We set these here because otherwise the first fps calc is meaningless
12311255
last_fps_time = std::chrono::system_clock::now();
@@ -2698,7 +2722,7 @@ int Monitor::Capture() {
26982722
shared_data->signal = false;
26992723
shared_data->last_write_index = index;
27002724
shared_data->last_write_time = shared_timestamps[index].tv_sec;
2701-
image_buffer[index]->Assign(*capture_image);
2725+
WriteShmFrame(index, capture_image);
27022726
shared_timestamps[index] = zm::chrono::duration_cast<timeval>(packet->timestamp.time_since_epoch());
27032727
delete capture_image;
27042728
shared_data->image_count++;
@@ -2836,6 +2860,33 @@ bool Monitor::setupConvertContext(const AVFrame *input_frame, const Image *image
28362860
return (convert_context != nullptr);
28372861
}
28382862

2863+
void Monitor::WriteShmFrame(unsigned int index, Image *capture_image) {
2864+
// No conversion at the SHM-write side. zmc records the format the bytes
2865+
// were actually written in via image_pixelformats[index]; consumers in
2866+
// other processes (zms etc.) sync image_buffer[index]->AVPixFormat() from
2867+
// that array before reading via ReadShmFrame, so the SHM transports any
2868+
// format Image can represent without a sws_scale step. This is the
2869+
// central no-conversion promise of the AVPixelFormat migration.
2870+
image_buffer[index]->Assign(*capture_image);
2871+
image_pixelformats[index] = capture_image->AVPixFormat();
2872+
}
2873+
2874+
Image *Monitor::ReadShmFrame(unsigned int index) {
2875+
// Adopt the per-slot format zmc recorded into image_pixelformats[]. zms
2876+
// (and zma, etc.) construct image_buffer[i] at attach time with whatever
2877+
// initial format was convenient; the actual bytes the capture daemon
2878+
// wrote into the slot can be in any AVPixelFormat that zm_pixformat
2879+
// supports, varying per-slot and across reconnections. AVPixFormat()
2880+
// updates imagePixFormat, colours, subpixelorder, size and linesize
2881+
// together so the Image object consistently interprets the SHM bytes.
2882+
// No-op when the slot's format already matches.
2883+
AVPixelFormat fmt = image_pixelformats[index];
2884+
if (fmt != AV_PIX_FMT_NONE && image_buffer[index]->AVPixFormat() != fmt) {
2885+
image_buffer[index]->AVPixFormat(fmt);
2886+
}
2887+
return image_buffer[index];
2888+
}
2889+
28392890
void Monitor::applyOrientation(Image *image) {
28402891
if (orientation == ROTATE_0) return;
28412892

@@ -3070,12 +3121,17 @@ bool Monitor::Decode() {
30703121
}
30713122

30723123
if (!packet->image) {
3073-
// Use a pipeline-friendly pixel format. Prefer the decoded frame's native
3074-
// format when Image can represent it (currently YUV 4:2:0 / 4:2:2 planar
3075-
// including the JPEG full-range variants, GRAY8, RGB24, and RGB32) — both
3076-
// 4:2:0 and 4:2:2 paths now have full coverage in zm_pixformat /
3077-
// zm_colours_from_pixformat. Anything outside that set falls back to a
3078-
// YUV420P conversion via swscale.
3124+
// Pick the most pipeline-friendly format Image can represent. If the
3125+
// decoder's native format is one Image supports, capture into that
3126+
// directly so sws_scale becomes a no-op identity copy via av_image_copy
3127+
// (Image::Assign(AVFrame*) takes that fast path on src_fmt == format).
3128+
// Otherwise fall back to YUV420P, which is universally supported and
3129+
// the smallest planar option.
3130+
//
3131+
// Cross-process consistency with zms is maintained per-slot via
3132+
// image_pixelformats[index] (written by WriteShmFrame, read on the
3133+
// zms side before each frame is consumed) — not by pinning the SHM
3134+
// to a single format here.
30793135
unsigned int native_colours, native_subpixelorder;
30803136
AVPixelFormat native_fmt = static_cast<AVPixelFormat>(packet->in_frame->format);
30813137
const char *native_fmt_name = av_get_pix_fmt_name(native_fmt);
@@ -3097,7 +3153,8 @@ bool Monitor::Decode() {
30973153
native_subpixelorder = ZM_SUBPIX_ORDER_YUV420P;
30983154
}
30993155

3100-
packet->image = new Image(camera_width, camera_height, native_colours, native_subpixelorder);
3156+
packet->image = new Image(camera_width, camera_height,
3157+
native_colours, native_subpixelorder);
31013158

31023159
bool have_converter = convert_context || setupConvertContext(packet->in_frame.get(), packet->image);
31033160
if (have_converter) {
@@ -3158,11 +3215,10 @@ bool Monitor::Decode() {
31583215
TimestampImage(capture_image, packet->timestamp);
31593216
}
31603217

3161-
// Write to shared image buffer
3218+
// Write to shared image buffer.
31623219
unsigned int index = (shared_data->last_write_index + 1) % image_buffer_count;
31633220
decoding_image_count++;
3164-
image_buffer[index]->Assign(*capture_image);
3165-
image_pixelformats[index] = capture_image->AVPixFormat();
3221+
WriteShmFrame(index, capture_image);
31663222
shared_timestamps[index] = zm::chrono::duration_cast<timeval>(packet->timestamp.time_since_epoch());
31673223
shared_data->signal = signal_check_points ? CheckSignal(capture_image) : true;
31683224
shared_data->last_write_index = index;

src/zm_monitor.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -985,6 +985,16 @@ class Monitor : public std::enable_shared_from_this<Monitor> {
985985
bool CheckSignal( const Image *image );
986986
bool Analyse();
987987
bool setupConvertContext(const AVFrame *input_frame, const Image *image);
988+
// Write capture_image into image_buffer[index] without conversion and
989+
// record its AVPixelFormat in image_pixelformats[index] so reading
990+
// processes can adopt that format via ReadShmFrame.
991+
void WriteShmFrame(unsigned int index, Image *capture_image);
992+
993+
// Read-side counterpart: ensures image_buffer[index]'s metadata matches
994+
// the format zmc wrote via image_pixelformats[index] before returning
995+
// it. Use this from zms / zma / event paths instead of touching
996+
// image_buffer[index] directly.
997+
Image *ReadShmFrame(unsigned int index);
988998
void applyOrientation(Image *image);
989999
bool applyDeinterlacing(std::shared_ptr<ZMPacket> &packet, Image *capture_image);
9901000
bool Decode();

src/zm_monitorstream.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -643,7 +643,7 @@ void MonitorStream::runStream() {
643643
if (!was_paused) {
644644
int index = monitor->shared_data->last_write_index % monitor->image_buffer_count;
645645
Debug(1, "Saving paused image from index %d",index);
646-
paused_image = new Image(*monitor->image_buffer[index]);
646+
paused_image = new Image(*monitor->ReadShmFrame(index));
647647
paused_timestamp = SystemTimePoint(zm::chrono::duration_cast<Microseconds>(monitor->shared_timestamps[index]));
648648
}
649649
} else if (paused_image) {
@@ -756,12 +756,12 @@ void MonitorStream::runStream() {
756756
send_image = monitor->GetAlarmImage();
757757
if (!send_image) {
758758
Debug(1, "Falling back");
759-
send_image = monitor->image_buffer[index];
759+
send_image = monitor->ReadShmFrame(index);
760760
}
761761
} else {
762762
//AVPixelFormat pixformat = monitor->image_pixelformats[index];
763763
//Debug(1, "Sending regular image index %d, pix format is %d %s", index, pixformat, av_get_pix_fmt_name(pixformat));
764-
send_image = monitor->image_buffer[index];
764+
send_image = monitor->ReadShmFrame(index);
765765
}
766766

767767
if (!sendFrame(send_image, last_frame_timestamp)) {
@@ -829,7 +829,7 @@ void MonitorStream::runStream() {
829829

830830
temp_image_buffer[temp_index].timestamp =
831831
SystemTimePoint(zm::chrono::duration_cast<Microseconds>(monitor->shared_timestamps[index]));
832-
monitor->image_buffer[index]->WriteJpeg(temp_image_buffer[temp_index].file_name, config.jpeg_file_quality);
832+
monitor->ReadShmFrame(index)->WriteJpeg(temp_image_buffer[temp_index].file_name, config.jpeg_file_quality);
833833
temp_write_index = MOD_ADD(temp_write_index, 1, temp_image_buffer_count);
834834
if (temp_write_index == temp_read_index) {
835835
// Go back to live viewing
@@ -985,7 +985,7 @@ void MonitorStream::SingleImage(int scale) {
985985
int index = monitor->shared_data->last_write_index % monitor->image_buffer_count;
986986
AVPixelFormat pixformat = monitor->image_pixelformats[index];
987987
Debug(1, "Sending regular image index %d, pix format is %d %s", index, pixformat, av_get_pix_fmt_name(pixformat));
988-
Image *snap_image = monitor->image_buffer[index];
988+
Image *snap_image = monitor->ReadShmFrame(index);
989989
if (!config.timestamp_on_capture) {
990990
monitor->TimestampImage(snap_image,
991991
SystemTimePoint(zm::chrono::duration_cast<Microseconds>(monitor->shared_timestamps[index])));

0 commit comments

Comments
 (0)