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.36
+mod_version=0.4.40
 maven_group=com.ejclaw.videoplayer
 archives_base_name=video_player
 

src/main/java/com/ejclaw/videoplayer/VideoPlayerClient.java

@@ -15,6 +15,7 @@ import net.fabricmc.api.Environment;
 import net.fabricmc.fabric.api.client.event.lifecycle.v1.ClientBlockEntityEvents;
 import net.fabricmc.fabric.api.client.event.lifecycle.v1.ClientLifecycleEvents;
 import net.fabricmc.fabric.api.client.event.lifecycle.v1.ClientTickEvents;
+import net.fabricmc.fabric.api.client.networking.v1.ClientPlayConnectionEvents;
 import net.fabricmc.fabric.api.client.networking.v1.ClientPlayNetworking;
 import net.fabricmc.fabric.api.client.rendering.v1.BlockEntityRendererRegistry;
 import net.fabricmc.fabric.api.client.rendering.v1.level.LevelRenderEvents;
@@ -84,6 +85,13 @@ public class VideoPlayerClient implements ClientModInitializer {
             }
         });
 
+        // On every server join, create video_player_cache/ inside whatever game directory this
+        // client is actually running from (vanilla .minecraft or a custom-launcher dir such as
+        // .mc_custom) and log the resolved path. The directory then exists up front rather than
+        // only appearing once the first download starts, and the log line shows exactly where it
+        // lives so the active install can be confirmed.
+        ClientPlayConnectionEvents.JOIN.register((handler, sender, client) -> VideoCache.ensureCacheDir());
+
         // Wipe video_player_cache/ on game exit so preloaded clips don't pile up across
         // sessions. Cache entries are re-broadcast by the server on every JOIN, so a freshly
         // started game will repopulate the cache automatically.

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

@@ -381,7 +381,7 @@ public final class VideoCache {
                 return;
             }
 
-            URLConnection raw = URI.create(url).toURL().openConnection();
+            URLConnection raw = URI.create(encodeUrl(url)).toURL().openConnection();
             raw.setConnectTimeout(10_000);
             raw.setReadTimeout(30_000);
             raw.setRequestProperty("User-Agent", "video_player/" + VideoPlayerMod.MOD_ID);
@@ -522,6 +522,53 @@ public final class VideoCache {
         }
     }
 
+    /**
+     * Percent-encode any non-ASCII characters in the URL so it is a valid wire URL for both the
+     * cache download ({@link HttpURLConnection}) and playback (FFmpeg). Callers keep the ORIGINAL
+     * {@code url} string as the cache key — the {@link #sha256(String)} filename and {@link #READY}
+     * map are unchanged — and only encode at the point a request is actually made, so
+     * {@link #lookup(String)} still matches the anchor's raw URL.
+     *
+     * <p>Without this, a URL with a non-ASCII path segment (e.g. {@code .../음악퀴즈/...}) is sent
+     * verbatim and the server answers HTTP 400, so every preload fails silently and the disk
+     * cache stays empty — clients then fall back to live streaming for every video. The multi-arg
+     * {@link URI} constructor encodes each component; an already-encoded URL round-trips unchanged
+     * (decode-then-encode is idempotent for these paths). On any parse failure we fall back to the
+     * raw string rather than dropping the request.
+     */
+    public static String encodeUrl(String url) {
+        if (url == null) return null;
+        try {
+            URI u = URI.create(url);
+            URI enc = new URI(u.getScheme(), u.getAuthority(), u.getPath(), u.getQuery(), u.getFragment());
+            return enc.toASCIIString();
+        } catch (Throwable t) {
+            return url;
+        }
+    }
+
+    /**
+     * Create the {@code <gameDir>/video_player_cache/} directory eagerly and log the resolved
+     * absolute path, so it exists (and is visible to the user) regardless of which game directory
+     * Minecraft is actually running from — vanilla {@code .minecraft} or a custom launcher dir
+     * (e.g. {@code .mc_custom}). Downloads also create it on demand; this just makes it appear up
+     * front and surfaces the exact location in the log for diagnosis.
+     */
+    public static void ensureCacheDir() {
+        Path dir = cacheDir();
+        if (dir == null) {
+            VideoPlayerMod.LOG.warn("[{}] cache dir: no game dir available yet", VideoPlayerMod.MOD_ID);
+            return;
+        }
+        try {
+            Files.createDirectories(dir);
+            VideoPlayerMod.LOG.info("[{}] cache dir ready: {}", VideoPlayerMod.MOD_ID, dir.toAbsolutePath());
+        } catch (Throwable t) {
+            VideoPlayerMod.LOG.warn("[{}] could not create cache dir {}: {}",
+                VideoPlayerMod.MOD_ID, dir, t.toString());
+        }
+    }
+
     /**
      * Best-effort filename extension from the URL path so FFmpeg's container probe gets a hint
      * (e.g. {@code .webm} for a webm stream). Falls back to {@code .bin}.

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,50 +26,104 @@ 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 (url.isEmpty() || !be.isAutoplay()) {
-            if (e != null) {
+            if (ENTRIES.containsKey(pos)) stop(pos);
-                stop(pos);
-            }
             return null;
         }
 
-        if (e != null && e.url.equals(be.getUrl())) {
+        // 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;
+        }
+
+        // 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);
+                stop(pos); // URL genuinely changed (e.g. next quiz video) — replace.
             }
 
             VideoBackend backend = WatermediaProbe.isAvailable() ? new WatermediaBackend() : new JavaCvBackend();
-        // If /videopreload already cached the URL to disk, hand the local file path to FFmpeg
+            // If the URL was preloaded to disk, hand the local file path to FFmpeg instead of the
-        // instead of the HTTP URL — eliminates the network read entirely. Falls back to the
+            // HTTP URL — eliminates the network read entirely. Falls back to the live URL on a
-        // live URL when the cache miss or the download hasn't finished yet.
+            // cache miss. The lookup key stays the anchor's raw URL (matching how the download
-        Path cached = VideoCache.lookup(be.getUrl());
+            // published it); only the live URL handed to FFmpeg on a miss is percent-encoded, so a
-        String source = cached != null ? cached.toAbsolutePath().toString() : be.getUrl();
+            // 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(be.getUrl(), backend);
+            Entry created = new Entry(pos, url, backend);
             ENTRIES.put(pos, created);
             return created.id;
         }
+    }
 
     public static Identifier currentTexture(BlockPos pos) {
         Entry e = ENTRIES.get(pos);
@@ -92,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;
@@ -102,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,
+            // fire for some edge case the BE may be gone from the level while our Entry still
-            // etc.), the BE will be gone from the level but our Entry still holds an open
+            // holds an open audio line. But under shader packs getBlockEntity can transiently
-            // audio line. Catch it here and stop next tick.
+            // 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)) {
+                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());
+                // Do NOT tear the decoder down on a transient upload error — that was the trigger
-                e.close();
+                // for the shader-pack recreate storm. Keep playing; log the real exception once so
-                it.remove();
+                // 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;
+                }
             }
         }
     }
@@ -143,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));
+            // Deterministic id from the block position (not System.identityHashCode of this
-            this.id = Identifier.fromNamespaceAndPath(VideoPlayerMod.MOD_ID, "dynamic/" + tag);
+            // 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) {