fix: third getUserMedia contention source still truncating webcam recordings - #19
Merged
Merged
Conversation
…camera Confirmed with live inspection of a real recorded session: the webcam file was still truncating to a fraction of a second (0.4s against a 30.3s screen recording) even after PR #18's stream-sharing fix between the floating preview and the actual recording. Found a third, independent getUserMedia() call PR #18 didn't cover: useVideoDevices opens its own temporary camera stream just to read device labels (Chromium hides labels until some getUserMedia grant exists this session), gated only by enabled=(webcamEnabled || openId==="webcam") and a module-level "have we done this once" flag. webcamEnabled stays true for the entire duration of a webcam-enabled recording, so if this hook's first label-fetch happens to land at/after recording start rather than before it, its independent stream request contends with useScreenRecorder's own concurrent session on the same physical camera - the exact failure mode PR #18 already diagnosed, just from a source it didn't know about. Device labels aren't needed while a recording is already in progress, so the fix is to skip the probe entirely during an active recording rather than have it compete for the camera.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Live-inspected a real recorded session after PR #18 merged: webcam file was still only 0.4s long against a 30.3s screen recording - the truncation bug PR #18 targeted was not actually resolved.
Found a third, independent
getUserMedia()call site PR #18 didn't cover:useVideoDevicesopens its own temporary camera stream purely to read device labels (Chromium hides labels until some getUserMedia grant exists this session). It's gated byenabled=(webcamEnabled || openId==="webcam"), andwebcamEnabledstaystruefor the entire duration of a webcam-enabled recording - so if this hook's one-time label fetch happens to land at/after recording start instead of before it, its independent stream contends withuseScreenRecorder's own concurrent session on the same physical camera. Same failure mode PR #18 diagnosed, different source.Fix: skip the label-probing
getUserMedia()call entirely while a recording is already in progress (LaunchWindow.tsxnow passes!recordingthrough). Device labels aren't needed mid-recording.Test plan
tsc --noEmitcleannpm test- 94/94 files, 819/819 tests passingI'm not 100% certain this is the only remaining contention source - flagging that directly rather than overclaiming, given this is the third attempt at this same bug.