From e31635ef24a8ae66c7bbb9a5a410ef42ba9a9f0f Mon Sep 17 00:00:00 2001 From: tkrmagid Date: Tue, 2 Jun 2026 00:59:44 +0900 Subject: [PATCH] Fix black-screen video on unaligned widths (swscale row padding) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit swscale aligns the RGBA linesize to a SIMD boundary, so any video whose width*4 isn't a multiple of the alignment (e.g. 1674-wide → 6720 B/row vs 6696) yields a padded plane larger than width*height*4. The old copy path tripped a `need > expected → skip` guard and dropped every video frame, leaving a black screen while audio (independent path) still played. Read Frame.imageStride and strip the row padding with a row-by-row memCopy into a tightly-packed slot (width*height*4), matching the texture's expected size. Falls back to deriving the stride from buffer size when the field is absent. Single-shot memCopy retained when stride == rowBytes (aligned widths). Co-Authored-By: Claude Opus 4 --- gradle.properties | 2 +- .../client/playback/JavaCvBackend.java | 77 +++++++++++++------ 2 files changed, 55 insertions(+), 24 deletions(-) diff --git a/gradle.properties b/gradle.properties index 8af22c6..38ad183 100644 --- a/gradle.properties +++ b/gradle.properties @@ -5,7 +5,7 @@ org.gradle.configuration-cache=false # Mod mod_id=video_player -mod_version=0.4.32 +mod_version=0.4.33 maven_group=com.ejclaw.videoplayer archives_base_name=video_player diff --git a/src/main/java/com/ejclaw/videoplayer/client/playback/JavaCvBackend.java b/src/main/java/com/ejclaw/videoplayer/client/playback/JavaCvBackend.java index 806284c..8490f03 100644 --- a/src/main/java/com/ejclaw/videoplayer/client/playback/JavaCvBackend.java +++ b/src/main/java/com/ejclaw/videoplayer/client/playback/JavaCvBackend.java @@ -276,6 +276,14 @@ public class JavaCvBackend implements VideoBackend { Class frameCls = Class.forName(FRAME_CLASS); Field imageField = frameCls.getField("image"); Field samplesField = frameCls.getField("samples"); + // Row stride (bytes) of frame.image[0]. swscale aligns the RGBA linesize to a SIMD + // boundary (32/64 B), so for any width whose width*4 isn't already aligned the plane + // is padded: e.g. 1674-wide → linesize 6720 instead of 6696. We read this to strip + // the padding row-by-row; resolved defensively so a Frame without the field just + // falls back to deriving the stride from the buffer size below. + Field strideField; + try { strideField = frameCls.getField("imageStride"); } + catch (Throwable t) { strideField = null; } // Java2DFrameConverter is no longer used now that we read RGBA bytes directly, // but we still resolve its class so a future code path could fall back to it if a // grabber refuses setPixelFormat. Keep the lookup defensive. @@ -323,26 +331,40 @@ public class JavaCvBackend implements VideoBackend { // primary memory-churn problem; 0.4.10 fixed that; 0.4.11 adds the ring on // top to absorb the burst-then-stall caused by SourceDataLine backpressure // pacing only at audio-buffer granularity. - int need = src.remaining(); - // Reviewer-mandated sanity bounds: memCopy is a raw native copy with no - // fence against overrun. Validate against (a) the source buffer's own - // capacity (so a corrupt plane can't read past it) and (b) the expected - // RGBA frame size (width*height*4) (so an unexpectedly oversized plane - // can't smash the dst slot we'll allocate). If either fails, skip this - // frame and continue — the next grab() will give us a fresh one. - int expected = width * height * 4; - if (need > src.capacity()) { - VideoPlayerMod.LOG.warn("[{}] frame overruns source capacity (need={}, cap={}); skipping", - VideoPlayerMod.MOD_ID, need, src.capacity()); - need = 0; - } else if (need > expected) { - VideoPlayerMod.LOG.warn("[{}] frame larger than expected RGBA size (need={}, expected={}); skipping", - VideoPlayerMod.MOD_ID, need, expected); - need = 0; + // The destination texture is tightly packed: one frame = width*height*4 + // bytes with no row padding (consumeFrame copies ringBytes[idx] straight + // into a w*h*4 GPU texture and rejects anything larger). swscale, however, + // hands us an RGBA plane whose linesize is SIMD-aligned, so for any width + // where width*4 isn't a multiple of the alignment the source rows carry + // trailing padding (e.g. 1674-wide → 6720 B/row vs the 6696 B we want). + // Copying the padded buffer verbatim both overruns the tight dst slot and + // shears the image; we must strip the pad row-by-row. + int rowBytes = width * 4; // bytes of real pixels per row + int dstBytes = rowBytes * height; // tightly-packed frame size + // Source row stride: prefer Frame.imageStride (authoritative), else derive + // it from the buffer size. Guard against a stride smaller than a full row + // (bogus field) by falling back to the tight rowBytes. + int strideBytes = rowBytes; + if (strideField != null) { + try { + int s = strideField.getInt(frame); + if (s >= rowBytes) strideBytes = s; + } catch (Throwable ignored) {} + } else if (height > 0) { + int s = src.remaining() / height; + if (s >= rowBytes) strideBytes = s; } - if (need > 0) { + // Sanity bound: the strided read must stay within the source buffer. memCopy + // is a raw native copy with no overrun fence, so if the plane is smaller than + // stride*height (corrupt/truncated) skip this frame and grab a fresh one. + boolean ok = dstBytes > 0 && (long) strideBytes * height <= src.capacity(); + if (!ok && dstBytes > 0) { + VideoPlayerMod.LOG.warn("[{}] frame plane too small (stride={}, h={}, cap={}); skipping", + VideoPlayerMod.MOD_ID, strideBytes, height, src.capacity()); + } + if (ok) { int srcPos = src.position(); - long srcAddr = MemoryUtil.memAddress(src) + srcPos; + long srcBase = MemoryUtil.memAddress(src) + srcPos; synchronized (frameLock) { // Recheck shutdown inside the lock: stopWorker() flipped running=false // before signaling, so worker is the only writer here and grabber.close() @@ -350,12 +372,21 @@ public class JavaCvBackend implements VideoBackend { // the contract obvious to future readers. if (!running.get() || closed) break; int idx = ringTail; - if (ringBufs[idx] == null || ringBufs[idx].capacity() < need) { - ringBufs[idx] = ByteBuffer.allocateDirect(need).order(ByteOrder.nativeOrder()); + if (ringBufs[idx] == null || ringBufs[idx].capacity() < dstBytes) { + ringBufs[idx] = ByteBuffer.allocateDirect(dstBytes).order(ByteOrder.nativeOrder()); } - long dstAddr = MemoryUtil.memAddress(ringBufs[idx]); - MemoryUtil.memCopy(srcAddr, dstAddr, need); - ringBytes[idx] = need; + long dstBase = MemoryUtil.memAddress(ringBufs[idx]); + if (strideBytes == rowBytes) { + MemoryUtil.memCopy(srcBase, dstBase, dstBytes); + } else { + // Strip swscale row padding: copy only the real pixels per row. + for (int y = 0; y < height; y++) { + MemoryUtil.memCopy(srcBase + (long) y * strideBytes, + dstBase + (long) y * rowBytes, + rowBytes); + } + } + ringBytes[idx] = dstBytes; ringTail = (idx + 1) % FRAME_RING_SLOTS; if (ringCount < FRAME_RING_SLOTS) { ringCount++;