fix: corrupted scaled/converted output for rotated monitors (SWScale buffer alignment)

src/zm_image.cpp

@@ -3359,8 +3359,11 @@ void Image::Scale(const unsigned int new_width, const unsigned int new_height) {
 
   SWScale swscale;
   swscale.init();
+  // Both buffers use Image's align-32 layout: `buffer` is ours, and
+  // scale_buffer is adopted by AssignDirect below, which derives
+  // size/linesize with av_image_* at align=32.
   if (swscale.Convert(buffer, allocation, scale_buffer, scale_buffer_size,
-                      format, format, width, height, new_width, new_height) < 0) {
+                      format, format, width, height, new_width, new_height, 32, 32) < 0) {
     Error("Scale: sws_scale conversion failed (%ux%u %s -> %ux%u)",
           width, height, av_get_pix_fmt_name(format), new_width, new_height);
     DumpBuffer(scale_buffer, ZM_BUFTYPE_ZM);

src/zm_libvnc_camera.cpp

@@ -235,18 +235,20 @@ int VncCamera::Capture(std::shared_ptr<ZMPacket> &zm_packet) {
         width,
         height);
 
+  // The VNC framebuffer is packed rows (alignment 1); directbuffer is an
+  // Image buffer (WriteBuffer), which is always align-32.
   int rc = scale.Convert(
              mVncData.buffer,
              mRfb->si.framebufferWidth * mRfb->si.framebufferHeight * 4,
-             //SWScale::GetBufferSize(AV_PIX_FMT_RGBA, mRfb->si.framebufferWidth, mRfb->si.framebufferHeight),
              directbuffer,
              width * height * colours,
              AV_PIX_FMT_RGBA,
              mImgPixFmt,
              mRfb->si.framebufferWidth,
              mRfb->si.framebufferHeight,
              width,
-             height);
+             height,
+             1, 32);
   return rc == 0 ? 1 : rc;
 }
 

src/zm_swscale.cpp

@@ -23,10 +23,7 @@
 #include "zm_logger.h"
 
 SWScale::SWScale() :
-  gotdefaults(false),
-  swscale_ctx(nullptr),
-  default_width(0),
-  default_height(0) {
+  swscale_ctx(nullptr) {
   Debug(4, "SWScale object created");
 }
 
@@ -56,23 +53,6 @@ SWScale::~SWScale() {
   Debug(4, "SWScale object destroyed");
 }
 
-int SWScale::SetDefaults(
-  enum _AVPIXELFORMAT in_pf,
-  enum _AVPIXELFORMAT out_pf,
-  unsigned int width,
-  unsigned int height) {
-
-  /* Assign the defaults */
-  default_input_pf = in_pf;
-  default_output_pf = out_pf;
-  default_width = width;
-  default_height = height;
-
-  gotdefaults = true;
-
-  return 0;
-}
-
 int SWScale::Convert(
   AVFrame *in_frame,
   AVFrame *out_frame
@@ -109,11 +89,13 @@ int SWScale::Convert(
   unsigned int width,
   unsigned int height,
   unsigned int new_width,
-  unsigned int new_height
+  unsigned int new_height,
+  int in_alignment,
+  int out_alignment
 ) {
-  Debug(1, "Convert: in_buffer %p in_buffer_size %zu out_buffer %p size %zu width %d height %d width %d height %d %d %d",
+  Debug(1, "Convert: in_buffer %p in_buffer_size %zu out_buffer %p size %zu width %d height %d width %d height %d %d %d align %d/%d",
         in_buffer, in_buffer_size, out_buffer, out_buffer_size, width, height, new_width, new_height,
-        in_pf, out_pf);
+        in_pf, out_pf, in_alignment, out_alignment);
   /* Parameter checking */
   if (in_buffer == nullptr) {
     Error("NULL Input buffer");
@@ -123,14 +105,14 @@ int SWScale::Convert(
     Error("NULL output buffer");
     return -1;
   }
-  //  if(in_pf == 0 || out_pf == 0) {
-  //    Error("Invalid input or output pixel formats");
-  //    return -2;
-  //  }
   if (!width || !height || !new_height || !new_width) {
     Error("Invalid width or height");
     return -3;
   }
+  if (in_alignment <= 0 || out_alignment <= 0) {
+    Error("Invalid buffer alignment %d/%d", in_alignment, out_alignment);
+    return -4;
+  }
 
   in_pf = fix_deprecated_pix_fmt(in_pf);
 
@@ -144,19 +126,19 @@ int SWScale::Convert(
             (out_pf)&0xff,((out_pf>>8)&0xff),((out_pf>>16)&0xff),((out_pf>>24)&0xff));
   }
 
-  int alignment = width % 32 ? 1 : 32;
-  /* Check the buffer sizes */
-  size_t needed_insize = GetBufferSize(in_pf, width, height);
+  /* 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",
       needed_insize,
       width,
       height,
       in_pf,
+      in_alignment,
       in_buffer_size);
   }
-  size_t needed_outsize = GetBufferSize(out_pf, new_width, new_height);
+  size_t needed_outsize = GetBufferSize(out_pf, new_width, new_height, out_alignment);
   if (needed_outsize > out_buffer_size) {
     Error("The output buffer is undersized for the output format. Required: %zu Available: %zu",
           needed_outsize,
@@ -174,22 +156,18 @@ int SWScale::Convert(
     return -6;
   }
 
-  /*
-  input_avframe->format = in_pf;
-  input_avframe->width = width;
-  input_avframe->height = height;
-  output_avframe->format = out_pf;
-  output_avframe->width = new_width;
-  output_avframe->height = new_height;
-  */
-  /* Fill in the buffers */
+  /* Fill in the buffers. The alignments describe how the caller's buffers
+   * are actually laid out — they are facts about the buffers, not tuning
+   * knobs. Guessing them from the dimensions (the old `width % 32 ? 1 : 32`
+   * heuristic) misread every align-32 Image whose width was not a multiple
+   * of 32, e.g. the 720- or 2160-wide output of a rotated monitor. */
   if (av_image_fill_arrays(input_avframe->data, input_avframe->linesize,
-                           (uint8_t*) in_buffer, in_pf, width, height, alignment) <= 0) {
+                           (uint8_t*) in_buffer, in_pf, width, height, in_alignment) <= 0) {
     Error("Failed filling input frame with input buffer");
     return -7;
   }
   if (av_image_fill_arrays(output_avframe->data, output_avframe->linesize,
-                           out_buffer, out_pf, new_width, new_height, alignment) <= 0) {
+                           out_buffer, out_pf, new_width, new_height, out_alignment) <= 0) {
     Error("Failed filling output frame with output buffer");
     return -8;
   }
@@ -214,8 +192,10 @@ int SWScale::Convert(
   enum _AVPIXELFORMAT in_pf,
   enum _AVPIXELFORMAT out_pf,
   unsigned int width,
-  unsigned int height) {
-  return Convert(in_buffer, in_buffer_size, out_buffer, out_buffer_size, in_pf, out_pf, width, height, width, height);
+  unsigned int height,
+  int in_alignment,
+  int out_alignment) {
+  return Convert(in_buffer, in_buffer_size, out_buffer, out_buffer_size, in_pf, out_pf, width, height, width, height, in_alignment, out_alignment);
 }
 
 int SWScale::Convert(
@@ -225,7 +205,8 @@ int SWScale::Convert(
   enum _AVPIXELFORMAT in_pf,
   enum _AVPIXELFORMAT out_pf,
   unsigned int width,
-  unsigned int height) {
+  unsigned int height,
+  int out_alignment) {
   if ( img->Width() != width ) {
     Error("Source image width differs. Source: %d Output: %d", img->Width(), width);
     return -12;
@@ -236,29 +217,10 @@ int SWScale::Convert(
     return -13;
   }
 
-  return Convert(img->Buffer(), img->Size(), out_buffer, out_buffer_size, in_pf, out_pf, width, height);
-}
-
-int SWScale::ConvertDefaults(const Image* img, uint8_t* out_buffer, const size_t out_buffer_size) {
-
-  if ( !gotdefaults ) {
-    Error("Defaults are not set");
-    return -24;
-  }
-
-  return Convert(img,out_buffer,out_buffer_size,default_input_pf,default_output_pf,default_width,default_height);
-}
-
-int SWScale::ConvertDefaults(const uint8_t* in_buffer, const size_t in_buffer_size, uint8_t* out_buffer, const size_t out_buffer_size) {
-
-  if ( !gotdefaults ) {
-    Error("Defaults are not set");
-    return -24;
-  }
-
-  return Convert(in_buffer,in_buffer_size,out_buffer,out_buffer_size,default_input_pf,default_output_pf,default_width,default_height);
+  // Image buffers are always laid out by av_image_* with align=32
+  return Convert(img->Buffer(), img->Size(), out_buffer, out_buffer_size, in_pf, out_pf, width, height, 32, out_alignment);
 }
 
-size_t SWScale::GetBufferSize(enum _AVPIXELFORMAT pf, unsigned int width, unsigned int height) {
-  return av_image_get_buffer_size(pf, width, height, 1);
+size_t SWScale::GetBufferSize(enum _AVPIXELFORMAT pf, unsigned int width, unsigned int height, int alignment) {
+  return av_image_get_buffer_size(pf, width, height, alignment);
 }

src/zm_swscale.h

@@ -12,24 +12,20 @@ class SWScale {
   SWScale();
   ~SWScale();
   bool init();
-  int SetDefaults(enum _AVPIXELFORMAT in_pf, enum _AVPIXELFORMAT out_pf, unsigned int width, unsigned int height);
-  int ConvertDefaults(const Image* img, uint8_t* out_buffer, const size_t out_buffer_size);
-  int ConvertDefaults(const uint8_t* in_buffer, const size_t in_buffer_size, uint8_t* out_buffer, const size_t out_buffer_size);
-  int Convert( AVFrame *in_frame, AVFrame *out_frame );
-  int Convert(const Image* img, uint8_t* out_buffer, const size_t out_buffer_size, enum _AVPIXELFORMAT in_pf, enum _AVPIXELFORMAT out_pf, unsigned int width, unsigned int height);
-  int Convert(const uint8_t* in_buffer, const size_t in_buffer_size, uint8_t* out_buffer, const size_t out_buffer_size, enum _AVPIXELFORMAT in_pf, enum _AVPIXELFORMAT out_pf, unsigned int width, unsigned int height);
-  int Convert(const uint8_t* in_buffer, const size_t in_buffer_size, uint8_t* out_buffer, const size_t out_buffer_size, enum _AVPIXELFORMAT in_pf, enum _AVPIXELFORMAT out_pf, unsigned int width, unsigned int height, unsigned int new_width, unsigned int new_height);
-  static size_t GetBufferSize(enum _AVPIXELFORMAT in_pf, unsigned int width, unsigned int height);
+  int Convert(AVFrame *in_frame, AVFrame *out_frame);
+  // A buffer's row alignment cannot be derived from its dimensions — the
+  // caller must state how each buffer is laid out. ZM Image buffers are
+  // always align-32 (av_image_* with align=32); raw device buffers (V4L2,
+  // VNC framebuffers) are packed rows (alignment 1).
+  int Convert(const Image* img, uint8_t* out_buffer, const size_t out_buffer_size, enum _AVPIXELFORMAT in_pf, enum _AVPIXELFORMAT out_pf, unsigned int width, unsigned int height, int out_alignment);
+  int Convert(const uint8_t* in_buffer, const size_t in_buffer_size, uint8_t* out_buffer, const size_t out_buffer_size, enum _AVPIXELFORMAT in_pf, enum _AVPIXELFORMAT out_pf, unsigned int width, unsigned int height, int in_alignment, int out_alignment);
+  int Convert(const uint8_t* in_buffer, const size_t in_buffer_size, uint8_t* out_buffer, const size_t out_buffer_size, enum _AVPIXELFORMAT in_pf, enum _AVPIXELFORMAT out_pf, unsigned int width, unsigned int height, unsigned int new_width, unsigned int new_height, int in_alignment, int out_alignment);
+  static size_t GetBufferSize(enum _AVPIXELFORMAT in_pf, unsigned int width, unsigned int height, int alignment);
 
  protected:
-  bool gotdefaults;
   struct SwsContext* swscale_ctx;
   av_frame_ptr input_avframe;
   av_frame_ptr output_avframe;
-  enum _AVPIXELFORMAT default_input_pf;
-  enum _AVPIXELFORMAT default_output_pf;
-  unsigned int default_width;
-  unsigned int default_height;
 };
 
 #endif // ZM_SWSCALE_H

src/zm_videostore.cpp

@@ -1153,14 +1153,19 @@ int VideoStore::writeVideoFramePacket(const std::shared_ptr<ZMPacket> zm_packet)
           zm_packet->get_out_frame(video_out_ctx->width, video_out_ctx->height, chosen_codec_data->sw_pix_fmt);
           av_frame_ref(frame.get(), zm_packet->out_frame.get());
 
+          // The destination is out_frame's buffer, which get_out_frame laid
+          // out at alignment (width % 32 ? 1 : 32) — mirror that choice here
+          // so sws writes the layout the encoder will read via
+          // out_frame->linesize.
           swscale.Convert(
               zm_packet->image,
               frame->buf[0]->data,
               zm_packet->codec_imgsize,
               zm_packet->image->AVPixFormat(),
               chosen_codec_data->sw_pix_fmt,
               video_out_ctx->width,
-              video_out_ctx->height
+              video_out_ctx->height,
+              (video_out_ctx->width % 32) ? 1 : 32
               );
         }
       } else if (zm_packet->in_frame) {

tests/zm_image.cpp

@@ -25,6 +25,7 @@ extern "C" {
 #include <libavutil/imgutils.h>
 }
 
+#include <cstdlib>
 #include <cstring>
 
 namespace {
@@ -134,6 +135,52 @@ TEST_CASE("Image::Rotate YUV420P odd dimensions", "[image]") {
   REQUIRE(dst.data[1][(cw - 1) * dst.stride[1] + 0] == 211);
 }
 
+// Regression: SWScale::Convert guessed each buffer's row alignment from
+// `width % 32 ? 1 : 32`. Image buffers are ALWAYS laid out align-32
+// (av_image_fill_arrays/av_image_get_buffer_size with align=32), so for any
+// width not divisible by 32 the converter read luma rows 16 bytes short and
+// chroma planes from packed (wrong) offsets. Rotating a 1280x720 monitor
+// yields exactly such a width (720 % 32 == 16): every Scale() of a rotated
+// frame came out sheared with garbage chroma, while unrotated monitors
+// (width % 32 == 0) were untouched.
+TEST_CASE("Image::Scale YUV420P with non-32-multiple width", "[image]") {
+  bootstrap_image_config();
+  const int w = 720, h = 1280;  // rotated 1280x720, as ROTATE_90/270 produces
+  Image image(w, h, ZM_COLOUR_GRAY8, ZM_SUBPIX_ORDER_YUV420P);
+  REQUIRE(image.Buffer() != nullptr);
+
+  // Column-banded luma (every row identical: left half 50, right half 200)
+  // and uniform chroma. Any per-row drift or plane-offset error shows up as
+  // rows that differ from each other or chroma that isn't the fill value.
+  Planes src = plane_view(image, AV_PIX_FMT_YUV420P, w, h);
+  for (int y = 0; y < h; y++) {
+    uint8_t *row = src.data[0] + static_cast<size_t>(y) * src.stride[0];
+    memset(row, 50, w / 2);
+    memset(row + w / 2, 200, w - w / 2);
+  }
+  memset(src.data[1], 100, static_cast<size_t>(src.stride[1]) * (h / 2));
+  memset(src.data[2], 160, static_cast<size_t>(src.stride[2]) * (h / 2));
+
+  image.Scale(w / 2, h / 2);
+  REQUIRE(image.Width() == static_cast<unsigned int>(w / 2));
+  REQUIRE(image.Height() == static_cast<unsigned int>(h / 2));
+
+  Planes dst = plane_view(image, AV_PIX_FMT_YUV420P, w / 2, h / 2);
+  // Identical input rows must produce identical output rows. Sample the
+  // middle of each band, away from the edge where bilinear blends.
+  for (int y : {0, h / 8, h / 4, h / 2 - 1}) {
+    const uint8_t *row = dst.data[0] + static_cast<size_t>(y) * dst.stride[0];
+    INFO("output luma row " << y);
+    CHECK(std::abs(static_cast<int>(row[w / 8]) - 50) <= 4);
+    CHECK(std::abs(static_cast<int>(row[w / 4 + w / 8]) - 200) <= 4);
+  }
+  for (int y : {0, h / 8, h / 4 - 1}) {
+    INFO("output chroma row " << y);
+    CHECK(std::abs(static_cast<int>(dst.data[1][static_cast<size_t>(y) * dst.stride[1] + w / 8]) - 100) <= 4);
+    CHECK(std::abs(static_cast<int>(dst.data[2][static_cast<size_t>(y) * dst.stride[2] + w / 8]) - 160) <= 4);
+  }
+}
+
 TEST_CASE("Image::Flip YUV420P", "[image]") {
   bootstrap_image_config();
   const int w = 640, h = 480;