Skip to content

0.4.84 fixes#1720

Merged
richiemcilroy merged 21 commits intomainfrom
0.4.84-fixes
Apr 9, 2026
Merged

0.4.84 fixes#1720
richiemcilroy merged 21 commits intomainfrom
0.4.84-fixes

Conversation

@richiemcilroy
Copy link
Copy Markdown
Member

@richiemcilroy richiemcilroy commented Apr 9, 2026

Greptile Summary

This PR is a patch release (0.4.84) with a collection of bug fixes across the desktop app and web player. Key improvements include: camera preview state handling after resume, correct recording controls positioning for window/area capture targets, camera-only mode upload and screenshot fixes, HLS live segments retry logic, and macOS dock visibility synchronization.

Confidence Score: 4/5

Safe to merge after addressing the HLS re-initialization bug; all other changes are well-scoped fixes.

One P1 finding: adding videoLoaded to the HLS setup effect dependency array causes the HLS player to be destroyed and recreated when the first fragment loads during live-recording playback, producing a noticeable interruption. All other changes (camera preview state machine, recording controls positioning, camera-only mode, force_exit, dock visibility) are correct bug fixes.

apps/web/app/s/[videoId]/_components/HLSVideoPlayer.tsx — the videoLoaded dependency in the HLS setup effect

Vulnerabilities

No security concerns identified. The unsafe block in force_exit is a deliberate POSIX _exit call to bypass atexit handlers on shutdown — it does not introduce a security vulnerability.

Important Files Changed

Filename Overview
apps/web/app/s/[videoId]/_components/HLSVideoPlayer.tsx Adds live-segment retry logic and upload progress handling; videoLoaded in HLS setup deps causes the HLS instance to be torn down and restarted when the video first loads, breaking live-segment playback continuity.
apps/desktop/src-tauri/src/camera.rs Fixes stale-frame flash on resume by draining the camera receive buffer, and correctly handles state changes while paused with the needs_full_reconfigure flag and GPU surface reconfiguration.
apps/desktop/src-tauri/src/lib.rs Replaces std::process::exit with a POSIX _exit-backed force_exit to avoid macOS shutdown hangs; guards camera cleanup/restore behind main_window_visible and camera_cleanup_done to prevent spurious camera feed preservation.
apps/desktop/src-tauri/src/fake_window.rs Adds calculate_recording_controls_position_for_target to position recording controls relative to the captured window or area bounds rather than always using cursor position.
apps/desktop/src-tauri/src/windows.rs Adds capture_target field to InProgressRecording, uses it for smart controls positioning, and calls sync_macos_dock_visibility at panel creation/error paths for correct macOS dock state.
apps/desktop/src-tauri/src/recording.rs Uses desktopMP4 upload mode for camera-only recordings, passes capture_target to InProgressRecording, and switches screenshot source for camera-only to the output MP4 path.
crates/recording/src/instant_recording.rs Extends health check to treat a non-empty output.mp4 as healthy, covering camera-only recordings that produce an MP4 instead of segments.
crates/recording/src/feeds/camera.rs Changes sync channel capacity from 0 (rendezvous) to 1, preventing a potential sender block when the receiver is not immediately ready to consume the done signal.

Sequence Diagram

sequenceDiagram
    participant UI as Desktop UI
    participant Rec as recording.rs
    participant Win as windows.rs
    participant FW as fake_window.rs
    participant Cam as camera.rs

    UI->>Rec: start_recording(inputs)
    Rec->>Rec: Determine upload_mode
    Rec->>Win: ShowCapWindow::InProgressRecording
    Win->>FW: calculate_recording_controls_position_for_target
    FW-->>Win: pos_x, pos_y
    Win->>Win: set_position + sync_macos_dock_visibility

    Note over Cam: On resume from pause
    Cam->>Cam: drain camera_rx buffer
    Cam->>Cam: needs_full_reconfigure=true
    Cam->>Cam: update_state_uniforms
    Cam->>Cam: resize_window + reconfigure_gpu_surface
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/web/app/s/[videoId]/_components/HLSVideoPlayer.tsx
Line: 354

Comment:
**HLS player destroyed and recreated when `videoLoaded` becomes `true`**

Adding `videoLoaded` to the dependency array means the entire effect — which creates, configures, and attaches the HLS instance — re-runs every time `videoLoaded` flips from `false` to `true`. This causes the cleanup to destroy the active HLS instance and immediately create a new one.

For live segments (`isLiveSegments=true`), this happens inside `Hls.Events.FRAG_LOADED`: the handler sets `videoLoaded = true`, which triggers the re-run mid-stream. The HLS connection is torn down and restarted exactly once, producing a visible interruption when viewing a recording that is still in progress.

The intent appears to be giving the `FRAG_LOADED` closure a fresh value of `videoLoaded`, but a ref avoids the re-init:

```typescript
const videoLoadedRef = useRef(videoLoaded);
useEffect(() => { videoLoadedRef.current = videoLoaded; }, [videoLoaded]);

// Inside the HLS effect:
hls.on(Hls.Events.FRAG_LOADED, () => {
    if (isLiveSegments && !videoLoadedRef.current) {
        setVideoLoaded(true);
        ...
    }
});
```

Then remove `videoLoaded` from the dependency array.

```suggestion
	}, [playbackSrc, isLiveSegments, reloadPlayback, router]);
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "style(desktop): apply clippy suggestion ..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

@paragon-review
Copy link
Copy Markdown

paragon-review bot commented Apr 9, 2026

Paragon Review Skipped

Hi @richiemcilroy! Your Polarity credit balance is insufficient to complete this review.

Please visit https://home.polarity.cc to add more credits and continue using Paragon reviews.

setHlsInitFailed(true);
}
}, [playbackSrc, isLiveSegments, videoRef.current]);
}, [playbackSrc, isLiveSegments, reloadPlayback, router, videoLoaded]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 HLS player destroyed and recreated when videoLoaded becomes true

Adding videoLoaded to the dependency array means the entire effect — which creates, configures, and attaches the HLS instance — re-runs every time videoLoaded flips from false to true. This causes the cleanup to destroy the active HLS instance and immediately create a new one.

For live segments (isLiveSegments=true), this happens inside Hls.Events.FRAG_LOADED: the handler sets videoLoaded = true, which triggers the re-run mid-stream. The HLS connection is torn down and restarted exactly once, producing a visible interruption when viewing a recording that is still in progress.

The intent appears to be giving the FRAG_LOADED closure a fresh value of videoLoaded, but a ref avoids the re-init:

const videoLoadedRef = useRef(videoLoaded);
useEffect(() => { videoLoadedRef.current = videoLoaded; }, [videoLoaded]);

// Inside the HLS effect:
hls.on(Hls.Events.FRAG_LOADED, () => {
    if (isLiveSegments && !videoLoadedRef.current) {
        setVideoLoaded(true);
        ...
    }
});

Then remove videoLoaded from the dependency array.

Suggested change
}, [playbackSrc, isLiveSegments, reloadPlayback, router, videoLoaded]);
}, [playbackSrc, isLiveSegments, reloadPlayback, router]);
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/app/s/[videoId]/_components/HLSVideoPlayer.tsx
Line: 354

Comment:
**HLS player destroyed and recreated when `videoLoaded` becomes `true`**

Adding `videoLoaded` to the dependency array means the entire effect — which creates, configures, and attaches the HLS instance — re-runs every time `videoLoaded` flips from `false` to `true`. This causes the cleanup to destroy the active HLS instance and immediately create a new one.

For live segments (`isLiveSegments=true`), this happens inside `Hls.Events.FRAG_LOADED`: the handler sets `videoLoaded = true`, which triggers the re-run mid-stream. The HLS connection is torn down and restarted exactly once, producing a visible interruption when viewing a recording that is still in progress.

The intent appears to be giving the `FRAG_LOADED` closure a fresh value of `videoLoaded`, but a ref avoids the re-init:

```typescript
const videoLoadedRef = useRef(videoLoaded);
useEffect(() => { videoLoadedRef.current = videoLoaded; }, [videoLoaded]);

// Inside the HLS effect:
hls.on(Hls.Events.FRAG_LOADED, () => {
    if (isLiveSegments && !videoLoadedRef.current) {
        setVideoLoaded(true);
        ...
    }
});
```

Then remove `videoLoaded` from the dependency array.

```suggestion
	}, [playbackSrc, isLiveSegments, reloadPlayback, router]);
```

How can I resolve this? If you propose a fix, please make it concise.

@richiemcilroy richiemcilroy merged commit 48645d5 into main Apr 9, 2026
17 of 18 checks passed
onClick={() => {
if (!selectedWindow()) {
setOriginalCameraBounds(null);
setLockedIcon(windowIcon.data ?? null);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

windowIcon.data is an accessor; storing it directly means lockedIcon() becomes a function and src ends up receiving a function. Probably want the resolved value here.

Suggested change
setLockedIcon(windowIcon.data ?? null);
setLockedIcon(windowIcon.data?.() ?? null);

}
});

hls.on(Hls.Events.FRAG_LOADED, () => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Avoid reading videoLoaded inside the HLS setup effect; it forces videoLoaded into deps and can cause the whole Hls instance to be torn down/recreated when the first fragment arrives. Since setVideoLoaded(true) is idempotent, you can simplify this handler.

Suggested change
hls.on(Hls.Events.FRAG_LOADED, () => {
hls.on(Hls.Events.FRAG_LOADED, () => {
if (isLiveSegments) {
setVideoLoaded(true);
if (!hasPlayedOnceRef.current) {
setShowPlayButton(true);
}
}
});

setHlsInitFailed(true);
}
}, [playbackSrc, isLiveSegments, videoRef.current]);
}, [playbackSrc, isLiveSegments, reloadPlayback, router, videoLoaded]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

videoLoaded in this dependency array will re-run the HLS init effect when setVideoLoaded(true) fires (live segments: FRAG_LOADED), which looks like it would restart playback.

Suggested change
}, [playbackSrc, isLiveSegments, reloadPlayback, router, videoLoaded]);
}, [playbackSrc, isLiveSegments, reloadPlayback, router]);

return;
}

if (isMounted) setPlatform("macos");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This sets platform to macos for any non-Windows/Linux UA (Android/iOS/ChromeOS, etc). If this is only meant to distinguish desktop platforms, it might be safer to only set macos when the UA actually contains Mac, otherwise keep platform as null.

Suggested change
if (isMounted) setPlatform("macos");
if (userAgent.includes("Mac")) {
if (isMounted) setPlatform("macos");
} else {
if (isMounted) setPlatform(null);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant