Hls recording fixes

[richiemcilroy]

Fixes stop recording freezing on Display captures by adding layered timeouts across the pipeline. Removes redundant local MP4 assembly (server handles it), making stop near-instant. Ensures all failure paths clear the UI.

Greptile Summary

This PR eliminates on-device MP4 assembly at stop time, letting the server reconstruct the final video from HLS segments. Stop is now near-instant and can no longer freeze the UI on Display captures thanks to layered timeouts: 5 s for capturer stop, a 2 s drain deadline for leftover frames, and a 10 s overall pipeline stop timeout.

• P2 – Encoder stop sentinel can be lost: in macos_fragmented_m4s.rs, changing send to try_send means the None stop sentinel is silently dropped if the 60-frame encoder channel is full; the encoder OS thread will keep running until video_tx is dropped after the pipeline timeout.

Confidence Score: 4/5

Safe to merge; one P2 resource-leak edge case in the encoder stop path worth addressing

All P0/P1 concerns are resolved: layered timeouts prevent UI freeze, failure paths correctly clear the UI, and segment-presence is a valid health proxy. The only remaining issue is the encoder stop sentinel potentially being dropped under high load (try_send on a full channel), which is a resource leak rather than a crash or data-loss scenario.

crates/recording/src/output_pipeline/macos_fragmented_m4s.rs — stop sentinel may be dropped when encoder channel is full

Vulnerabilities

No security concerns identified.

Important Files Changed

Filename	Overview
crates/recording/src/output_pipeline/macos_fragmented_m4s.rs	Changed send to try_send for M4S encoder stop sentinel; if channel is full the sentinel is silently dropped
apps/desktop/src-tauri/src/recording.rs	Improved stop-failure propagation with StopFailureContext, new segment-based screenshot path, and simplified post-stop upload flow
crates/recording/src/instant_recording.rs	Removed local MP4 assembly on stop; replaced with lightweight segment-presence health check
crates/recording/src/output_pipeline/core.rs	Added 5 s timeout to video source stop, hard deadline to drain loop, and 10 s timeout to overall pipeline stop
apps/desktop/src-tauri/src/http_client.rs	Added connect_timeout(10 s) and overall timeout(30 s) to HttpClient; RetryableHttpClient gets connect_timeout only, using per-request timeouts at call sites
apps/desktop/src-tauri/src/lib.rs	New create_screenshot_source_from_segments helper builds a minimal temp MP4 from init.mp4 + first .m4s segment
apps/desktop/src-tauri/src/upload.rs	Added failed-segment warning log before final manifest upload; added emit_upload_complete on signal-complete failure path
crates/recording/src/sources/screen_capture/macos.rs	Added 5 s timeout around capturer.stop() to prevent hang on Display captures
crates/enc-avfoundation/src/mp4.rs	Test: collapsed nested if to satisfy collapsible_if Clippy lint
crates/recording/tests/hardware_instant_recording.rs	Test updated to verify segment-based output instead of assembled MP4

Sequence Diagram

sequenceDiagram
    participant UI
    participant stop_recording
    participant OutputPipeline
    participant VideoSource
    participant SegmentUploader
    participant Server

    UI->>stop_recording: stop_recording()
    stop_recording->>OutputPipeline: drop stop_token
    OutputPipeline->>VideoSource: stop() [5 s timeout]
    VideoSource-->>OutputPipeline: ok / timeout
    OutputPipeline->>OutputPipeline: drain loop [2 s deadline, max 500 frames]
    OutputPipeline-->>stop_recording: done_fut [10 s timeout]
    alt stop succeeded
        stop_recording->>SegmentUploader: await handle
        SegmentUploader->>Server: upload final manifest
        Server-->>SegmentUploader: signal_recording_complete
        SegmentUploader->>UI: emit_upload_complete
    else stop failed
        stop_recording->>SegmentUploader: handle.abort()
        stop_recording->>UI: emit_upload_complete
    end

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: crates/recording/src/output_pipeline/macos_fragmented_m4s.rs
Line: 219-231

Comment:
**`try_send` may silently drop the encoder stop sentinel**

When the encoder channel holds its full 60 frames, `try_send(None)` returns `TrySendError::Full` and only logs a warning — the `None` sentinel is never delivered. The encoder thread blocks on `video_rx.recv()` and will not exit until `video_tx` is eventually dropped once the muxer state is cleaned up after the 10 s pipeline timeout fires. Under sustained high encoder load this leaks an OS thread per recording stop. The old blocking `send` was safe here because `stop()` is a synchronous method called from inside an `async` tokio task (via `muxer.lock().await.stop()`). Consider either a short-deadline blocking send as a fallback, or a separate atomic stop-flag the encoder polls between frames to handle the full-channel case:

```rust
// fallback: drain one slot by force, then send sentinel
if state.video_tx.try_send(None).is_err() {
    // channel is full — set a stop flag the encoder loop checks
    state.stop_requested.store(true, Ordering::Release);
}
```

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

---

This is a comment left during a code review.
Path: apps/desktop/src-tauri/src/recording.rs
Line: 1877-1917

Comment:
**Thumbnail always uploaded regardless of segment-upload outcome**

In the old code the thumbnail upload was gated on `video_upload_succeeded`. The new code unconditionally runs `compress_image` + `singlepart_uploader` after `screenshot_task.await`, even when `!upload_succeeded`. If intentional (the video was pre-created and its presigned URL is still valid), this is fine — but it is a behavioural change worth confirming: a screenshot will now be pushed for recordings whose segments never fully reached the server.

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

_{Reviews (1): Last reviewed commit: "Enhance instant recording test media che..." | Re-trigger Greptile}

Greptile also left 2 inline comments on this PR.

[paragon-review]

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.

[tembo]

RetryableHttpClient has a connect timeout but no overall request timeout. A stalled request could hang for a long time even with retries. Consider adding .timeout(...) like HttpClient.

Suggested change

	.connect_timeout(std::time::Duration::from_secs(10))
	reqwest::Client::builder()
	.connect_timeout(std::time::Duration::from_secs(10))
	.timeout(std::time::Duration::from_secs(30))
	.retry(

[tembo]

create_screenshot_source_from_segments writes .screenshot_source.mp4 into the segments directory. If any upload/cleanup path ever scans that directory, this temp file could get picked up accidentally. Consider writing the temp MP4 under the OS temp dir instead and returning that path.

[tembo]

Minor nit: the retry loop sleeps before checking Disconnected, so a closed channel will still delay stop by 50ms. You can avoid the extra sleep and collapse the initial try_send + retry loop into one loop that only sleeps on Full (same pattern applies to the camera muxer stop too).

Suggested change

	for _ in 0..5 {
	for attempt in 0..=5 {
	match state.video_tx.try_send(None) {
	Ok(()) => return,
	Err(std::sync::mpsc::TrySendError::Disconnected(_)) => {
	trace!("M4S encoder channel closed during stop");
	return;
	}
	Err(std::sync::mpsc::TrySendError::Full(_)) => {
	if attempt < 5 {
	std::thread::sleep(Duration::from_millis(50));
	}
	}
	}
	}

[tembo]

These warnings will be way easier to act on if they include the pipeline output path (especially when multiple recordings are stopping concurrently).

Suggested change

	warn!(
	warn!(
	path = %self.path.display(),
	"first_timestamp channel was dropped without sending a value, defaulting to now"
	);