v0.4.28: wipe video_player_cache on game shutdown

Add ClientLifecycleEvents.CLIENT_STOPPING handler that deletes every
file under <gameDir>/video_player_cache/. Quieter sibling of clearAll()
- no chat notify and no concurrent-download race handling needed since
at shutdown no other code is touching the directory.

Cache repopulation on next launch is already handled: the server's
ServerPlayConnectionEvents.JOIN handler re-broadcasts a PreloadPayload
for every preload_urls and cache_entries entry on every join, so a
freshly wiped cache auto-refills from the server's named entries
without any user action.

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

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

@@ -3,6 +3,7 @@ package com.ejclaw.videoplayer;
 import com.ejclaw.videoplayer.block.VideoAnchorBlockEntity;
 import com.ejclaw.videoplayer.client.MusicQuizClient;
 import com.ejclaw.videoplayer.client.net.ClientNetworking;
+import com.ejclaw.videoplayer.client.playback.VideoCache;
 import com.ejclaw.videoplayer.client.playback.VideoPlayback;
 import com.ejclaw.videoplayer.client.render.VideoAnchorRenderer;
 import com.ejclaw.videoplayer.item.VideoStickItem;
@@ -12,6 +13,7 @@ import net.fabricmc.api.ClientModInitializer;
 import net.fabricmc.api.EnvType;
 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.ClientPlayNetworking;
 import net.fabricmc.fabric.api.client.rendering.v1.BlockEntityRendererRegistry;
@@ -82,6 +84,11 @@ public class VideoPlayerClient implements ClientModInitializer {
             }
         });
 
+        // 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.
+        ClientLifecycleEvents.CLIENT_STOPPING.register(client -> VideoCache.wipeOnShutdown());
+
         VideoPlayerMod.LOG.info("[{}] client initialized", VideoPlayerMod.MOD_ID);
     }
 

src/main/java/com/ejclaw/videoplayer/block/VideoAnchorBlockEntity.java

@@ -3,7 +3,11 @@ package com.ejclaw.videoplayer.block;
 import com.ejclaw.videoplayer.registry.VideoPlayerBlockEntities;
 import net.minecraft.core.BlockPos;
 import net.minecraft.core.Direction;
+import net.minecraft.core.HolderLookup;
 import net.minecraft.nbt.CompoundTag;
+import net.minecraft.network.protocol.Packet;
+import net.minecraft.network.protocol.game.ClientGamePacketListener;
+import net.minecraft.network.protocol.game.ClientboundBlockEntityDataPacket;
 import net.minecraft.world.level.block.entity.BlockEntity;
 import net.minecraft.world.level.block.state.BlockState;
 import net.minecraft.world.level.storage.ValueInput;
@@ -123,6 +127,35 @@ public class VideoAnchorBlockEntity extends BlockEntity {
         out.putBoolean("autoplay", autoplay);
     }
 
+    /**
+     * Vanilla chunk-load BE sync. The base implementation returns an empty tag, which means
+     * when a client first sees this BE (chunk loads or player walks into range) it gets default
+     * values — url="" in particular makes the renderer no-op and the panel appears invisible.
+     *
+     * <p>Returning {@link #toNbt()} here carries the custom fields in the vanilla packet, so
+     * we don't depend on the {@code SyncAnchorPayload} arriving before the chunk's block-update
+     * packet (there's a race: {@code level.setBlock} queues a deferred chunk broadcast while
+     * {@code ServerPlayNetworking.send} writes immediately; if the payload wins, the client
+     * drops it because the BE doesn't exist yet, then the chunk packet creates the BE with
+     * defaults). It also fixes "player walks far away and comes back" — that path has no
+     * SyncAnchorPayload at all, just vanilla chunk re-sync.
+     */
+    @Override
+    public CompoundTag getUpdateTag(HolderLookup.Provider provider) {
+        return toNbt();
+    }
+
+    /**
+     * Triggers a {@link ClientboundBlockEntityDataPacket} whenever the chunk tracker decides
+     * this BE needs to push an update. Default implementation returns {@code null} (no packet
+     * sent on BE change). Combined with {@link #getUpdateTag} above, every BE-state change a
+     * client sees carries the full config.
+     */
+    @Override
+    public Packet<ClientGamePacketListener> getUpdatePacket() {
+        return ClientboundBlockEntityDataPacket.create(this);
+    }
+
     @Override
     protected void loadAdditional(ValueInput in) {
         super.loadAdditional(in);

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

@@ -180,27 +180,24 @@ public class JavaCvBackend implements VideoBackend {
         // stale srcAddr — closing the grabber there frees the av_frame plane and the next
         // memcpy crashes inside StubRoutines::jbyte_disjoint_arraycopy (exactly the 4K-delete
         // crash dump we saw). So the safe rule is: only the decoder thread touches the
-        // grabber. External stop signals `running=false`, stops the audio line, interrupts the
-        // worker, and joins briefly; the worker's own `finally` calls grabber.close(). Inside
-        // the loop, grab() unblocks via the rw_timeout/timeout options (3 s, set in runLoop)
-        // even on a stuck HTTP read, so the join below normally returns within a frame.
+        // grabber. External stop signals `running=false`, stops the audio line, interrupts
+        // the worker, and returns immediately; the worker's own `finally` calls
+        // grabber.close() whenever grab()/start() eventually returns.
+        //
+        // We deliberately do NOT join the worker. close() runs on the client tick thread (via
+        // VideoPlayback.tick → Entry.close), and the worker can spend several seconds inside
+        // the native FFmpeg probe at the top of runLoop — probesize=8 MB and
+        // analyzeduration=2 s do not honor the `running` flag and Thread.interrupt() doesn't
+        // unblock native I/O. A bounded join() there (the old 2 s) is exactly the "place then
+        // immediately delete freezes the game for a moment" symptom: the worker hasn't
+        // entered the grab() loop yet, so flipping running=false has no effect on it until
+        // start() returns. The worker is a daemon, the audio line is already silenced above,
+        // and Entry has been removed from the active map by the caller — nothing observable
+        // depends on the grabber having been closed before this method returns.
         Thread t = worker;
         worker = null;
         if (t != null) {
             t.interrupt();
-            try {
-                t.join(2000);
-            } catch (InterruptedException ie) {
-                Thread.currentThread().interrupt();
-            }
-            if (t.isAlive()) {
-                // Worker still blocked in native grab() — let it finish on its own. Its
-                // finally still closes the grabber when grab() eventually returns / throws.
-                // No native pointers leak in the meantime because we don't touch them here.
-                VideoPlayerMod.LOG.warn(
-                    "[{}] decoder did not exit within 2 s of stop; orphaning until next grab() returns",
-                    VideoPlayerMod.MOD_ID);
-            }
         }
         ready = false;
     }

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

@@ -226,6 +226,40 @@ public final class VideoCache {
         IN_FLIGHT.clear();
     }
 
+    /**
+     * Wipe the cache directory at game shutdown so the user doesn't accumulate disk usage
+     * across sessions. Distinct from {@link #clearAll()}: at shutdown there are no concurrent
+     * downloads to race and no player to chat-notify, so we skip the chat ping but still bump
+     * the epoch (defensive — any straggler write inside the in-flight {@code .part} stream
+     * will detect the bump and self-clean) and drop indexes.
+     */
+    public static void wipeOnShutdown() {
+        CACHE_EPOCH.incrementAndGet();
+        READY.clear();
+        IN_FLIGHT.clear();
+        int deleted = 0;
+        int failed  = 0;
+        try {
+            Path dir = cacheDir();
+            if (dir != null && Files.isDirectory(dir)) {
+                try (var stream = Files.newDirectoryStream(dir)) {
+                    for (Path p : stream) {
+                        try {
+                            if (Files.isRegularFile(p) && Files.deleteIfExists(p)) deleted++;
+                        } catch (Throwable t) {
+                            failed++;
+                        }
+                    }
+                }
+            }
+        } catch (Throwable t) {
+            VideoPlayerMod.LOG.warn("[{}] wipeOnShutdown failed: {}",
+                VideoPlayerMod.MOD_ID, t.toString());
+        }
+        VideoPlayerMod.LOG.info("[{}] wipeOnShutdown: deleted={} failed={}",
+            VideoPlayerMod.MOD_ID, deleted, failed);
+    }
+
     /** Caller-supplied: current set of URLs that are fully cached, for diagnostics. */
     public static Set<String> readyUrls() {
         return new HashSet<>(READY.keySet());

src/main/java/com/ejclaw/videoplayer/command/VideoPlaceCommand.java

@@ -22,10 +22,30 @@ import net.minecraft.server.level.ServerPlayer;
 import net.minecraft.server.permissions.Permissions;
 import net.minecraft.world.level.block.Block;
 
-/** SPEC §4.5.1 — {@code /videoPlace <pos> <facing> <width> <height> <url>} */
+/**
+ * SPEC §4.5.1 — {@code /videoPlace} has two accepted forms:
+ *
+ * <ul>
+ *   <li><b>Legacy 5-arg</b>: {@code /videoPlace <pos> <facing> <width> <height> <url>}
+ *       — preserved for existing command blocks. Volume defaults to 50% and muted=false.
+ *       The {@code url} here is a single-token string so it doesn't swallow the {@code volume}
+ *       slot of the new form.</li>
+ *   <li><b>New 6-arg</b>: {@code /videoPlace <pos> <facing> <width> <height> <volume> <url>}
+ *       — {@code volume} is 0..100 (percent) or {@code -1} to start muted. The percent form
+ *       mirrors the GUI slider; {@code -1} is a CLI shortcut so admins don't need a follow-up
+ *       {@code /videoMute} step. {@code url} is greedy here, so URLs containing spaces (rare
+ *       but possible after URL-decoding) still work.</li>
+ * </ul>
+ *
+ * <p>Brigadier disambiguates the two by the type of the 5th argument: integer → new form,
+ * non-integer token → legacy form.
+ */
 public final class VideoPlaceCommand {
     private VideoPlaceCommand() {}
 
+    /** Default volume (percent) applied to the legacy 5-arg form. */
+    private static final int DEFAULT_VOLUME_PCT = 50;
+
     public static void register(CommandDispatcher<CommandSourceStack> dispatcher) {
         dispatcher.register(build("videoPlace"));
     }
@@ -38,12 +58,33 @@ public final class VideoPlaceCommand {
                 .then(Commands.argument("facing", StringArgumentType.word())
                     .then(Commands.argument("width", IntegerArgumentType.integer(1, 32))
                         .then(Commands.argument("height", IntegerArgumentType.integer(1, 32))
-                            .then(Commands.argument("url", StringArgumentType.greedyString())
-                                .executes(VideoPlaceCommand::run))))));
+                            // New form: volume (int) + greedy url
+                            .then(Commands.argument("volume", IntegerArgumentType.integer(-1, 100))
+                                .then(Commands.argument("url", StringArgumentType.greedyString())
+                                    .executes(ctx -> runNew(ctx))))
+                            // Legacy form: single-token url, no volume slot. Single-token string
+                            // is intentional so "<int> https://..." cannot be parsed as a legacy
+                            // url that happens to start with a number — Brigadier first tries the
+                            // new branch and only falls through here if "volume" isn't an int.
+                            .then(Commands.argument("url", StringArgumentType.string())
+                                .executes(ctx -> runLegacy(ctx)))))));
     }
 
-    private static int run(com.mojang.brigadier.context.CommandContext<CommandSourceStack> ctx)
+    private static int runNew(com.mojang.brigadier.context.CommandContext<CommandSourceStack> ctx)
             throws CommandSyntaxException {
+        int volumeArg = IntegerArgumentType.getInteger(ctx, "volume");
+        String url = StringArgumentType.getString(ctx, "url");
+        return runWithValues(ctx, volumeArg, url);
+    }
+
+    private static int runLegacy(com.mojang.brigadier.context.CommandContext<CommandSourceStack> ctx)
+            throws CommandSyntaxException {
+        String url = StringArgumentType.getString(ctx, "url");
+        return runWithValues(ctx, DEFAULT_VOLUME_PCT, url);
+    }
+
+    private static int runWithValues(com.mojang.brigadier.context.CommandContext<CommandSourceStack> ctx,
+                                     int volumeArg, String rawUrl) throws CommandSyntaxException {
         CommandSourceStack src = ctx.getSource();
         ServerLevel level = src.getLevel();
         BlockPos pos = BlockPosArgument.getLoadedBlockPos(ctx, "pos");
@@ -54,7 +95,12 @@ public final class VideoPlaceCommand {
         }
         int width  = IntegerArgumentType.getInteger(ctx, "width");
         int height = IntegerArgumentType.getInteger(ctx, "height");
-        String raw = StringArgumentType.getString(ctx, "url").trim();
+        // -1 is the CLI mute shortcut; the BE keeps the underlying volume so an admin can
+        // /videoMute false later without re-typing a level. Anything 0..100 sets %-volume and
+        // clears mute.
+        boolean placeMuted = volumeArg < 0;
+        float placeVolume = placeMuted ? 0.5F : (volumeArg / 100.0F);
+        String raw = rawUrl == null ? "" : rawUrl.trim();
         // Accept either an http(s) URL or a /videoCache add <name> entry: resolveUrlOrName()
         // returns the canonical URL in both cases, or null when a non-URL string didn't match
         // any named entry.
@@ -75,6 +121,8 @@ public final class VideoPlaceCommand {
         be.setWidth(width);
         be.setHeight(height);
         be.setUrl(url);
+        be.setVolume(placeVolume);
+        be.setMuted(placeMuted);
 
         CompoundTag nbt = be.toNbt();
         for (ServerPlayer p : PlayerLookup.tracking(level, pos)) {