diff --git a/crates/compositor/src/live.rs b/crates/compositor/src/live.rs index c486da8e..552d2264 100644 --- a/crates/compositor/src/live.rs +++ b/crates/compositor/src/live.rs @@ -62,6 +62,12 @@ fn webcam_seek_time(screen_source_time_sec: f64, webcam_offset_sec: f64) -> f64 struct PrefetchedClip { sdec: Decoder, wdec: Decoder, + /// `wdec` est-il la VRAIE caméra, ou le remplaçant écran (pas de caméra déclarée, ou + /// déclarée et illisible) ? Voyage avec la paire de décodeurs parce que c'est d'ELLE que + /// la réponse dépend, pas de la scène : deux clips de la même timeline peuvent avoir l'un + /// une caméra qui s'ouvre et l'autre un fichier mort, et le pool (`PooledClip`) réactive + /// des paires ouvertes plusieurs bascules plus tôt. Cf. `open_webcam_or_stand_in`. + webcam_decoder_is_real: bool, webcam_offset_sec: f64, idx: u32, /// Piste curseur du clip à venir, préchargée ici pour la même raison que les décodeurs : @@ -89,8 +95,15 @@ struct PrefetchedClip { /// valide plutôt qu'un `Option` à dérouler sur tout le chemin chaud, et rien ne le dessine /// puisque la composition ne pose une vignette que si le document déclare une caméra. /// -/// Avec une caméra déclarée dont le fichier ne s'ouvre pas, c'est l'inverse : la vignette -/// EST dessinée et affiche l'écran. Ce cas-là méritait une trace, et n'en avait aucune. +/// Avec une caméra déclarée dont le fichier ne s'ouvre pas, le chemin, lui, reste parfaitement +/// plausible : `webcam_is_real` répond vrai, la vignette était donc dessinée — sur le +/// remplaçant, c'est-à-dire l'enregistrement d'écran dupliqué dans son propre coin. Vu en vrai +/// avec un `.mp4` webcam de 0 octet, laissé non finalisé par le helper de capture natif. +/// +/// D'où le `bool` rendu à côté du décodeur : « ce que je te rends est-il VRAIMENT la caméra ? ». +/// Seule l'ouverture peut répondre — aucune inspection du chemin ne sait qu'un fichier est mort +/// — et c'est cette réponse, et pas le chemin, qui décide de dessiner la vignette (voir +/// `should_draw_webcam`). /// /// ponytail: on garde le remplaçant plutôt que de passer `wdec` en `Option`, ce qui /// toucherait 22 sites dont le pool de décodeurs et la boucle de composition `unsafe`. À faire @@ -100,21 +113,35 @@ unsafe fn open_webcam_or_stand_in( screen_path: &str, webcam_path: &str, gpu: &Gpu, -) -> Result { +) -> Result<(Decoder, bool)> { if !webcam_is_real(webcam_path, screen_path) { - return Decoder::open(screen_path, gpu); + return Ok((Decoder::open(screen_path, gpu)?, false)); } match Decoder::open(webcam_path, gpu) { - Ok(d) => Ok(d), + Ok(d) => Ok((d, true)), Err(e) => { eprintln!( - "WARNING: caméra déclarée mais illisible ({webcam_path}) : {e}. La vignette caméra affichera l'enregistrement d'écran ; le média est à relier." + "WARNING: caméra déclarée mais illisible ({webcam_path}) : {e}. La vignette caméra ne sera pas dessinée ; le média est à relier." ); - Decoder::open(screen_path, gpu) + Ok((Decoder::open(screen_path, gpu)?, false)) } } } +/// Faut-il dessiner la vignette caméra ? Il faut les DEUX moitiés : +/// +/// - le document déclare une caméra — `webcam_is_real`, un test de chemins ; +/// - et son décodeur s'est vraiment ouvert — `decoder_is_real`, ce que seul +/// `open_webcam_or_stand_in` sait, transporté jusqu'ici par `PrefetchedClip`/`Player`. +/// +/// Le test de chemins seul ne suffit pas : il a répondu « vraie caméra » pour un `.mp4` webcam +/// de 0 octet (fichier non finalisé par le helper de capture), la vignette a été dessinée, et +/// le décodeur derrière elle était le remplaçant écran — l'utilisateur voyait son propre +/// enregistrement d'écran répliqué dans le petit rectangle caméra. +fn should_draw_webcam(webcam_path: &str, screen_path: &str, decoder_is_real: bool) -> bool { + webcam_is_real(webcam_path, screen_path) && decoder_is_real +} + unsafe fn open_and_seek_clip( screen_path: &str, webcam_path: &str, @@ -124,7 +151,7 @@ unsafe fn open_and_seek_clip( ) -> Result { let source_time_sec = source_time_sec.max(0.0); let mut sdec = Decoder::open(screen_path, gpu)?; - let mut wdec = open_webcam_or_stand_in(screen_path, webcam_path, gpu)?; + let (mut wdec, webcam_decoder_is_real) = open_webcam_or_stand_in(screen_path, webcam_path, gpu)?; let sf = sdec.seek_to(source_time_sec)?; let mut wf = wdec.seek_to(webcam_seek_time(source_time_sec, webcam_offset_sec))?; if wf.is_null() { @@ -135,7 +162,7 @@ unsafe fn open_and_seek_clip( } let idx = (source_time_sec * sdec.fps()).round().max(0.0) as u32; let cursor_track = CursorTrack::load(&format!("{screen_path}.cursor.json"), 0.0, 24.0 * 3600.0).ok(); - Ok(PrefetchedClip { sdec, wdec, webcam_offset_sec, idx, cursor_track }) + Ok(PrefetchedClip { sdec, wdec, webcam_decoder_is_real, webcam_offset_sec, idx, cursor_track }) } /// Nombre de paires de décodeurs INACTIVES gardées ouvertes en plus de la paire active. @@ -253,6 +280,10 @@ pub struct Player { sdec: Decoder, wdec: Decoder, gpu: Gpu, + /// Même question que `PrefetchedClip::webcam_decoder_is_real`, pour la paire ACTIVE : + /// `wdec` est-il la caméra ou le remplaçant écran ? Mis à jour à chaque bascule de clip + /// (`swap_active`), lu par la boucle de rendu pour décider de dessiner la vignette. + webcam_decoder_is_real: bool, webcam_offset_sec: f64, has_current_frame: bool, use_current_on_next_step: bool, @@ -261,7 +292,7 @@ pub struct Player { impl Player { pub unsafe fn open(screen: &str, webcam: &str, gpu: &Gpu) -> Result { - let wdec = open_webcam_or_stand_in(screen, webcam, gpu)?; + let (wdec, webcam_decoder_is_real) = open_webcam_or_stand_in(screen, webcam, gpu)?; Ok(Player { sdec: Decoder::open(screen, gpu)?, wdec, @@ -271,6 +302,7 @@ impl Player { feature_level: gpu.feature_level, backend: gpu.backend, }, + webcam_decoder_is_real, webcam_offset_sec: 0.0, has_current_frame: false, use_current_on_next_step: false, @@ -348,11 +380,16 @@ impl Player { let outgoing = PrefetchedClip { sdec: std::mem::replace(&mut self.sdec, incoming.sdec), wdec: std::mem::replace(&mut self.wdec, incoming.wdec), + // Suit son décodeur dans les deux sens : la paire sortante emporte sa réponse vers + // le pool (elle sera réactivée sans réouverture, donc sans personne pour la + // recalculer), l'entrante impose la sienne au player. + webcam_decoder_is_real: self.webcam_decoder_is_real, webcam_offset_sec: self.webcam_offset_sec, idx: self.idx, // Le curseur est re-dérivé du chemin à la réactivation ; inutile de le trimballer. cursor_track: None, }; + self.webcam_decoder_is_real = incoming.webcam_decoder_is_real; self.webcam_offset_sec = incoming.webcam_offset_sec; self.idx = incoming.idx; self.has_current_frame = true; @@ -372,6 +409,14 @@ impl Player { open_and_seek_clip(screen, webcam, webcam_offset_sec, source_time_sec, &self.gpu) } + /// Le décodeur webcam ACTIF est-il la vraie caméra ? `false` quand c'est le remplaçant + /// écran — aucune caméra déclarée, ou une caméra déclarée dont le fichier ne s'ouvre pas. + /// La boucle de rendu en a besoin parce que la seule autre source d'information dont elle + /// dispose, le chemin webcam du clip, ment dans le second cas (cf. `should_draw_webcam`). + pub fn webcam_decoder_is_real(&self) -> bool { + self.webcam_decoder_is_real + } + /// Temps source courant du décodeur écran — utilisé par `render_thread` pour détecter le /// franchissement de la fin de fenêtre du clip actif pendant la lecture libre, et pour /// calculer la cible de `step` en lecture libre. `pub` (pas `pub(crate)`) : le harnais @@ -1424,8 +1469,16 @@ unsafe fn render_thread( cfg.mblur_n = ip.mblur_taps; cfg.cursor = ip.cursor_show; // A clip with no camera must not draw the PiP box — the decoder behind it is the - // screen video, so drawing it duplicates the recording into its own corner. - let has_real_webcam = webcam_is_real(&active_webcam_path, &active_screen_path); + // screen video, so drawing it duplicates the recording into its own corner. The + // paths alone cannot answer that: a declared camera whose file will not open (the + // 0-byte MP4 an unfinalized capture leaves behind) keeps a perfectly plausible + // path, and the decoder behind its box is the screen fallback just the same. So + // ask the player what it actually opened. + let has_real_webcam = should_draw_webcam( + &active_webcam_path, + &active_screen_path, + player.webcam_decoder_is_real(), + ); comp.set_live_params(LiveParams { bg_color: ip.bg_color, shadow_scale: ip.shadow_scale, @@ -1930,6 +1983,37 @@ mod tests { assert!(webcam_is_real("/rec/recording-1-webcam.webm", "/rec/recording-1.mp4")); } + #[test] + fn a_camera_whose_file_would_not_open_draws_nothing() { + // Reproduced on a real machine: the native capture helper left a 0-byte webcam + // MP4, so `Decoder::open` failed and the webcam decoder fell back to the SCREEN + // file — while the path stayed as plausible as any other, which is why the string + // test still says "real camera" on the very same input. + assert!(webcam_is_real("/rec/recording-1-webcam.mp4", "/rec/recording-1.mp4")); + assert!(!should_draw_webcam( + "/rec/recording-1-webcam.mp4", + "/rec/recording-1.mp4", + false, + )); + } + + #[test] + fn a_camera_that_did_open_is_drawn() { + assert!(should_draw_webcam( + "/rec/recording-1-webcam.webm", + "/rec/recording-1.mp4", + true, + )); + } + + #[test] + fn a_clip_without_a_camera_draws_nothing_however_well_its_decoder_opened() { + // The stand-in decoder always opens — it IS the screen file — so the path test + // remains the half of the answer that catches "this clip has no camera at all". + assert!(!should_draw_webcam("", "/rec/recording-1.mp4", true)); + assert!(!should_draw_webcam("/rec/recording-1.mp4", "/rec/recording-1.mp4", true)); + } + // --- transport handed to an export and back ------------------------------- // A real `LiveView` needs a D3D device and a decoder; the transport is the only // part an export touches, so these exercise it through `PreviewTransport` alone. diff --git a/electron/electron-env.d.ts b/electron/electron-env.d.ts index 789eb7e5..f75fe1f1 100644 --- a/electron/electron-env.d.ts +++ b/electron/electron-env.d.ts @@ -145,6 +145,11 @@ interface Window { message?: string; discarded?: boolean; error?: string; + /** + * A camera was recorded but produced nothing usable, so the session was + * saved without it. Still a success — the screen video is intact. + */ + webcamDropped?: boolean; }>; pauseNativeWindowsRecording: () => Promise<{ success: boolean; diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index d4c09486..1fe1c8c7 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -75,6 +75,8 @@ import { toHelperRect } from "../native-bridge/helperCoordinates"; import { isSalvageableFragmentedCapture, NATIVE_WINDOWS_SALVAGEABLE_OUTPUT_BYTES, + readWebcamFormat, + readWebcamUnavailable, terminateNativeWindowsCapture, waitForNativeWindowsCaptureStop, } from "../recording/nativeWindowsCaptureStop"; @@ -1324,25 +1326,6 @@ function sendNativeWindowsStopCommand(proc: ChildProcessWithoutNullStreams) { return true; } -function readNativeWindowsWebcamFormat(output: string) { - const lines = output.split(/\r?\n/).filter((line) => line.includes('"event":"webcam-format"')); - const lastLine = lines.at(-1); - if (!lastLine) { - return null; - } - - try { - return JSON.parse(lastLine) as { - width?: number; - height?: number; - fps?: number; - deviceName?: string; - }; - } catch { - return null; - } -} - function readNativeWindowsEncoderSelection(output: string) { const lines = output .split(/\r?\n/) @@ -2449,7 +2432,7 @@ export function registerIpcHandlers( cursorCaptureMode === "editable-overlay" ? Math.max(0, captureStartedAtMs - cursorStartTimeMs) : 0; - const webcamFormat = readNativeWindowsWebcamFormat(nativeWindowsCaptureOutput); + const webcamFormat = readWebcamFormat(nativeWindowsCaptureOutput); const encoderSelection = readNativeWindowsEncoderSelection(nativeWindowsCaptureOutput); // Captured now because stop may have no helper left to ask. A helper // killed mid-recording is exactly the case where this matters most. @@ -2466,12 +2449,30 @@ export function registerIpcHandlers( onRecordingStateChange(true, source.name); } + // Reported at start, not at stop: the helper decides the camera is a + // lost cause during its own init — before it announces "Recording + // started", so the warning is already in the buffer here — and telling + // the user now, while the take is still worth restarting, beats telling + // them at the end. Keyed on the helper's own event rather than on a + // missing `webcamFormat`: absence of the format line also means "the + // line could not be parsed", which would put a red toast on a recording + // whose camera is working perfectly. + const webcamUnavailable = + request.webcam.enabled && readWebcamUnavailable(nativeWindowsCaptureOutput); + if (webcamUnavailable) { + console.warn("[native-wgc] recording without a camera; the helper could not open it", { + deviceId: request.webcam.deviceId, + deviceName: request.webcam.deviceName, + }); + } + return { success: true, recordingId, path: outputPath, helperPath, videoEncoderSelection: encoderSelection?.video ?? null, + webcamUnavailable, }; } catch (error) { console.error("Failed to start native Windows recording:", error); @@ -2863,8 +2864,21 @@ export function registerIpcHandlers( let webcamVideoPath: string | undefined; if (preferredWebcamPath) { try { - await fs.access(preferredWebcamPath, fsConstants.R_OK); - webcamVideoPath = preferredWebcamPath; + // Size, not just existence. A camera that opened but delivered no + // frame still gets a file created for it, and its `Finalize()` then + // fails, leaving nought bytes on disk. Admitting that file put a + // camera track in the document pointing at something no demuxer can + // read, and the preview compositor answers an unreadable camera by + // drawing the SCREEN recording inside the little camera rectangle — + // which is how a webcam that never recorded showed up as the desktop + // duplicated into its own corner (getopenscreen/openscreen#387). + const webcamStat = await fs.stat(preferredWebcamPath); + webcamVideoPath = webcamStat.size > 0 ? preferredWebcamPath : undefined; + if (!webcamVideoPath) { + console.warn("[native-wgc] the webcam file is empty; saving without a camera", { + path: preferredWebcamPath, + }); + } } catch { webcamVideoPath = undefined; } @@ -2887,6 +2901,14 @@ export function registerIpcHandlers( path: screenVideoPath, session, recovered, + // `preferredWebcamPath` is non-null only for a take that asked for a + // camera, so the pair means "a camera was requested and none survived". + // This is the second, quieter way to lose one: the helper opened the + // device happily and then never got a frame out of it, so it reports no + // `webcam-unavailable` and the start-time notice stays silent. Left + // unreported, the user would find out in the editor — which is exactly + // the silence this change exists to end. + webcamDropped: Boolean(preferredWebcamPath) && !webcamVideoPath, message: recovered ? "Native Windows recording recovered from a failed stop" : "Native Windows recording session stored successfully", diff --git a/electron/native/wgc-capture/src/dshow_webcam_capture.cpp b/electron/native/wgc-capture/src/dshow_webcam_capture.cpp index 7e3f8b7a..f39e4b06 100644 --- a/electron/native/wgc-capture/src/dshow_webcam_capture.cpp +++ b/electron/native/wgc-capture/src/dshow_webcam_capture.cpp @@ -112,40 +112,19 @@ DirectShowWebcamCapture::~DirectShowWebcamCapture() { delete impl_; } -bool DirectShowWebcamCapture::initialize( - const std::wstring& deviceId, - const std::wstring& deviceName, - const std::wstring& directShowClsid, - int requestedWidth, - int requestedHeight, - int requestedFps) { - (void)deviceId; - stop(); - delete impl_; - impl_ = nullptr; - impl_ = new Impl(); - fps_ = std::clamp(requestedFps > 0 ? requestedFps : 30, 1, 60); - - HRESULT hr = CoInitializeEx(nullptr, COINIT_MULTITHREADED); - if (SUCCEEDED(hr)) { - impl_->comInitialized = true; - } else if (hr != RPC_E_CHANGED_MODE) { - return succeeded(hr, "CoInitializeEx(DirectShow webcam)"); - } - - if (directShowClsid.empty()) { - std::cerr << "ERROR: DirectShow webcam fallback requires a resolved filter CLSID" << std::endl; - return false; - } - - CLSID selectedClsid{}; - if (FAILED(CLSIDFromString(directShowClsid.c_str(), &selectedClsid))) { - std::cerr << "ERROR: DirectShow webcam fallback received an invalid filter CLSID" << std::endl; - return false; - } - selectedDeviceName_ = deviceName.empty() ? directShowClsid : deviceName; +bool DirectShowWebcamCapture::buildGraph(const CLSID& sourceClsid, const GUID* preferredSubtype) { + // Every attempt starts from empty filters. A RenderStream that fails can + // leave pins connected behind it, and retrying on top of that half-built + // graph is how you get a second failure that says nothing about the format. + impl_->mediaControl.Reset(); + impl_->nullRenderer.Reset(); + impl_->sampleGrabber.Reset(); + impl_->sampleGrabberFilter.Reset(); + impl_->captureFilter.Reset(); + impl_->captureGraph.Reset(); + impl_->graph.Reset(); - if (!succeeded(CoCreateInstance(selectedClsid, nullptr, CLSCTX_INPROC_SERVER, IID_PPV_ARGS(&impl_->captureFilter)), + if (!succeeded(CoCreateInstance(sourceClsid, nullptr, CLSCTX_INPROC_SERVER, IID_PPV_ARGS(&impl_->captureFilter)), "CoCreateInstance(DirectShow webcam filter)")) { return false; } @@ -176,6 +155,9 @@ bool DirectShowWebcamCapture::initialize( AM_MEDIA_TYPE requestedType{}; requestedType.majortype = MEDIATYPE_Video; requestedType.formattype = FORMAT_VideoInfo; + if (preferredSubtype) { + requestedType.subtype = *preferredSubtype; + } if (!succeeded(impl_->sampleGrabber->SetMediaType(&requestedType), "SetMediaType(DirectShow video)")) { return false; } @@ -193,16 +175,90 @@ bool DirectShowWebcamCapture::initialize( return false; } - if (!succeeded(impl_->captureGraph->RenderStream( - &PIN_CATEGORY_CAPTURE, - &MEDIATYPE_Video, - impl_->captureFilter.Get(), - impl_->sampleGrabberFilter.Get(), - impl_->nullRenderer.Get()), - "RenderStream(DirectShow webcam)")) { + return succeeded(impl_->captureGraph->RenderStream( + &PIN_CATEGORY_CAPTURE, + &MEDIATYPE_Video, + impl_->captureFilter.Get(), + impl_->sampleGrabberFilter.Get(), + impl_->nullRenderer.Get()), + "RenderStream(DirectShow webcam)"); +} + +bool DirectShowWebcamCapture::initialize( + const std::wstring& deviceId, + const std::wstring& deviceName, + const std::wstring& directShowClsid, + int requestedWidth, + int requestedHeight, + int requestedFps) { + (void)deviceId; + stop(); + delete impl_; + impl_ = nullptr; + impl_ = new Impl(); + fps_ = std::clamp(requestedFps > 0 ? requestedFps : 30, 1, 60); + + HRESULT hr = CoInitializeEx(nullptr, COINIT_MULTITHREADED); + if (SUCCEEDED(hr)) { + impl_->comInitialized = true; + } else if (hr != RPC_E_CHANGED_MODE) { + return succeeded(hr, "CoInitializeEx(DirectShow webcam)"); + } + + if (directShowClsid.empty()) { + std::cerr << "ERROR: DirectShow webcam fallback requires a resolved filter CLSID" << std::endl; + return false; + } + + CLSID selectedClsid{}; + if (FAILED(CLSIDFromString(directShowClsid.c_str(), &selectedClsid))) { + std::cerr << "ERROR: DirectShow webcam fallback received an invalid filter CLSID" << std::endl; + return false; + } + selectedDeviceName_ = deviceName.empty() ? directShowClsid : deviceName; + + // The camera's own format first, a forced RGB32 conversion only if we cannot + // read it. + // + // Order matters, and not for style. Leaving the grabber unconstrained lets + // intelligent connect hand through the source's native output, which is what + // every camera that works today relies on — asking for RGB32 up front made + // OBS Virtual Camera connect as RGB32 and then deliver no frames at all, a + // straight regression. But this class can only unpack YUY2, NV12 and RGB32, + // so a camera speaking anything else used to be rejected after the graph was + // already built, and the recording simply had no webcam. NVIDIA Broadcast is + // exactly that: absent from Media Foundation, so it can only arrive here, and + // it connects with a subtype this file cannot read + // (getopenscreen/openscreen#387). Naming a concrete subtype on the retry is + // what makes DirectShow insert a colour converter for it. + if (!buildGraph(selectedClsid, nullptr)) { + return false; + } + if (!resolveConnectedFormat(requestedWidth, requestedHeight, false)) { + std::cerr << "WARNING: DirectShow webcam speaks a format this build cannot unpack; " + "asking for RGB32 so the graph converts it" + << std::endl; + if (!buildGraph(selectedClsid, &MEDIASUBTYPE_RGB32)) { + return false; + } + if (!resolveConnectedFormat(requestedWidth, requestedHeight, true)) { + return false; + } + } + + impl_->sampleGrabber->SetBufferSamples(TRUE); + impl_->sampleGrabber->SetOneShot(FALSE); + if (!succeeded(impl_->graph.As(&impl_->mediaControl), "QueryInterface(IMediaControl)")) { return false; } + return true; +} + +bool DirectShowWebcamCapture::resolveConnectedFormat( + int requestedWidth, + int requestedHeight, + bool reportUnsupported) { AM_MEDIA_TYPE connectedType{}; if (!succeeded(impl_->sampleGrabber->GetConnectedMediaType(&connectedType), "GetConnectedMediaType(DirectShow webcam)")) { return false; @@ -214,8 +270,13 @@ bool DirectShowWebcamCapture::initialize( } else if (connectedType.subtype == MEDIASUBTYPE_RGB32) { pixelFormat_ = PixelFormat::Bgra; } else { - std::cerr << "ERROR: Unsupported DirectShow webcam media subtype " - << guidToString(connectedType.subtype) << std::endl; + // Silent on the first pass: the caller answers an unreadable format by + // rebuilding the graph with a conversion, and an ERROR line for a case + // that is about to be handled reads like a failure that never happened. + if (reportUnsupported) { + std::cerr << "ERROR: Unsupported DirectShow webcam media subtype " + << guidToString(connectedType.subtype) << std::endl; + } freeMediaType(connectedType); return false; } @@ -242,12 +303,6 @@ bool DirectShowWebcamCapture::initialize( sourceStride_ = pixelFormat_ == PixelFormat::Bgra ? width_ * 4 : ((width_ + 3) / 4) * 4; } - impl_->sampleGrabber->SetBufferSamples(TRUE); - impl_->sampleGrabber->SetOneShot(FALSE); - if (!succeeded(impl_->graph.As(&impl_->mediaControl), "QueryInterface(IMediaControl)")) { - return false; - } - return true; } diff --git a/electron/native/wgc-capture/src/dshow_webcam_capture.h b/electron/native/wgc-capture/src/dshow_webcam_capture.h index 3debcbe0..cf510e2d 100644 --- a/electron/native/wgc-capture/src/dshow_webcam_capture.h +++ b/electron/native/wgc-capture/src/dshow_webcam_capture.h @@ -50,6 +50,24 @@ class DirectShowWebcamCapture { struct Impl; void captureLoop(); + /** + * Builds source -> sample grabber -> null renderer and connects it. + * + * `preferredSubtype` is what the grabber will accept: pass a concrete one + * (RGB32) to make DirectShow's intelligent connect insert a colour converter, + * or nullptr to accept whatever the camera offers natively. Returns false + * without leaving the graph half-built, so the caller can retry with a + * different constraint. + */ + bool buildGraph(const CLSID& sourceClsid, const GUID* preferredSubtype); + /** + * Reads back what the graph actually negotiated and records how to unpack it. + * + * Returns false for a subtype this class cannot decode, which is a retryable + * outcome rather than a failure — `reportUnsupported` is what tells it apart + * from the last attempt, whose rejection is worth logging. + */ + bool resolveConnectedFormat(int requestedWidth, int requestedHeight, bool reportUnsupported); Impl* impl_ = nullptr; std::thread thread_; diff --git a/electron/recording/nativeWindowsCaptureStop.test.ts b/electron/recording/nativeWindowsCaptureStop.test.ts index 900df9e1..acbee862 100644 --- a/electron/recording/nativeWindowsCaptureStop.test.ts +++ b/electron/recording/nativeWindowsCaptureStop.test.ts @@ -6,6 +6,8 @@ import { isSalvageableFragmentedCapture, NATIVE_WINDOWS_SALVAGEABLE_OUTPUT_BYTES, readStoppedPath, + readWebcamFormat, + readWebcamUnavailable, terminateNativeWindowsCapture, waitForNativeWindowsCaptureStop, } from "./nativeWindowsCaptureStop"; @@ -79,6 +81,76 @@ describe("readStoppedPath", () => { }); }); +describe("readWebcamUnavailable", () => { + it("sees the helper giving up on the camera", () => { + const output = + '{"event":"ready","schemaVersion":2}\n' + + "WARNING: Failed to initialize native webcam capture; continuing without webcam\n" + + '{"event":"warning","code":"webcam-unavailable","message":"Failed to initialize native webcam capture"}\n' + + "Recording started\n"; + expect(readWebcamUnavailable(output)).toBe(true); + }); + + it("is false for a run whose camera worked", () => { + const output = + '{"event":"webcam-format","schemaVersion":2,"width":1920,"height":1080,"fps":30,"deviceName":"Camera (NVIDIA Broadcast)"}\n' + + "Recording started\n"; + expect(readWebcamUnavailable(output)).toBe(false); + }); +}); + +describe("readWebcamFormat", () => { + it("reads the negotiated camera format", () => { + const output = + '{"event":"webcam-format","schemaVersion":2,"width":1920,"height":1080,"fps":30,"deviceName":"Camera (NVIDIA Broadcast)"}\n'; + expect(readWebcamFormat(output)).toMatchObject({ + width: 1920, + height: 1080, + deviceName: "Camera (NVIDIA Broadcast)", + }); + }); + + // Captured verbatim from a real run: the helper's stderr diagnostic and its + // stdout event land in the same drained chunk, with no newline between them. + // Parsing that line whole throws, which used to read as "no camera at all". + it("reads the event even when stderr is glued onto the front of it", () => { + const output = + "INFO: DirectShow webcam connected subtype NV12 720x1280 " + + 'stride=2880{"event":"webcam-format","schemaVersion":2,"width":720,"height":1280,"fps":30,"deviceName":"OBS Virtual Camera"}\n'; + expect(readWebcamFormat(output)).toMatchObject({ + width: 720, + height: 1280, + deviceName: "OBS Virtual Camera", + }); + }); + + // A friendly name is free text from the driver, so it can contain the very + // character that used to end the slice. Cutting the object there dropped a + // camera that was working perfectly. + it("reads a device name that contains a closing brace", () => { + const output = + '{"event":"webcam-format","schemaVersion":2,"width":1280,"height":720,"fps":30,"deviceName":"Camera } Studio"}\n'; + expect(readWebcamFormat(output)).toMatchObject({ + width: 1280, + deviceName: "Camera } Studio", + }); + }); + + it("reads a device name whose escaped quote precedes a brace", () => { + const output = + '{"event":"webcam-format","schemaVersion":2,"width":640,"height":480,"fps":30,"deviceName":"Cam \\"X\\" } 2"}\n'; + expect(readWebcamFormat(output)).toMatchObject({ deviceName: 'Cam "X" } 2' }); + }); + + it("is null when the object is cut off mid-way", () => { + expect(readWebcamFormat('{"event":"webcam-format","width":1280')).toBe(null); + }); + + it("is null when the helper never announced a camera", () => { + expect(readWebcamFormat("Recording started\n")).toBe(null); + }); +}); + describe("isSalvageableFragmentedCapture", () => { const big = NATIVE_WINDOWS_SALVAGEABLE_OUTPUT_BYTES * 8; diff --git a/electron/recording/nativeWindowsCaptureStop.ts b/electron/recording/nativeWindowsCaptureStop.ts index badec52d..24f047f3 100644 --- a/electron/recording/nativeWindowsCaptureStop.ts +++ b/electron/recording/nativeWindowsCaptureStop.ts @@ -94,6 +94,92 @@ export function readAbandonedStep(output: string) { return output.match(STOP_TIMEOUT_EVENT_PATTERN)?.[1] ?? null; } +/** + * Did the helper give up on the camera and record the screen alone? + * + * It says so and then carries on, which is the right call — a screen-and-audio + * take the user can still edit beats losing the whole recording over one + * device. What was missing was anyone listening: nothing read this event, so a + * recording started WITH a camera came back without one, and without a word + * (getopenscreen/openscreen#387). + * + * A substring test and not a per-line parse, for the reason below. + */ +export function readWebcamUnavailable(output: string) { + return output.includes('"code":"webcam-unavailable"'); +} + +/** + * Index of the `}` that closes the object starting at `start`, or -1. + * + * Stopping at the first `}` is wrong for a value that contains one, and a + * camera's friendly name is free text straight from the driver — "Camera } + * Studio" is unusual but nothing forbids it, and cutting the object there turns + * a working camera into one that reported no format at all. So brace depth is + * counted, and braces inside a JSON string are skipped along with anything an + * escape protects. + */ +function findObjectEnd(output: string, start: number) { + let depth = 0; + let inString = false; + let escaped = false; + for (let index = start; index < output.length; index += 1) { + const ch = output[index]; + if (escaped) { + escaped = false; + continue; + } + if (ch === "\\" && inString) { + escaped = true; + continue; + } + if (ch === '"') { + inString = !inString; + continue; + } + if (inString) continue; + if (ch === "{") depth += 1; + else if (ch === "}") { + depth -= 1; + if (depth === 0) return index; + } + } + return -1; +} + +/** + * The format the helper negotiated with the camera, or null if it never said. + * + * Slices the object out of the buffer rather than parsing a line whole. Both + * helper streams are drained into this one string, diagnostics go to stderr and + * protocol to stdout, and a chunk boundary routinely glues them together — real + * output contains lines like + * `INFO: DirectShow webcam connected subtype NV12 {"event":"webcam-format",…}`. + * `JSON.parse` on that throws, and a working camera reads as one that said + * nothing at all. + */ +export function readWebcamFormat(output: string) { + const start = output.lastIndexOf('{"event":"webcam-format"'); + if (start === -1) { + return null; + } + const end = findObjectEnd(output, start); + if (end === -1) { + return null; + } + + try { + return JSON.parse(output.slice(start, end + 1)) as { + width?: number; + height?: number; + fps?: number; + deviceName?: string; + }; + } catch { + return null; + } +} + /** * The most useful line of a failed helper run, for a toast. * diff --git a/src/components/launch/LaunchWindow.tsx b/src/components/launch/LaunchWindow.tsx index 7639936f..14046bc3 100644 --- a/src/components/launch/LaunchWindow.tsx +++ b/src/components/launch/LaunchWindow.tsx @@ -141,13 +141,19 @@ export function LaunchWindow() { // on its very first frame, and that `webcamDeviceId` is already the default // device when the button is clicked — so enabling the camera acquires the // right stream once instead of acquiring the default and then re-acquiring. + // + // Passing `webcamDeviceId` as the preferred device is what keeps the pick the + // user made in the editor's Rec stage: this window is destroyed and rebuilt + // for every recording, so the enumeration default would otherwise revert the + // camera to whatever the OS lists first on each take. const { devices: cameraDevices, + selectedDevice: selectedCamera, selectedDeviceId: selectedCameraId, setSelectedDeviceId: setSelectedCameraId, isLoading: isCameraDevicesLoading, error: cameraDevicesError, - } = useCameraDevices(true); + } = useCameraDevices(true, webcamDeviceId); // The microphone list stays lazy: enumerating it asks for mic permission, // which would light the OS "in use" indicator just for opening the HUD. const { @@ -163,12 +169,18 @@ export function LaunchWindow() { } }, [selectedMicId, micDevices, setMicrophoneDeviceId, setMicrophoneDeviceName]); + // Keyed on the chosen device's own fields, never on the `cameraDevices` array. + // That array is rebuilt on every `devicechange`, and mirroring the selection + // back on each rebuild put this effect in a tug-of-war with the preference + // adoption inside `useCameraDevices`: the two wrote each other's value on + // every commit and the HUD spun without ever settling. + const selectedCameraLabel = selectedCamera?.label; useEffect(() => { if (selectedCameraId) { setWebcamDeviceId(selectedCameraId); - setWebcamDeviceName(cameraDevices.find((d) => d.deviceId === selectedCameraId)?.label); + setWebcamDeviceName(selectedCameraLabel); } - }, [selectedCameraId, cameraDevices, setWebcamDeviceId, setWebcamDeviceName]); + }, [selectedCameraId, selectedCameraLabel, setWebcamDeviceId, setWebcamDeviceName]); useEffect(() => { let cancelled = false; @@ -628,10 +640,31 @@ export function LaunchWindow() { setMicrophoneEnabled(!microphoneEnabled); }, [controlsLocked, microphoneEnabled, setMicrophoneEnabled]); + /** + * Write a camera choice back to the main-process recording prefs. + * + * The HUD used to be a reader of that SSOT and never a writer, while being + * destroyed and rebuilt for every recording — so a camera picked here lived + * exactly as long as one take, and the editor's Rec stage kept showing the + * previous device. Best-effort on purpose: failing to persist a preference + * must not stop a recording. + */ + const persistCameraPrefs = useCallback( + (patch: { camEnabled?: boolean; camDeviceId?: string }) => { + void window.electronAPI?.setRecordingPrefs?.(patch).catch((error) => { + console.warn("Failed to persist the camera preference:", error); + }); + }, + [], + ); + const toggleWebcam = useCallback(() => { if (controlsLocked) return; - void setWebcamEnabled(!webcamEnabled); - }, [controlsLocked, setWebcamEnabled, webcamEnabled]); + const next = !webcamEnabled; + void setWebcamEnabled(next).then((ok) => { + if (ok) persistCameraPrefs({ camEnabled: next }); + }); + }, [controlsLocked, persistCameraPrefs, setWebcamEnabled, webcamEnabled]); // Selecting a device never switches it on. If the device is already live the // recorder re-acquires on the id change; if it isn't, this just records which @@ -650,8 +683,9 @@ export function LaunchWindow() { setSelectedCameraId(device.deviceId); setWebcamDeviceId(device.deviceId); setWebcamDeviceName(device.label); + persistCameraPrefs({ camDeviceId: device.deviceId }); }, - [setSelectedCameraId, setWebcamDeviceId, setWebcamDeviceName], + [persistCameraPrefs, setSelectedCameraId, setWebcamDeviceId, setWebcamDeviceName], ); const toggleDeviceSettings = useCallback(() => { diff --git a/src/hooks/useCameraDevices.loop.test.tsx b/src/hooks/useCameraDevices.loop.test.tsx new file mode 100644 index 00000000..97eb70df --- /dev/null +++ b/src/hooks/useCameraDevices.loop.test.tsx @@ -0,0 +1,128 @@ +// @vitest-environment jsdom +import { act, render, waitFor } from "@testing-library/react"; +import { useEffect, useState } from "react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { useCameraDevices } from "./useCameraDevices"; + +/** + * The HUD wires this hook into a cycle: it mirrors `selectedDevice` into the + * recorder's `webcamDeviceId`, and hands that same value straight back as + * `preferredDeviceId`. Testing the hook alone cannot see that — it takes both + * halves in one tree — and the first version of the preference adoption spun at + * ~1500 renders per 500 ms once a remembered-but-absent camera reappeared, with + * the two effects writing each other's value on every commit. `webcamDeviceId` + * is also a dependency of the recorder's getUserMedia effect, so the spin would + * have hammered the camera hardware from the always-on recording toolbar. + */ + +const devicechangeListeners: Array<() => void> = []; +let currentDevices: Array<{ kind: string; deviceId: string; label: string; groupId: string }> = []; + +Object.defineProperty(global.navigator, "mediaDevices", { + value: { + enumerateDevices: vi.fn(async () => currentDevices), + getUserMedia: vi.fn(), + addEventListener: (_: string, cb: () => void) => devicechangeListeners.push(cb), + removeEventListener: vi.fn(), + }, + configurable: true, +}); + +const TWO_CAMERAS = [ + { kind: "videoinput", deviceId: "cam1", label: "VCam Camera", groupId: "g1" }, + { kind: "videoinput", deviceId: "cam2", label: "Logitech StreamCam", groupId: "g2" }, +]; + +let renders = 0; +const observed = { selected: "", webcam: undefined as string | undefined }; + +/** Mirrors LaunchWindow's wiring: the hook's output feeds its own input. */ +function Hud({ seedPrefs }: { seedPrefs: (set: (id: string) => void) => void }) { + const [webcamDeviceId, setWebcamDeviceId] = useState(undefined); + const [, setWebcamDeviceName] = useState(undefined); + + // useScreenRecorder seeds this from the main-process prefs, over IPC. + useEffect(() => { + seedPrefs(setWebcamDeviceId); + }, [seedPrefs]); + + const { selectedDevice, selectedDeviceId } = useCameraDevices(true, webcamDeviceId); + + const selectedLabel = selectedDevice?.label; + useEffect(() => { + if (selectedDeviceId) { + setWebcamDeviceId(selectedDeviceId); + setWebcamDeviceName(selectedLabel); + } + }, [selectedDeviceId, selectedLabel]); + + renders += 1; + observed.selected = selectedDeviceId; + observed.webcam = webcamDeviceId; + return null; +} + +describe("useCameraDevices inside the HUD's write-back cycle", () => { + beforeEach(() => { + devicechangeListeners.length = 0; + renders = 0; + currentDevices = [...TWO_CAMERAS]; + }); + + it("settles when a remembered camera that was absent finally appears", async () => { + let seedNow: ((id: string) => void) | null = null; + render( + { + seedNow = set; + }} + />, + ); + + // Enumeration wins the race and falls back to the first device. + await waitFor(() => { + expect(observed.selected).toBe("cam1"); + }); + + // The prefs land afterwards, naming a camera that is not plugged in yet — + // a virtual camera whose app has not been started, say. + await act(async () => { + seedNow?.("camX"); + }); + expect(observed.selected).toBe("cam1"); + expect(observed.webcam).toBe("camX"); + + // The user starts NVIDIA Broadcast, so camX shows up. + currentDevices = [ + ...TWO_CAMERAS, + { kind: "videoinput", deviceId: "camX", label: "Camera (NVIDIA Broadcast)", groupId: "g3" }, + ]; + const before = renders; + await act(async () => { + for (const notify of devicechangeListeners) notify(); + await new Promise((resolve) => setTimeout(resolve, 300)); + }); + + // Both halves agree on the remembered camera, and the tree is quiet. + expect(observed.selected).toBe("camX"); + expect(observed.webcam).toBe("camX"); + expect(renders - before).toBeLessThan(20); + }); + + it("stays quiet when a devicechange brings no actual change", async () => { + render( undefined} />); + + await waitFor(() => { + expect(observed.selected).toBe("cam1"); + }); + + const before = renders; + await act(async () => { + for (const notify of devicechangeListeners) notify(); + await new Promise((resolve) => setTimeout(resolve, 200)); + }); + + expect(observed.selected).toBe("cam1"); + expect(renders - before).toBeLessThan(10); + }); +}); diff --git a/src/hooks/useCameraDevices.test.ts b/src/hooks/useCameraDevices.test.ts index 2bef6f04..a622ba78 100644 --- a/src/hooks/useCameraDevices.test.ts +++ b/src/hooks/useCameraDevices.test.ts @@ -85,6 +85,84 @@ describe("useCameraDevices", () => { expect(result.current.isLoading).toBe(false); }); + it("should prefer the restored device over the first enumerated one", async () => { + const { result } = renderHook(() => useCameraDevices(true, "cam2")); + + await waitFor(() => { + expect(result.current.selectedDeviceId).toBe("cam2"); + }); + }); + + // The prefs arrive over IPC and routinely land after enumeration has already + // settled on the first device. Losing the user's camera at that point is what + // made a HUD rebuilt for a new recording revert to a virtual camera that emits + // nothing, while the native helper was told to capture it by name. + it("should adopt the restored device when it arrives after enumeration", async () => { + const { result, rerender } = renderHook( + ({ preferred }: { preferred?: string }) => useCameraDevices(true, preferred), + { initialProps: { preferred: undefined as string | undefined } }, + ); + + await waitFor(() => { + expect(result.current.selectedDeviceId).toBe("cam1"); + }); + + rerender({ preferred: "cam2" }); + + await waitFor(() => { + expect(result.current.selectedDeviceId).toBe("cam2"); + }); + }); + + it("should ignore a restored device that is no longer plugged in", async () => { + const { result } = renderHook(() => useCameraDevices(true, "cam-that-left")); + + await waitFor(() => { + expect(result.current.selectedDeviceId).toBe("cam1"); + }); + }); + + /** + * The HUD feeds this hook's own output back in: `LaunchWindow` writes + * `selectedDeviceId` into the recorder's `webcamDeviceId`, and hands that same + * value back as `preferredDeviceId`. A rule that re-asserts the preference on + * every render would ping-pong with that write-back forever, so the loop this + * closes has to settle. Counts renders rather than asserting a value: a + * converging hook renders a handful of times, one that oscillates renders + * without end. + */ + it("settles instead of oscillating when its own selection is fed back as the preference", async () => { + let renders = 0; + const { result, rerender } = renderHook( + ({ preferred }: { preferred?: string }) => { + renders += 1; + return useCameraDevices(true, preferred); + }, + { initialProps: { preferred: undefined as string | undefined } }, + ); + + await waitFor(() => { + expect(result.current.selectedDeviceId).toBe("cam1"); + }); + + // The HUD hands back whatever this hook just selected, then the late prefs + // name a different camera, which the hook adopts and hands back in turn. + rerender({ preferred: result.current.selectedDeviceId }); + rerender({ preferred: "cam2" }); + + await waitFor(() => { + expect(result.current.selectedDeviceId).toBe("cam2"); + }); + + const settled = renders; + rerender({ preferred: result.current.selectedDeviceId }); + await new Promise((resolve) => setTimeout(resolve, 50)); + + // Only the render this test asked for; the effect no longer has anything to say. + expect(renders - settled).toBeLessThanOrEqual(2); + expect(result.current.selectedDeviceId).toBe("cam2"); + }); + it("should fall back to first available device when selected device is unplugged", async () => { const { result } = renderHook(() => useCameraDevices(true)); diff --git a/src/hooks/useCameraDevices.ts b/src/hooks/useCameraDevices.ts index bf402c7f..0466cc40 100644 --- a/src/hooks/useCameraDevices.ts +++ b/src/hooks/useCameraDevices.ts @@ -6,13 +6,35 @@ export interface CameraDevice { groupId: string; } -export function useCameraDevices(enabled: boolean = false) { +/** + * @param preferredDeviceId The camera the session already settled on — typically + * the one restored from the recording prefs. It wins over "first in the list", + * because the list order is the OS enumeration order and the first entry is + * routinely a virtual camera that emits nothing. Without this, a HUD rebuilt for + * a new recording silently reverted the user's pick to that first device, and + * only the *name* reached the native helper, so the preview showed the chosen + * camera while the recording captured the other one. + */ +export function useCameraDevices(enabled: boolean = false, preferredDeviceId?: string) { const [devices, setDevices] = useState([]); const [selectedDeviceId, setSelectedDeviceId] = useState(""); const [isLoading, setIsLoading] = useState(false); const [error, setError] = useState(null); + // `loadDevices` runs long after the render that scheduled it — on a + // `devicechange` that may arrive at any moment — so it reads these two + // through refs rather than closing over them. + // + // Synchronised in an effect and not during render: React may discard a render + // without committing it, and a ref written there keeps the value anyway. The + // selection would then be resolved against a device the committed tree never + // agreed on. Declared above the loading effect so the refs are current before + // the first enumeration reads them. const selectedDeviceIdRef = useRef(selectedDeviceId); - selectedDeviceIdRef.current = selectedDeviceId; + const preferredDeviceIdRef = useRef(preferredDeviceId); + useEffect(() => { + selectedDeviceIdRef.current = selectedDeviceId; + preferredDeviceIdRef.current = preferredDeviceId; + }, [selectedDeviceId, preferredDeviceId]); useEffect(() => { if (!enabled) return; @@ -39,7 +61,11 @@ export function useCameraDevices(enabled: boolean = false) { const currentId = selectedDeviceIdRef.current; const stillAvailable = videoInputs.some((d) => d.deviceId === currentId); if (!currentId || !stillAvailable) { - setSelectedDeviceId(videoInputs[0]?.deviceId ?? ""); + const preferredId = preferredDeviceIdRef.current; + const preferred = preferredId + ? videoInputs.find((d) => d.deviceId === preferredId) + : undefined; + setSelectedDeviceId(preferred?.deviceId ?? videoInputs[0]?.deviceId ?? ""); } setIsLoading(false); } @@ -60,5 +86,25 @@ export function useCameraDevices(enabled: boolean = false) { }; }, [enabled]); - return { devices, selectedDeviceId, setSelectedDeviceId, isLoading, error }; + // The preference is restored over IPC while this list is being enumerated, so + // it routinely lands *after* the effect above has already fallen back to the + // first device. Adopting it here is what makes the two async sources converge + // on the same camera instead of racing. + useEffect(() => { + if (!enabled || !preferredDeviceId) return; + if (preferredDeviceId === selectedDeviceId) return; + if (!devices.some((d) => d.deviceId === preferredDeviceId)) return; + setSelectedDeviceId(preferredDeviceId); + }, [enabled, preferredDeviceId, devices, selectedDeviceId]); + + // The selected entry itself, so callers can react to "which camera is chosen" + // without depending on the identity of `devices`. `loadDevices` rebuilds that + // array on every `devicechange`, and a caller that mirrors the selection back + // into its own state — the HUD does — would re-fire on each rebuild and fight + // the effect above for the value, swapping the two on every commit and never + // settling. Depending on this object's fields instead makes the write-back + // fire only when the chosen camera really changed. + const selectedDevice = devices.find((d) => d.deviceId === selectedDeviceId); + + return { devices, selectedDevice, selectedDeviceId, setSelectedDeviceId, isLoading, error }; } diff --git a/src/hooks/useScreenRecorder.ts b/src/hooks/useScreenRecorder.ts index a6da24d7..5668b0f9 100644 --- a/src/hooks/useScreenRecorder.ts +++ b/src/hooks/useScreenRecorder.ts @@ -20,6 +20,7 @@ import type { CursorCaptureMode, RecordedVideoAssetInput } from "@/lib/recording import { requestCameraAccess } from "@/lib/requestCameraAccess"; import { loadUserPreferences, saveUserPreferences } from "@/lib/userPreferences"; import { createRecorderHandle, type RecorderHandle } from "./recorderHandle"; +import { webcamDeviceIdentityFrom } from "./webcamDeviceIdentity"; const TARGET_FRAME_RATE = 60; const MIN_FRAME_RATE = 30; @@ -187,6 +188,23 @@ export async function finalizeWebcamAsset( export function useScreenRecorder(): UseScreenRecorderReturn { const t = useScopedT("editor"); + /** + * `t` through a ref, for the callbacks that must not be rebuilt when it + * changes identity. + * + * `finalizeNativeWindowsRecording` is one of them: it sits in the dependency + * array of the unmount effect below, whose cleanup bumps `countdownRunId` and + * discards any native recording in flight. Recreating that callback therefore + * re-runs the effect, and its cleanup silently cancels the countdown a + * recording is starting from — the take never begins, with nothing logged. + */ + const tRef = useRef(t); + // In an effect, not during render: a render React discards still leaves a ref + // written there, and a later recording error would then be worded by a UI that + // never reached the screen. + useEffect(() => { + tRef.current = t; + }, [t]); const [recording, setRecording] = useState(false); const [paused, setPaused] = useState(false); const [saving, setSaving] = useState(false); @@ -318,6 +336,18 @@ export function useScreenRecorder(): UseScreenRecorderReturn { } }, []); + /** + * The camera to name in a native capture request. See + * `webcamDeviceIdentityFrom` for why it is read off the live track rather than + * off this hook's two separate pieces of state. Must be called before + * `stopWebcamPreviewStream()`, which is why the Windows path captures it up + * front instead of at the point of use. + */ + const readWebcamDeviceIdentity = useCallback( + () => webcamDeviceIdentityFrom(webcamStream.current, webcamDeviceId, webcamDeviceName), + [webcamDeviceId, webcamDeviceName], + ); + const stopWebcamPreviewStream = useCallback(() => { if (!webcamStream.current) { return; @@ -600,6 +630,15 @@ export function useScreenRecorder(): UseScreenRecorderReturn { } clearNativeRecordingState(); + // The other way a camera goes missing, and the quieter one: the device + // opened, so nothing warned at start, but it never produced a frame and + // the file it left behind was empty. Say so before the editor opens + // without a camera and leaves the user to work out why. Through `tRef` + // because this callback has to stay referentially stable — see the ref's + // own comment. + if (result.webcamDropped) { + toast.error(tRef.current("recording.cameraCaptureUnavailable")); + } if (result.session) { await window.electronAPI.setCurrentRecordingSession(result.session); } else if (result.path) { @@ -1058,11 +1097,16 @@ export function useScreenRecorder(): UseScreenRecorderReturn { const displayId = Number(selectedSource.display_id); const sourceType = selectedSource.id.startsWith("window:") ? "window" : "display"; const windowHandle = parseWindowHandleFromSourceId(selectedSource.id); + let webcamIdentity = { deviceId: webcamDeviceId, deviceName: webcamDeviceName }; if (webcamEnabled) { await waitForWebcamReady(); if (!isCountdownRunActive(countdownRunToken)) { return true; } + // Read the device off the live track before letting go of it: this is + // the only moment where the id and the name are known to describe the + // same camera (see readWebcamDeviceIdentity). + webcamIdentity = readWebcamDeviceIdentity(); // Release the renderer-side validation stream before asking the native // helper to open the same device: most webcams only allow one exclusive // capture session, and native (Media Foundation/DirectShow) now owns @@ -1098,8 +1142,8 @@ export function useScreenRecorder(): UseScreenRecorderReturn { }, webcam: { enabled: webcamEnabled, - deviceId: webcamDeviceId, - deviceName: webcamDeviceName, + deviceId: webcamIdentity.deviceId, + deviceName: webcamIdentity.deviceName, width: 0, height: 0, fps: WEBCAM_TARGET_FRAME_RATE, @@ -1113,6 +1157,14 @@ export function useScreenRecorder(): UseScreenRecorderReturn { throw new Error(result.error ?? "Native Windows capture failed."); } + // The take goes on without the camera rather than failing, so this is the + // only moment the user can learn about it while it is still cheap to stop + // and retry. Left unsaid, the camera's absence was discovered in the + // editor, long after the moment was gone. + if (result.webcamUnavailable) { + toast.error(t("recording.cameraCaptureUnavailable")); + } + // Tell the user when the helper silently switched away from the default // GPU encoder; an explicit software-preferred selection needs no notice. setSoftwareEncoderFallbackNoticeVisible( @@ -1248,8 +1300,9 @@ export function useScreenRecorder(): UseScreenRecorderReturn { }, webcam: { enabled: webcamEnabled, - deviceId: webcamDeviceId, - deviceName: webcamDeviceName, + // Same pairing rule as the Windows path; here the stream is still + // open, so the identity can be read at the point of use. + ...readWebcamDeviceIdentity(), width: 0, height: 0, fps: WEBCAM_TARGET_FRAME_RATE, diff --git a/src/hooks/webcamDeviceIdentity.test.ts b/src/hooks/webcamDeviceIdentity.test.ts new file mode 100644 index 00000000..db72bb64 --- /dev/null +++ b/src/hooks/webcamDeviceIdentity.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, it } from "vitest"; +import { webcamDeviceIdentityFrom } from "./webcamDeviceIdentity"; + +function streamWith(track: Partial | null): MediaStream { + return { getVideoTracks: () => (track ? [track] : []) } as unknown as MediaStream; +} + +describe("webcamDeviceIdentityFrom", () => { + it("takes both halves from the track the browser opened", () => { + const stream = streamWith({ + label: "Camera (NVIDIA Broadcast)", + getSettings: () => ({ deviceId: "nvidia-id" }), + } as Partial); + + expect(webcamDeviceIdentityFrom(stream, "stale-id", "VCam Camera")).toEqual({ + deviceId: "nvidia-id", + deviceName: "Camera (NVIDIA Broadcast)", + }); + }); + + /** + * The bug this exists to prevent. The HUD's own enumeration defaulted the NAME + * to the first device — a virtual camera that emits nothing — while the prefs + * restored the ID of the camera the user actually picked. Chromium honoured + * the id and previewed the right camera; the native helper matched on the name + * and recorded the wrong one, producing a zero-byte webcam file. + */ + it("never pairs one camera's id with another camera's name", () => { + const stream = streamWith({ + label: "Logitech StreamCam (046d:0893)", + getSettings: () => ({ deviceId: "logitech-id" }), + } as Partial); + + const identity = webcamDeviceIdentityFrom(stream, "vcam-id", "VCam Camera"); + + expect(identity.deviceId).toBe("logitech-id"); + expect(identity.deviceName).toBe("Logitech StreamCam (046d:0893)"); + }); + + it("falls back to the known selection when there is no stream", () => { + expect(webcamDeviceIdentityFrom(null, "picked-id", "Picked Camera")).toEqual({ + deviceId: "picked-id", + deviceName: "Picked Camera", + }); + }); + + it("falls back to the known selection when there is no video track", () => { + expect(webcamDeviceIdentityFrom(streamWith(null), "picked-id", "Picked Camera")).toEqual({ + deviceId: "picked-id", + deviceName: "Picked Camera", + }); + }); + + // Chromium withholds labels until camera permission has been granted, and + // reports an empty deviceId under some constraint combinations. Neither is a + // reason to send an empty name to a helper that matches devices by name. + it("keeps the known values when the track answers with empty strings", () => { + const stream = streamWith({ + label: "", + getSettings: () => ({ deviceId: "" }), + } as Partial); + + expect(webcamDeviceIdentityFrom(stream, "picked-id", "Picked Camera")).toEqual({ + deviceId: "picked-id", + deviceName: "Picked Camera", + }); + }); +}); diff --git a/src/hooks/webcamDeviceIdentity.ts b/src/hooks/webcamDeviceIdentity.ts new file mode 100644 index 00000000..070a67d4 --- /dev/null +++ b/src/hooks/webcamDeviceIdentity.ts @@ -0,0 +1,40 @@ +export interface WebcamDeviceIdentity { + deviceId: string | undefined; + deviceName: string | undefined; +} + +/** + * The camera to name in a native capture request, read off the track the browser + * actually opened rather than off two separate pieces of React state. + * + * Those two used to be fed by two independent async sources — the id restored + * from the recording prefs over IPC, the name from the HUD window's own + * `enumerateDevices()` — and whichever settled last won on its own. A request + * could therefore carry one camera's id next to another camera's name, and since + * Chromium selects by id while the native Windows helper matches by NAME, the + * preview showed the chosen camera while the recording captured a different one + * (getopenscreen/openscreen#387). + * + * `track.label` and `track.getSettings().deviceId` describe the same device by + * construction, which is what makes the pair impossible to mismatch. The + * fallbacks cover the cases where the track cannot answer: no stream yet, or a + * label withheld until camera permission has been granted. + * + * Lives in its own module rather than inside `useScreenRecorder` so it can be + * tested without dragging in the i18n context that hook depends on. + */ +export function webcamDeviceIdentityFrom( + stream: MediaStream | null | undefined, + fallbackDeviceId: string | undefined, + fallbackDeviceName: string | undefined, +): WebcamDeviceIdentity { + const track = stream?.getVideoTracks()[0]; + if (!track) { + return { deviceId: fallbackDeviceId, deviceName: fallbackDeviceName }; + } + + return { + deviceId: track.getSettings?.().deviceId || fallbackDeviceId, + deviceName: track.label || fallbackDeviceName, + }; +} diff --git a/src/i18n/locales/ar/editor.json b/src/i18n/locales/ar/editor.json index d2a469a5..02223d13 100644 --- a/src/i18n/locales/ar/editor.json +++ b/src/i18n/locales/ar/editor.json @@ -42,6 +42,7 @@ "cameraDenied": "تم رفض الوصول إلى الكاميرا. سيستمر التسجيل بدون كاميرا الويب.", "cameraDisconnected": "تم فصل كاميرا الويب.", "cameraNotFound": "لم يتم العثور على كاميرا.", + "cameraCaptureUnavailable": "تعذّر فتح الكاميرا. يجري التسجيل بدون كاميرا.", "permissionDenied": "تم رفض إذن التسجيل. يرجى السماح بتسجيل الشاشة.", "accessibilityAllowAndRetry": "اسمح بوصول تسهيلات الاستخدام لـ OpenScreen، ثم اضغط على التسجيل مرة أخرى لبدء العد التنازلي.", "selectSource": "يرجى تحديد مصدر للتسجيل" diff --git a/src/i18n/locales/en/editor.json b/src/i18n/locales/en/editor.json index 390f687b..739c637b 100644 --- a/src/i18n/locales/en/editor.json +++ b/src/i18n/locales/en/editor.json @@ -42,6 +42,7 @@ "cameraDenied": "Camera access denied. Recording will continue without webcam.", "cameraDisconnected": "Webcam disconnected.", "cameraNotFound": "Camera not found.", + "cameraCaptureUnavailable": "The camera could not be opened. Recording without it.", "permissionDenied": "Recording permission denied. Please allow screen recording.", "accessibilityAllowAndRetry": "Allow Accessibility access for OpenScreen, then press record again to start the countdown.", "selectSource": "Please select a source to record" diff --git a/src/i18n/locales/es/editor.json b/src/i18n/locales/es/editor.json index a3bb887f..4d882215 100644 --- a/src/i18n/locales/es/editor.json +++ b/src/i18n/locales/es/editor.json @@ -34,6 +34,7 @@ "cameraDenied": "Acceso a la cámara denegado. La grabación continuará sin cámara web.", "cameraDisconnected": "Cámara web desconectada.", "cameraNotFound": "Cámara no encontrada.", + "cameraCaptureUnavailable": "No se pudo abrir la cámara. Grabando sin ella.", "permissionDenied": "Permiso de grabación denegado. Por favor permite la grabación de pantalla.", "accessibilityAllowAndRetry": "Permite el acceso de accesibilidad para OpenScreen y luego pulsa grabar de nuevo para iniciar la cuenta atrás.", "selectSource": "Por favor selecciona una fuente para grabar" diff --git a/src/i18n/locales/fr/editor.json b/src/i18n/locales/fr/editor.json index 245ee857..d560fa07 100644 --- a/src/i18n/locales/fr/editor.json +++ b/src/i18n/locales/fr/editor.json @@ -40,6 +40,7 @@ "cameraDenied": "Accès à la caméra refusé. L'enregistrement continuera sans webcam.", "cameraDisconnected": "Webcam déconnectée.", "cameraNotFound": "Caméra introuvable.", + "cameraCaptureUnavailable": "Impossible d'ouvrir la caméra. Enregistrement sans elle.", "permissionDenied": "Permission d'enregistrement refusée. Veuillez autoriser l'enregistrement d'écran.", "accessibilityAllowAndRetry": "Autorisez l'accès Accessibilité pour OpenScreen, puis appuyez de nouveau sur enregistrer pour lancer le compte à rebours.", "selectSource": "Veuillez sélectionner une source à enregistrer" diff --git a/src/i18n/locales/it/editor.json b/src/i18n/locales/it/editor.json index ab2e9f46..ba0afe4d 100644 --- a/src/i18n/locales/it/editor.json +++ b/src/i18n/locales/it/editor.json @@ -42,6 +42,7 @@ "cameraDenied": "Accesso alla fotocamera negato. La registrazione continuerà senza webcam.", "cameraDisconnected": "Webcam disconnessa.", "cameraNotFound": "Fotocamera non trovata.", + "cameraCaptureUnavailable": "Impossibile aprire la fotocamera. Registrazione senza di essa.", "permissionDenied": "Autorizzazione di registrazione negata. Consenti la registrazione dello schermo.", "accessibilityAllowAndRetry": "Consenti l'accesso all'accessibilità per OpenScreen, poi premi di nuovo registra per avviare il conto alla rovescia.", "selectSource": "Seleziona una sorgente da registrare" diff --git a/src/i18n/locales/ja-JP/editor.json b/src/i18n/locales/ja-JP/editor.json index d4cb0efe..946d62eb 100644 --- a/src/i18n/locales/ja-JP/editor.json +++ b/src/i18n/locales/ja-JP/editor.json @@ -43,6 +43,7 @@ "permissionDenied": "録画の権限が拒否されました。画面録画を許可してください。", "cameraDisconnected": "ウェブカメラが切断されました。", "cameraNotFound": "カメラが見つかりません。", + "cameraCaptureUnavailable": "カメラを開けませんでした。カメラなしで録画します。", "accessibilityAllowAndRetry": "OpenScreenにアクセシビリティアクセスを許可してから、もう一度録画を押してカウントダウンを開始してください。", "selectSource": "録画するソースを選択してください" }, diff --git a/src/i18n/locales/ko-KR/editor.json b/src/i18n/locales/ko-KR/editor.json index d06b5a09..def66d9d 100644 --- a/src/i18n/locales/ko-KR/editor.json +++ b/src/i18n/locales/ko-KR/editor.json @@ -43,6 +43,7 @@ "permissionDenied": "녹화 권한이 거부되었습니다. 화면 녹화를 허용해 주세요.", "cameraDisconnected": "웹캠 연결이 끊어졌습니다.", "cameraNotFound": "카메라를 찾을 수 없습니다.", + "cameraCaptureUnavailable": "카메라를 열 수 없습니다. 카메라 없이 녹화합니다.", "accessibilityAllowAndRetry": "OpenScreen의 손쉬운 사용 접근을 허용한 다음, 카운트다운을 시작하려면 다시 녹화를 누르세요.", "selectSource": "녹화할 소스를 선택해 주세요" }, diff --git a/src/i18n/locales/pt-BR/editor.json b/src/i18n/locales/pt-BR/editor.json index 4c0b3ddc..a8870b0f 100644 --- a/src/i18n/locales/pt-BR/editor.json +++ b/src/i18n/locales/pt-BR/editor.json @@ -42,6 +42,7 @@ "cameraDenied": "Acesso à câmera negado. A gravação continuará sem webcam.", "cameraDisconnected": "Webcam desconectada.", "cameraNotFound": "Câmera não encontrada.", + "cameraCaptureUnavailable": "Não foi possível abrir a câmera. Gravando sem ela.", "permissionDenied": "Permissão de gravação negada. Por favor, permita a gravação de tela.", "accessibilityAllowAndRetry": "Permita o acesso de Acessibilidade para o OpenScreen e pressione gravar novamente para iniciar a contagem regressiva.", "selectSource": "Por favor, selecione uma fonte para gravar" diff --git a/src/i18n/locales/ru/editor.json b/src/i18n/locales/ru/editor.json index 89a3e6fc..dd06f469 100644 --- a/src/i18n/locales/ru/editor.json +++ b/src/i18n/locales/ru/editor.json @@ -42,6 +42,7 @@ "cameraDenied": "Доступ к камере запрещён. Запись продолжится без веб-камеры.", "cameraDisconnected": "Веб-камера отключена.", "cameraNotFound": "Камера не найдена.", + "cameraCaptureUnavailable": "Не удалось открыть камеру. Запись идёт без неё.", "permissionDenied": "Разрешение на запись запрещено. Пожалуйста, разрешите запись экрана.", "accessibilityAllowAndRetry": "Разрешите OpenScreen доступ к Универсальному доступу, затем снова нажмите запись, чтобы начать обратный отсчет.", "selectSource": "Пожалуйста, выберите источник для записи" diff --git a/src/i18n/locales/tr/editor.json b/src/i18n/locales/tr/editor.json index 0a679795..f19f4a0f 100644 --- a/src/i18n/locales/tr/editor.json +++ b/src/i18n/locales/tr/editor.json @@ -35,6 +35,7 @@ "permissionDenied": "Kayıt izni reddedildi. Lütfen ekran kaydına izin verin.", "cameraDisconnected": "Webcam bağlantısı kesildi.", "cameraNotFound": "Kamera bulunamadı.", + "cameraCaptureUnavailable": "Kamera açılamadı. Kamerasız kaydediliyor.", "accessibilityAllowAndRetry": "OpenScreen için Erişilebilirlik erişimine izin verin, ardından geri sayımı başlatmak için tekrar kayda basın.", "selectSource": "Lütfen kayıt için bir kaynak seçin" }, diff --git a/src/i18n/locales/vi/editor.json b/src/i18n/locales/vi/editor.json index 1456ffe0..4527cb0a 100644 --- a/src/i18n/locales/vi/editor.json +++ b/src/i18n/locales/vi/editor.json @@ -42,6 +42,7 @@ "cameraDenied": "Quyền truy cập máy ảnh bị từ chối. Sẽ tiếp tục ghi hình không có webcam.", "cameraDisconnected": "Webcam bị ngắt kết nối.", "cameraNotFound": "Không tìm thấy máy ảnh.", + "cameraCaptureUnavailable": "Không thể mở máy ảnh. Đang ghi mà không có máy ảnh.", "permissionDenied": "Quyền ghi hình bị từ chối. Vui lòng cho phép ghi màn hình.", "accessibilityAllowAndRetry": "Cho phép OpenScreen truy cập Trợ năng, sau đó nhấn ghi lại để bắt đầu đếm ngược.", "selectSource": "Vui lòng chọn một nguồn để ghi" diff --git a/src/i18n/locales/zh-CN/editor.json b/src/i18n/locales/zh-CN/editor.json index 0c61fddb..75098e96 100644 --- a/src/i18n/locales/zh-CN/editor.json +++ b/src/i18n/locales/zh-CN/editor.json @@ -42,6 +42,7 @@ "cameraDenied": "摄像头权限被拒绝。录制将继续,但不包含摄像头画面。", "cameraDisconnected": "摄像头已断开连接。", "cameraNotFound": "未找到摄像头。", + "cameraCaptureUnavailable": "无法打开摄像头,正在不使用摄像头录制。", "permissionDenied": "录屏权限被拒绝。请允许屏幕录制。", "accessibilityAllowAndRetry": "允许 OpenScreen 使用辅助功能权限,然后再次按录制以开始倒计时。", "selectSource": "请选择要录制的源" diff --git a/src/i18n/locales/zh-TW/editor.json b/src/i18n/locales/zh-TW/editor.json index 29e9b4ef..42ad8711 100644 --- a/src/i18n/locales/zh-TW/editor.json +++ b/src/i18n/locales/zh-TW/editor.json @@ -43,6 +43,7 @@ "permissionDenied": "錄影權限被拒絕。請允許螢幕錄製。", "cameraDisconnected": "網路攝影機已中斷連線。", "cameraNotFound": "找不到攝影機。", + "cameraCaptureUnavailable": "無法開啟攝影機,將在沒有攝影機的情況下錄製。", "accessibilityAllowAndRetry": "允許 OpenScreen 使用輔助使用權限,然後再次按下錄製以開始倒數。", "selectSource": "請選擇要錄製的來源" }, diff --git a/src/lib/nativeWindowsRecording.ts b/src/lib/nativeWindowsRecording.ts index 313eff9b..5f6af14e 100644 --- a/src/lib/nativeWindowsRecording.ts +++ b/src/lib/nativeWindowsRecording.ts @@ -47,6 +47,12 @@ export type NativeWindowsRecordingStartResult = { error?: string; /** Helper-reported encoder selection: "default", "software-preferred", or "software-fallback". */ videoEncoderSelection?: string | null; + /** + * A camera was asked for and the helper could not open it, so this take is + * screen and audio only. Still a success — the recording is worth keeping — + * but the user has to be told, or they discover it in the editor. + */ + webcamUnavailable?: boolean; }; export function parseWindowHandleFromSourceId(sourceId?: string | null) { diff --git a/technical-documentation/architecture/recording.md b/technical-documentation/architecture/recording.md index 7d766fd3..21375d89 100644 --- a/technical-documentation/architecture/recording.md +++ b/technical-documentation/architecture/recording.md @@ -59,6 +59,8 @@ Two consequences follow, and both were once bugs: Electron resolves selected sources, devices, and paths before launching the helper. The helper does not guess a DirectShow camera: Windows receives the resolved selection. A helper error is reported explicitly rather than silently switching a Windows native feature to browser capture. +Two things follow from Windows matching a camera by **name** while Chromium selects one by **id**. First, the renderer reads both halves off the `MediaStreamTrack` it opened rather than from separate state, so a request can never carry one camera's id beside another's name — the failure that let the HUD preview show the chosen camera while the recording captured a different one. Second, a camera the helper cannot open is a warning (`webcam-unavailable`), not a failed recording: the take continues as screen and audio, and the renderer says so at that moment instead of leaving the absence to be discovered in the editor. The DirectShow fallback negotiates the camera's own format first and only asks for RGB32 — inserting a colour converter — when that format is one the helper cannot unpack, which is the only way devices absent from Media Foundation, such as NVIDIA Broadcast, can be captured at all. + ## Output files and sidecars Windows and macOS both write their screen video as a fragmented MP4 — `MFCreateFMPEG4MediaSink` with a one-second `MF_MPEG4SINK_MIN_FRAGMENT_DURATION`, and `AVAssetWriter.movieFragmentInterval` respectively. A plain MP4 has no index until the writer's final call emits `moov`, so a helper that is force-exited before that leaves every captured frame on disk and no way to read them; that is why a frozen recording used to cost the whole file rather than its tail (issues #252 / #292 / #327). Fragmenting does not stop the freeze, it stops the freeze from destroying the recording. Windows falls back to the plain container if the fragmented sink is unavailable and reports which one it used in the `encoder-selection` event. Linux still writes a plain MP4: `frag_keyframe+empty_moov` would make the output permanently non-seekable and the native Linux path has no re-index step, so the editor's scrub cost has to be measured first.