fix(scanner): take the luminance fast path for packed camera frames (#1303)

Author: bmc08gt

libs/codes/kikcode/src/commonMain/kotlin/com/getcode/codes/kikcode/LuminancePlane.kt

@@ -0,0 +1,74 @@
+package com.getcode.codes.kikcode
+
+/**
+ * The contract between a camera frame's luminance (Y) plane and the native code scanner.
+ *
+ * `kikCodeScan` does `memcpy(greyscale.data, image, height * width)` — it reads exactly
+ * [scannedByteCount] bytes from the front of whatever buffer it is handed, and assumes those bytes
+ * are a tightly packed `width * height` greyscale image. Cameras do not always hand us that: rows
+ * are commonly padded out to a hardware alignment, so a plane's row stride can exceed its width.
+ *
+ * Both platforms got this wrong in different directions, which is why the rule lives here:
+ *  - Android guarded the unpadding with `pixelStride != -1`, which is vacuously true for a
+ *    YUV_420_888 Y plane, so it ran an O(width * height) per-pixel copy on *every* frame — ~1.5 ms
+ *    at 1080p — even when the plane was already packed.
+ *  - iOS never unpadded at all, and read its stride with `CVPixelBufferGetBytesPerRow` (which
+ *    reports a whole-buffer value for planar formats) instead of
+ *    `CVPixelBufferGetBytesPerRowOfPlane(_, 0)`. It survives only because the 1080p capture width
+ *    happens to be 64-aligned and therefore unpadded.
+ *
+ * ## Why this shares the decision and not the bytes
+ *
+ * [unpad] is deliberately *not* part of the iOS-facing surface. Handing a plane across the
+ * Kotlin/Native bridge would convert `Data` to `ByteArray`, copying the whole ~2 MB frame — far
+ * worse than the copy this is meant to avoid. Callers ask [isTightlyPacked] whether a copy is
+ * needed and then move the bytes in their own native code, so the shared piece stays branch-only
+ * and allocation-free.
+ */
+object LuminancePlane {
+
+    /**
+     * Whether the plane can be handed to the scanner as-is.
+     *
+     * When true the first [scannedByteCount] bytes are already the image the scanner expects, so the
+     * buffer can be passed through with no copy. When false the caller must repack it — see [unpad]
+     * for the reference implementation.
+     *
+     * A YUV_420_888 Y plane always reports a [pixelStride] of 1 (the format guarantees the Y plane
+     * is never interleaved), so in practice only [rowStride] decides this; the pixel-stride term is
+     * kept so the rule stays correct if that guarantee ever loosens.
+     */
+    fun isTightlyPacked(width: Int, rowStride: Int, pixelStride: Int): Boolean =
+        rowStride == width && pixelStride == 1
+
+    /** How many bytes the scanner reads for a `width x height` frame. */
+    fun scannedByteCount(width: Int, height: Int): Int = width * height
+
+    /**
+     * Repacks a padded or interleaved plane into a tightly packed `width * height` buffer.
+     *
+     * Returns [data] untouched when [isTightlyPacked] already holds, so the common case costs
+     * nothing. Note the returned array may be *longer* than [scannedByteCount] — the scanner only
+     * reads the front of it.
+     *
+     * This is the JVM/Android path. iOS repacks in Swift over the raw plane pointer rather than
+     * calling this, to keep the frame out of the Kotlin/Native bridge.
+     */
+    fun unpad(
+        data: ByteArray,
+        width: Int,
+        height: Int,
+        rowStride: Int,
+        pixelStride: Int,
+    ): ByteArray {
+        if (isTightlyPacked(width, rowStride, pixelStride)) return data
+
+        val cleanData = ByteArray(scannedByteCount(width, height))
+        for (y in 0 until height) {
+            for (x in 0 until width) {
+                cleanData[y * width + x] = data[y * rowStride + x * pixelStride]
+            }
+        }
+        return cleanData
+    }
+}

libs/codes/kikcode/src/commonTest/kotlin/com/getcode/codes/kikcode/LuminancePlaneTest.kt

@@ -0,0 +1,135 @@
+package com.getcode.codes.kikcode
+
+import kotlin.test.Test
+import kotlin.test.assertContentEquals
+import kotlin.test.assertEquals
+import kotlin.test.assertFalse
+import kotlin.test.assertSame
+import kotlin.test.assertTrue
+
+/**
+ * Covers the packing rule every analyzed camera frame goes through, on both platforms.
+ *
+ * Android's guard used to read `pixelStride != -1`, which is vacuously true for a YUV_420_888 Y
+ * plane (whose pixel stride is always 1), so the `||` short-circuited to always-true and the
+ * per-pixel copy ran on every frame even for tightly packed buffers. iOS had the opposite bug: it
+ * never unpadded at all. These tests run on both targets so the rule cannot drift again.
+ */
+class LuminancePlaneTest {
+
+    /** Android's pre-fix guard, kept verbatim so we can assert the two agree on output. */
+    private fun legacyUnpad(
+        data: ByteArray,
+        width: Int,
+        height: Int,
+        rowStride: Int,
+        pixelStride: Int,
+    ): ByteArray {
+        if (width != rowStride || pixelStride != -1) {
+            val cleanData = ByteArray(width * height)
+            for (y in 0 until height) {
+                for (x in 0 until width) {
+                    cleanData[y * width + x] = data[y * rowStride + x * pixelStride]
+                }
+            }
+            return cleanData
+        }
+        return data
+    }
+
+    private fun plane(height: Int, rowStride: Int): ByteArray =
+        ByteArray(rowStride * height) { (it % 251).toByte() }
+
+    @Test
+    fun `a plane is tightly packed only when the row stride matches the width`() {
+        assertTrue(LuminancePlane.isTightlyPacked(width = 1920, rowStride = 1920, pixelStride = 1))
+        // 64-byte row alignment, the usual source of padding on both platforms
+        assertFalse(LuminancePlane.isTightlyPacked(width = 1440, rowStride = 1472, pixelStride = 1))
+        assertFalse(LuminancePlane.isTightlyPacked(width = 1000, rowStride = 1024, pixelStride = 1))
+        // interleaved planes are never packed, even when the arithmetic happens to line up
+        assertFalse(LuminancePlane.isTightlyPacked(width = 64, rowStride = 64, pixelStride = 2))
+    }
+
+    @Test
+    fun `the scanner reads exactly width times height bytes`() {
+        assertEquals(1920 * 1080, LuminancePlane.scannedByteCount(1920, 1080))
+    }
+
+    @Test
+    fun `tightly packed plane takes the fast path and avoids a copy`() {
+        val width = 640
+        val height = 480
+        val data = plane(height, rowStride = width)
+
+        val result = LuminancePlane.unpad(data, width, height, rowStride = width, pixelStride = 1)
+
+        assertSame(data, result, "tightly packed plane should be returned without copying")
+    }
+
+    @Test
+    fun `padded plane still strips row padding`() {
+        val width = 640
+        val height = 480
+        val rowStride = 768 // 128 bytes of row padding
+        val data = plane(height, rowStride)
+
+        val result = LuminancePlane.unpad(data, width, height, rowStride, pixelStride = 1)
+
+        assertEquals(width * height, result.size)
+        for (y in 0 until height) {
+            for (x in 0 until width) {
+                assertEquals(data[y * rowStride + x], result[y * width + x], "mismatch at ($x,$y)")
+            }
+        }
+    }
+
+    @Test
+    fun `interleaved plane still honours pixel stride`() {
+        val width = 32
+        val height = 16
+        val pixelStride = 2
+        val rowStride = width * pixelStride
+        val data = plane(height, rowStride)
+
+        val result = LuminancePlane.unpad(data, width, height, rowStride, pixelStride)
+
+        assertEquals(width * height, result.size)
+        for (y in 0 until height) {
+            for (x in 0 until width) {
+                assertEquals(data[y * rowStride + x * pixelStride], result[y * width + x])
+            }
+        }
+    }
+
+    /**
+     * The Android fix is a pure speedup: for every plane geometry the camera can hand us, the bytes
+     * the scanner sees must be byte-identical to what the old code produced.
+     */
+    @Test
+    fun `output is byte-identical to the pre-fix implementation`() {
+        val geometries = listOf(
+            Triple(640, 480, 640),
+            Triple(640, 480, 768),
+            Triple(1280, 720, 1280),
+            Triple(1280, 720, 1408),
+            Triple(1920, 1080, 1920),
+            Triple(1920, 1080, 2048),
+        )
+
+        for ((width, height, rowStride) in geometries) {
+            val data = plane(height, rowStride)
+            val fixed = LuminancePlane.unpad(data, width, height, rowStride, pixelStride = 1)
+            val legacy = legacyUnpad(data, width, height, rowStride, pixelStride = 1)
+
+            // The fast path hands back the backing array, which is longer than width*height when
+            // the buffer is over-allocated; the scanner only reads the first width*height.
+            val scanned = LuminancePlane.scannedByteCount(width, height)
+            assertTrue(fixed.size >= scanned, "${width}x$height/$rowStride too small")
+            assertContentEquals(
+                legacy.copyOf(scanned),
+                fixed.copyOf(scanned),
+                "content drift at ${width}x$height rowStride=$rowStride",
+            )
+        }
+    }
+}

vendor/kik/scanner/build.gradle.kts

@@ -36,4 +36,10 @@ dependencies {
     api(project(":libs:codes:kikcode"))
     implementation(project(":libs:encryption:ed25519"))
     implementation(project(":vendor:opencv:sdk"))
+
+    androidTestImplementation(libs.junit)
+    androidTestImplementation(libs.androidx.junit)
+    androidTestImplementation(libs.androidx.test.runner)
+    androidTestImplementation(libs.kotlin.test.junit)
+    androidTestImplementation(libs.kotlinx.coroutines.core)
 }