fix: carry the zone's alarm colour on the alarmed and filtered pixel methods

The alarmed/filtered pixel check methods handed Overlay() the raw GRAY8
scoring mask. Overlay keys on a non-zero source pixel and copies that byte, so
the highlight took whatever shape one byte has in the destination format: the
red channel on an RGB32 monitor, which is the whole reason alarms have always
come out red; all three channels on RGB24, giving white; luma alone on YUV420.
The zone's configured Alarm Colour was honoured only on the blob path, which
goes through HighlightEdges.

Generalise HighlightEdges into BuildHighlight, which takes an edges_only flag
and otherwise fills every marked pixel, and build the highlight for the pixel
methods the same way the blob path already builds its outline: in the
capture's own pixel format, carrying alarm_rgb, once scoring is finished with
the GRAY8 mask. HighlightEdges stays as a thin wrapper so the blob path and
its callers are unchanged.

This is a behaviour change: monitors left on the default red see no
difference, but a zone configured with any other Alarm Colour now paints that
colour instead of red or white.

Reverting the zone hunk fails the new test in all three of its sections.
Full suite: 148 cases, 12535 assertions.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WBHBB95RBX7D9p8ge2WDZb

src/zm_image.cpp

@@ -1175,14 +1175,15 @@ void Image::Assign(const Image &image) {
   }
 }
 
-Image *Image::HighlightEdges(
+Image *Image::BuildHighlight(
   Rgb colour,
   unsigned int p_colours,
   unsigned int p_subpixelorder,
-  const Box *limits
+  const Box *limits,
+  bool edges_only
 ) {
   if ( imagePixFormat != AV_PIX_FMT_GRAY8 ) {
-    Panic("Attempt to highlight image edges when colours = %d", colours);
+    Panic("Attempt to highlight image when colours = %d", colours);
   }
 
   /* Convert the colour's RGBA subpixel order into the image's subpixel order */
@@ -1215,12 +1216,13 @@ Image *Image::HighlightEdges(
       const uint8_t* p = buffer + (y * src_linesize) + lo_x;
       uint8_t* phigh = high_buff + (y * dst_linesize) + lo_x;
       for ( unsigned int x = lo_x; x <= hi_x; x++, p++, phigh++ ) {
-        bool edge = false;
+        bool mark = false;
         if ( *p ) {
-          edge = (x > 0 && !*(p-1)) || (x < (width-1) && !*(p+1))
+          mark = !edges_only
+              || (x > 0 && !*(p-1)) || (x < (width-1) && !*(p+1))
               || (y > 0 && !*(p-src_linesize)) || (y < (height-1) && !*(p+src_linesize));
         }
-        if ( edge ) {
+        if ( mark ) {
           *phigh = colour;
         }
       }
@@ -1230,12 +1232,13 @@ Image *Image::HighlightEdges(
       const uint8_t* p = buffer + (y * src_linesize) + lo_x;
       uint8_t* phigh = high_buff + (y * dst_linesize) + lo_x * 3;
       for ( unsigned int x = lo_x; x <= hi_x; x++, p++, phigh += 3 ) {
-        bool edge = false;
+        bool mark = false;
         if ( *p ) {
-          edge = (x > 0 && !*(p-1)) || (x < (width-1) && !*(p+1))
+          mark = !edges_only
+              || (x > 0 && !*(p-1)) || (x < (width-1) && !*(p+1))
               || (y > 0 && !*(p-src_linesize)) || (y < (height-1) && !*(p+src_linesize));
         }
-        if ( edge ) {
+        if ( mark ) {
           RED_PTR_RGBA(phigh) = RED_VAL_RGBA(colour);
           GREEN_PTR_RGBA(phigh) = GREEN_VAL_RGBA(colour);
           BLUE_PTR_RGBA(phigh) = BLUE_VAL_RGBA(colour);
@@ -1247,12 +1250,13 @@ Image *Image::HighlightEdges(
       const uint8_t* p = buffer + (y * src_linesize) + lo_x;
       Rgb* phigh = (Rgb*)(high_buff + (y * dst_linesize) + (lo_x << 2));
       for ( unsigned int x = lo_x; x <= hi_x; x++, p++, phigh++ ) {
-        bool edge = false;
+        bool mark = false;
         if ( *p ) {
-          edge = (x > 0 && !*(p-1)) || (x < (width-1) && !*(p+1))
+          mark = !edges_only
+              || (x > 0 && !*(p-1)) || (x < (width-1) && !*(p+1))
               || (y > 0 && !*(p-src_linesize)) || (y < (height-1) && !*(p+src_linesize));
         }
-        if ( edge ) {
+        if ( mark ) {
           *phigh = colour;
         }
       }
@@ -1260,24 +1264,25 @@ Image *Image::HighlightEdges(
   } else if ( zm_is_yuv420(p_pixfmt) ) {
     // Single alarm colour over a transparent (Y=0) background; Overlay() onto
     // a YUV420 image keys on a non-zero luma marker. Write the colour's luma
-    // at each edge pixel and its chroma at the shared 2x2 chroma sample.
+    // at each marked pixel and its chroma at the shared 2x2 chroma sample.
     const YUV yuv = brg_to_yuv(colour);
     const uint8_t Yc = Y_VAL(yuv), Uc = U_VAL(yuv), Vc = V_VAL(yuv);
     uint8_t *hplane[4] = {};
     int hstride[4] = {};
     if (av_image_fill_arrays(hplane, hstride, high_buff, p_pixfmt, width, height, 32) < 0) {
-      Error("HighlightEdges: av_image_fill_arrays failed for YUV420 %ux%u", width, height);
+      Error("Highlight: av_image_fill_arrays failed for YUV420 %ux%u", width, height);
       return high_image;
     }
     for ( unsigned int y = lo_y; y <= hi_y; y++ ) {
       const uint8_t* p = buffer + (y * src_linesize) + lo_x;
       for ( unsigned int x = lo_x; x <= hi_x; x++, p++ ) {
-        bool edge = false;
+        bool mark = false;
         if ( *p ) {
-          edge = (x > 0 && !*(p-1)) || (x < (width-1) && !*(p+1))
+          mark = !edges_only
+              || (x > 0 && !*(p-1)) || (x < (width-1) && !*(p+1))
               || (y > 0 && !*(p-src_linesize)) || (y < (height-1) && !*(p+src_linesize));
         }
-        if ( edge ) {
+        if ( mark ) {
           hplane[0][y * hstride[0] + x] = Yc ? Yc : 1;  // keep the luma marker non-zero
           hplane[1][(y / 2) * hstride[1] + (x / 2)] = Uc;
           hplane[2][(y / 2) * hstride[2] + (x / 2)] = Vc;

src/zm_image.h

@@ -277,7 +277,13 @@ class Image {
                 uint8 size = 1,
                 Rgb fg_colour = kRGBWhite,
                 Rgb bg_colour = kRGBBlack);
-  Image *HighlightEdges( Rgb colour, unsigned int p_colours, unsigned int p_subpixelorder, const Box *limits=0 );
+  // Build a colour highlight image from this GRAY8 mask. edges_only traces the
+  // outline of each marked region (what the blob check method shows); false
+  // fills every marked pixel (what the alarmed/filtered pixel methods show).
+  Image *BuildHighlight( Rgb colour, unsigned int p_colours, unsigned int p_subpixelorder, const Box *limits=0, bool edges_only=true );
+  Image *HighlightEdges( Rgb colour, unsigned int p_colours, unsigned int p_subpixelorder, const Box *limits=0 ) {
+    return BuildHighlight(colour, p_colours, p_subpixelorder, limits, true);
+  }
   //Image *HighlightEdges( Rgb colour, const Polygon &polygon );
   void Timestamp(const char *label, SystemTimePoint when, const Vector2 &coord, int label_size);
   void Colourise(const unsigned int p_reqcolours, const unsigned int p_reqsubpixelorder);

src/zm_zone.cpp

@@ -154,6 +154,24 @@ void Zone::SetScore(unsigned int nScore) {
   stats.score_ = nScore;
 }  // end void Zone::SetScore(unsigned int nScore)
 
+// Replace the GRAY8 analysis mask with a highlight in the capture's own pixel
+// format, carrying this zone's alarm colour, and take ownership of it in
+// `image`. Overlay() onto the analysis image is then a same-format copy.
+// monitor->Colours()==1 aliases both GRAY8 and planar YUV420P, so resolve the
+// real format first: a true GRAY8 monitor has no colour to carry, so upgrade
+// its highlight to RGB24; YUV420P keeps its format and carries the alarm
+// colour in chroma.
+void Zone::BuildAlarmHighlight(Image *mask, bool edges_only) {
+  AVPixelFormat capture_fmt = zm_pixformat_from_colours(monitor->Colours(), monitor->SubpixelOrder());
+  if (capture_fmt == AV_PIX_FMT_GRAY8) {
+    image = mask->BuildHighlight(alarm_rgb, ZM_COLOUR_RGB24, ZM_SUBPIX_ORDER_RGB, &polygon.Extent(), edges_only);
+  } else {
+    image = mask->BuildHighlight(alarm_rgb, monitor->Colours(), monitor->SubpixelOrder(), &polygon.Extent(), edges_only);
+  }
+  // 'image' is now detached from the mask we were handed, so that is ours to free.
+  delete mask;
+}
+
 void Zone::SetAlarmImage(const Image* srcImage) {
   if ( image )
     delete image;
@@ -795,15 +813,19 @@ bool Zone::CheckAlarms(const Image *delta_image) {
       // aliases both GRAY8 and planar YUV420P, so resolve the real format first:
       // a true GRAY8 monitor has no colour to carry, so upgrade its highlight to
       // RGB24; YUV420P keeps its format and carries the alarm colour in chroma.
-      AVPixelFormat capture_fmt = zm_pixformat_from_colours(monitor->Colours(), monitor->SubpixelOrder());
-      if (capture_fmt == AV_PIX_FMT_GRAY8) {
-        image = diff_image->HighlightEdges(alarm_rgb, ZM_COLOUR_RGB24, ZM_SUBPIX_ORDER_RGB, &polygon.Extent());
-      } else {
-        image = diff_image->HighlightEdges(alarm_rgb, monitor->Colours(), monitor->SubpixelOrder(), &polygon.Extent());
-      }
-
-      // Only need to delete this when 'image' becomes detached and points somewhere else
-      delete diff_image;
+      BuildAlarmHighlight(diff_image, true);
+      diff_image = nullptr;
+    } else if ((type < PRECLUSIVE) && (monitor->GetOptSaveJPEGs() > 1)) {
+      // Alarmed/filtered pixels: the mask stays GRAY8 through the analysis
+      // above because the scoring reads it as one byte per pixel. Overlay()
+      // onto the analysis image then keys on a non-zero source pixel, so a
+      // GRAY8 mask painted an undifferentiated highlight -- writing the mask
+      // value into the red channel on an RGB32 monitor (hence "alarms are
+      // always red"), into all three on RGB24 (white), and into luma alone on
+      // YUV420 -- and the zone's configured Alarm Colour was honoured only on
+      // the blob path. Build the same colour highlight the blob path builds,
+      // filled rather than outlined, now that scoring is done with the mask.
+      BuildAlarmHighlight(diff_image, false);
       diff_image = nullptr;
     }  // end if ( (type < PRECLUSIVE) && (check_method >= BLOBS) && (monitor->GetOptSaveJPEGs() > 1)
 

src/zm_zone.h

@@ -226,6 +226,7 @@ class Zone {
   int GetExtendAlarmFrames();
   void SetScore(unsigned int nScore);
   void SetAlarmImage(const Image* srcImage);
+  void BuildAlarmHighlight(Image *mask, bool edges_only);
 
   inline const Image *getPgImage() const { return pg_image; }
   inline const Range *getRanges() const { return ranges; }

tests/CMakeLists.txt

@@ -41,6 +41,7 @@ set(TEST_SOURCES
   zm_vector2.cpp
   zm_zone.cpp
   zm_zone_stride.cpp
+  zm_zone_highlight.cpp
   zm_image_linesize.cpp
   zm_logger_rotate.cpp
   zm_ffmpeg_camera.cpp

tests/zm_zone_highlight.cpp

@@ -0,0 +1,217 @@
+/*
+ * This file is part of the ZoneMinder Project. See AUTHORS file for Copyright information
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "zm_catch2.h"
+
+#include "zm_config.h"
+#include "zm_image.h"
+#include "zm_monitor.h"
+#include "zm_rgb.h"
+#include "zm_zone.h"
+
+#include <algorithm>
+#include <cstring>
+#include <memory>
+#include <vector>
+
+// The alarmed/filtered pixel check methods used to hand Overlay() the raw GRAY8
+// scoring mask. Overlay keys on a non-zero source pixel and copies it, so the
+// highlight took whatever shape that byte had in the destination format: the
+// red channel on an RGB32 monitor (which is why alarms were "always red"), all
+// three channels on RGB24 (white), luma alone on YUV420. The zone's configured
+// Alarm Colour was only ever honoured on the blob path, via HighlightEdges.
+//
+// The highlight must now carry the zone's alarm colour on every check method
+// and colour depth, filled for the pixel methods and outlined for blobs.
+namespace {
+
+void EnsureConfig() {
+  if (!config.font_file_location) config.font_file_location = "";
+  if (!config.event_close_mode) config.event_close_mode = "idle";
+}
+
+class TestMonitor : public Monitor {
+ public:
+  TestMonitor(unsigned int w, unsigned int h, unsigned int c) : Monitor() {
+    width = w;
+    height = h;
+    colours = c;
+    // >1 is what gates building the highlight at all.
+    savejpegs = 2;
+  }
+};
+
+std::shared_ptr<Monitor> MakeMonitor(unsigned int w, unsigned int h, unsigned int c) {
+  return std::static_pointer_cast<Monitor>(std::make_shared<TestMonitor>(w, h, c));
+}
+
+Polygon FullFramePolygon(unsigned int w, unsigned int h) {
+  std::vector<Vector2> vertices = {Vector2(0, 0), Vector2(w - 1, 0),
+                                   Vector2(w - 1, h - 1), Vector2(0, h - 1)};
+  return Polygon(vertices);
+}
+
+std::unique_ptr<Image> MakeSaturatedDelta(unsigned int w, unsigned int h) {
+  auto img = std::unique_ptr<Image>(
+      new Image(w, h, ZM_COLOUR_GRAY8, ZM_SUBPIX_ORDER_NONE));
+  for (unsigned int y = 0; y < h; y++)
+    for (unsigned int x = 0; x < w; x++) *img->Buffer(x, y) = 255;
+  return img;
+}
+
+// A colour no channel-smearing bug can produce by accident: it is not white,
+// not pure red, and its three channels are all different.
+constexpr Rgb kAlarmColour = 0x20c0ff;
+constexpr uint8_t kR = 0x20, kG = 0xc0, kB = 0xff;
+
+// Monitor::SubpixelOrder() reads through the camera, which a bare test Monitor
+// does not have, so which byte holds which channel is not pinned down here.
+// Checking the three bytes as a set still fails for every way the old code got
+// this wrong: white is {ff,ff,ff}, red-channel-only is {xx,00,00}.
+bool ChannelsAre(const uint8_t *px, uint8_t a, uint8_t b, uint8_t c) {
+  std::vector<uint8_t> got = {px[0], px[1], px[2]};
+  std::vector<uint8_t> want = {a, b, c};
+  std::sort(got.begin(), got.end());
+  std::sort(want.begin(), want.end());
+  return got == want;
+}
+
+}  // namespace
+
+TEST_CASE("Zone alarm highlight carries the zone's alarm colour", "[Zone]") {
+  EnsureConfig();
+
+  // 720 is deliberately not 32-byte aligned in either depth, so this also
+  // exercises the padded-stride path the striping fix covers.
+  const unsigned int w = 720, h = 480;
+
+  SECTION("RGB24 monitor: filled with the alarm colour, not white") {
+    auto monitor = MakeMonitor(w, h, ZM_COLOUR_RGB24);
+    Zone zone(monitor, 1, "full", Zone::ACTIVE, FullFramePolygon(w, h),
+              kAlarmColour, Zone::ALARMED_PIXELS,
+              /*min_pixel_threshold*/ 10, /*max_pixel_threshold*/ 0,
+              /*min_alarm_pixels*/ 1, /*max_alarm_pixels*/ static_cast<int>(w * h));
+
+    auto delta = MakeSaturatedDelta(w, h);
+    REQUIRE(zone.CheckAlarms(delta.get()));
+
+    const Image *alarm = zone.AlarmImage();
+    REQUIRE(alarm != nullptr);
+    REQUIRE(alarm->Colours() == ZM_COLOUR_RGB24);
+
+    const uint8_t *px = alarm->Buffer(w / 2, h / 2);
+    CHECK(ChannelsAre(px, kR, kG, kB));
+    // The old behaviour wrote the mask byte into all three channels.
+    CHECK_FALSE((px[0] == 0xff && px[1] == 0xff && px[2] == 0xff));
+  }
+
+  SECTION("RGB32 monitor: alarm colour, not just the red channel") {
+    auto monitor = MakeMonitor(w, h, ZM_COLOUR_RGB32);
+    Zone zone(monitor, 1, "full", Zone::ACTIVE, FullFramePolygon(w, h),
+              kAlarmColour, Zone::ALARMED_PIXELS,
+              /*min_pixel_threshold*/ 10, /*max_pixel_threshold*/ 0,
+              /*min_alarm_pixels*/ 1, /*max_alarm_pixels*/ static_cast<int>(w * h));
+
+    auto delta = MakeSaturatedDelta(w, h);
+    REQUIRE(zone.CheckAlarms(delta.get()));
+
+    const Image *alarm = zone.AlarmImage();
+    REQUIRE(alarm != nullptr);
+    REQUIRE(alarm->Colours() == ZM_COLOUR_RGB32);
+
+    const uint8_t *px = alarm->Buffer(w / 2, h / 2);
+    CHECK(ChannelsAre(px, kR, kG, kB));
+    // The old behaviour left two of the three channels at zero.
+    CHECK_FALSE((px[1] == 0 && px[2] == 0));
+  }
+
+  SECTION("the highlight is filled, not just outlined, for alarmed pixels") {
+    auto monitor = MakeMonitor(w, h, ZM_COLOUR_RGB24);
+    Zone zone(monitor, 1, "full", Zone::ACTIVE, FullFramePolygon(w, h),
+              kAlarmColour, Zone::ALARMED_PIXELS,
+              /*min_pixel_threshold*/ 10, /*max_pixel_threshold*/ 0,
+              /*min_alarm_pixels*/ 1, /*max_alarm_pixels*/ static_cast<int>(w * h));
+
+    auto delta = MakeSaturatedDelta(w, h);
+    REQUIRE(zone.CheckAlarms(delta.get()));
+
+    const Image *alarm = zone.AlarmImage();
+    REQUIRE(alarm != nullptr);
+
+    // Every pixel alarmed, so every pixel must be painted -- an outline would
+    // leave the interior black.
+    unsigned int painted = 0;
+    for (unsigned int y = 0; y < h; y++) {
+      const uint8_t *row = alarm->Buffer(0, y);
+      for (unsigned int x = 0; x < w; x++)
+        if (row[x * 3] || row[x * 3 + 1] || row[x * 3 + 2]) painted++;
+    }
+    CHECK(painted == w * h);
+  }
+}
+
+// The highlight is built from a GRAY8 mask into a wider destination whose row
+// stride is padded independently of the source's. Indexing the destination
+// with the source's stride sheared the highlight diagonally across the frame
+// and ran past the end of the destination on the last rows -- the same
+// width-vs-linesize family as the striping fixes, but inside the highlight
+// builder, where it stayed latent until the pixel check methods started using
+// it. Run the same assertion at a padded width and an aligned one so a failure
+// says whether the stride is to blame.
+TEST_CASE("Image::BuildHighlight lands the highlight where the mask marked it", "[Image]") {
+  EnsureConfig();
+
+  auto check = [](unsigned int w, unsigned int h) {
+    Image mask(w, w, h, ZM_COLOUR_GRAY8, ZM_SUBPIX_ORDER_NONE);
+    memset(mask.Buffer(), 0, static_cast<size_t>(w) * h);
+
+    // One solid rectangle, well inside the frame.
+    const unsigned int r_lo_x = 100, r_hi_x = 199, r_lo_y = 50, r_hi_y = 149;
+    for (unsigned int y = r_lo_y; y <= r_hi_y; y++)
+      memset(mask.Buffer() + static_cast<size_t>(y) * w + r_lo_x, 0xff,
+             r_hi_x - r_lo_x + 1);
+
+    std::unique_ptr<Image> high(mask.BuildHighlight(
+        kAlarmColour, ZM_COLOUR_RGB24, ZM_SUBPIX_ORDER_RGB, nullptr, false));
+    REQUIRE(high != nullptr);
+
+    unsigned int painted = 0, first_y = h, last_y = 0, first_x = w, last_x = 0;
+    for (unsigned int y = 0; y < h; y++) {
+      const uint8_t *row = high->Buffer(0, y);
+      for (unsigned int x = 0; x < w; x++) {
+        if (row[x * 3] || row[x * 3 + 1] || row[x * 3 + 2]) {
+          painted++;
+          first_y = std::min(first_y, y);
+          last_y = std::max(last_y, y);
+          first_x = std::min(first_x, x);
+          last_x = std::max(last_x, x);
+        }
+      }
+    }
+
+    INFO("width " << w << " mask linesize " << mask.LineSize()
+                  << " highlight linesize " << high->LineSize());
+    CHECK(painted == (r_hi_x - r_lo_x + 1) * (r_hi_y - r_lo_y + 1));
+    CHECK(first_y == r_lo_y);
+    CHECK(last_y == r_hi_y);
+    CHECK(first_x == r_lo_x);
+    CHECK(last_x == r_hi_x);
+  };
+
+  SECTION("720 wide, GRAY8 row padded to 736") { check(720, 480); }
+  SECTION("640 wide, GRAY8 row already aligned") { check(640, 480); }
+}