diff --git a/gradle.properties b/gradle.properties
index 68bbb2a..9890a63 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.39
+mod_version=0.4.40
maven_group=com.ejclaw.videoplayer
archives_base_name=video_player
diff --git a/src/main/java/com/ejclaw/videoplayer/client/playback/VideoPlayback.java b/src/main/java/com/ejclaw/videoplayer/client/playback/VideoPlayback.java
index 8ab512b..a5deebf 100644
--- a/src/main/java/com/ejclaw/videoplayer/client/playback/VideoPlayback.java
+++ b/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;
*
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.
+ *
+ *
Shader-pack robustness. 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/"), 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 ENTRIES = new HashMap<>();
+ /** ConcurrentHashMap: render-state extraction can run off-thread under shader packs. */
+ private static final Map 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/"), 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) {