fix(linux): Hyprland cursor telemetry + anchor recording start to the first real frame#158
Closed
eduardoaugustolb wants to merge 11 commits into
Closed
Conversation
Documents the investigation (ruled out app-level cursor constraint, hardware-cursor-plane theory, EGL renegotiation noise, useSystemPicker) and the actual root cause: Electron's desktopCapturer reuses one cursor-less PipeWire session across picker + recording, and xdg-desktop-portal-hyprland lacks Metadata cursor mode entirely. Fix sources cursor position from Hyprland's own IPC socket instead of the broken screen.getCursorScreenPoint(), feeding the existing provider:"none" generic-cursor overlay pipeline. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The Linux capture path used the legacy chromeMediaSource constraints, which have no cursor toggle at all (unlike the cursor constraint available via getDisplayMedia). Mirrors the Windows path: system audio still goes through the legacy desktop-audio API since Electron's loopback audio isn't supported on Linux. This alone doesn't fix cursor visibility on Hyprland (see the design spec for the real root cause and fix), but it's the correct constraint to request regardless of compositor.
…ay cursor The editor's cursor overlay (src/lib/cursor/nativeCursor.ts) always draws a generic cursor for Linux recordings, since Linux cursor sessions always produce provider:"none" position telemetry. Requesting cursor:"always" from getDisplayMedia was based on the (incorrect, now-superseded) assumption that Linux had no overlay at all — on any setup where that constraint actually bakes the cursor into the raw video (X11, or a Wayland compositor with Embedded cursor mode support, unlike Hyprland's portal), it would double up with the overlay. Requesting "never" matches how Windows already avoids this in editable-overlay mode.
hasEditableCursorRecording gated the whole cursor-overlay feature (preview and export) to win32/darwin only, regardless of whether valid recording data existed. This predates the Hyprland telemetry fix and made sense while Linux's position samples were always frozen garbage -- but it also means the newly-fixed, real position data silently never got drawn. Linux never captures real cursor assets either way (provider is always "none"), same as the existing macOS telemetry fallback, so it renders the same default arrow asset from position samples alone.
captureSample stamped timeMs with Date.now() after awaiting the socket query, so every sample's timestamp was biased later than the moment the cursor was actually at that position by one IPC round-trip. That reads as a constant lag between the drawn cursor and the recording during playback. Stamp it right before the query goes out instead.
recordingId.current (Date.now() right after acquiring the track) was used as cursor telemetry's zero-point, but on Linux/PipeWire the track can still be muted at that moment -- mid DMA-BUF modifier renegotiation (see main.ts) or renegotiating again from applyConstraints just above. Every telemetry sample ends up timestamped slightly before the frame that should show that position actually exists, which plays back as the cursor lagging the recording, confirmed even scrubbing to a single paused frame (so it wasn't a live-playback rendering artifact). Starts listening for the track's unmute event right after track acquisition (resolves immediately if already unmuted) and uses that resolved time -- instead of recordingId.current -- as the cursor session's startTimeMs. Fire-and-forget like the original call, so recorder.start() and the recording UI state are unaffected; only the cursor telemetry's anchor point changes. recordingId.current is left untouched for file naming.
MediaRecorder's internal frame-0 timeline starts whenever the track delivers its first real frame, not when recorder.start() is called. On Linux/PipeWire the desktop-capture portal can still be negotiating (DMA-BUF modifier renegotiation) for several seconds after the track reports unmuted, so anchoring duration accounting and cursor telemetry to Date.now() at button-press time let both drift out of sync with the actual video content: declared duration exceeded the real content span (playback jumped to the end early) and the cursor overlay lagged the picture. waitForFirstVideoFrame() now blocks on an actual rendered frame (via requestVideoFrameCallback, capped at 3s) before recorder.start() is ever called, so recordingId, segmentStartedAt, and the cursor-telemetry start time all anchor to the same verified wall-clock instant.
📝 WalkthroughWalkthroughAdds Hyprland IPC cursor sampling and normalized recording support, routes Hyprland Linux sessions through it, enables Linux editable cursor overlays, and updates browser recording capture and timeline initialization. ChangesHyprland cursor telemetry
Linux browser recording flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RecordingFactory
participant HyprlandCursorRecordingSession
participant HyprlandIPC
participant HyprlandSocket
RecordingFactory->>HyprlandCursorRecordingSession: create on Linux with HYPRLAND_INSTANCE_SIGNATURE
HyprlandCursorRecordingSession->>HyprlandIPC: query cursor position
HyprlandIPC->>HyprlandSocket: send j/cursorpos
HyprlandSocket-->>HyprlandIPC: return JSON coordinates
HyprlandIPC-->>HyprlandCursorRecordingSession: return parsed position or null
HyprlandCursorRecordingSession-->>RecordingFactory: emit normalized cursor samples
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Author
|
Fechando para reabrir sem os commits de docs/plans do superpowers (não deveriam fazer parte deste PR). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/plans/2026-07-22-hyprland-cursor-position.md`:
- Around line 308-339: Update the captureSample method to record requestedAtMs
immediately before awaiting queryHyprlandCursorPos, then use that captured
timestamp to calculate samples’ timeMs instead of calling Date.now() after the
IPC response. Preserve the existing position fallback and sampling behavior.
In `@docs/superpowers/specs/2026-07-22-hyprland-cursor-position-design.md`:
- Around line 92-94: Label the fenced code block containing the Hyprland socket
path with an appropriate language identifier such as text to satisfy the
fenced-code-language lint rule.
- Around line 13-18: Update the cursor-flow specification in the sections
describing Linux capture and verification to reflect the finalized behavior:
Linux should request cursor: "never" and use the editable overlay path to
prevent duplicate cursors. Remove or revise the outdated statement that Linux
requests cursor: "always" and requires no editor change, including the
corresponding repeated guidance in the later section.
In `@electron/native-bridge/cursor/recording/hyprlandCursorIpc.test.ts`:
- Around line 111-116: Add a test alongside the existing queryHyprlandCursorPos
socket tests that starts a server accepting the cursor-position request while
intentionally neither responding nor closing; assert queryHyprlandCursorPos
resolves to null after its timeout, and clean up the server and socket
resources.
In `@electron/native-bridge/cursor/recording/hyprlandCursorRecordingSession.ts`:
- Around line 41-52: Update HyprlandCursorRecordingSession’s sampling lifecycle
to track the active captureSample() promise, await it in stop() after clearing
the interval, and only then construct the result. Return a stable snapshot of
this.samples rather than the mutable array so no in-flight sample can alter the
returned recording data.
- Around line 19-91: Add same-package tests for HyprlandCursorRecordingSession,
mocking Electron’s screen API and queryHyprlandCursorPos. Cover coordinate
normalization, initial IPC failure fallback, prevention of overlapping capture
requests, maxSamples bounding, and timestamps based on the pre-request time;
exercise start, capture scheduling, and stop behavior through the public session
API.
- Around line 55-70: Initialize a non-null fallback cursor position for the
Hyprland recording session, using the existing Electron cursor point, so
captureSample emits flat samples when the initial queryHyprlandCursorPos request
fails. Update the lastPosition fallback path while preserving replacement with
the IPC position after a successful response.
In `@src/hooks/useScreenRecorder.ts`:
- Around line 1268-1309: Update the Linux system-audio branch around the paired
video and audio capture promises to retain the audio stream when the video
capture fails, stop all audio tracks before propagating the video error, and
preserve the existing video-only fallback for audio capture errors. Follow the
cleanup pattern used for the outer screen-vs-mic race, while keeping the
combined-stream behavior unchanged on success.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 45b15c79-afca-428d-bbbe-ccf72388d2af
📒 Files selected for processing (8)
docs/superpowers/plans/2026-07-22-hyprland-cursor-position.mddocs/superpowers/specs/2026-07-22-hyprland-cursor-position-design.mdelectron/native-bridge/cursor/recording/factory.tselectron/native-bridge/cursor/recording/hyprlandCursorIpc.test.tselectron/native-bridge/cursor/recording/hyprlandCursorIpc.tselectron/native-bridge/cursor/recording/hyprlandCursorRecordingSession.tssrc/components/video-editor/VideoEditor.tsxsrc/hooks/useScreenRecorder.ts
Comment on lines
+308
to
+339
| private async captureSample() { | ||
| if (this.isSampling || !this.socketPath) { | ||
| return; | ||
| } | ||
| this.isSampling = true; | ||
| try { | ||
| const position = (await queryHyprlandCursorPos(this.socketPath)) ?? this.lastPosition; | ||
| if (!position) { | ||
| return; | ||
| } | ||
| this.lastPosition = position; | ||
|
|
||
| const display = | ||
| this.options.getDisplayBounds() ?? screen.getDisplayNearestPoint(position).bounds; | ||
| const width = Math.max(1, display.width); | ||
| const height = Math.max(1, display.height); | ||
|
|
||
| this.samples.push({ | ||
| timeMs: Math.max(0, Date.now() - this.startTimeMs), | ||
| cx: clamp((position.x - display.x) / width, 0, 1), | ||
| cy: clamp((position.y - display.y) / height, 0, 1), | ||
| visible: true, | ||
| }); | ||
|
|
||
| if (this.samples.length > this.options.maxSamples) { | ||
| this.samples.shift(); | ||
| } | ||
| } finally { | ||
| this.isSampling = false; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the implementation plan aligned with the timing fix.
Line 326 timestamps after the IPC response. Capture requestedAtMs before await queryHyprlandCursorPos(...) and use it for timeMs, matching the finalized session and avoiding round-trip cursor lag.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-07-22-hyprland-cursor-position.md` around lines
308 - 339, Update the captureSample method to record requestedAtMs immediately
before awaiting queryHyprlandCursorPos, then use that captured timestamp to
calculate samples’ timeMs instead of calling Date.now() after the IPC response.
Preserve the existing position fallback and sampling behavior.
Comment on lines
+13
to
+18
| - **App-level cursor request**: `useScreenRecorder.ts`'s Linux capture path used the legacy | ||
| `chromeMediaSource` constraints, which have no cursor toggle at all (unlike the `cursor` | ||
| constraint available via `getDisplayMedia`). This was fixed (Linux now requests | ||
| `cursor: "always"` via `getDisplayMedia`, mirroring the Windows path) and is a real, | ||
| independent improvement, but it did **not** fix the reported bug — frame extraction from | ||
| real recordings, before and after, showed zero cursor pixels either way. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the spec to match the finalized cursor flow.
It says Linux requests cursor: "always" and needs no editor change, while this PR’s objective is cursor: "never" to avoid duplicate cursors and enabling the Linux editable overlay. Leaving both statements here makes the verification guidance misleading.
Also applies to: 56-61
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/specs/2026-07-22-hyprland-cursor-position-design.md` around
lines 13 - 18, Update the cursor-flow specification in the sections describing
Linux capture and verification to reflect the finalized behavior: Linux should
request cursor: "never" and use the editable overlay path to prevent duplicate
cursors. Remove or revise the outdated statement that Linux requests cursor:
"always" and requires no editor change, including the corresponding repeated
guidance in the later section.
Comment on lines
+92
to
+94
| ``` | ||
| ${XDG_RUNTIME_DIR}/hypr/${HYPRLAND_INSTANCE_SIGNATURE}/.socket.sock | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Label the fenced block.
Add a language such as text to satisfy markdownlint’s fenced-code-language rule.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 92-92: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/specs/2026-07-22-hyprland-cursor-position-design.md` around
lines 92 - 94, Label the fenced code block containing the Hyprland socket path
with an appropriate language identifier such as text to satisfy the
fenced-code-language lint rule.
Source: Linters/SAST tools
Comment on lines
+111
to
+116
| it("resolves with null when the socket doesn't exist", async () => { | ||
| const socketPath = createTestSocketPath(); | ||
| // Never started a server on this path. | ||
|
|
||
| await expect(queryHyprlandCursorPos(socketPath)).resolves.toBeNull(); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Test the accepted-but-unresponsive socket path.
This only covers an immediate connection error. Add a server that accepts j/cursorpos but neither responds nor closes, then assert resolution to null; otherwise the required timeout behavior can regress without detection.
As per coding guidelines, “Add a test for every new behavior in the same package as the code under test.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron/native-bridge/cursor/recording/hyprlandCursorIpc.test.ts` around
lines 111 - 116, Add a test alongside the existing queryHyprlandCursorPos socket
tests that starts a server accepting the cursor-position request while
intentionally neither responding nor closing; assert queryHyprlandCursorPos
resolves to null after its timeout, and clean up the server and socket
resources.
Source: Coding guidelines
Comment on lines
+19
to
+91
| export class HyprlandCursorRecordingSession implements CursorRecordingSession { | ||
| private samples: CursorRecordingSample[] = []; | ||
| private interval: NodeJS.Timeout | null = null; | ||
| private startTimeMs = 0; | ||
| private isSampling = false; | ||
| private lastPosition: { x: number; y: number } | null = null; | ||
| private readonly socketPath: string | null; | ||
|
|
||
| constructor(private readonly options: HyprlandCursorRecordingSessionOptions) { | ||
| this.socketPath = options.socketPath ?? resolveHyprlandSocketPath(); | ||
| } | ||
|
|
||
| async start(): Promise<void> { | ||
| this.samples = []; | ||
| this.lastPosition = null; | ||
| this.startTimeMs = this.options.startTimeMs ?? Date.now(); | ||
| await this.captureSample(); | ||
| this.interval = setInterval(() => { | ||
| void this.captureSample(); | ||
| }, this.options.sampleIntervalMs); | ||
| } | ||
|
|
||
| async stop(): Promise<CursorRecordingData> { | ||
| if (this.interval) { | ||
| clearInterval(this.interval); | ||
| this.interval = null; | ||
| } | ||
|
|
||
| return { | ||
| version: 2, | ||
| provider: "none", | ||
| samples: this.samples, | ||
| assets: [], | ||
| }; | ||
| } | ||
|
|
||
| private async captureSample() { | ||
| if (this.isSampling || !this.socketPath) { | ||
| return; | ||
| } | ||
| this.isSampling = true; | ||
| // Stamp the sample with the time the query was *sent*, not when the response | ||
| // came back. Stamping after the await biases every sample's timeMs later than | ||
| // when the cursor was actually there by one IPC round-trip, which reads as a | ||
| // constant lag between the cursor overlay and the recording during playback. | ||
| const requestedAtMs = Date.now(); | ||
| try { | ||
| const position = (await queryHyprlandCursorPos(this.socketPath)) ?? this.lastPosition; | ||
| if (!position) { | ||
| return; | ||
| } | ||
| this.lastPosition = position; | ||
|
|
||
| const display = | ||
| this.options.getDisplayBounds() ?? screen.getDisplayNearestPoint(position).bounds; | ||
| const width = Math.max(1, display.width); | ||
| const height = Math.max(1, display.height); | ||
|
|
||
| this.samples.push({ | ||
| timeMs: Math.max(0, requestedAtMs - this.startTimeMs), | ||
| cx: clamp((position.x - display.x) / width, 0, 1), | ||
| cy: clamp((position.y - display.y) / height, 0, 1), | ||
| visible: true, | ||
| }); | ||
|
|
||
| if (this.samples.length > this.options.maxSamples) { | ||
| this.samples.shift(); | ||
| } | ||
| } finally { | ||
| this.isSampling = false; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Add same-package tests for the session behavior.
Mock Electron’s screen and the IPC helper to cover normalization, initial IPC failure fallback, overlap prevention, bounded samples, and pre-request timestamps. This is the core new recording behavior and currently has no automated regression coverage.
As per coding guidelines, “Add a test for every new behavior in the same package as the code under test.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron/native-bridge/cursor/recording/hyprlandCursorRecordingSession.ts`
around lines 19 - 91, Add same-package tests for HyprlandCursorRecordingSession,
mocking Electron’s screen API and queryHyprlandCursorPos. Cover coordinate
normalization, initial IPC failure fallback, prevention of overlapping capture
requests, maxSamples bounding, and timestamps based on the pre-request time;
exercise start, capture scheduling, and stop behavior through the public session
API.
Source: Coding guidelines
Comment on lines
+41
to
+52
| async stop(): Promise<CursorRecordingData> { | ||
| if (this.interval) { | ||
| clearInterval(this.interval); | ||
| this.interval = null; | ||
| } | ||
|
|
||
| return { | ||
| version: 2, | ||
| provider: "none", | ||
| samples: this.samples, | ||
| assets: [], | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Wait for an in-flight sample before returning recording data.
clearInterval() does not await a captureSample() already blocked on IPC. That call can push into this.samples after stop() returns, so persisted cursor data can miss the final sample while the returned object mutates afterward. Track the active sampling promise, await it in stop(), and return a stable snapshot.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron/native-bridge/cursor/recording/hyprlandCursorRecordingSession.ts`
around lines 41 - 52, Update HyprlandCursorRecordingSession’s sampling lifecycle
to track the active captureSample() promise, await it in stop() after clearing
the interval, and only then construct the result. Return a stable snapshot of
this.samples rather than the mutable array so no in-flight sample can alter the
returned recording data.
Comment on lines
+55
to
+70
| private async captureSample() { | ||
| if (this.isSampling || !this.socketPath) { | ||
| return; | ||
| } | ||
| this.isSampling = true; | ||
| // Stamp the sample with the time the query was *sent*, not when the response | ||
| // came back. Stamping after the await biases every sample's timeMs later than | ||
| // when the cursor was actually there by one IPC round-trip, which reads as a | ||
| // constant lag between the cursor overlay and the recording during playback. | ||
| const requestedAtMs = Date.now(); | ||
| try { | ||
| const position = (await queryHyprlandCursorPos(this.socketPath)) ?? this.lastPosition; | ||
| if (!position) { | ||
| return; | ||
| } | ||
| this.lastPosition = position; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve a flat fallback when the first IPC request fails.
When IPC is unavailable before any successful response, lastPosition is null and this returns without emitting samples. Since the factory selects this session whenever the Hyprland signature exists, a startup socket race can make the cursor disappear entirely rather than degrading to the prior flat telemetry behavior. Seed a fallback position (for example, Electron’s existing cursor point) and emit it until IPC succeeds.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron/native-bridge/cursor/recording/hyprlandCursorRecordingSession.ts`
around lines 55 - 70, Initialize a non-null fallback cursor position for the
Hyprland recording session, using the existing Electron cursor point, so
captureSample emits flat samples when the initial queryHyprlandCursorPos request
fails. Update the lastPosition fallback path while preserving replacement with
the IPC position after a successful response.
Comment on lines
+1268
to
+1309
| if (platform === "linux" && systemAudioEnabled) { | ||
| // Electron's getDisplayMedia loopback audio (main.ts's | ||
| // setDisplayMediaRequestHandler) only supports win32/macOS, so system | ||
| // audio on Linux still has to go through the legacy desktop-audio | ||
| // constraints and get merged with the getDisplayMedia video track. | ||
| const [videoOnlyStream, audioOnlyStream] = await Promise.all([ | ||
| navigator.mediaDevices.getDisplayMedia({ | ||
| video: { | ||
| cursor, | ||
| width: { max: TARGET_WIDTH }, | ||
| height: { max: TARGET_HEIGHT }, | ||
| frameRate: { ideal: TARGET_FRAME_RATE }, | ||
| } as MediaTrackConstraints, | ||
| audio: false, | ||
| } as DisplayMediaStreamOptions), | ||
| navigator.mediaDevices | ||
| .getUserMedia({ | ||
| audio: { | ||
| mandatory: { | ||
| chromeMediaSource: CHROME_MEDIA_SOURCE, | ||
| chromeMediaSourceId: selectedSource.id, | ||
| }, | ||
| }, | ||
| video: false, | ||
| } as unknown as MediaStreamConstraints) | ||
| .catch((audioErr) => { | ||
| console.warn( | ||
| "System audio capture failed, falling back to video-only:", | ||
| audioErr, | ||
| ); | ||
| toast.error(t("recording.systemAudioUnavailable")); | ||
| return null; | ||
| }), | ||
| ]); | ||
|
|
||
| const combined = new MediaStream(); | ||
| for (const track of videoOnlyStream.getVideoTracks()) combined.addTrack(track); | ||
| if (audioOnlyStream) { | ||
| for (const track of audioOnlyStream.getAudioTracks()) combined.addTrack(track); | ||
| } | ||
| return combined; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Orphaned system-audio stream leaks when the paired video getDisplayMedia call fails.
Promise.all([videoOnlyStream, audioOnlyStream]) rejects as soon as videoOnlyStream rejects, but audioOnlyStream already swallows its own errors via .catch(() => null), so it never causes the Promise.all to reject — it just keeps resolving independently. If the video capture fails/rejects after the audio getUserMedia call has already succeeded, that live audio track is never stopped: it leaks and keeps the OS-level capture indicator active.
This is exactly the class of bug already fixed for the outer screen-vs-mic race at Lines 1387-1399 ("the screen capture rejects it would otherwise orphan a mic stream"). Apply the same pattern here.
🛠️ Proposed fix
if (platform === "linux" && systemAudioEnabled) {
- const [videoOnlyStream, audioOnlyStream] = await Promise.all([
- navigator.mediaDevices.getDisplayMedia({
- video: {
- cursor,
- width: { max: TARGET_WIDTH },
- height: { max: TARGET_HEIGHT },
- frameRate: { ideal: TARGET_FRAME_RATE },
- } as MediaTrackConstraints,
- audio: false,
- } as DisplayMediaStreamOptions),
- navigator.mediaDevices
- .getUserMedia({
- audio: {
- mandatory: {
- chromeMediaSource: CHROME_MEDIA_SOURCE,
- chromeMediaSourceId: selectedSource.id,
- },
- },
- video: false,
- } as unknown as MediaStreamConstraints)
- .catch((audioErr) => {
- console.warn(
- "System audio capture failed, falling back to video-only:",
- audioErr,
- );
- toast.error(t("recording.systemAudioUnavailable"));
- return null;
- }),
- ]);
+ const audioOnlyStreamPromise = navigator.mediaDevices
+ .getUserMedia({
+ audio: {
+ mandatory: {
+ chromeMediaSource: CHROME_MEDIA_SOURCE,
+ chromeMediaSourceId: selectedSource.id,
+ },
+ },
+ video: false,
+ } as unknown as MediaStreamConstraints)
+ .catch((audioErr) => {
+ console.warn(
+ "System audio capture failed, falling back to video-only:",
+ audioErr,
+ );
+ toast.error(t("recording.systemAudioUnavailable"));
+ return null;
+ });
+
+ let videoOnlyStream: MediaStream;
+ try {
+ videoOnlyStream = await navigator.mediaDevices.getDisplayMedia({
+ video: {
+ cursor,
+ width: { max: TARGET_WIDTH },
+ height: { max: TARGET_HEIGHT },
+ frameRate: { ideal: TARGET_FRAME_RATE },
+ } as MediaTrackConstraints,
+ audio: false,
+ } as DisplayMediaStreamOptions);
+ } catch (videoErr) {
+ void audioOnlyStreamPromise.then((s) =>
+ s?.getTracks().forEach((track) => track.stop()),
+ );
+ throw videoErr;
+ }
+ const audioOnlyStream = await audioOnlyStreamPromise;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (platform === "linux" && systemAudioEnabled) { | |
| // Electron's getDisplayMedia loopback audio (main.ts's | |
| // setDisplayMediaRequestHandler) only supports win32/macOS, so system | |
| // audio on Linux still has to go through the legacy desktop-audio | |
| // constraints and get merged with the getDisplayMedia video track. | |
| const [videoOnlyStream, audioOnlyStream] = await Promise.all([ | |
| navigator.mediaDevices.getDisplayMedia({ | |
| video: { | |
| cursor, | |
| width: { max: TARGET_WIDTH }, | |
| height: { max: TARGET_HEIGHT }, | |
| frameRate: { ideal: TARGET_FRAME_RATE }, | |
| } as MediaTrackConstraints, | |
| audio: false, | |
| } as DisplayMediaStreamOptions), | |
| navigator.mediaDevices | |
| .getUserMedia({ | |
| audio: { | |
| mandatory: { | |
| chromeMediaSource: CHROME_MEDIA_SOURCE, | |
| chromeMediaSourceId: selectedSource.id, | |
| }, | |
| }, | |
| video: false, | |
| } as unknown as MediaStreamConstraints) | |
| .catch((audioErr) => { | |
| console.warn( | |
| "System audio capture failed, falling back to video-only:", | |
| audioErr, | |
| ); | |
| toast.error(t("recording.systemAudioUnavailable")); | |
| return null; | |
| }), | |
| ]); | |
| const combined = new MediaStream(); | |
| for (const track of videoOnlyStream.getVideoTracks()) combined.addTrack(track); | |
| if (audioOnlyStream) { | |
| for (const track of audioOnlyStream.getAudioTracks()) combined.addTrack(track); | |
| } | |
| return combined; | |
| } | |
| if (platform === "linux" && systemAudioEnabled) { | |
| const audioOnlyStreamPromise = navigator.mediaDevices | |
| .getUserMedia({ | |
| audio: { | |
| mandatory: { | |
| chromeMediaSource: CHROME_MEDIA_SOURCE, | |
| chromeMediaSourceId: selectedSource.id, | |
| }, | |
| }, | |
| video: false, | |
| } as unknown as MediaStreamConstraints) | |
| .catch((audioErr) => { | |
| console.warn( | |
| "System audio capture failed, falling back to video-only:", | |
| audioErr, | |
| ); | |
| toast.error(t("recording.systemAudioUnavailable")); | |
| return null; | |
| }); | |
| let videoOnlyStream: MediaStream; | |
| try { | |
| videoOnlyStream = await navigator.mediaDevices.getDisplayMedia({ | |
| video: { | |
| cursor, | |
| width: { max: TARGET_WIDTH }, | |
| height: { max: TARGET_HEIGHT }, | |
| frameRate: { ideal: TARGET_FRAME_RATE }, | |
| } as MediaTrackConstraints, | |
| audio: false, | |
| } as DisplayMediaStreamOptions); | |
| } catch (videoErr) { | |
| void audioOnlyStreamPromise.then((s) => | |
| s?.getTracks().forEach((track) => track.stop()), | |
| ); | |
| throw videoErr; | |
| } | |
| const audioOnlyStream = await audioOnlyStreamPromise; | |
| const combined = new MediaStream(); | |
| for (const track of videoOnlyStream.getVideoTracks()) combined.addTrack(track); | |
| if (audioOnlyStream) { | |
| for (const track of audioOnlyStream.getAudioTracks()) combined.addTrack(track); | |
| } | |
| return combined; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/useScreenRecorder.ts` around lines 1268 - 1309, Update the Linux
system-audio branch around the paired video and audio capture promises to retain
the audio stream when the video capture fails, stop all audio tracks before
propagating the video error, and preserve the existing video-only fallback for
audio capture errors. Follow the cleanup pattern used for the outer
screen-vs-mic race, while keeping the combined-stream behavior unchanged on
success.
4 tasks
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
1 participant
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
cursor:neverfrom the portal and relying on the editor's editable overlay on Linux.recordingId, and cursor telemetry start time) to the first real captured frame instead of toDate.now()at button-press time. On Linux/PipeWire, the desktop-capture portal can still be negotiating (DMA-BUF modifier renegotiation) for several seconds after the track reports unmuted; anchoring to wall-clock "button pressed" time let the declared recording duration exceed the video's real content span (playback would jump to the end early) and let the cursor overlay drift out of sync with the picture.waitForFirstVideoFrame()now blocks on an actual rendered frame (viarequestVideoFrameCallback, capped at 3s) beforerecorder.start()is called, so everything anchors to the same verified instant.Fixes #99 (comment)
Test plan
tsc --noEmitpasses cleanSummary by CodeRabbit
New Features
Bug Fixes