From 57326dc6a7d2fcd187cc45edba2dac4433894ac1 Mon Sep 17 00:00:00 2001 From: ded Date: Tue, 20 Oct 2026 01:51:23 +0300 Subject: [PATCH 1/6] G5: stills at a reduced render size are resampled once and kept at that size (#1812) The media pool honoured the requested size only for movies; stills came back at full size and the renderer resampled them on every frame. A 21120 px wide plate at Quarter was re-filtered each frame, and four of them (1.85 GB as f32) did not fit the 1 GiB pool, so they were also decoded again and again. Stills are now resampled once with effectcraft_raster::resample (the same call the renderer made, so pixels are unchanged) and the pool keeps the reduced copy under the sized key. #1812 project at Quarter, CLI bench: 410 ms -> 33 ms a frame (26 ms with the layer cache); desktop playback 2% -> 60-99% of real time. Co-Authored-By: Claude Opus 5.5 --- crates/media/src/pool.rs | 79 +++++++++++++++++++++++------------ crates/media/tests/footage.rs | 26 ++++++++++++ 2 files changed, 78 insertions(+), 27 deletions(-) diff --git a/crates/media/src/pool.rs b/crates/media/src/pool.rs index 9a8ec5c2..d3b6da0d 100644 --- a/crates/media/src/pool.rs +++ b/crates/media/src/pool.rs @@ -708,15 +708,15 @@ impl Inner { } } - /// The frame of `footage` at `t`; a movie's resampled to `size` when it can be made at that - /// size directly (see [`FootageSource::frame_at_size`]). + /// The frame of `footage` at `t`, resampled to `size` (see [`FootageSource::frame_at_size`]): + /// a movie's made at that size directly, a still's resampled once and kept at that size. fn frame_at(self: &Arc, footage: &Footage, t: Tick, playback: bool, size: Option<(u32, u32)>, float: FloatCurve) -> Result> { if !footage.has_video { return Err(MediaError::Unsupported(format!("{}: no video", footage.path))); } let mut loc = Self::locate(footage, t, float); let movie = loc.media_t.is_some(); - if movie && let Some(size) = size { + if let Some(size) = size { loc.key.size = size; } let mut c = lock(&self.cache); @@ -773,34 +773,59 @@ impl Inner { #[cfg(target_arch = "wasm32")] fn prefetch(self: &Arc, _footage: &Footage, _t: Tick, _size: Option<(u32, u32)>, _float: FloatCurve) {} + /// A still (image file or sequence frame) of the footage's own size, at `loc.key.size` when + /// one is set: resampled as the renderer would ([`FootageSource::frame_at_size`]), so the + /// pool keeps the reduced frame and a reduced-resolution render neither decodes nor resamples + /// a large still again for every frame (#1812: 21120-px plates at Quarter). + fn still_at_size(img: Image, loc: &Loc, footage: &Footage) -> Image { + let (w, h) = loc.key.size; + if w == 0 || h == 0 || (img.width, img.height) != (footage.width, footage.height) || (w, h) == (img.width, img.height) { + return img; + } + effectcraft_raster::resample(&img, w, h) + } + fn decode(&self, loc: &Loc, footage: &Footage) -> Result { + match loc.media_t { + None => self.decode_still(loc, footage).map(|img| Self::still_at_size(img, loc, footage)), + Some(_) => self.decode_media(loc, footage), + } + } + + fn decode_still(&self, loc: &Loc, footage: &Footage) -> Result { + let op = AlphaOp::of(footage, loc.float); + let path = &loc.key.path; + if &**path == PLACEHOLDER { + return Ok(colour_bars(footage.width, footage.height)); + } + let bytes = self.read(path)?; + if let Some(img) = crate::dpx::decode(path, &bytes)? { + return Ok(dynamic_to_image(&img, op)); + } + if let Some(img) = crate::layered::decode(path, &bytes, footage, op)? { + return Ok(img); + } + // (an OpenEXR compression the decoder lacks came out transparent or as a missing + // channel error) + if path.to_ascii_lowercase().ends_with(".exr") + && let Some(why) = crate::exr_channels::unsupported_compression(&bytes) + { + return Err(MediaError::Decode(format!("{path}: {why}"))); + } + let loaded = crate::probe::load_oriented(&bytes, crate::still_format(path, &bytes)); + let img = match loaded { + Ok(img) => img, + // A multi-layer OpenEXR file without an unnamed RGB layer: its colour layer. + Err(e) => crate::exr_channels::cached_layered_image(path, bytes).ok_or_else(|| MediaError::Decode(format!("{path}: {e}")))?, + }; + Ok(dynamic_to_image(&img, op)) + } + + fn decode_media(&self, loc: &Loc, footage: &Footage) -> Result { let op = AlphaOp::of(footage, loc.float); let path = &loc.key.path; match loc.media_t { - None if &**path == PLACEHOLDER => Ok(colour_bars(footage.width, footage.height)), - None => { - let bytes = self.read(path)?; - if let Some(img) = crate::dpx::decode(path, &bytes)? { - return Ok(dynamic_to_image(&img, op)); - } - if let Some(img) = crate::layered::decode(path, &bytes, footage, op)? { - return Ok(img); - } - // (an OpenEXR compression the decoder lacks came out transparent or as a missing - // channel error) - if path.to_ascii_lowercase().ends_with(".exr") - && let Some(why) = crate::exr_channels::unsupported_compression(&bytes) - { - return Err(MediaError::Decode(format!("{path}: {why}"))); - } - let loaded = crate::probe::load_oriented(&bytes, crate::still_format(path, &bytes)); - let img = match loaded { - Ok(img) => img, - // A multi-layer OpenEXR file without an unnamed RGB layer: its colour layer. - Err(e) => crate::exr_channels::cached_layered_image(path, bytes).ok_or_else(|| MediaError::Decode(format!("{path}: {e}")))?, - }; - Ok(dynamic_to_image(&img, op)) - } + None => self.decode_still(loc, footage), Some(mt) if gif_anim::is_animated(footage) => { let anim = self.gif(path)?; let frame = anim.frame_at(Tick(mt.0)).ok_or_else(|| MediaError::Decode(format!("{path}: no frames")))?; diff --git a/crates/media/tests/footage.rs b/crates/media/tests/footage.rs index 13ca605e..9e68e3f6 100644 --- a/crates/media/tests/footage.rs +++ b/crates/media/tests/footage.rs @@ -580,6 +580,32 @@ fn sequence_gaps_show_placeholders() { let _ = std::fs::remove_dir_all(&dir); } +/// #1812: a still asked for at a reduced size comes back as the renderer would resample it, and +/// is kept at that size: later frames at that size neither decode nor resample it again (a +/// parallax project's 21120-px plates were decoded and resampled for every frame at Quarter). +#[test] +fn stills_at_reduced_sizes_are_resampled_once() { + let dir = std::env::temp_dir().join(format!("ec-still-size-{}", std::process::id())); + std::fs::create_dir_all(&dir).expect("temp dir"); + let p = dir.join("plate.png"); + let px = |x: u32, y: u32| { + image::Rgba([(x * 7 % 256) as u8, (y * 13 % 256) as u8, ((x ^ y) % 256) as u8, if (x + y).is_multiple_of(5) { 0 } else { 255 - (x % 128) as u8 }]) + }; + image::RgbaImage::from_fn(613, 97, px).save(&p).expect("png"); + let f = probe(&p).expect("probe"); + let pool = MediaPool::new(); + let full = pool.frame(ItemId(1), &f, Tick::ZERO).expect("frame"); + for (w, h) in [(306, 48), (153, 24), (77, 12)] { + let sized = pool.frame_at_size(ItemId(1), &f, Tick::ZERO, w, h).expect("frame"); + assert_eq!(*sized, effectcraft_raster::resample(&full, w, h), "{w}×{h}: the renderer's resample"); + let misses = pool.stats().misses; + let again = pool.frame_at_size(ItemId(1), &f, Tick(3 * TICKS_PER_SECOND), w, h).expect("frame"); + assert!(Arc::ptr_eq(&sized, &again), "{w}×{h}: kept at that size"); + assert_eq!(pool.stats().misses, misses, "{w}×{h}: not decoded again"); + } + let _ = std::fs::remove_dir_all(&dir); +} + #[test] fn png_with_alpha() { let Some(p) = alpha_png() else { return }; From fd1250d224a5db35f7cb551667ce7bd6d7096145 Mon Sep 17 00:00:00 2001 From: ded Date: Tue, 20 Oct 2026 03:13:48 +0300 Subject: [PATCH 2/6] G5: still footage layers are kept in the layer cache across times (#1812) Footage layers had no layer cache key, so a still made a new layer buffer every frame on every preview worker, and the GPU compositor uploaded it again each time (its upload cache is keyed by buffer). Stills (without a proxy) are now keyed like text and shape layers, without the source time; movies and sequences are unchanged. Co-Authored-By: Claude Opus 5.5 --- crates/render/src/cache.rs | 18 ++++++++++++++---- crates/render/src/tests_footage_size.rs | 25 +++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/crates/render/src/cache.rs b/crates/render/src/cache.rs index bae12d1e..3fabe473 100644 --- a/crates/render/src/cache.rs +++ b/crates/render/src/cache.rs @@ -22,7 +22,7 @@ use std::hash::{Hash, Hasher}; use std::sync::{Arc, Mutex}; use effectcraft_effects::Buf; -use effectcraft_project::{ItemKind, Layer, LayerSource, Node, PropGroup}; +use effectcraft_project::{FootageKind, ItemKind, Layer, LayerSource, Node, PropGroup}; use crate::eval::EvalCtx; @@ -481,7 +481,7 @@ fn hash_values(h: &mut KeyHasher, ctx: &EvalCtx, layer: &Layer, g: &PropGroup) { } /// Cache key for the processed (source → masks → effects) buffer of `layer` at the context -/// time, or `None` when the layer must not be cached (footage, precomps, cameras, adjustment +/// time, or `None` when the layer must not be cached (movie footage, precomps, cameras, adjustment /// layers…). pub fn layer_key(ctx: &EvalCtx, layer: &Layer, scale: f64, draft: bool, blur: bool) -> Option { // Time effects see property values at other times. Keyframes are part of the hashed @@ -514,6 +514,13 @@ fn reads_other_times(layer: &Layer) -> bool { }) } +/// A still image (no proxy): its layer is cached like a text or shape layer, so the GPU +/// compositor uploads it once, not every frame (#1812: four 21120 px plates re-uploaded by every +/// preview worker ran the GPU out of memory). Movies and sequences stay in the media pool only. +fn still_footage(ctx: &EvalCtx, item: effectcraft_project::ItemId) -> bool { + ctx.project.item(item).is_some_and(|it| it.proxy.is_none() && matches!(&it.kind, ItemKind::Footage(f) if f.kind == FootageKind::Still)) +} + fn has_expression(g: &PropGroup) -> bool { g.children.iter().any(|c| match c { Node::Prop(p) => p.has_expression(), @@ -552,14 +559,17 @@ fn key_any(ctx: &EvalCtx, layer: &Layer, scale: f64, draft: bool, blur: bool, fo hash_debug(&mut h, s); } LayerSource::Text | LayerSource::Shape => {} - LayerSource::Footage { item } if footage => { + LayerSource::Footage { item } if footage || still_footage(ctx, *item) => { let it = ctx.project.item(*item)?; let ItemKind::Footage(f) = &it.kind else { return None }; hash_debug(&mut h, f); // A proxy (and its Use Proxy switch) changes the pixels. hash_debug(&mut h, &it.proxy); item.hash(&mut h); - ctx.source_time(layer).0.hash(&mut h); + // A still is the same at every time (unless a proxy, which may be a movie, stands in). + if f.kind != FootageKind::Still || it.proxy.is_some() { + ctx.source_time(layer).0.hash(&mut h); + } // Frame blending mixes neighbouring source frames. ctx.comp.enable_frame_blending.hash(&mut h); } diff --git a/crates/render/src/tests_footage_size.rs b/crates/render/src/tests_footage_size.rs index 88df726e..065941d8 100644 --- a/crates/render/src/tests_footage_size.rs +++ b/crates/render/src/tests_footage_size.rs @@ -81,3 +81,28 @@ fn reduced_renders_take_footage_resampled_by_the_source() { Renderer::new(&p, &sized, RenderOpts { scale: 0.5, ..Default::default() }).comp_frame(cid, Tick::ZERO); assert!(sized.sizes.unwrap().into_inner().unwrap().is_empty()); } + +/// #1812: a still's layer is kept in the layer cache across times (its texture then uploads to +/// the GPU once, not every frame); a movie's is fetched again at every time. +#[test] +fn still_layers_are_cached_across_times_and_movies_are_not() { + for (kind, fetches) in [(FootageKind::Still, 1), (FootageKind::Video, 3)] { + let (mut p, cid) = project(BitDepth::Bpc8, false); + let Some(LayerSource::Footage { item }) = p.comp(cid).map(|c| c.layers[0].source.clone()) else { panic!("footage layer") }; + if let Some(ItemKind::Footage(f)) = p.item_mut(item).map(|it| &mut it.kind) { + f.kind = kind; + } + let cache = crate::LayerCache::default(); + let src = Clip { sizes: Some(Mutex::default()) }; + let opts = RenderOpts { scale: 0.5, ..Default::default() }; + for t in [0.0, 0.25, 0.5] { + let t = Tick::from_seconds_f64(t); + let mut r = Renderer::new(&p, &src, opts); + r.cache = Some(&cache); + let frame = r.comp_frame(cid, t); + assert_eq!(frame, Renderer::new(&p, &src, opts).comp_frame(cid, t), "{kind:?} at {t:?}"); + } + // (each uncached render above fetches once more) + assert_eq!(src.sizes.unwrap().into_inner().unwrap().len(), fetches + 3, "{kind:?}"); + } +} From c33d5c20df766508bea366c4cbe3754c7999741d Mon Sep 17 00:00:00 2001 From: ded Date: Tue, 20 Oct 2026 04:01:07 +0300 Subject: [PATCH 3/6] G5: a still asked for at its own size is the unsized frame (#1812) Review: frame_at_size at a still's native size made a second cache key, so the pool decoded and kept a second full-size copy of a frame thumbnail() or frame() already had. Co-Authored-By: Claude Opus 5.5 --- crates/media/src/pool.rs | 3 ++- crates/media/tests/footage.rs | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/media/src/pool.rs b/crates/media/src/pool.rs index d3b6da0d..6bc4bd8d 100644 --- a/crates/media/src/pool.rs +++ b/crates/media/src/pool.rs @@ -716,7 +716,8 @@ impl Inner { } let mut loc = Self::locate(footage, t, float); let movie = loc.media_t.is_some(); - if let Some(size) = size { + // A still asked for at its own size is the unsized frame (one decode, one entry). + if let Some(size) = size.filter(|s| movie || *s != (footage.width, footage.height)) { loc.key.size = size; } let mut c = lock(&self.cache); diff --git a/crates/media/tests/footage.rs b/crates/media/tests/footage.rs index 9e68e3f6..2e882d73 100644 --- a/crates/media/tests/footage.rs +++ b/crates/media/tests/footage.rs @@ -595,6 +595,11 @@ fn stills_at_reduced_sizes_are_resampled_once() { let f = probe(&p).expect("probe"); let pool = MediaPool::new(); let full = pool.frame(ItemId(1), &f, Tick::ZERO).expect("frame"); + // At its own size it is the unsized frame: no second decode, no second full-size copy. + let misses = pool.stats().misses; + let native = pool.frame_at_size(ItemId(1), &f, Tick::ZERO, f.width, f.height).expect("frame"); + assert!(Arc::ptr_eq(&full, &native), "the native size reuses the unsized frame"); + assert_eq!(pool.stats().misses, misses); for (w, h) in [(306, 48), (153, 24), (77, 12)] { let sized = pool.frame_at_size(ItemId(1), &f, Tick::ZERO, w, h).expect("frame"); assert_eq!(*sized, effectcraft_raster::resample(&full, w, h), "{w}×{h}: the renderer's resample"); From ea1f2fe9840c5741aaa4d4f6dab37eca8c579a64 Mon Sep 17 00:00:00 2001 From: ded Date: Tue, 20 Oct 2026 04:21:14 +0300 Subject: [PATCH 4/6] G5: a still decoded larger than its nominal size is resampled like the renderer does (#1812) Review: still_at_size returned a smart object's pixels unresampled when they differed from the footage's nominal size. It now follows footage_buf: pixels larger than nominal are resampled to the requested size, smaller ones are left for the renderer to scale up. Co-Authored-By: Claude Opus 5.5 --- crates/media/src/pool.rs | 13 ++++++++----- crates/media/tests/footage.rs | 8 ++++++++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/crates/media/src/pool.rs b/crates/media/src/pool.rs index 6bc4bd8d..062bdac5 100644 --- a/crates/media/src/pool.rs +++ b/crates/media/src/pool.rs @@ -774,13 +774,16 @@ impl Inner { #[cfg(target_arch = "wasm32")] fn prefetch(self: &Arc, _footage: &Footage, _t: Tick, _size: Option<(u32, u32)>, _float: FloatCurve) {} - /// A still (image file or sequence frame) of the footage's own size, at `loc.key.size` when - /// one is set: resampled as the renderer would ([`FootageSource::frame_at_size`]), so the - /// pool keeps the reduced frame and a reduced-resolution render neither decodes nor resamples - /// a large still again for every frame (#1812: 21120-px plates at Quarter). + /// A still (image file or sequence frame) at `loc.key.size` when one is set: resampled as the + /// renderer would ([`FootageSource::frame_at_size`]), so the pool keeps the reduced frame and a + /// reduced-resolution render neither decodes nor resamples a large still again for every frame + /// (#1812: 21120-px plates at Quarter). Like the renderer's `footage_buf`, an image decoded + /// larger than the footage's nominal size (a smart object's pixels) is resampled too, and one + /// decoded smaller is not (the renderer scales the layer instead). fn still_at_size(img: Image, loc: &Loc, footage: &Footage) -> Image { let (w, h) = loc.key.size; - if w == 0 || h == 0 || (img.width, img.height) != (footage.width, footage.height) || (w, h) == (img.width, img.height) { + let smaller = img.width == 0 || footage.width as f64 / img.width as f64 > 1.0 + 1e-9; + if w == 0 || h == 0 || smaller || (w, h) == (img.width, img.height) { return img; } effectcraft_raster::resample(&img, w, h) diff --git a/crates/media/tests/footage.rs b/crates/media/tests/footage.rs index 2e882d73..0722e5d4 100644 --- a/crates/media/tests/footage.rs +++ b/crates/media/tests/footage.rs @@ -608,6 +608,14 @@ fn stills_at_reduced_sizes_are_resampled_once() { assert!(Arc::ptr_eq(&sized, &again), "{w}×{h}: kept at that size"); assert_eq!(pool.stats().misses, misses, "{w}×{h}: not decoded again"); } + // Pixels larger than the nominal size (a smart object's) are resampled as the renderer does; + // smaller ones are left as they are (the renderer scales the layer up instead). + let larger = Footage { width: 306, height: 48, ..f.clone() }; + let sized = MediaPool::new().frame_at_size(ItemId(1), &larger, Tick::ZERO, 153, 24).expect("frame"); + assert_eq!(*sized, effectcraft_raster::resample(&full, 153, 24)); + let smaller = Footage { width: 1226, height: 194, ..f.clone() }; + let kept = MediaPool::new().frame_at_size(ItemId(1), &smaller, Tick::ZERO, 306, 48).expect("frame"); + assert_eq!(*kept, *full); let _ = std::fs::remove_dir_all(&dir); } From e14cdd6009796c7ee56e82e8373c1d0ef0c896d8 Mon Sep 17 00:00:00 2001 From: ded Date: Tue, 20 Oct 2026 04:21:14 +0300 Subject: [PATCH 5/6] Layer cache: animated masks with mask motion blur key the layer time Review: mask motion blur draws an animated mask at the shutter's other times, but the layer key hashed its value at this time only. Two frames with the same mask value (a hold before a move) shared a cached layer, with the wrong blur. This applied to solids, text and shapes already, and to stills since this PR. Co-Authored-By: Claude Opus 5.5 --- crates/render/src/cache.rs | 26 ++++++++++++++++++++++++++ crates/render/src/tests_mask_blur.rs | 25 +++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/crates/render/src/cache.rs b/crates/render/src/cache.rs index 3fabe473..17794d46 100644 --- a/crates/render/src/cache.rs +++ b/crates/render/src/cache.rs @@ -521,6 +521,27 @@ fn still_footage(ctx: &EvalCtx, item: effectcraft_project::ItemId) -> bool { ctx.project.item(item).is_some_and(|it| it.proxy.is_none() && matches!(&it.kind, ItemKind::Footage(f) if f.kind == FootageKind::Still)) } +/// Does the layer have an animated mask that motion blur may draw at other times (Layer ▸ Mask ▸ +/// Motion Blur On, or Same As Layer with the layer blurred)? +fn mask_motion_blur(ctx: &EvalCtx, layer: &Layer, blur: bool) -> bool { + use effectcraft_project::{GroupKind, MaskMotionBlur}; + fn animated(g: &PropGroup) -> bool { + g.children.iter().any(|c| match c { + Node::Prop(p) => !p.keys.is_empty() || p.has_expression(), + Node::Group(sub) => animated(sub), + }) + } + ctx.comp.enable_motion_blur + && layer.masks().is_some_and(|m| { + m.groups().any(|g| match g.kind { + GroupKind::Mask { motion_blur, .. } => { + (motion_blur == MaskMotionBlur::On || (motion_blur == MaskMotionBlur::SameAsLayer && blur)) && animated(g) + } + _ => false, + }) + }) +} + fn has_expression(g: &PropGroup) -> bool { g.children.iter().any(|c| match c { Node::Prop(p) => p.has_expression(), @@ -635,6 +656,11 @@ fn key_any(ctx: &EvalCtx, layer: &Layer, scale: f64, draft: bool, blur: bool, fo effectcraft_effects::paint::cache_key(g, lt).0.hash(&mut h); } } + // Mask motion blur draws animated masks at the shutter's other times too: their values at + // this time don't tell two frames apart (a hold before a move). + if mask_motion_blur(ctx, layer, blur) { + layer.layer_time(ctx.time).0.hash(&mut h); + } if time_dependent(layer, effects != Some(0)) { layer.layer_time(ctx.time).0.hash(&mut h); // Effects also see the layer's time mapping. diff --git a/crates/render/src/tests_mask_blur.rs b/crates/render/src/tests_mask_blur.rs index 12b88bbb..07bc1e95 100644 --- a/crates/render/src/tests_mask_blur.rs +++ b/crates/render/src/tests_mask_blur.rs @@ -41,6 +41,31 @@ fn alpha_at_25(p: &Project, cid: ItemId, motion_blur: bool) -> f32 { Renderer::new(p, &NoFootage, RenderOpts { motion_blur, ..Default::default() }).comp_frame(cid, half).get(25, 50)[3] } +/// A mask that holds from frame 0 to 2, then moves: frames 1 and 2 have the same mask value, but at +/// frame 2 the shutter already sees it move. The layer cache must not serve frame 1's layer there. +#[test] +fn mask_motion_blur_keys_the_layer_cache_by_time() { + let (mut p, cid) = project(true); + let rect = |x: f64| Value::Path(ShapePath::rect([x, 50.0], 20.0, 100.0)); + let l = &mut p.comp_mut(cid).unwrap().layers[0]; + let Some(Node::Group(m)) = l.props.sub_mut("masks").unwrap().children.first_mut() else { panic!("mask") }; + m.get_mut("path").unwrap().keys = vec![ + Keyframe::new(Tick::ZERO, rect(20.0)), + Keyframe::new(FrameRate::FPS_30.tick_of(2), rect(20.0)), + Keyframe::new(FrameRate::FPS_30.tick_of(3), rect(60.0)), + ]; + let cache = crate::LayerCache::default(); + let render = |t: Tick, cached: bool| { + let mut r = Renderer::new(&p, &NoFootage, RenderOpts { motion_blur: true, ..Default::default() }); + r.cache = cached.then_some(&cache); + r.comp_frame(cid, t) + }; + let (f1, f2) = (FrameRate::FPS_30.tick_of(1), FrameRate::FPS_30.tick_of(2)); + assert_ne!(render(f1, false), render(f2, false), "the shutter at frame 2 sees the move"); + render(f1, true); + assert_eq!(render(f2, true), render(f2, false)); +} + #[test] fn mask_motion_blur_follows_the_render_and_the_comp() { let (p, cid) = project(true); From 4cf6580b19539e02863bdf03b071a55698ee144c Mon Sep 17 00:00:00 2001 From: ded Date: Tue, 20 Oct 2026 10:42:22 +0300 Subject: [PATCH 6/6] G5: a reduced still's cache key includes its nominal size (#1812) Review: two footage items of one file with different nominal sizes could share a sized entry, though one is resampled and the other kept as decoded. Co-Authored-By: Claude Opus 5.5 --- crates/media/src/pool.rs | 10 ++++++++-- crates/media/tests/footage.rs | 7 ++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/crates/media/src/pool.rs b/crates/media/src/pool.rs index 062bdac5..09c9bb1d 100644 --- a/crates/media/src/pool.rs +++ b/crates/media/src/pool.rs @@ -45,6 +45,9 @@ struct Key { matte: [u32; 3], /// A movie frame made at this size for a reduced-resolution render ((0, 0): as decoded). size: (u32, u32), + /// A still at a reduced `size`: the footage's nominal size, which decides whether it is + /// resampled (see `Inner::still_at_size`); (0, 0) otherwise. + nominal: (u32, u32), } struct Entry { @@ -678,7 +681,7 @@ impl Inner { FloatCurve::Gamma24 => 2, } << 2); let matte = if footage.alpha == AlphaMode::Premultiplied { footage.premul_color.map(f32::to_bits) } else { [0; 3] }; - let key = |path: &str, frame| Key { path: path.into(), frame, interp, matte, size: (0, 0) }; + let key = |path: &str, frame| Key { path: path.into(), frame, interp, matte, size: (0, 0), nominal: (0, 0) }; match footage.kind { FootageKind::Sequence if !footage.sequence.is_empty() => { let i = Self::frame_index(footage.frame_rate, t, footage.sequence_frames(), footage.loop_count); @@ -719,6 +722,9 @@ impl Inner { // A still asked for at its own size is the unsized frame (one decode, one entry). if let Some(size) = size.filter(|s| movie || *s != (footage.width, footage.height)) { loc.key.size = size; + if !movie { + loc.key.nominal = (footage.width, footage.height); + } } let mut c = lock(&self.cache); // sequential playback of a movie: read the next frame ahead @@ -1074,7 +1080,7 @@ mod tests { fn lru_respects_budget_and_recycles() { let mut c = Lru::default(); let img = || Arc::new(Image::new(16, 16)); // 4 KiB + overhead - let k = |i| Key { path: "a".into(), frame: i, interp: 0, matte: [0; 3], size: (0, 0) }; + let k = |i| Key { path: "a".into(), frame: i, interp: 0, matte: [0; 3], size: (0, 0), nominal: (0, 0) }; let budget = 3 * (4096 + 64); for i in 0..10 { c.insert(k(i), img(), budget); diff --git a/crates/media/tests/footage.rs b/crates/media/tests/footage.rs index 0722e5d4..f510109e 100644 --- a/crates/media/tests/footage.rs +++ b/crates/media/tests/footage.rs @@ -610,11 +610,12 @@ fn stills_at_reduced_sizes_are_resampled_once() { } // Pixels larger than the nominal size (a smart object's) are resampled as the renderer does; // smaller ones are left as they are (the renderer scales the layer up instead). + // One pool, one file, one requested size: the two decisions don't share a cache entry. let larger = Footage { width: 306, height: 48, ..f.clone() }; - let sized = MediaPool::new().frame_at_size(ItemId(1), &larger, Tick::ZERO, 153, 24).expect("frame"); - assert_eq!(*sized, effectcraft_raster::resample(&full, 153, 24)); let smaller = Footage { width: 1226, height: 194, ..f.clone() }; - let kept = MediaPool::new().frame_at_size(ItemId(1), &smaller, Tick::ZERO, 306, 48).expect("frame"); + let sized = pool.frame_at_size(ItemId(2), &larger, Tick::ZERO, 153, 24).expect("frame"); + assert_eq!(*sized, effectcraft_raster::resample(&full, 153, 24)); + let kept = pool.frame_at_size(ItemId(3), &smaller, Tick::ZERO, 153, 24).expect("frame"); assert_eq!(*kept, *full); let _ = std::fs::remove_dir_all(&dir); }