fix: bound retained camera frames to their age and input

Author: richiemcilroy

apps/desktop-gpui/src/app_windows.rs

@@ -4507,6 +4507,12 @@ pub fn deliver_camera_frame(
     #[cfg(not(target_os = "macos"))] frame: crate::camera_window::CameraPreviewFrame,
     cx: &mut App,
 ) -> bool {
+    if !crate::feeds::Feeds::global(cx)
+        .read(cx)
+        .accepts_camera_preview(frame.timestamp)
+    {
+        return false;
+    }
     let Some(handle) = cx.global::<AppWindows>().camera else {
         return false;
     };

apps/desktop-gpui/src/camera_window.rs

@@ -923,6 +923,7 @@ struct CameraPreviewView {
     frame_dims: Option<(usize, usize)>,
     retained: Option<Arc<gpui::RenderImage>>,
     retained_dims: Option<(usize, usize)>,
+    retained_captured_at: Option<Instant>,
     reveal_started: Option<Instant>,
     retained_invalidated: bool,
     selection_revision: u64,
@@ -963,6 +964,7 @@ impl CameraPreviewView {
                     && retained.captured_at.elapsed() < Duration::from_secs(60)
             });
         let retained_dims = retained.as_ref().map(|retained| retained.frame_dims);
+        let retained_captured_at = retained.as_ref().map(|retained| retained.captured_at);
         if let Some(retained) = &retained {
             let expiry = cx
                 .background_executor()
@@ -1005,6 +1007,7 @@ impl CameraPreviewView {
             frame_dims: None,
             retained,
             retained_dims,
+            retained_captured_at,
             reveal_started: None,
             retained_invalidated: false,
             selection_revision: 0,
@@ -1061,6 +1064,21 @@ impl CameraPreviewView {
 
 impl Render for CameraPreviewView {
     fn render(&mut self, window: &mut Window, _cx: &mut Context<Self>) -> impl IntoElement {
+        if self
+            .frame_revision
+            .is_some_and(|revision| revision != self.selection_revision)
+        {
+            #[cfg(target_os = "macos")]
+            {
+                self.latest_frame = None;
+            }
+            #[cfg(not(target_os = "macos"))]
+            if let Some(image) = self.latest_frame.take() {
+                let _ = window.drop_image(image);
+            }
+            self.frame_dims = None;
+            self.frame_revision = None;
+        }
         if (self.retained_invalidated
             || self
                 .reveal_started
@@ -1129,6 +1147,7 @@ impl Render for CameraPreviewView {
             let overlay = div().absolute().inset_0().child(
                 gpui::img(image)
                     .size_full()
+                    .rounded(px(radius))
                     .object_fit(gpui::ObjectFit::Cover),
             );
             container = if self.reveal_started.is_some() {
@@ -1211,6 +1230,7 @@ impl Render for CameraPreviewView {
 
 #[cfg(not(target_os = "macos"))]
 pub struct CameraPreviewFrame {
+    pub timestamp: cap_timestamp::Timestamp,
     pub image: Arc<gpui::RenderImage>,
     pub dims: (usize, usize),
 }
@@ -1249,6 +1269,7 @@ impl Render for CameraToolbarView {
 /// `release_blur_resources` behaviour (`camera.rs:1477-1484`).
 #[cfg(target_os = "macos")]
 struct BlurBridge {
+    epoch: u64,
     tx: flume::Sender<camera_blur::BlurJob>,
     /// The first blurred output may land while the window is inactive, where
     /// a notify alone may not present (the unit-2 first-frame finding); the
@@ -1350,21 +1371,27 @@ impl CameraWindow {
         let image = (preview.frame_revision == Some(preview.selection_revision))
             .then(|| preview.latest_frame.clone())
             .flatten();
-        let Some(image) = image.or_else(|| preview.retained.clone()) else {
+        let Some((image, captured_at)) = image
+            .map(|image| (image, Instant::now()))
+            .or_else(|| preview.retained.clone().zip(preview.retained_captured_at))
+        else {
             return;
         };
+        let remaining = Duration::from_secs(60).saturating_sub(captured_at.elapsed());
+        if remaining.is_zero() {
+            return;
+        }
         let Some(frame_dims) = preview.frame_dims.or(preview.retained_dims) else {
             return;
         };
-        let captured_at = Instant::now();
         cx.default_global::<ParkedCameraPreview>().0 = Some(RetainedCameraPreview {
             image,
             camera,
             state: self.state,
             captured_at,
             frame_dims,
         });
-        let expiry = cx.background_executor().timer(Duration::from_secs(60));
+        let expiry = cx.background_executor().timer(remaining);
         cx.spawn(async move |_, cx| {
             expiry.await;
             cx.update(|cx| {
@@ -1514,6 +1541,9 @@ impl CameraWindow {
     ) {
         #[cfg(target_os = "macos")]
         {
+            let Some(epoch) = Feeds::global(cx).read(cx).camera_preview_epoch() else {
+                return;
+            };
             use core_foundation::base::TCFType as _;
             use core_video::pixel_buffer::{CVPixelBuffer, CVPixelBufferRef};
 
@@ -1544,7 +1574,7 @@ impl CameraWindow {
                         ring_generation: converted.generation,
                         mode,
                     };
-                    match self.ensure_blur_bridge(window, cx).tx.try_send(job) {
+                    match self.ensure_blur_bridge(epoch, window, cx).tx.try_send(job) {
                         Ok(()) => {}
                         // Worker busy: drop this frame and keep the last
                         // painted one -- the bounded(1) latest-wins shape of
@@ -1652,7 +1682,19 @@ impl CameraWindow {
     }
 
     #[cfg(target_os = "macos")]
-    fn ensure_blur_bridge(&mut self, window: &Window, cx: &mut Context<Self>) -> &BlurBridge {
+    fn ensure_blur_bridge(
+        &mut self,
+        epoch: u64,
+        window: &Window,
+        cx: &mut Context<Self>,
+    ) -> &BlurBridge {
+        if self
+            .blur
+            .as_ref()
+            .is_some_and(|bridge| bridge.epoch != epoch)
+        {
+            self.blur = None;
+        }
         if self.blur.is_none() {
             let (job_tx, job_rx) = flume::bounded::<camera_blur::BlurJob>(1);
             let (out_tx, out_rx) = flume::bounded::<camera_blur::BlurOutput>(2);
@@ -1668,7 +1710,7 @@ impl CameraWindow {
             let pump = cx.spawn(async move |this, cx| {
                 while let Ok(output) = out_rx.recv_async().await {
                     let first = match this.update(cx, |this: &mut CameraWindow, cx| {
-                        this.blurred_frame_arrived(output, cx)
+                        this.blurred_frame_arrived(output, epoch, cx)
                     }) {
                         Ok(first) => first,
                         Err(_) => break,
@@ -1685,6 +1727,7 @@ impl CameraWindow {
                 }
             });
             self.blur = Some(BlurBridge {
+                epoch,
                 tx: job_tx,
                 first_output_pending: true,
                 _pump: pump,
@@ -1700,11 +1743,14 @@ impl CameraWindow {
     fn blurred_frame_arrived(
         &mut self,
         output: camera_blur::BlurOutput,
+        epoch: u64,
         cx: &mut Context<Self>,
     ) -> bool {
         // A stale output can land after the mode flips back to Off; the raw
         // path is already painting again, so drop it.
-        if self.state.background_blur == BlurMode::Off {
+        if self.state.background_blur == BlurMode::Off
+            || Feeds::global(cx).read(cx).camera_preview_epoch() != Some(epoch)
+        {
             return false;
         }
         let first = self

apps/desktop-gpui/src/feeds.rs

@@ -117,6 +117,13 @@ async fn camera_input_operation<T>(
     operation.await.map(Some)
 }
 
+fn camera_preview_frame_is_fresh(
+    timestamp: cap_timestamp::Timestamp,
+    ready_at: Option<cap_timestamp::Timestamps>,
+) -> bool {
+    ready_at.is_some_and(|ready_at| timestamp.checked_duration_since(ready_at).is_some())
+}
+
 fn configuration_result(
     current_epoch: u64,
     epoch: u64,
@@ -642,7 +649,11 @@ fn run_camera_preview_worker(config: CameraPreviewWorkerConfig) {
             recording.publish(&image, dims, frame.timestamp, applied_mask);
         }
         if active.load(Ordering::Acquire) {
-            match preview_tx.try_send(crate::camera_window::CameraPreviewFrame { image, dims }) {
+            match preview_tx.try_send(crate::camera_window::CameraPreviewFrame {
+                image,
+                dims,
+                timestamp: frame.timestamp,
+            }) {
                 Ok(()) | Err(flume::TrySendError::Full(_)) => {}
                 Err(flume::TrySendError::Disconnected(_)) => {
                     #[cfg(target_os = "linux")]
@@ -667,6 +678,7 @@ pub struct Feeds {
     microphone_settings: Option<microphone::MicrophoneDeviceSettings>,
     applied_settings: crate::store::RecordingDeviceSettings,
     camera_input_pending: bool,
+    camera_preview_not_before: Option<cap_timestamp::Timestamps>,
     mic_input_pending: bool,
     mic_input_released: bool,
     microphone_error: Option<String>,
@@ -728,6 +740,7 @@ impl Feeds {
             microphone_settings: None,
             applied_settings: crate::store::RecordingDeviceSettings::default(),
             camera_input_pending: false,
+            camera_preview_not_before: None,
             mic_input_pending: false,
             mic_input_released: false,
             microphone_error: None,
@@ -870,6 +883,7 @@ impl Feeds {
         self.camera_settings = settings;
         self.applied_settings.camera = None;
         self.camera_input_pending = false;
+        self.camera_preview_not_before = None;
         self.camera_error = None;
         cx.notify();
 
@@ -886,6 +900,20 @@ impl Feeds {
         self.camera_epoch
     }
 
+    pub(crate) fn camera_preview_epoch(&self) -> Option<u64> {
+        (self.camera.is_some()
+            && !self.camera_preview_parked
+            && !self.camera_input_pending
+            && self.camera_error.is_none()
+            && self.camera_preview_not_before.is_some())
+        .then_some(self.camera_epoch)
+    }
+
+    pub(crate) fn accepts_camera_preview(&self, timestamp: cap_timestamp::Timestamp) -> bool {
+        self.camera_preview_epoch().is_some()
+            && camera_preview_frame_is_fresh(timestamp, self.camera_preview_not_before)
+    }
+
     pub fn camera_configuration_result(&self, epoch: u64) -> Option<Result<(), String>> {
         configuration_result(
             self.camera_epoch,
@@ -910,6 +938,7 @@ impl Feeds {
         }
 
         self.camera_preview_parked = true;
+        self.camera_preview_not_before = None;
         self.applied_settings.camera = None;
         self.camera_input_pending = false;
         self.camera_epoch += 1;
@@ -949,6 +978,7 @@ impl Feeds {
     fn start_camera_preview(&mut self, selection: SelectedCamera, cx: &mut Context<Self>) {
         self.camera_error = None;
         self.camera_input_pending = true;
+        self.camera_preview_not_before = None;
         self.applied_settings.camera = None;
         let epoch = self.camera_epoch;
         let settings = self.camera_settings;
@@ -1010,7 +1040,10 @@ impl Feeds {
                 }
                 this.camera_input_pending = false;
                 match result {
-                    Ok(settings) => this.applied_settings.camera = settings.camera,
+                    Ok(settings) => {
+                        this.applied_settings.camera = settings.camera;
+                        this.camera_preview_not_before = Some(cap_timestamp::Timestamps::now());
+                    }
                     Err(error) => {
                         tracing::error!("camera input failed: {error}");
                         this.camera_error = Some(error);
@@ -1470,6 +1503,43 @@ fn db_fs(samples: &MicrophoneSamples) -> f64 {
 mod tests {
     use super::*;
 
+    #[test]
+    fn preview_rejects_frames_queued_before_input_readiness() {
+        let ready = cap_timestamp::Timestamps::now();
+        let queued = ready
+            .instant()
+            .checked_sub(Duration::from_millis(1))
+            .unwrap();
+        assert!(!camera_preview_frame_is_fresh(
+            cap_timestamp::Timestamp::Instant(queued),
+            Some(ready)
+        ));
+        assert!(!camera_preview_frame_is_fresh(
+            cap_timestamp::Timestamp::Instant(ready.instant()),
+            None
+        ));
+        assert!(camera_preview_frame_is_fresh(
+            cap_timestamp::Timestamp::Instant(ready.instant()),
+            Some(ready)
+        ));
+    }
+
+    #[cfg(target_os = "macos")]
+    #[test]
+    fn preview_checks_native_camera_capture_clock() {
+        let ready = cap_timestamp::Timestamps::now();
+        assert!(!camera_preview_frame_is_fresh(
+            cap_timestamp::Timestamp::MachAbsoluteTime(cap_timestamp::MachAbsoluteTimestamp::new(
+                0
+            )),
+            Some(ready)
+        ));
+        assert!(camera_preview_frame_is_fresh(
+            cap_timestamp::Timestamp::MachAbsoluteTime(cap_timestamp::MachAbsoluteTimestamp::now()),
+            Some(ready)
+        ));
+    }
+
     #[tokio::test]
     async fn same_device_format_change_discards_queued_previous_configuration() {
         let gate = tokio::sync::Mutex::new(());

apps/desktop/src/routes/camera.tsx

@@ -432,13 +432,24 @@ function LegacyCameraPreviewPage(props: {
 	const retainedCanvas = document.createElement("canvas");
 	const { rawOptions } = useRecordingOptions();
 	let retainedFrameTimeout: ReturnType<typeof setTimeout> | undefined;
+	let retainedFrameCapturedAt: number | undefined;
 
 	const clearRetainedFrame = () => {
 		clearTimeout(retainedFrameTimeout);
+		retainedFrameCapturedAt = undefined;
 		setHasRetainedFrame(false);
 		retainedCanvas.width = 0;
 		retainedCanvas.height = 0;
 	};
+	const scheduleRetainedFrameExpiry = () => {
+		clearTimeout(retainedFrameTimeout);
+		if (retainedFrameCapturedAt === undefined) return;
+		const remaining = 60_000 - (performance.now() - retainedFrameCapturedAt);
+		retainedFrameTimeout = setTimeout(
+			clearRetainedFrame,
+			Math.max(0, remaining),
+		);
+	};
 	const [frameDimensions, setFrameDimensions] = createSignal<{
 		width: number;
 		height: number;
@@ -502,13 +513,13 @@ function LegacyCameraPreviewPage(props: {
 			controls?.hasRenderedFrame() && rawOptions.cameraID && !props.issue();
 		controls?.dispose();
 		if (canRetain && retainedCanvas.width > 0) {
+			retainedFrameCapturedAt = performance.now();
 			setHasRetainedFrame(true);
 		} else if (!rawOptions.cameraID || props.issue()) {
 			clearRetainedFrame();
 		}
 		setHasFrame(false);
-		clearTimeout(retainedFrameTimeout);
-		retainedFrameTimeout = setTimeout(clearRetainedFrame, 60_000);
+		scheduleRetainedFrameExpiry();
 		if (
 			socket &&
 			socket.readyState !== WebSocket.CLOSING &&
@@ -601,11 +612,11 @@ function LegacyCameraPreviewPage(props: {
 				rawOptions.cameraID &&
 				!props.issue()
 			) {
+				retainedFrameCapturedAt = performance.now();
 				setHasRetainedFrame(true);
 			}
 			setHasFrame(false);
-			clearTimeout(retainedFrameTimeout);
-			retainedFrameTimeout = setTimeout(clearRetainedFrame, 60_000);
+			scheduleRetainedFrameExpiry();
 			scheduleReconnect();
 		});