From 8c0e19ceb6f10d525de9dff26e9f7b82f0dadf1c Mon Sep 17 00:00:00 2001 From: gamer147 Date: Tue, 21 Jul 2026 15:15:23 -0400 Subject: [PATCH] fix: avoid AGF decode during movie preroll --- docs/phase-a-slice-plan.md | 11 ++++++++ engine/Age.Engine.Tests/MovieOpcodeTests.cs | 27 +++++++++++++++++++ engine/Age.Engine.Tests/TestSupport.cs | 2 ++ engine/Age.Engine/Vm/VirtualMachine.cs | 5 +++- godot/GodotAdvHost.cs | 30 ++++++++++++++++----- 5 files changed, 67 insertions(+), 8 deletions(-) diff --git a/docs/phase-a-slice-plan.md b/docs/phase-a-slice-plan.md index f4762bf..b7d1683 100644 --- a/docs/phase-a-slice-plan.md +++ b/docs/phase-a-slice-plan.md @@ -1244,6 +1244,17 @@ the existing test bootstrap still finds root script fixtures through `Paths.Scri and changing that unrelated bootstrap was outside this movie slice. Final validation: engine **132/132**, Godot build with zero warnings, threaded `SELFTEST OK`, and opcode-map lint clean. +**Pre-roll decode-warning follow-up (2026-07-21).** SC0000 could print +`AGF decode failed CHAPTER.AGF: expected an ACGF image` immediately before successful MPEG playback. +This was a port-side compositor race, not corrupt media or bad catalog resolution: op `0x236` exposed the +movie's `.AGF` resource binding while the VFS read/DirectShow graph setup was still synchronous, and the +first-frame resolver fell through to the still-image AGF decoder. Movie surfaces are now registered before +the VFS read, remain blank until their first dynamic frame, and the VM publishes the movie resource binding +only after synchronous host setup returns. The same pending-frame guard also covers modal movies. A focused +regression locks the blank-during-setup boundary; validation is **271/271** engine tests, a zero-warning Godot +build, and threaded `SELFTEST OK`. The headless screenshot harness did not terminate at its requested page; +the subsequent live SC0000 recheck passed with normal playback and no spurious AGF warning. + ### Phase A — SC0000 textbox/control-strip one-shot blend correction DONE (2026-07-11) The lower white panel noted after movie publication and the white-outline controls had one shared cause, diff --git a/engine/Age.Engine.Tests/MovieOpcodeTests.cs b/engine/Age.Engine.Tests/MovieOpcodeTests.cs index def615e..f8344c9 100644 --- a/engine/Age.Engine.Tests/MovieOpcodeTests.cs +++ b/engine/Age.Engine.Tests/MovieOpcodeTests.cs @@ -100,6 +100,33 @@ public class MovieOpcodeTests Assert.Equal(0x33, vm.Gfx.SnapshotVisibleObjects().Single().SurfaceResId); } + [Fact] + public void PlayMovieKeepsCreatedSurfaceBlankDuringSynchronousHostSetup() + { + var table = OpcodeTableJson.Load(Paths.OpcodesJson); + var script = ScriptAssembler.Assemble(table, "MOVIE-PREROLL", new List<(int, Operand[])> + { + (0x1f8, new[] { new Operand(0, 5), new Operand(0, 800), new Operand(0, 600), new Operand(0, 0) }), + (0x1fb, new[] + { + new Operand(0, 1), new Operand(0, 5), new Operand(0, 0), new Operand(0, 0), + new Operand(0, 800), new Operand(0, 600), new Operand(0, 0), new Operand(0, 0), + }), + (0x236, new[] { new Operand(0, 0x33), new Operand(0, 5), new Operand(0, 2), new Operand(0, 0) }), + (0x2, System.Array.Empty()), + }, System.Array.Empty()); + var host = new RecordingHost(); + VirtualMachine? vm = null; + long resourceDuringSetup = -1; + host.OnPlayMovie = () => resourceDuringSetup = vm!.Gfx.SnapshotVisibleObjects().Single().SurfaceResId; + vm = new VirtualMachine(script, table, host); + + vm.Run(); + + Assert.Equal(0, resourceDuringSetup); + Assert.Equal(0x33, vm.Gfx.SnapshotVisibleObjects().Single().SurfaceResId); + } + [Fact] public void QueryMovieStopTimeReturnsTheValueRetainedByPlayMovie() { diff --git a/engine/Age.Engine.Tests/TestSupport.cs b/engine/Age.Engine.Tests/TestSupport.cs index 82c381c..9a42f2b 100644 --- a/engine/Age.Engine.Tests/TestSupport.cs +++ b/engine/Age.Engine.Tests/TestSupport.cs @@ -38,6 +38,7 @@ internal class RecordingHost : IHost public readonly List SfxReleases = new(); public readonly List<(int Target, long Duration)> BgmFades = new(); public readonly List<(long Resource, int Surface, long Flags, long SyncMask)> Movies = new(); + public System.Action? OnPlayMovie; public long? MovieStopTimeMs; public readonly List<(long Resource, int Surface, long Flags)> ModalMovies = new(); public readonly List ClearedRenderTargets = new(); @@ -151,6 +152,7 @@ internal class RecordingHost : IHost public long? PlayMovieToSurface(long resourceId, int surfaceSlot, long movieFlags, long syncMask) { Movies.Add((resourceId, surfaceSlot, movieFlags, syncMask)); + OnPlayMovie?.Invoke(); return MovieStopTimeMs; } public void PlayModalMovieToSurface(long rawResourceId, int surfaceSlot, long movieFlags) diff --git a/engine/Age.Engine/Vm/VirtualMachine.cs b/engine/Age.Engine/Vm/VirtualMachine.cs index c946c31..d4fbd49 100644 --- a/engine/Age.Engine/Vm/VirtualMachine.cs +++ b/engine/Age.Engine/Vm/VirtualMachine.cs @@ -1422,10 +1422,13 @@ public sealed class VirtualMachine Gfx.RemapObjectSurface(movieHandle, surfaceSlot, 0); surfaceSlot = 0; } + // Graph construction is synchronous. Keep the existing created surface blank during that + // boundary; publishing the movie resource first would let a concurrent compositor mistake + // the MPEG payload's .AGF name for a still image before the host registers/decodes it. + long? stopTimeMs = _host.PlayMovieToSurface(resourceId, surfaceSlot, Read(a[2]), Read(a[3])); // The native CMovieToTexture renderer replaces the pixels of the already-created surface. // Retain the same resource binding so the compositor resolves live movie frames for its objects. Gfx.SetSurface(surfaceSlot, resourceId, 0); - long? stopTimeMs = _host.PlayMovieToSurface(resourceId, surfaceSlot, Read(a[2]), Read(a[3])); Gfx.SetMovieStopTime(surfaceSlot, stopTimeMs); return pc + 1; // native cmd size 9 resumes at the next instruction; playback is asynchronous } diff --git a/godot/GodotAdvHost.cs b/godot/GodotAdvHost.cs index f9006a8..10b0fcd 100644 --- a/godot/GodotAdvHost.cs +++ b/godot/GodotAdvHost.cs @@ -762,8 +762,14 @@ public sealed class GodotAdvHost : IHost public (RgbaImage Image, string Name, int AssetId, bool IsDynamic)? ResolveResIdTexture(long resId) { lock (_imageLock) + { if (_movieFrames.TryGetValue(resId, out var movie)) return (movie.Image, movie.Name, movie.RawIndex, true); + // Movie payloads use the same .AGF extension as still images. While DirectShow is opening + // the graph (or before its first sample arrives), keep the already-created surface blank + // instead of falling through to AgfDecoder and misclassifying the MPEG program stream. + if (_movieBySurface.Values.Contains(resId)) return null; + } var asset = _res.ResolveRawTexture(resId); var image = asset != null ? Decode(asset) : null; return asset != null && image != null ? (image, asset.Name, asset.RawIndex, false) : null; @@ -825,16 +831,18 @@ public sealed class GodotAdvHost : IHost long syncMask, bool modal, out long? stopTimeMs) { stopTimeMs = null; + // Publish the movie identity before the potentially long VFS read and DirectShow graph setup. + // The compositor can therefore distinguish a legitimate blank pre-roll surface from a still AGF. + ReleaseSurface(surfaceSlot); + lock (_imageLock) + { + _movieBySurface[surfaceSlot] = resourceId; + _completedMovies.Remove(resourceId); + } + _slotDims[surfaceSlot] = (800, 600); // SC0000 creates this native-sized surface immediately beforehand. try { var movie = _res.ReadMovie(asset); - ReleaseSurface(surfaceSlot); - lock (_imageLock) - { - _movieBySurface[surfaceSlot] = resourceId; - _completedMovies.Remove(resourceId); - } - _slotDims[surfaceSlot] = (800, 600); // SC0000 creates this native-sized surface immediately beforehand. _timeline?.Event("movie-start", new() { ["resource"] = resourceId, ["surface"] = surfaceSlot, ["file"] = movie.Name, @@ -844,6 +852,14 @@ public sealed class GodotAdvHost : IHost } catch (System.Exception e) { + lock (_imageLock) + { + if (_movieBySurface.TryGetValue(surfaceSlot, out long registered) && registered == resourceId) + _movieBySurface.Remove(surfaceSlot); + _movieFrames.Remove(resourceId); + _completedMovies.Remove(resourceId); + } + _slotDims.Remove(surfaceSlot); Godot.GD.Print($"movie read failed {asset.Name}: {e.Message}"); return false; }