Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
type: fixed
date: 2026-08-21
---

The camera bubble now recovers on its own when macOS refuses to start its video, instead of sitting as a black circle until you clicked it.
163 changes: 150 additions & 13 deletions templates/clips/desktop/src-tauri/src/native_screen/custom_capture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -959,6 +959,34 @@ struct CustomScreenCaptureWriterState {
session_start_time: Option<(i64, i32)>,
finished: bool,
failed: Option<String>,
/// Per-track append bookkeeping, keyed by the labels in `track_labels`.
/// Only interesting when something goes wrong: `appendSampleBuffer`
/// reports a bare `false` plus an `AVErrorUnknown`, naming neither the
/// track nor the timestamp, so a mid-recording writer death is otherwise
/// undiagnosable from a user's log.
append_stats: std::collections::HashMap<&'static str, TrackAppendStats>,
}

/// Track labels used for append diagnostics. Static strings so the stats map
/// keys stay allocation-free on the realtime capture callbacks.
mod track_labels {
pub(super) const VIDEO: &str = "video";
pub(super) const SYSTEM_AUDIO: &str = "system-audio";
pub(super) const MIC_AUDIO: &str = "mic-audio";
pub(super) const MIXED_AUDIO: &str = "mixed-audio";
}

/// What we know about one writer input's append history. Enough to answer the
/// two questions a writer failure raises: which track broke, and was its
/// timeline still monotonic when it did.
#[derive(Default, Clone, Copy)]
struct TrackAppendStats {
appended: u64,
last_pts_seconds: Option<f64>,
/// Count of samples whose PTS did not advance past the previous one.
/// AVAssetWriter rejects a non-monotonic timeline, so a non-zero count
/// here beside a failure is the answer rather than a coincidence.
pts_regressions: u64,
}

// SAFETY: `Retained<AnyObject>` is `!Send`/`!Sync` by default because objc2
Expand Down Expand Up @@ -1558,6 +1586,7 @@ impl CustomScreenCaptureWriter {
session_start_time: None,
finished: false,
failed: None,
append_stats: std::collections::HashMap::new(),
})),
mixer: mixer.map(|m| Arc::new(Mutex::new(m))),
started: Arc::new(AtomicBool::new(false)),
Expand Down Expand Up @@ -1675,6 +1704,35 @@ impl CustomScreenCaptureWriter {
.and_then(|guard| guard.failed.clone())
}

/// One line describing what each writer input managed to append. Paired
/// with a failure it separates "this track died" from "this track was
/// never fed", which the failure string alone cannot say.
fn append_stats_summary(&self) -> String {
let Ok(guard) = self.inner.lock() else {
return "append stats unavailable (writer lock poisoned)".to_string();
};
if guard.append_stats.is_empty() {
return "no samples appended on any track".to_string();
}
let mut tracks: Vec<_> = guard.append_stats.iter().collect();
tracks.sort_by_key(|(track, _)| **track);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Fix the unsized Rust sort key

append_stats_summary collects HashMap::iter() entries, so track is &&str; **track dereferences to the unsized str, which cannot be used as the sort_by_key result. This prevents the modified Rust target from compiling. Sort by *track (or the tuple's reference) instead.

Additional Info
Found by 1 of 2 incremental review agents; confirmed from the current source and Rust type behavior.

Fix in Builder

tracks
.iter()
.map(|(track, stats)| {
format!(
"{track}: appended={} last_pts={} regressions={}",
stats.appended,
stats
.last_pts_seconds
.map(|v| format!("{v:.6}s"))
.unwrap_or_else(|| "none".to_string()),
stats.pts_regressions
)
})
.collect::<Vec<_>>()
.join(" | ")
}

/// Accumulated pause offset in seconds. Every appended sample skips this
/// much wall-clock time so a pause/resume leaves no gap in the file.
pub(super) fn pause_offset(&self) -> f64 {
Expand Down Expand Up @@ -1719,10 +1777,14 @@ impl CustomScreenCaptureWriter {
if guard.finished || guard.failed.is_some() {
return;
}
let input = match of_type {
SCStreamOutputType::Screen => Some(guard.video_input.clone()),
SCStreamOutputType::Audio => guard.system_audio_input.clone(),
SCStreamOutputType::Microphone => guard.mic_audio_input.clone(),
let (input, track) = match of_type {
SCStreamOutputType::Screen => (Some(guard.video_input.clone()), track_labels::VIDEO),
SCStreamOutputType::Audio => {
(guard.system_audio_input.clone(), track_labels::SYSTEM_AUDIO)
}
SCStreamOutputType::Microphone => {
(guard.mic_audio_input.clone(), track_labels::MIC_AUDIO)
}
};
let Some(input) = input else {
return;
Expand All @@ -1731,6 +1793,7 @@ impl CustomScreenCaptureWriter {
Ok(timing) if timing.presentation_time_stamp.is_valid() => timing,
_ => return,
};
let source_pts = timing.presentation_time_stamp.as_seconds();

unsafe {
if !self.ensure_session_started(&mut guard, timing.presentation_time_stamp) {
Expand All @@ -1748,15 +1811,27 @@ impl CustomScreenCaptureWriter {
let pause_offset = self.pause_offset();
match retimed_sample_copy(sample, &timing, base, pause_offset) {
Ok(copy) => {
self.append_sample_ptr(&mut guard, &input, copy.as_ptr());
// Report the rebased PTS, not the source one — that is
// the timeline the writer actually validates.
let rebased_pts = copy
.sample_timing_info(0)
.ok()
.and_then(|t| t.presentation_time_stamp.as_seconds());
self.append_sample_ptr(
&mut guard,
&input,
copy.as_ptr(),
track,
rebased_pts,
);
}
Err(err) => {
drop(guard);
self.fail(format!("sample retime failed: {err}"));
self.fail(format!("sample retime failed on {track}: {err}"));
}
}
} else {
self.append_sample_ptr(&mut guard, &input, sample.as_ptr());
self.append_sample_ptr(&mut guard, &input, sample.as_ptr(), track, source_pts);
}
}
}
Expand Down Expand Up @@ -1822,8 +1897,18 @@ impl CustomScreenCaptureWriter {
return;
};
for buffer in &emitted {
let pts = buffer
.sample_timing_info(0)
.ok()
.and_then(|t| t.presentation_time_stamp.as_seconds());
unsafe {
self.append_sample_ptr(&mut guard, &input, buffer.as_ptr());
self.append_sample_ptr(
&mut guard,
&input,
buffer.as_ptr(),
track_labels::MIXED_AUDIO,
pts,
);
}
if guard.failed.is_some() {
break;
Expand Down Expand Up @@ -1936,6 +2021,8 @@ impl CustomScreenCaptureWriter {
guard: &mut CustomScreenCaptureWriterState,
input: &objc2::rc::Retained<objc2::runtime::AnyObject>,
sample_ptr: *mut std::ffi::c_void,
track: &'static str,
pts_seconds: Option<f64>,
) {
use objc2::msg_send;

Expand All @@ -1945,10 +2032,43 @@ impl CustomScreenCaptureWriter {
// count and periodically log so sustained backpressure is visible.
let dropped = self.dropped_samples.fetch_add(1, Ordering::Relaxed) + 1;
if dropped == 1 || dropped % 100 == 0 {
eprintln!("[mixer] writer input not ready; dropped {dropped} sample(s) so far");
eprintln!(
"[mixer] writer input not ready on {track}; dropped {dropped} sample(s) so far"
);
}
return;
}
// Record the timeline BEFORE the append so a failure report describes
// the sample that was actually rejected, not the last good one.
let stats = guard.append_stats.entry(track).or_default();
let previous_pts = stats.last_pts_seconds;
if let Some(pts) = pts_seconds {
if previous_pts.is_some_and(|last| pts <= last) {
stats.pts_regressions += 1;
let regressions = stats.pts_regressions;
if regressions == 1 || regressions % 100 == 0 {
crate::logfile::diagnostic(&format!(
"[capture-health] {track} PTS did not advance: {:.6}s after {:.6}s ({regressions} so far)",
pts,
previous_pts.unwrap_or(f64::NAN)
));
}
}
stats.last_pts_seconds = Some(pts);
}
let appended_before = stats.appended;
stats.appended += 1;
let pts_report = |outcome: &str| {
format!(
"AVAssetWriter appendSampleBuffer {outcome} on {track} (pts={}, previous={}, appended={appended_before})",
pts_seconds
.map(|v| format!("{v:.6}s"))
.unwrap_or_else(|| "unknown".to_string()),
previous_pts
.map(|v| format!("{v:.6}s"))
.unwrap_or_else(|| "none".to_string()),
)
};
// `appendSampleBuffer:` throws Objective-C exceptions on bad input
// (format/timestamp/state). Those can't be caught by `catch_unwind`
// and would abort the app, so contain them here.
Expand All @@ -1962,15 +2082,18 @@ impl CustomScreenCaptureWriter {
Ok(false) => {
self.appends_closed.store(true, Ordering::SeqCst);
guard.failed = Some(format!(
"AVAssetWriter appendSampleBuffer failed{}",
"{}{}",
pts_report("failed"),
av_writer_error_suffix(&guard.writer)
));
}
Err(exc) => {
self.appends_closed.store(true, Ordering::SeqCst);
let detail = describe_objc_exception(exc);
eprintln!("[mixer] appendSampleBuffer raised Objective-C exception: {detail}");
guard.failed = Some(format!("AVAssetWriter appendSampleBuffer raised: {detail}"));
eprintln!(
"[mixer] appendSampleBuffer raised Objective-C exception on {track}: {detail}"
);
guard.failed = Some(format!("{}: {detail}", pts_report("raised")));
}
}
}
Expand Down Expand Up @@ -2012,8 +2135,18 @@ impl CustomScreenCaptureWriter {
Ok(buffers) => {
if let Some(input) = guard.mixed_audio_input.clone() {
for buffer in &buffers {
let pts = buffer
.sample_timing_info(0)
.ok()
.and_then(|t| t.presentation_time_stamp.as_seconds());
unsafe {
self.append_sample_ptr(&mut guard, &input, buffer.as_ptr());
self.append_sample_ptr(
&mut guard,
&input,
buffer.as_ptr(),
track_labels::MIXED_AUDIO,
pts,
);
}
if guard.failed.is_some() {
break;
Expand Down Expand Up @@ -3909,6 +4042,10 @@ fn spawn_capture_watchdog(
crate::logfile::diagnostic(&format!(
"[capture-health] writer closed unexpectedly; finalizing partial recording: {writer_error}"
));
crate::logfile::diagnostic(&format!(
"[capture-health] append stats at failure — {}",
writer.append_stats_summary()
));
if let Ok(guard) = stream.lock() {
let _ = guard.stop_capture();
}
Expand Down
26 changes: 25 additions & 1 deletion templates/clips/desktop/src/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1438,7 +1438,7 @@
}, [serverUrl]);

useEffect(() => {
checkAuth();

Check warning on line 1441 in templates/clips/desktop/src/app.tsx

View workflow job for this annotation

GitHub Actions / Lint & format

typescript(no-floating-promises)

Promises must be awaited, add void operator to ignore.
}, [checkAuth]);

// Push the current server URL to the Rust meetings watcher so it can
Expand Down Expand Up @@ -1568,7 +1568,7 @@
if (method === "GET") {
const params = new URLSearchParams();
for (const [key, value] of Object.entries(body)) {
if (value != null) params.set(key, String(value));

Check warning on line 1571 in templates/clips/desktop/src/app.tsx

View workflow job for this annotation

GitHub Actions / Lint & format

typescript(no-base-to-string)

'value' will use Object's default stringification format ('[object Object]') when stringified.

Check warning on line 1571 in templates/clips/desktop/src/app.tsx

View workflow job for this annotation

GitHub Actions / Lint & format

eslint(eqeqeq)

Expected !== and instead saw !=
}
const qs = params.toString();
if (qs) url += `?${qs}`;
Expand Down Expand Up @@ -2018,33 +2018,33 @@
cancelled = true;
};
}
Promise.all(
meetings.map(async (meeting) => {
if (!meeting.scheduledStart)
return [meeting.id, { available: false }] as const;
try {
const availability = await invoke<RewindMeetingHistoryAvailability>(
"rewind_meeting_history_status",
{ scheduledStart: meeting.scheduledStart },
);
return [meeting.id, availability] as const;
} catch (error) {
return [
meeting.id,
{
available: false,
reason:
error instanceof Error
? error.message
: "Earlier local meeting audio is unavailable.",
},
] as const;
}
}),
).then((entries) => {
if (!cancelled)
setRewindMeetingHistoryAvailability(Object.fromEntries(entries));
});

Check warning on line 2047 in templates/clips/desktop/src/app.tsx

View workflow job for this annotation

GitHub Actions / Lint & format

typescript(no-floating-promises)

Promises must be awaited, add void operator to ignore.
return () => {
cancelled = true;
};
Expand Down Expand Up @@ -2696,6 +2696,7 @@
let stopPump: (() => void) | null = null;
let fellBackToPump = false;
let stream: MediaStream | null = null;
let unlistenUnrendered: (() => void) | null = null;

const startPump = (reason: string) => {
if (cancelled || stopPump || !stream) return;
Expand Down Expand Up @@ -2723,7 +2724,7 @@
s.getTracks().forEach((t) => t.stop());
return;
}
await loadDevices();

Check warning on line 2727 in templates/clips/desktop/src/app.tsx

View workflow job for this annotation

GitHub Actions / Lint & format

react-hooks(exhaustive-deps)

React Hook useEffect has a missing dependency: 'loadDevices'
stream = s;
bubbleStreamRef.current = s;
// Open the bubble window. It's a pure renderer — the bubble
Expand Down Expand Up @@ -2756,11 +2757,30 @@
webrtcHandle = null;
startPump(reason);
};
// ICE reaching `connected` proves the transport works, nothing more.
// WKWebView can refuse to play the received track (no user gesture in
// the bubble page, or its window briefly had no on-screen area), and
// that failure is invisible from here — so the bubble reports it and
// we fall back to the pump. Without this the safety net below only
// ever fired on ICE failure, which is not how this breaks in practice.
listen("clips:bubble-webrtc-unrendered", (ev) => {
startCanvasFallback(
`bubble reported no rendered frames ${JSON.stringify(ev.payload)}`,
);
})
.then((u) => {
if (cancelled) {
u();
return;
}
unlistenUnrendered = u;
})
.catch(() => {});
webrtcHandle = startBubbleWebrtc({
stream: s,
onConnected: () => {
console.log(
"[clips-popover] bubble WebRTC connected — video is live",
"[clips-popover] bubble WebRTC transport connected — waiting for the bubble to confirm playback",
);
},
onFailure: startCanvasFallback,
Expand Down Expand Up @@ -2805,6 +2825,10 @@
!!webrtcHandle,
!!stopPump,
);
if (unlistenUnrendered) {
unlistenUnrendered();
unlistenUnrendered = null;
}
if (webrtcHandle) {
webrtcHandle.stop();
webrtcHandle = null;
Expand Down
76 changes: 76 additions & 0 deletions templates/clips/desktop/src/lib/bubble-playback.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { describe, expect, it } from "vitest";

import {
BUBBLE_RENDER_GRACE_MS,
isRenderingWebrtc,
shouldClaimWebrtcPath,
shouldReportUnrendered,
} from "./bubble-playback";

const base = {
trackArrivedAt: 1_000,
now: 1_000 + BUBBLE_RENDER_GRACE_MS,
paused: true,
videoWidth: 0,
alreadyReported: false,
};

describe("isRenderingWebrtc", () => {
it("requires decoded frames, not just an unpaused element", () => {
expect(isRenderingWebrtc({ paused: false, videoWidth: 0 })).toBe(false);
expect(isRenderingWebrtc({ paused: false, videoWidth: 1280 })).toBe(true);
expect(isRenderingWebrtc({ paused: true, videoWidth: 1280 })).toBe(false);
});
});

describe("shouldClaimWebrtcPath", () => {
it("takes the surface once frames are decoded", () => {
expect(
shouldClaimWebrtcPath({ fallbackRequested: false, videoWidth: 1280 }),
).toBe(true);
});

it("refuses a playing event that decoded nothing", () => {
expect(
shouldClaimWebrtcPath({ fallbackRequested: false, videoWidth: 0 }),
).toBe(false);
});

it("refuses a late playing event after the canvas pump was requested", () => {
expect(
shouldClaimWebrtcPath({ fallbackRequested: true, videoWidth: 1280 }),
).toBe(false);
});
});

describe("shouldReportUnrendered", () => {
it("reports a track that never produced frames within the grace window", () => {
expect(shouldReportUnrendered(base)).toBe(true);
});

it("keeps waiting while the grace window has not elapsed", () => {
expect(shouldReportUnrendered({ ...base, now: base.now - 1 })).toBe(false);
});

it("stays quiet when there is no track to render", () => {
expect(shouldReportUnrendered({ ...base, trackArrivedAt: null })).toBe(
false,
);
});

it("stays quiet once frames are on screen", () => {
expect(
shouldReportUnrendered({ ...base, paused: false, videoWidth: 1280 }),
).toBe(false);
});

it("reports a blocked element that WebKit left unpaused but frameless", () => {
expect(shouldReportUnrendered({ ...base, paused: false })).toBe(true);
});

it("reports once per track so the fallback is not requested in a loop", () => {
expect(shouldReportUnrendered({ ...base, alreadyReported: true })).toBe(
false,
);
});
});
Loading
Loading