Skip to content

segmented MP4 upload pipeline with live HLS playback and server-side muxing#1716

Merged
richiemcilroy merged 27 commits intomainfrom
hls-recording-reliability
Apr 7, 2026
Merged

segmented MP4 upload pipeline with live HLS playback and server-side muxing#1716
richiemcilroy merged 27 commits intomainfrom
hls-recording-reliability

Conversation

@richiemcilroy
Copy link
Copy Markdown
Member

@richiemcilroy richiemcilroy commented Apr 7, 2026

  • Replaces the growing-file progressive upload with a segment-by-segment upload pipeline where each fMP4 segment (video and audio) is uploaded to S3 as it completes
  • Segments are served as a live HLS stream via hls.js for instant web playback during recording
  • When recording finishes, the media server muxes segments into a final result.mp4 with faststart
  • Adds DashAudioSegmentEncoder producing CMAF-compatible init + m4s segments for HLS audio
  • Desktop falls back to full-file upload if segment upload fails entirely

Greptile Summary

This PR replaces the growing-file multipart upload with a per-segment pipeline: fMP4 segments are uploaded to S3 in real-time, served as a live HLS stream via hls.js, and muxed into a final result.mp4 by the media server when recording ends. The architecture is well-structured with semaphore-bounded concurrency, retry logic, a graceful full-file fallback, and correct no-store cache headers for the live HLS playlist.

  • Breaking auth change: validateMediaServerSecret now hard-rejects all protected media-server routes (including the previously open /process endpoint) when MEDIA_SERVER_WEBHOOK_SECRET is unset, and the webhook receiver applies the same hard-reject. Self-hosted deployments without this env var will have all background video processing silently broken after upgrading.
  • Silent partial mux: muxSegmentsAsync continues muxing when fewer than 50% of video segments fail to download, potentially producing a truncated or corrupted result.mp4 without surfacing the gap to the caller.

Confidence Score: 4/5

Safe to merge after addressing the breaking auth requirement for self-hosted deployments

The segmented upload pipeline is well-implemented with semaphore-bounded concurrency, retry logic, and a graceful full-file fallback. One P1 finding: the hard requirement for MEDIA_SERVER_WEBHOOK_SECRET silently breaks all background video processing in existing deployments that haven't set it. All other findings are P2 quality or reliability concerns.

apps/media-server/src/routes/video.ts and apps/web/app/api/webhooks/media-server/progress/route.ts for the auth breaking change

Important Files Changed

Filename Overview
apps/desktop/src-tauri/src/upload.rs Adds SegmentUploader with concurrent upload tasks; manifest snapshot is captured and mutex dropped before the network write, allowing two tasks to upload manifests out of order
apps/media-server/src/routes/video.ts Adds /mux-segments endpoint and validateMediaServerSecret; secret now required for all protected endpoints — breaking for deployments without the env var
apps/web/app/api/playlist/route.ts Adds segments-master/video/audio playlist types served from S3 manifest; no-store cache-control is correct for live HLS polling
apps/web/app/api/upload/[...route]/recording-complete.ts New endpoint verifies segment manifest completeness and triggers media-server muxing with idempotent row-lock on videoUploads
apps/web/app/api/webhooks/media-server/progress/route.ts Webhook now hard-requires MEDIA_SERVER_WEBHOOK_SECRET; deployments without the env var will have the webhook permanently return 401
apps/web/app/s/[videoId]/_components/HLSVideoPlayer.tsx Adds isLiveSegments mode with aggressive retry config; hlsInitFailed state gates the error UI correctly; overlay opacity inversion fix is correct
packages/database/schema.ts Adds desktopSegments to the video source type union; non-breaking schema extension
packages/web-domain/src/Video.ts Adds SegmentsSource class and SegmentManifest schema; normalizeSegmentEntry handles legacy number entries with a 3.0s default duration assumption

Sequence Diagram

sequenceDiagram
    participant Desktop as Desktop (Tauri)
    participant S3
    participant WebApp as Web App
    participant MediaServer as Media Server
    participant Browser

    Desktop->>WebApp: POST /api/desktop/video/create?recordingMode=desktopSegments
    WebApp-->>Desktop: videoId + upload credentials
    Desktop->>Desktop: Start recording (fMP4 pipeline)
    loop Per segment
        Desktop->>S3: PUT segments/video/segment_NNN.m4s
        Desktop->>S3: PUT segments/manifest.json (is_complete=false)
    end
    Browser->>WebApp: GET /api/playlist?videoType=segments-master
    WebApp->>S3: GET manifest.json
    WebApp-->>Browser: HLS master playlist
    Browser->>WebApp: GET /api/playlist?videoType=segments-video
    WebApp-->>Browser: HLS media playlist (signed segment URLs)
    Browser->>S3: Fetch segment files (live HLS)
    Desktop->>S3: PUT manifest.json (is_complete=true)
    Desktop->>WebApp: POST /api/upload/recording-complete
    WebApp->>MediaServer: POST /video/mux-segments
    MediaServer->>S3: Download all segments
    MediaServer->>MediaServer: FFmpeg concat + mux to result.mp4
    MediaServer->>S3: PUT result.mp4
    MediaServer->>WebApp: POST /api/webhooks/media-server/progress (phase=complete)
    WebApp->>WebApp: Update source to desktopMP4
    WebApp->>S3: Delete segments/ prefix (async)
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/media-server/src/routes/video.ts
Line: 37-48

Comment:
**Breaking change: `MEDIA_SERVER_WEBHOOK_SECRET` now required for all endpoints**

`validateMediaServerSecret` returns `false` (→ 401) whenever the env var is absent. It is now applied to the previously-unauthenticated `/process` endpoint as well as `/process/:jobId/cancel`, `/cleanup`, and `/force-cleanup`. The webhook receiver in `apps/web/app/api/webhooks/media-server/progress/route.ts` was simultaneously tightened from `if (webhookSecret) { check }` (allow-all when unset) to `if (!webhookSecret || header !== secret) { reject }`. Any self-hosted deployment that has not yet added `MEDIA_SERVER_WEBHOOK_SECRET` to its environment will have all background video processing silently broken after this upgrade.

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/media-server/src/routes/video.ts
Line: 1072-1085

Comment:
**Silent partial-failure continues muxing with missing segments**

When fewer than 50% of video segments fail to download, `muxSegmentsAsync` logs a warning and continues muxing. Depending on codec and key-frame placement, the missing `.m4s` segments produce either a visibly glitched video or a silently truncated `result.mp4`, and the webhook payload does not surface the gap to the caller. Consider failing the job outright, or at minimum including a `segmentsMissing` count in the progress payload so the web app can surface a degraded-quality notice.

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/upload.rs
Line: 1046-1060

Comment:
**Manifest can transiently regress during concurrent uploads**

Each spawned task acquires the state mutex, updates it, drops the lock, then calls `Self::upload_manifest`. Because the lock is released before the network write, two concurrent tasks can compute consistent snapshots A (older) and B (newer), upload B first, then overwrite it with A — briefly making the live HLS manifest appear to lose a segment. For a viewer polling during recording this is cosmetic, but it could cause a brief "not found" flash. Serializing manifest writes through a dedicated channel, or holding the lock across the upload call, would eliminate the race.

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

Reviews (1): Last reviewed commit: "Add explicit types for cues, subtitles, ..." | Re-trigger Greptile

Greptile also left 3 inline comments on this PR.

Made-with: Cursor
@richiemcilroy
Copy link
Copy Markdown
Member Author

@greptileai please review the PR

@paragon-review
Copy link
Copy Markdown

paragon-review bot commented Apr 7, 2026

Paragon Summary

This pull request review identified 2 issues across 1 category in 52 files. The review analyzed code changes, potential bugs, security vulnerabilities, performance issues, and code quality concerns using automated analysis tools.

This PR makes changes across apps, crates, packages directories.

Key changes:

  • Modifies rs, ts, tsx files

Confidence score: 3/5

  • This PR has moderate risk due to 2 high-priority issues that should be addressed
  • Score reflects significant bugs, performance issues, or architectural concerns
  • Review high-priority findings carefully before merging

52 files reviewed, 2 comments

Severity breakdown: High: 2

Dashboard

@richiemcilroy richiemcilroy merged commit 50b72ed into main Apr 7, 2026
17 checks passed
if (ownerId) {
const segmentsPrefix = `${ownerId}/${videoId}/segments/`;
Effect.gen(function* () {
const bucketId = Option.fromNullable(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

listed.Contents entries can include an undefined Key, which would end up sending { Key: undefined } to S3. Also, the bucketId cast looks unnecessary since videos.bucket is already typed as S3BucketId | null.

Suggested change
const bucketId = Option.fromNullable(
const bucketId = Option.fromNullable(currentVideo.bucket);
const [bucket] = yield* S3Buckets.getBucketAccess(bucketId);
let totalDeleted = 0;
let continuationToken: string | undefined;
do {
const listed = yield* bucket.listObjects({
prefix: segmentsPrefix,
continuationToken,
});
if (listed.Contents && listed.Contents.length > 0) {
const objects = listed.Contents.flatMap((c: { Key?: string }) =>
c.Key ? [{ Key: c.Key }] : [],
);
if (objects.length > 0) {
yield* bucket.deleteObjects(objects);
totalDeleted += objects.length;
}
}
continuationToken = listed.IsTruncated
? listed.NextContinuationToken
: undefined;
} while (continuationToken);

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.

2 participants