Enforce the upload contract before a video leaves the device

[jlocala1]

Uploads reached the server as 4K HEVC at ~28 Mbps with the index at the end of the file, while the recorder's own config asked for 1080p at 5 Mbps. Phones could not play them back.

This adds an upload-side gate: probe every video before it is uploaded, and re-encode only if it breaches the contract. It is deliberately complementary to the source-level fixes #143 rather than a replacement for them — see "how this relates to the H.264 work" below.

What was measured

Real clips from the same phone, profiled with ffprobe:

	resolution	bitrate	codec	faststart	container
pre-07-21	3840x2160	28.80 Mbps	hevc	no	qt
2026-07-29	1920x1080	8.24 Mbps	hevc	no	qt

The resolution breach is fixed and not by anything here — targetResolution is not a setting but a bias in a weighted format vote, <Camera> ranks its preview output above ours, the preview wants a format at least as large as the screen, and every modern iPhone screen is taller than 1920px. A 1080p format therefore took an aspect-ratio penalty weighted 100x and 4K won by roughly 20x. VisionCamera 5.2.0 rescored that comparison; confirmed on a device.

What remains on the 07-29 build is why this PR exists: 8.24 Mbps against a 5 Mbps pin that has been set since 07-20, and no faststart.

What this changes

• upload-contract.ts — the policy, pure and unit-tested: H.264, long edge <= 1920, <= 5 Mbps, AAC. 18 tests, including fixtures taken from the two real recordings above and the portrait case.
• ensure-upload-contract.ts — the runtime. Probe, decide, re-encode only on breach. A compliant file costs one probe and is uploaded untouched.
• upload-manager.ts — applied on both upload units. uploadUnit is chosen by the destination server, so gating only the merged path would silently disable the contract for any server that asks for segments.
• use-recorder.ts / recorder.tsx — log the negotiated capture format and warn when it breaches, so a lost vote cannot go unnoticed for weeks again; and name the video output's resolutionBias explicitly. That is margin, not a fix: 5.2.0 wins by ~13% on the largest screens, an explicit bias makes it ~6x.
• Import normalization failures are no longer swallowed. Falling back to the original bytes is right; being silent about it is how a 4K master enters a draft looking like one that was normalized.

Conditioned segment copies go to a stable path under the draft dir rather than a cache temp name: segment uploads resume byte-wise via TUS HEAD + PATCH, so a resumed run must send the bytes it began with. Re-encoding on resume would splice a second encode into a half-finished transfer.

How this relates to the H.264 work in #143

No overlap in code — this rebases onto it cleanly. Forcing H.264 at the recorder is the better fix for the codec breach and this PR does not duplicate it. The gate covers what the source misses today (bitrate, faststart) and, more durably, catches the case where a setting silently stops being honoured — which is exactly what happened with targetResolution. If the recorder becomes fully compliant, this gate becomes a no-op probe. That is the intended end state, not a wasted path.

Two things found while measuring, worth checking independently of this PR:

1. The faststart patch in patches/react-native-vision-camera+5.2.0.patch sets shouldOptimizeForNetworkUse on HybridFrameRecorder. With enablePersistentRecorder unset, HybridCameraVideoOutput.createRecorder builds HybridVideoRecorder, which wraps AVCaptureMovieFileOutput — no faststart API at all. HybridFrameRecorder is only instantiated by HybridCameraVideoFrameOutput. Recordings still measure moov at 99.9%.
2. targetBitRate: 5_000_000 still is not landing: 8.24 Mbps at 1080p, which is close to the default rate scaled by the pixel drop (28.8 / 4 = 7.2).

Verification

• tsc --noEmit clean, 140 tests pass, expo lint 0 errors.
• The decision logic is covered by unit tests against real measured probe values.
• Not verified on a device. I have been removed from the PulseCam Dev TestFlight group, so I cannot install a build carrying this. That is the main thing missing before it should merge, and I would rather say so than imply otherwise.

Draft while that is outstanding. Happy to drop any part of this if you would rather close the remaining breaches at the source.

[morepriyam]

Both findings check out against the VisionCamera 5.2.0 source — thank you for measuring this properly.

1. The faststart patch was dead code, confirmed. HybridCameraVideoOutput.createRecorder returns HybridVideoRecorder, which wraps AVCaptureMovieFileOutput — no shouldOptimizeForNetworkUse (or any faststart) API exists on it. The patched HybridFrameRecorder is only instantiated by HybridCameraVideoFrameOutput, which this app doesn't record through. The patch is removed from feat: standardize pipeline on H.264 for web playback #143 and the in-code comments now state that raw per-clip files remain moov-at-end.
2. The bitrate-not-landing mechanism is visible in the source. HybridCameraVideoOutput.configure silently returns when output.connection(with: .video) is nil, and setBitRate bails on the supportedOutputSettingsKeys guard with only a log — and both run inside the beginConfiguration/commitConfiguration batch before the session is live. Your 8.24 Mbps ≈ default-scaled-by-pixels reading is consistent with the settings never being applied. Also relevant: VideoOutputSettings exposes only codec, so there is no JS-side path to re-apply the bitrate post-ready the way feat: standardize pipeline on H.264 for web playback #143 does for the codec.

Net: #143 keeps the codec fix at the source (its setOutputSettings runs post-cameraReady, when the connection exists) and drops every claim it can't back; this gate is the right owner of bitrate + faststart enforcement for raw clips, with mieweb/pulsevault#58 as the server-side backstop for anything that slips past both. The three compose cleanly.

Re device verification: let's get your TestFlight access restored so this can come out of draft.

[Copilot]

Pull request overview

Adds an upload-side enforcement gate to ensure every video leaving the device meets a strict playback/upload contract (H.264, <=1920 long edge, <=5 Mbps target, AAC), complementing recorder-side configuration by probing and conditionally re-encoding only when a clip breaches the contract.

Changes:

• Introduces a pure, unit-tested upload contract policy and a runtime “probe → decide → (maybe) transcode” gate, applied to both merged and segmented upload paths.
• Tightens/standardizes import normalization to guarantee H.264 across the pipeline and improves e2e expectations for audio-less sources.
• Improves operational robustness: stable per-draft conditioned segment output paths for resumable uploads; adds a VisionCamera iOS patch via patch-package and logs negotiated capture format + import-normalization failures.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/utils/upload-contract.ts	Defines the upload contract decision policy and bitrate derivation logic.
src/utils/upload-contract.test.ts	Unit tests covering decision logic, including “real upload” probes.
src/utils/ensure-upload-contract.ts	Implements the runtime enforcement (probe + conditional transcode) and cached/stable output for segments.
src/utils/file-store.ts	Adds stable drafts/{draftId}/upload/ destination for conditioned segment copies.
src/features/upload/upload-manager.ts	Applies the contract gate to both merged and segmented upload units and persists conditioned merged paths.
src/utils/import-normalization.ts	Exports shared helpers/constants; changes import normalization to allow passthrough only for H.264; explicitly sets codec on re-encode.
src/utils/import-normalization.test.ts	Updates fixture expectations reflecting HEVC import normalization.
src/utils/import-pipeline.e2e.test.ts	Updates e2e expectations (HEVC fixtures now re-encode; audio-less outputs remain audio-less).
src/features/recorder/use-recorder.ts	Logs negotiated capture resolution; forces H.264 on iOS video output; makes import-normalization failures visible.
src/app/recorder.tsx	Adds explicit resolutionBias ordering to strengthen 1080p negotiation.
patches/react-native-vision-camera+5.2.0.patch	Patch-package patch to enable shouldOptimizeForNetworkUse for the frame recorder writer.
package.json	Adds patch-package and runs it on postinstall; bumps react-native-video-trim git ref.
package-lock.json	Lockfile updates for patch-package and updated react-native-video-trim ref.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[morepriyam]

Working on the tests for these changes now, and verifying the whole thing works end to end — device runs for the capture/upload paths (codec, bitrate, moov placement) plus browser playback checks, across #143, #142, and the pulsevault backstop. Will report results here.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/utils/ensure-upload-contract.ts:55

• ensureUploadContract calls probeVideo(path) with the raw path string. On Android, merged.path can be a bare filesystem path (see upload-manager.ts:602-604), and other call sites normalize to file:// before using native APIs. Normalizing here avoids probing failing open and silently skipping enforcement on Android.

export async function ensureUploadContract(path: string): Promise<ContractResult> {
  const probe = await probeVideo(path).catch((e: unknown) => {
    console.warn('[contract] probe failed; uploading the original', e);
    return null;
  });
  if (!probe) {
    return { path, changed: false, reasons: [], failure: 'could not probe the file' };
  }

  const decision = decideUploadContract(probe);
  if (decision.action === 'passthrough') {
    return { path, changed: false, reasons: [] };
  }

src/utils/ensure-upload-contract.ts:73

• After normalizing the input to a file:// URI, compress() should also use the normalized URI (not the original path) to avoid platform-dependent failures. Additionally, returning a normalized output path keeps the ContractResult.path contract of being immediately usable by Expo File.

  const result = await compress(path, { ...decision.options, outputExt: 'mp4' }).catch(
    (e: unknown) => {
      console.warn('[contract] re-encode failed; uploading the original', decision.reasons, e);
      return null;
    },
  );
  if (!result) {
    return {
      path,
      changed: false,
      reasons: decision.reasons,
      failure: `could not re-encode (${decision.reasons.join(', ')})`,
    };
  }

  return { path: result.outputPath, changed: true, reasons: decision.reasons };
}

src/utils/ensure-upload-contract.ts:106

• If uploadDest(draftId, segmentId) already exists but is empty/corrupt (e.g. prior crash during move), File.move(dest) may fail because the destination exists. Deleting any existing dest before moving avoids getting stuck in a retry loop where the corrupted file prevents parking the conditioned bytes.

  try {
    // compress() writes into the OS-purgeable cache dir; move it somewhere a resume can find it.
    await new File(toFileUri(result.path)).move(dest);
    return { ...result, path: dest.uri };

src/utils/upload-contract.ts:12

• The module doc defines the upload contract as including faststart, but the decision logic explicitly cannot observe/enforce moov placement (and there is no faststart-specific step in the gate). As written, a clip can be considered compliant (or normalized) while still being moov-at-end. Please clarify where faststart is actually enforced, or add an explicit faststart remux step in the upload gate.

 * Everything PulseCam uploads must satisfy:
 *
 *     H.264 · long edge <= 1920 · <= 5 Mbps · AAC · faststart
 *
 * This is not a new target — it is what the recorder already asks for

[morepriyam]

Reviewed and verified this against #143 locally.

Compose check. I fetched pull/142/head and rebased it onto #143's head. Zero conflicts, exactly as claimed. On the combined branch, tsc --noEmit is clean and all 156 tests pass, including the e2e suite with real ffmpeg. The two PRs really are complementary with no code overlap.

Your findings check out. I verified the HybridFrameRecorder analysis against the vision-camera source. createRecorder does wrap AVCaptureMovieFileOutput, which has no faststart API at all, so that patch was dead code. #143 dropped it and now documents that raw per-clip files stay moov-at-end, with faststart handled by the merge layer, this gate, and the pulsevault backstop.

One measurement update in your favor. #143 now bumps vision-camera to 5.2.2, because 5.2.0's setOutputSettings crashed the whole app with an uncatchable NSException (upstream mrousavy/react-native-vision-camera#4037, fixed in #4081). A side effect of that fix is that codec and bitrate get written together in one minimal dict, so the 5 Mbps target actually lands now. A 2-segment trim and merge from an iPhone 17 Pro Max probes h264, 1080p, 5.50 Mbps, AAC, faststart. On current builds a compliant file costs this gate exactly one probe, which is the end state you described. The gate is still worth having for the exact reason you argued: it catches the next setting that silently stops being honored.

Landing order stands: #143 first, then this rebases and lands as the enforcement layer. LGTM.