fix(stills): preserve request size and snapshot time in cache keys

CHANGELOG.md

@@ -126,6 +126,10 @@ the public-API contract.
   after its response head reports the hop that answered and its status. The observer does not alter
   request policy and requires hosts to redact source URLs.
 
+- **Still extraction respects requested size and precise snapshot position on cache hits.**
+  Reusing one extractor for different thumbnail widths or snapshot bounds no longer returns the
+  first size rendered. Snapshot requests no longer share a coarse time bucket across frame boundaries.
+
 - **A remote disc image on an extensionless URL is recognized by its `Content-Type` (AE#860, thanks @qoli).**
   The HTTP disc adapter was gated on the `.iso` / `.img` / `.udf` extension alone, so an ISO behind an
   opaque proxy path reached libavformat as raw filesystem bytes and failed with `Invalid data found

Sources/AetherEngine/FrameExtractor/FrameCache.swift

@@ -1,25 +1,27 @@
 import Foundation
 import CoreGraphics
 
-/// Size-bounded LRU cache of rendered frames keyed by `(mode, position bucket)`.
+/// Count-bounded LRU cache keyed by mode, requested position, and requested dimensions.
 /// Per-mode count limit caps the cache at `thumbnailLimit + snapshotLimit` CGImages.
-/// Key omits target size: one extractor serves one fixed-size surface, so a repeat
-/// position at a different maxWidth/maxSize returns the first-rendered image.
 /// Not thread-safe; FrameExtractor owns the only instance and touches it actor-isolated.
 final class FrameCache {
+    private struct Key: Hashable {
+        let position: Double
+        let width: CGFloat
+        let height: CGFloat
+    }
+
     private let thumbnailLimit: Int
     private let snapshotLimit: Int
     private let thumbnailBucketSeconds: Double
-    // Snapshots are frame-accurate, so a 0.1 s grid (~3 frames at 30 fps) suffices.
-    private static let snapshotBucketSeconds: Double = 0.1
 
     // Each FrameMode case needs a matching store + order pair below, plus
     // mirroring in get/set/clear. Add both when introducing a new case.
     /// Use order per mode, front = MRU; holds bucket keys, `store` holds payloads.
-    private var thumbnailOrder: [Int] = []
-    private var snapshotOrder: [Int] = []
-    private var thumbnailStore: [Int: CGImage] = [:]
-    private var snapshotStore: [Int: CGImage] = [:]
+    private var thumbnailOrder: [Key] = []
+    private var snapshotOrder: [Key] = []
+    private var thumbnailStore: [Key: CGImage] = [:]
+    private var snapshotStore: [Key: CGImage] = [:]
 
     init(thumbnailLimit: Int, snapshotLimit: Int, thumbnailBucketSeconds: Double) {
         precondition(thumbnailBucketSeconds > 0, "thumbnailBucketSeconds must be positive")
@@ -28,18 +30,20 @@ final class FrameCache {
         self.thumbnailBucketSeconds = thumbnailBucketSeconds
     }
 
-    private func bucket(_ seconds: Double, mode: FrameMode) -> Int {
-        let grid = mode == .thumbnail ? thumbnailBucketSeconds : Self.snapshotBucketSeconds
-        let scaled = max(0, seconds) / grid
-        // Thumbnails floor to grid; snapshots round to nearest so a value within
-        // half a bucket of a stored position resolves to the same entry.
-        let rounded = mode == .thumbnail ? scaled.rounded(.down) : scaled.rounded()
-        // Audit BIT-105: `Int(_:)` traps past `Int.max`; every such position shares the last bucket.
-        return rounded < 0x1p62 ? Int(rounded) : 1 << 62
+    private func key(seconds: Double, mode: FrameMode, targetWidth: Int, maxSize: CGSize?) -> Key {
+        if mode == .thumbnail {
+            let bucket = (max(0, seconds) / thumbnailBucketSeconds).rounded(.down)
+            // Preserve the saturating thumbnail bucket for enormous finite requests.
+            return Key(position: min(bucket, 0x1p62), width: CGFloat(targetWidth), height: 0)
+        }
+        // Any two nearby requests can straddle a frame boundary. Accurate extraction must
+        // reuse only the same request, rather than guess a frame rate with a time bucket.
+        let size = maxSize.flatMap { $0.width > 0 && $0.height > 0 ? $0 : nil }
+        return Key(position: max(0, seconds), width: size?.width ?? 0, height: size?.height ?? 0)
     }
 
-    func get(mode: FrameMode, seconds: Double) -> CGImage? {
-        let key = bucket(seconds, mode: mode)
+    func get(mode: FrameMode, seconds: Double, targetWidth: Int = 0, maxSize: CGSize? = nil) -> CGImage? {
+        let key = key(seconds: seconds, mode: mode, targetWidth: targetWidth, maxSize: maxSize)
         switch mode {
         case .thumbnail:
             guard let img = thumbnailStore[key] else { return nil }
@@ -52,8 +56,9 @@ final class FrameCache {
         }
     }
 
-    func set(_ image: CGImage, mode: FrameMode, seconds: Double) {
-        let key = bucket(seconds, mode: mode)
+    func set(_ image: CGImage, mode: FrameMode, seconds: Double,
+             targetWidth: Int = 0, maxSize: CGSize? = nil) {
+        let key = key(seconds: seconds, mode: mode, targetWidth: targetWidth, maxSize: maxSize)
         switch mode {
         case .thumbnail:
             thumbnailStore[key] = image
@@ -73,14 +78,14 @@ final class FrameCache {
         snapshotStore.removeAll()
     }
 
-    private func touch(_ order: inout [Int], _ key: Int) {
+    private func touch(_ order: inout [Key], _ key: Key) {
         if let idx = order.firstIndex(of: key) {
             order.remove(at: idx)
         }
         order.insert(key, at: 0)
     }
 
-    private func evict(_ order: inout [Int], _ store: inout [Int: CGImage], limit: Int) {
+    private func evict(_ order: inout [Key], _ store: inout [Key: CGImage], limit: Int) {
         while store.count > limit, let lru = order.popLast() {
             store[lru] = nil
         }

Sources/AetherEngine/FrameExtractor/FrameExtractor.swift

@@ -54,7 +54,7 @@ public actor FrameExtractor {
     }
 
     /// Only used with a one-segment DataIOReader. No URL or provider fallback.
-    /// Refined results bypass the old 0.1 s snapshot bucket entirely.
+    /// Refined results bypass the rendered-frame cache so they carry the actual decoded PTS.
     func residentPreview(rawTarget: Double, refined: Bool, maxWidth: Int,
                                 isCancelled: @escaping @Sendable () -> Bool) async -> ScrubFrame? {
         guard !isShutDown, rawTarget.isFinite,
@@ -284,7 +284,7 @@ public actor FrameExtractor {
     private func produce(at seconds: Double, mode: FrameMode, targetWidth: Int, maxSize: CGSize?) async -> CGImage? {
         // Audit BIT-105: a 0/0 scrub fraction or `CMTime.indefinite.seconds` is a host bug, not a position.
         guard !isShutDown, seconds.isFinite, seconds >= 0 else { return nil }
-        if let hit = cache.get(mode: mode, seconds: seconds) {
+        if let hit = cache.get(mode: mode, seconds: seconds, targetWidth: targetWidth, maxSize: maxSize) {
             scheduleIdleClose()
             return hit
         }
@@ -331,7 +331,7 @@ public actor FrameExtractor {
             return FrameResult(image: image)
         }
         if let image = result.image, !token.isCancelled {
-            cache.set(image, mode: mode, seconds: seconds)
+            cache.set(image, mode: mode, seconds: seconds, targetWidth: targetWidth, maxSize: maxSize)
         }
         scheduleIdleClose()
         return result.image

Tests/AetherEngineTests/FrameCacheTests.swift

@@ -48,14 +48,53 @@ struct FrameCacheTests {
         #expect(cache.get(mode: .thumbnail, seconds: 3.0) === c)
     }
 
-    @Test("Snapshot uses a frame-accurate (sub-second) bucket")
-    func snapshotBucket() {
+    @Test("Snapshots preserve distinct requests across a frame boundary")
+    func snapshotPosition() {
         let cache = FrameCache(thumbnailLimit: 4, snapshotLimit: 4, thumbnailBucketSeconds: 1.0)
         let a = dummyImage(), b = dummyImage()
-        cache.set(a, mode: .snapshot, seconds: 10.10)
-        cache.set(b, mode: .snapshot, seconds: 10.90)
-        #expect(cache.get(mode: .snapshot, seconds: 10.12) === a)
-        #expect(cache.get(mode: .snapshot, seconds: 10.88) === b)
+        // Even sub-millisecond requests can lie on opposite sides of a frame boundary.
+        cache.set(a, mode: .snapshot, seconds: 1.0)
+        cache.set(b, mode: .snapshot, seconds: 1.0001)
+        #expect(cache.get(mode: .snapshot, seconds: 1.0) === a)
+        #expect(cache.get(mode: .snapshot, seconds: 1.0001) === b)
+        #expect(cache.get(mode: .snapshot, seconds: 1.0002) == nil)
+    }
+
+    @Test("Thumbnail cache distinguishes requested widths")
+    func thumbnailDimensions() {
+        let cache = FrameCache(thumbnailLimit: 4, snapshotLimit: 2, thumbnailBucketSeconds: 1.0)
+        let small = dummyImage(), large = dummyImage()
+        cache.set(small, mode: .thumbnail, seconds: 2, targetWidth: 160)
+        cache.set(large, mode: .thumbnail, seconds: 2, targetWidth: 640)
+        #expect(cache.get(mode: .thumbnail, seconds: 2.2, targetWidth: 160) === small)
+        #expect(cache.get(mode: .thumbnail, seconds: 2.2, targetWidth: 640) === large)
+        #expect(cache.get(mode: .thumbnail, seconds: 2.2, targetWidth: 320) == nil)
+    }
+
+    @Test("Snapshot cache distinguishes native and both requested dimensions")
+    func snapshotDimensions() {
+        let cache = FrameCache(thumbnailLimit: 2, snapshotLimit: 4, thumbnailBucketSeconds: 1.0)
+        let native = dummyImage(), wide = dummyImage(), short = dummyImage()
+        cache.set(native, mode: .snapshot, seconds: 2)
+        cache.set(wide, mode: .snapshot, seconds: 2, maxSize: CGSize(width: 320, height: 240))
+        cache.set(short, mode: .snapshot, seconds: 2, maxSize: CGSize(width: 320, height: 120))
+        #expect(cache.get(mode: .snapshot, seconds: 2) === native)
+        #expect(cache.get(mode: .snapshot, seconds: 2, maxSize: CGSize(width: 320, height: 240)) === wide)
+        #expect(cache.get(mode: .snapshot, seconds: 2, maxSize: CGSize(width: 320, height: 120)) === short)
+        #expect(cache.get(mode: .snapshot, seconds: 2, maxSize: CGSize(width: 640, height: 240)) == nil)
+    }
+
+    @Test("Size variants share the same per-mode count limit")
+    func sizeVariantEviction() {
+        let cache = FrameCache(thumbnailLimit: 2, snapshotLimit: 2, thumbnailBucketSeconds: 1.0)
+        let a = dummyImage(), b = dummyImage(), c = dummyImage()
+        cache.set(a, mode: .thumbnail, seconds: 1, targetWidth: 160)
+        cache.set(b, mode: .thumbnail, seconds: 1, targetWidth: 320)
+        _ = cache.get(mode: .thumbnail, seconds: 1, targetWidth: 160)
+        cache.set(c, mode: .thumbnail, seconds: 1, targetWidth: 640)
+        #expect(cache.get(mode: .thumbnail, seconds: 1, targetWidth: 160) === a)
+        #expect(cache.get(mode: .thumbnail, seconds: 1, targetWidth: 320) == nil)
+        #expect(cache.get(mode: .thumbnail, seconds: 1, targetWidth: 640) === c)
     }
 
     @Test("clear empties the cache")

docs/api.md

@@ -1193,7 +1193,7 @@ say so on the tracker rather than working around it.
 | `presentationAxisMap` | `PresentationAxisMap`: `sourceSeconds(forItemSeconds:)`, `itemSeconds(forSourceSeconds:)`, `shiftSeconds(atItemSeconds:)`, `seams` (each a `PresentationAxisMap.Seam` of `itemSeconds` and `shiftSeconds`), `isEmpty`. Readable off the main actor. Cue times and `sourceTime` live on the source axis; `AVPlayerItem.currentTime()` lives on the item axis, and they differ by the producer shift. Returns nil rather than a guess where no axis is established. |
 | `setNativeVideoFrameTimeObserver(_:)` | Takes a `NativeVideoFrameTimeObserver`, called per muxed frame on both axes (`NativeVideoFrameTime`: `source`, `item`, `segmentIndex`, `isKeyframe`, `epoch`). Called on the producer pump thread in **decode** order, so `source` is not monotonic under B-frames. `epoch` rises process-wide. |
 | `setSoftwareVideoFrameTimeObserver(_:)`, `softwarePresentationTimebase` | The software-path equivalent, a `SoftwareVideoFrameTimeObserver` over `SoftwareVideoFrameTime` (`presentation`, `generation`), in ascending presentation order, on the same axis as the cues. `generation` moves on every renderer flush. |
-| `FrameExtractor` | Off-playback stills: `thumbnail(at:maxWidth:)`, `snapshot(at:maxSize:)`, `prewarm()`, `shutdown()`, over a URL or an `IOReader`. Opens its own demuxer, so it needs a source that tolerates a second connection. `boundedSnapshot(at:maxSize:limits:cancellation:)` is the disposable form: one caller-supplied `ProbeLimits` / `ProbeCancellation` budget spans open, stream analysis, seek and decode, a stalled HTTP header included, and the result is a `BoundedStillFrame`. Use a fresh extractor, since an already-open context is refused rather than charged to the budget; the context closes on every outcome. |
+| `FrameExtractor` | Off-playback stills: `thumbnail(at:maxWidth:)`, `snapshot(at:maxSize:)`, `prewarm()`, `shutdown()`, over a URL or an `IOReader`. Cache entries include the requested dimensions, so one extractor may serve multiple sizes. Thumbnails reuse their position bucket; snapshots reuse only an identical requested time so adjacent frame requests remain distinct. Opens its own demuxer, so it needs a source that tolerates a second connection. `boundedSnapshot(at:maxSize:limits:cancellation:)` is the disposable form: one caller-supplied `ProbeLimits` / `ProbeCancellation` budget spans open, stream analysis, seek and decode, a stalled HTTP header included, and the result is a `BoundedStillFrame`. Use a fresh extractor, since an already-open context is refused rather than charged to the budget; the context closes on every outcome. |
 
 ## Now Playing and the audio session