v0.4.40: make video playback survive shader-pack multi-pass rendering

Under Iris/shader packs the world renders in several passes per frame, so
the block-entity renderer's extractRenderState (and thus getOrStart) runs
many times per frame and tick() can transiently miss the BE. The old design
recreated the backend + a fresh per-instance texture Identifier on every
miss and tore the entry down on the first absent-BE tick, producing a
runaway: hundreds of decoder threads/sec for one screen, the texture id
changing every pass ("Missing resource .../dynamic/<id>"), and a panel that
never stabilised — invisible for shader users while non-shader users saw it.

Fixes:
- Deterministic texture Identifier derived from BlockPos, so a recreate
  reuses the same id and the renderer never binds a released id.
- Serialise create/url-change under a lock + ConcurrentHashMap so concurrent
  passes can't double-spawn a decoder per anchor.
- Tolerate a missing BE for MISSING_GRACE_TICKS (~2s) before teardown
  instead of nuking on the first miss; real deletes still stop immediately
  via BLOCK_ENTITY_UNLOAD.
- Transient texture-upload errors no longer kill the decoder (logged once).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

gradle.properties

@@ -5,7 +5,7 @@ org.gradle.configuration-cache=false
 
 # Mod
 mod_id=video_player
-mod_version=0.4.39
+mod_version=0.4.40
 maven_group=com.ejclaw.videoplayer
 archives_base_name=video_player
 

src/main/java/com/ejclaw/videoplayer/client/playback/VideoPlayback.java

@@ -11,11 +11,11 @@ import net.minecraft.core.BlockPos;
 import net.minecraft.resources.Identifier;
 
 import java.nio.file.Path;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * SPEC §5 — per-anchor playback registry. Maps {@link BlockPos} → ({@link VideoBackend} +
@@ -26,67 +26,103 @@ import java.util.Set;
  * <p>When no backend is available on the classpath (e.g. JavaCV jar not installed by the
  * user), the texture stays at its initial placeholder pattern, so the anchor's screen quad
  * still renders as a visible surface.
+ *
+ * <p><b>Shader-pack robustness.</b> Under Iris/OptiFine-style shader packs the world is
+ * rendered in several passes per frame (shadow map, deferred, translucent, …), so the block
+ * entity renderer's {@code extractRenderState} — and therefore {@link #getOrStart} — runs many
+ * times per frame, sometimes off the main render thread. The earlier design re-created the
+ * backend + a fresh per-instance texture {@link Identifier} whenever it momentarily failed to
+ * find the existing entry, and {@link #tick()} tore the whole entry down on the first frame the
+ * anchor's BE wasn't found. With shaders that produced a runaway loop: hundreds of decoder
+ * threads spawned per second for a single screen, the texture id changed every pass (renderer
+ * logged "Missing resource …/dynamic/<id>"), and the panel never stabilised — i.e. the video
+ * was invisible for shader users while non-shader users saw it fine. This class is now hardened
+ * against that: the texture {@link Identifier} is derived deterministically from the
+ * {@link BlockPos} (so a recreate reuses the same id and the renderer never dangles), creation
+ * is serialised so concurrent passes can't spawn duplicate decoders, a missing BE is tolerated
+ * for {@link #MISSING_GRACE_TICKS} before teardown, and a transient upload error no longer kills
+ * the decoder.
  */
 @Environment(EnvType.CLIENT)
 public final class VideoPlayback {
     private VideoPlayback() {}
 
     private static final int PLACEHOLDER_SIZE = 32;
-    private static final Map<BlockPos, Entry> ENTRIES = new HashMap<>();
+    /** ConcurrentHashMap: render-state extraction can run off-thread under shader packs. */
+    private static final Map<BlockPos, Entry> ENTRIES = new ConcurrentHashMap<>();
+    /** Serialises the create / url-change path so multiple render passes can't double-spawn. */
+    private static final Object LOCK = new Object();
+    /**
+     * Consecutive {@link #tick()} calls an anchor's BE may be absent before we tear its playback
+     * down. Under shader packs {@code mc.level.getBlockEntity(pos)} can transiently miss during a
+     * question/anchor swap or an extra render pass; tearing down on the first miss is exactly what
+     * made the renderer recreate the decoder every pass. Tolerating a short absence (~2 s at 20
+     * tps) keeps the audio-on-delete safety net without the churn — a real delete still fires
+     * {@code BLOCK_ENTITY_UNLOAD} and {@link #stop(BlockPos)} immediately.
+     */
+    private static final int MISSING_GRACE_TICKS = 40;
 
     /**
      * Ensure a playback entry exists for this anchor and return its texture identifier.
      * Creates a backend + dynamic texture on first call. Returns {@code null} only if the
-     * URL is empty or autoplay is off.
+     * URL is empty or autoplay is off. Safe to call many times per frame / from multiple
+     * render passes: it reuses the existing entry and only ever creates one decoder per
+     * (pos, url).
      */
     public static Identifier getOrStart(VideoAnchorBlockEntity be) {
         BlockPos pos = be.getBlockPos();
-        Entry e = ENTRIES.get(pos);
+        String url = be.getUrl();
 
-        if (be.getUrl().isEmpty() || !be.isAutoplay()) {
-            if (e != null) {
-                stop(pos);
-            }
+        if (url.isEmpty() || !be.isAutoplay()) {
+            if (ENTRIES.containsKey(pos)) stop(pos);
             return null;
         }
 
-        if (e != null && e.url.equals(be.getUrl())) {
-            return e.id;
-        }
-        if (e != null) {
-            stop(pos);
+        // Fast path: existing entry for the same URL — just reuse it (and clear any pending
+        // missing-BE grace count, since the renderer clearly still sees this anchor).
+        Entry existing = ENTRIES.get(pos);
+        if (existing != null && existing.url.equals(url)) {
+            existing.missingTicks = 0;
+            return existing.id;
         }
 
-        VideoBackend backend = WatermediaProbe.isAvailable() ? new WatermediaBackend() : new JavaCvBackend();
-        // If /videopreload already cached the URL to disk, hand the local file path to FFmpeg
-        // instead of the HTTP URL — eliminates the network read entirely. Falls back to the
-        // live URL when the cache miss or the download hasn't finished yet. The lookup key stays
-        // the anchor's raw URL (matching how the download published it); only the live URL handed
-        // to FFmpeg on a miss is percent-encoded, so a non-ASCII path (e.g. ".../음악퀴즈/...")
-        // streams correctly instead of relying on FFmpeg's lenient raw handling.
-        Path cached = VideoCache.lookup(be.getUrl());
-        String source = cached != null
-            ? cached.toAbsolutePath().toString()
-            : VideoCache.encodeUrl(be.getUrl());
-        // Diagnostic: record, per anchor, whether this play resolved to the local cache file or
-        // fell back to a live network stream. A LIVE STREAM line on a video that doesn't show
-        // means a cache miss (download hadn't finished / failed) and the failure is on the
-        // network/decoder-open path; a CACHE line that still doesn't show means the local file
-        // decoded badly. This is the single fact that splits "cache problem" from "decode
-        // problem" when comparing two clients' logs.
-        if (cached != null) {
-            VideoPlayerMod.LOG.info("[{}] play {} at {} -> CACHE {}",
-                VideoPlayerMod.MOD_ID, be.getUrl(), pos.toShortString(), cached.getFileName());
-        } else {
-            VideoPlayerMod.LOG.info("[{}] play {} at {} -> LIVE STREAM (not cached yet)",
-                VideoPlayerMod.MOD_ID, be.getUrl(), pos.toShortString());
-        }
-        backend.play(source, be.isLoop());
-        backend.setVolume(be.isMuted() ? 0F : be.getVolume());
+        // Create / url-change path. Serialise so two render passes (or an off-thread extraction)
+        // can't both pass the check above and each spawn a decoder + texture for the same anchor.
+        synchronized (LOCK) {
+            Entry e = ENTRIES.get(pos);
+            if (e != null && e.url.equals(url)) {
+                e.missingTicks = 0;
+                return e.id;
+            }
+            if (e != null) {
+                stop(pos); // URL genuinely changed (e.g. next quiz video) — replace.
+            }
 
-        Entry created = new Entry(be.getUrl(), backend);
-        ENTRIES.put(pos, created);
-        return created.id;
+            VideoBackend backend = WatermediaProbe.isAvailable() ? new WatermediaBackend() : new JavaCvBackend();
+            // If the URL was preloaded to disk, hand the local file path to FFmpeg instead of the
+            // HTTP URL — eliminates the network read entirely. Falls back to the live URL on a
+            // cache miss. The lookup key stays the anchor's raw URL (matching how the download
+            // published it); only the live URL handed to FFmpeg on a miss is percent-encoded, so a
+            // non-ASCII path (e.g. ".../음악퀴즈/...") streams instead of relying on FFmpeg's lenient
+            // raw handling.
+            Path cached = VideoCache.lookup(url);
+            String source = cached != null
+                ? cached.toAbsolutePath().toString()
+                : VideoCache.encodeUrl(url);
+            if (cached != null) {
+                VideoPlayerMod.LOG.info("[{}] play {} at {} -> CACHE {}",
+                    VideoPlayerMod.MOD_ID, url, pos.toShortString(), cached.getFileName());
+            } else {
+                VideoPlayerMod.LOG.info("[{}] play {} at {} -> LIVE STREAM (not cached yet)",
+                    VideoPlayerMod.MOD_ID, url, pos.toShortString());
+            }
+            backend.play(source, be.isLoop());
+            backend.setVolume(be.isMuted() ? 0F : be.getVolume());
+
+            Entry created = new Entry(pos, url, backend);
+            ENTRIES.put(pos, created);
+            return created.id;
+        }
     }
 
     public static Identifier currentTexture(BlockPos pos) {
@@ -110,7 +146,7 @@ public final class VideoPlayback {
         e.backend.setVolume(be.isMuted() ? 0F : be.getVolume());
     }
 
-    /** Called every client tick to upload new frames into the GPU texture. */
+    /** Called every client tick / render frame to upload new frames into the GPU texture. */
     public static void tick() {
         Minecraft mc = Minecraft.getInstance();
         if (mc == null) return;
@@ -120,21 +156,30 @@ public final class VideoPlayback {
             BlockPos pos = me.getKey();
             Entry e = me.getValue();
             // Belt-and-suspenders for the audio-on-delete bug: if BLOCK_ENTITY_UNLOAD didn't
-            // fire for some edge case (dimension change, chunk torn down before event runs,
-            // etc.), the BE will be gone from the level but our Entry still holds an open
-            // audio line. Catch it here and stop next tick.
+            // fire for some edge case the BE may be gone from the level while our Entry still
+            // holds an open audio line. But under shader packs getBlockEntity can transiently
+            // miss for a frame or two, so we only tear down after MISSING_GRACE_TICKS consecutive
+            // misses — otherwise we'd nuke and respawn the decoder every render pass.
             if (mc.level == null || !(mc.level.getBlockEntity(pos) instanceof VideoAnchorBlockEntity)) {
-                e.close();
-                it.remove();
+                if (++e.missingTicks > MISSING_GRACE_TICKS) {
+                    e.close();
+                    it.remove();
+                }
                 continue;
             }
+            e.missingTicks = 0;
             if (!e.backend.isReady()) continue;
             try {
                 e.tryUpload();
             } catch (Throwable t) {
-                VideoPlayerMod.LOG.warn("[{}] texture upload failed: {}", VideoPlayerMod.MOD_ID, t.toString());
-                e.close();
-                it.remove();
+                // Do NOT tear the decoder down on a transient upload error — that was the trigger
+                // for the shader-pack recreate storm. Keep playing; log the real exception once so
+                // a genuinely fatal upload problem is still diagnosable.
+                if (!e.uploadErrorLogged) {
+                    VideoPlayerMod.LOG.warn("[{}] texture upload error (decoder kept alive): {}",
+                        VideoPlayerMod.MOD_ID, t.toString());
+                    e.uploadErrorLogged = true;
+                }
             }
         }
     }
@@ -161,18 +206,38 @@ public final class VideoPlayback {
         DynamicTexture texture;
         int texW = 0, texH = 0;
         boolean registered = false;
+        /** Consecutive ticks the BE has been missing (see {@link #MISSING_GRACE_TICKS}). */
+        volatile int missingTicks = 0;
+        /** One-shot guard so a per-frame upload failure logs once, not every frame. */
+        boolean uploadErrorLogged = false;
 
-        Entry(String url, VideoBackend backend) {
+        Entry(BlockPos pos, String url, VideoBackend backend) {
             this.url = url;
             this.backend = backend;
-            String tag = Integer.toHexString(System.identityHashCode(this));
-            this.id = Identifier.fromNamespaceAndPath(VideoPlayerMod.MOD_ID, "dynamic/" + tag);
+            // Deterministic id from the block position (not System.identityHashCode of this
+            // object). If the entry is ever recreated for the same anchor, the renderer's cached
+            // texture Identifier stays valid and TextureManager keeps a texture registered under
+            // it — so a recreate can never leave the renderer binding a released id ("Missing
+            // resource …/dynamic/<id>"), which was the visible shader-pack breakage.
+            this.id = Identifier.fromNamespaceAndPath(VideoPlayerMod.MOD_ID,
+                "dynamic/" + Long.toHexString(pos.asLong()));
             ensureTexture(PLACEHOLDER_SIZE, PLACEHOLDER_SIZE, true);
         }
 
         /** Allocate or resize the dynamic texture, registering it on first allocation. */
         private void ensureTexture(int w, int h, boolean fillPlaceholder) {
-            if (texture != null && w == texW && h == texH) return;
+            if (texture != null && w == texW && h == texH) {
+                // Re-register defensively in case the id was released by a prior teardown for this
+                // same position (deterministic ids are shared across an anchor's lifetime).
+                if (!registered) {
+                    Minecraft mc = Minecraft.getInstance();
+                    if (mc != null) {
+                        mc.getTextureManager().register(id, texture);
+                        registered = true;
+                    }
+                }
+                return;
+            }
             if (texture != null) texture.close();
             NativeImage img = new NativeImage(w, h, false);
             if (fillPlaceholder) {