From 2fbdcfe5f5233443dcdeaf53b37023939d336ba0 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 14:33:44 -0500 Subject: [PATCH] UI: cap live video decoders and bound slide-show grid thumbnail height (#1801) * UI: cap live video decoders and bound slide-show grid thumbnail height * address review: retry video feed creation when playback resumes * address review: reclaim the decoder of ended videos when the feed cap is reached * test: cover the video decoder cap and its retry * address review: seeking a video without a decoder retries the bounded decoder allocation --- crates/ui-egui/src/media.rs | 96 ++++++++++++++++++++++++++++--- crates/ui-egui/src/show/render.rs | 36 +++++++++--- 2 files changed, 117 insertions(+), 15 deletions(-) diff --git a/crates/ui-egui/src/media.rs b/crates/ui-egui/src/media.rs index a5429bc37..562b9236c 100644 --- a/crates/ui-egui/src/media.rs +++ b/crates/ui-egui/src/media.rs @@ -12,6 +12,9 @@ use egui::{Align2, Color32, CornerRadius, Pos2, Rect, Sense, Stroke, TextureHand use crate::icons::{self, Icon}; +/// Video shapes that get a decoder at once; more play their sound without a picture. +const MAX_VIDEO_FEEDS: usize = 16; + /// Who started a voice: the editor's control bar stops when you leave the slide, the slide show /// manages its own. #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -227,14 +230,10 @@ impl MediaHost { audio: clip.volume > 0.0, }; let source_changed = self.owners.get(&id).is_some_and(|o| o.source != media_key); - if clip.video && (source_changed || !self.videos.contains_key(&id)) { - let feed = match VideoFeed::new(bytes.clone()) { - Ok(f) => Some(f), - Err(e) => { - log::info!("video {id}: {e}"); - None - } - }; + // A shape whose feed could not be had (the cap, a decode error) tries again on each play. + let feedless = self.videos.get(&id).is_some_and(|v| v.feed.is_none()); + if clip.video && (source_changed || feedless || !self.videos.contains_key(&id)) { + let feed = self.new_feed(id, &bytes); self.videos.insert(id, Video { feed, frame: None, tex: None }); } else if !clip.video { self.videos.remove(&id); @@ -243,6 +242,35 @@ impl MediaHost { self.owners.insert(id, Owned { owner, clip: clip.clone(), source: media_key }); } + /// A decoder for the video of shape `id`, within the cap on live decoders; `None` past it or + /// when the video cannot be decoded (the shape then plays its sound only). + fn new_feed(&mut self, id: ShapeId, bytes: &Arc>) -> Option { + // One decoder and frame queue per video shape: past the cap a shape plays its sound only. + let mut live = self.videos.iter().filter(|(vid, v)| **vid != id && v.feed.is_some()).count(); + if live >= MAX_VIDEO_FEEDS { + // Videos that have ended no longer need their decoder: free it and keep the last + // picture (a replay makes a new feed). + for (vid, v) in self.videos.iter_mut() { + if *vid != id && v.feed.is_some() && self.player.status(key(*vid)).is_some_and(|s| s.state == PlayState::Ended) { + v.feed = None; + live -= 1; + } + } + } + if live >= MAX_VIDEO_FEEDS { + log::info!("video {id}: more than {MAX_VIDEO_FEEDS} videos playing; showing no picture"); + None + } else { + match VideoFeed::new(bytes.clone()) { + Ok(f) => Some(f), + Err(e) => { + log::info!("video {id}: {e}"); + None + } + } + } + } + pub fn pause(&mut self, id: ShapeId) { self.player.pause(key(id)); } @@ -280,6 +308,15 @@ impl MediaHost { self.play(doc_uid, doc, id, clip, Some(t), owner); self.pause(id); } + // A video whose decoder was reclaimed (or refused at the cap) tries for one again, so its picture follows the seek. + if clip.video && self.videos.get(&id).is_some_and(|v| v.feed.is_none()) { + let bytes = doc.media(clip.media).map(|m| m.data.clone()); + if let Some(feed) = bytes.and_then(|b| self.new_feed(id, &b)) + && let Some(v) = self.videos.get_mut(&id) + { + v.feed = Some(feed); + } + } self.player.seek(key(id), t); } @@ -804,4 +841,47 @@ mod tests { assert_eq!((a.rect.x, a.rect.y, a.rect.w, a.rect.h), (100.0, 50.0, 20.0, 20.0)); assert_eq!((b.rect.x, b.rect.y, b.rect.w, b.rect.h), (120.0, 50.0, 20.0, 20.0)); } + + /// Past `MAX_VIDEO_FEEDS` live decoders a video shape plays without a picture, and gets a + /// decoder again on its next play once one has been freed. + #[test] + fn video_decoders_are_capped_and_retried_when_one_frees_up() { + let mut doc = Presentation::default(); + let media = doc.add_media("clip.mp4", "video/mp4", include_bytes!("../../media/tests/fixtures/clip-h264.mp4").to_vec()).expect("media"); + let clip = MediaClip { media, volume: 0.0, video: true, ..Default::default() }; + let mut host = MediaHost::new(None); + let live = |h: &MediaHost| h.videos.values().filter(|v| v.feed.is_some()).count(); + for n in 0..=MAX_VIDEO_FEEDS as u32 { + host.play(1, &doc, ShapeId(5000 + n), &clip, None, Owner::Editor); + } + assert_eq!(live(&host), MAX_VIDEO_FEEDS); + let extra = ShapeId(5000 + MAX_VIDEO_FEEDS as u32); + assert!(host.videos.get(&extra).is_some_and(|v| v.feed.is_none()), "the shape past the cap has no picture"); + host.stop(ShapeId(5000)); + host.play(1, &doc, extra, &clip, None, Owner::Editor); + assert!(host.videos.get(&extra).is_some_and(|v| v.feed.is_some()), "a freed decoder is taken on the next play"); + assert_eq!(live(&host), MAX_VIDEO_FEEDS); + } + + /// Seeking a video that has no decoder (refused at the cap, or reclaimed) retries one without + /// restarting playback, so its picture follows the seek. + #[test] + fn seeking_a_feedless_video_retries_its_decoder() { + let mut doc = Presentation::default(); + let media = doc.add_media("clip.mp4", "video/mp4", include_bytes!("../../media/tests/fixtures/clip-h264.mp4").to_vec()).expect("media"); + let clip = MediaClip { media, volume: 0.0, video: true, ..Default::default() }; + let mut host = MediaHost::new(None); + for n in 0..=MAX_VIDEO_FEEDS as u32 { + host.play(1, &doc, ShapeId(7000 + n), &clip, None, Owner::Editor); + } + let extra = ShapeId(7000 + MAX_VIDEO_FEEDS as u32); + assert!(host.videos.get(&extra).is_some_and(|v| v.feed.is_none())); + // Still at the cap: a seek does not exceed it. + host.seek(1, &doc, extra, &clip, 0.0, Owner::Editor); + assert!(host.videos.get(&extra).is_some_and(|v| v.feed.is_none())); + host.stop(ShapeId(7000)); + host.seek(1, &doc, extra, &clip, 0.0, Owner::Editor); + assert!(host.videos.get(&extra).is_some_and(|v| v.feed.is_some()), "the seek got a decoder"); + assert_eq!(host.videos.values().filter(|v| v.feed.is_some()).count(), MAX_VIDEO_FEEDS); + } } diff --git a/crates/ui-egui/src/show/render.rs b/crates/ui-egui/src/show/render.rs index d6de7a9ff..29a0c8fb7 100644 --- a/crates/ui-egui/src/show/render.rs +++ b/crates/ui-egui/src/show/render.rs @@ -196,18 +196,40 @@ pub(super) fn part_textures(ctx: &egui::Context, doc: &Presentation, from: Optio ] } +/// Width of a slide-show grid thumbnail, in pixels. +const THUMB_WIDTH: f64 = 320.0; +/// Tallest grid thumbnail, so an extreme portrait slide cannot ask for a huge raster. +const THUMB_MAX_HEIGHT: f64 = 2048.0; + +/// The scale and pixel size of a grid thumbnail of a `width` x `height` slide. +fn thumb_size(width: f64, height: f64) -> (f64, f64, f64) { + let (sw, sh) = (width.max(1.0), height.max(1.0)); + let scale = (THUMB_WIDTH / sw).min(THUMB_MAX_HEIGHT / sh); + (scale, (sw * scale).round().max(1.0), (sh * scale).round().max(1.0)) +} + pub(super) fn render_thumb(ctx: &egui::Context, show: &mut Show, doc: &Presentation, i: usize) -> Option { if show.thumbs.len() >= GRID_CAP { return None; } - let w = 320.0_f64; - let h = (w * doc.slide_size.height / doc.slide_size.width.max(1.0)).round().max(1.0); - let img = deckcraft_render::render_slide( - doc, - i, - &RenderOpts { scale: w / doc.slide_size.width.max(1.0), size: Some((w as u32, h as u32)), ..Default::default() }, - ); + // 320 wide, unless the slide is so tall that this would pass the height limit: fit that box. + let (scale, w, h) = thumb_size(doc.slide_size.width, doc.slide_size.height); + let img = deckcraft_render::render_slide(doc, i, &RenderOpts { scale, size: Some((w as u32, h as u32)), ..Default::default() }); let tex = crate::textures::upload(ctx, format!("show-grid-{i}"), crate::textures::to_color_image(&img, false)); show.thumbs.push((i, tex.clone())); Some(tex) } + +#[cfg(test)] +mod thumb_size_tests { + use super::*; + + #[test] + fn ordinary_slides_are_320_wide_and_extreme_ones_stay_in_the_height_limit() { + let (scale, w, h) = thumb_size(960.0, 540.0); + assert_eq!((w, h), (320.0, 180.0)); + assert!((scale - 320.0 / 960.0).abs() < 1e-12); + let (_, w, h) = thumb_size(1.0, 1.0e9); + assert!(h <= THUMB_MAX_HEIGHT && w >= 1.0); + } +}