From cc2795c9f32a3ac24ffdf2f82585c15662898b26 Mon Sep 17 00:00:00 2001 From: Jonathan Locala Date: Mon, 3 Aug 2026 15:45:30 -0400 Subject: [PATCH 1/2] fix(upload): enforce the upload contract before a video leaves the device Recordings reached the server at 3840x2160 / ~28 Mbps HEVC with the index at the end of the file, while the recorder's own config asked for 1080p / 5 Mbps. Phones could not play them back. Measured on real clips from two builds, same phone: 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 already fixed upstream. `targetResolution` is not a setting but a bias in a weighted vote: VisionCamera scores every camera format against all outputs, and ranks its preview output above ours. The preview wants a format at least as large as the screen, every modern iPhone screen is taller than 1920px, and a 1080p format therefore took an aspect-ratio penalty weighted 100x. 4K won by roughly 20x. VisionCamera 5.2.0 rescored that comparison and 1080p now wins on its own -- confirmed on a device, above. So this does three things, in increasing order of how much they can be trusted: - Name the video output's resolutionBias explicitly, first in the constraint list. This is no longer the fix, it is margin: 5.2.0 wins by ~13% on the largest screens, and an explicit bias makes it ~6x. - Log the negotiated capture format on session start and warn when it breaches, so a lost vote can never again go unnoticed for weeks. - Gate uploads on the contract itself: probe, and re-encode only on a breach. A compliant file costs one probe and is uploaded untouched. The gate is the part that matters, and the 07-29 numbers are why: resolution is fixed, but bitrate is still 65% over a pin that has been set since 07-20, the codec is still HEVC (undecodable in Android Chrome), and the file is still not faststart. Settings are requests that a subsystem may ignore -- targetResolution did for weeks, targetBitRate and fileType still do. A gate does not ask. The gate sits in the upload path, not the export path: Share and Save-to-Photos keep full capture quality. The conditioned path is persisted so an after-kill resume re-uploads the same bytes rather than a fresh encode. Also stop swallowing import normalization failures: the fallback to the original bytes is correct, being silent about it is not. --- src/app/recorder.tsx | 15 +- src/features/recorder/use-recorder.ts | 43 +++++- src/features/upload/upload-manager.ts | 29 +++- src/utils/ensure-upload-contract.ts | 71 +++++++++ src/utils/import-normalization.ts | 8 +- src/utils/upload-contract.test.ts | 201 ++++++++++++++++++++++++++ src/utils/upload-contract.ts | 178 +++++++++++++++++++++++ 7 files changed, 536 insertions(+), 9 deletions(-) create mode 100644 src/utils/ensure-upload-contract.ts create mode 100644 src/utils/upload-contract.test.ts create mode 100644 src/utils/upload-contract.ts diff --git a/src/app/recorder.tsx b/src/app/recorder.tsx index be54715..0bc6a51 100644 --- a/src/app/recorder.tsx +++ b/src/app/recorder.tsx @@ -276,9 +276,20 @@ export default function RecorderScreen() { }, [device]); // Pinned 1080p output + 30fps so every recorded clip is format-uniform (fast-path merge). + // + // `resolutionBias` FIRST, deliberately. VisionCamera picks the capture format by scoring every + // format against a weighted list of constraints — weight is `count - index`, so earlier entries + // outrank later ones — and it auto-appends one `{ resolutionBias: output }` per output, in the + // order the outputs are given. `` puts its own preview output ahead of ours, and the + // preview asks for a format at least as large as the SCREEN. Every modern iPhone screen is taller + // than 1920px, so 1080p could never satisfy the preview and the vote elected 4K: clips shipped at + // 3840x2160 / ~23 Mbps while `useVideoOutput` below asked for 1080p / 5 Mbps. VisionCamera 5.2.0 + // rescored that case so 1080p wins on its own, but only by ~13% on the largest screens — naming + // the video output's bias explicitly, at the top of the list, turns a margin into a mandate. + // See `logNegotiatedResolution` in use-recorder.ts for the runtime check that this held. const constraints = useMemo( - () => [{ videoStabilizationMode: stabilization }, { fps: 30 }], - [stabilization], + () => [{ resolutionBias: videoOutput }, { videoStabilizationMode: stabilization }, { fps: 30 }], + [stabilization, videoOutput], ); const outputs = useMemo(() => [videoOutput], [videoOutput]); diff --git a/src/features/recorder/use-recorder.ts b/src/features/recorder/use-recorder.ts index 766b903..409c544 100644 --- a/src/features/recorder/use-recorder.ts +++ b/src/features/recorder/use-recorder.ts @@ -33,6 +33,35 @@ import { generateThumbnailFile, getDurationMs } from '@/utils/video'; import CallDetector from '../../../modules/expo-call-detector/src/CallDetectorModule'; import { useCallState } from './use-call-state'; +import { UPLOAD_MAX_LONG_EDGE } from '@/utils/upload-contract'; + +/** + * Report what the camera session ACTUALLY negotiated, and shout if it isn't what we asked for. + * + * `targetResolution` on the video output is a bias in a weighted vote across all outputs, not a + * setting — the preview output's preference outranked it and every clip came out 4K while the + * config said 1080p, silently, for weeks. The failure mode of a lost vote is indistinguishable + * from success unless something looks. This looks. + * + * Cheap (one property read on session start) and non-fatal by design: a device that can only + * offer something larger should still record, it just shouldn't do so unnoticed. + */ +function logNegotiatedResolution(output: { + currentResolution?: { width: number; height: number }; +}) { + const size = output.currentResolution; + if (!size) return; + const longEdge = Math.max(size.width, size.height); + if (longEdge > UPLOAD_MAX_LONG_EDGE) { + console.warn( + `[recorder] capture format negotiated to ${size.width}x${size.height}, above the ${UPLOAD_MAX_LONG_EDGE} ` + + `long-edge target — clips will be re-encoded before upload. Check the resolutionBias constraint order.`, + ); + return; + } + console.log(`[recorder] capture format: ${size.width}x${size.height}`); +} + // 'cinematic' is an iOS-only AVCaptureVideoStabilizationMode — CameraX has no equivalent, so // Android only cycles through the modes it can actually honor. The union type keeps 'cinematic' // on both platforms so persisted iOS prefs and shared UI maps still typecheck. @@ -365,7 +394,13 @@ export function useRecorder(initialDraftId?: string) { if (probe) { const decision = decideImport(probe); if (decision.action === 'normalize') { - const normalized = await compress(picked.uri, decision.options).catch(() => null); + const normalized = await compress(picked.uri, decision.options).catch((e: unknown) => { + // Falling back to the original bytes is the right call — a failed normalize should not + // block the import — but it must not be SILENT. This is how a 4K HDR master enters a + // draft looking exactly like a clip that was normalized successfully. + console.warn('[import] normalize failed; importing the original', decision.reasons, e); + return null; + }); if (normalized) { normalizedPath = normalized.outputPath; sourceUri = normalized.outputPath; @@ -472,7 +507,11 @@ export function useRecorder(initialDraftId?: string) { callActive, appActive, reportMicPriorityError, - onCameraReady: () => setCameraReady(true), + onCameraReady: () => { + // The session has started, so the output is attached and its negotiated format is readable. + logNegotiatedResolution(videoOutput); + setCameraReady(true); + }, toggleRecording, finalizeRecording, importClip: () => void importClip(), diff --git a/src/features/upload/upload-manager.ts b/src/features/upload/upload-manager.ts index 281d12a..606e13a 100644 --- a/src/features/upload/upload-manager.ts +++ b/src/features/upload/upload-manager.ts @@ -20,6 +20,7 @@ import { getDraftToken } from '@/db/secure-token'; import { getDraftTranscriptRow } from '@/db/transcripts'; import { linesToVtt } from '@/features/transcription/vtt'; import { parseTranscriptLines } from '@/features/transcription/whisper'; +import { ensureUploadContract } from '@/utils/ensure-upload-contract'; import { absolutize, toFileUri } from '@/utils/file-store'; import { effFile } from '@/utils/segment-window'; import { generateThumbnailFile } from '@/utils/video'; @@ -578,6 +579,26 @@ class BackgroundUploadManager { private async uploadMerged(session: UploadSession, signal: AbortSignal): Promise { const { draftId, destination, segments, merged } = session; if (!merged) throw new Error('Export is not ready yet'); + + // Bring the video into the upload contract (H.264 / <=1920 long edge / <=5 Mbps / AAC) before + // anything reads its bytes. A compliant file costs one probe and is returned untouched; only a + // breach pays for a re-encode. This is the gate, NOT the export step: Share and Save-to-Photos + // read `state.outputPath` directly and must keep full capture quality — only what leaves the + // device for a browser to play is constrained. + const contract = await ensureUploadContract(merged.path); + if (contract.changed) { + // Persist the conditioned path so an after-kill resume re-uploads the SAME bytes. Without + // this, resume would re-encode from the original and a TUS PATCH could continue a transfer + // with bytes from a different encode. (Re-running the gate on an already-conditioned file is + // a no-op passthrough, so this is a cost saving as well as a correctness one.) + merged.path = contract.path; + await setUploadMerged(draftId, merged); + } + if (contract.failure) { + // Fail open, loudly: the upload proceeds with the original bytes, but it is not silent. + console.warn(`[contract] uploading unconditioned video for ${draftId}: ${contract.failure}`); + } + // merged.path is a bare filesystem path on Android (RNVT) — normalize to a file:// URI or the // File API rejects it outright ("URI is not absolute"). const file = new File(toFileUri(merged.path)); @@ -652,7 +673,13 @@ class BackgroundUploadManager { const result = await this.uploadOne( draftId, destination, - { artifactId: destination.artifactId, filename: `${draftId}.mp4`, kind: 'video', name: draftName, file }, + { + artifactId: destination.artifactId, + filename: `${draftId}.mp4`, + kind: 'video', + name: draftName, + file, + }, destination.resourceUrl, checksum, signal, diff --git a/src/utils/ensure-upload-contract.ts b/src/utils/ensure-upload-contract.ts new file mode 100644 index 0000000..125c041 --- /dev/null +++ b/src/utils/ensure-upload-contract.ts @@ -0,0 +1,71 @@ +import { compress, probeVideo } from 'react-native-video-trim'; + +import { decideUploadContract } from './upload-contract'; + +/** + * What conditioning did to a file on its way to being uploaded. + * + * `path` is always usable — on any failure it falls back to the input, so a broken probe + * or a failed encode degrades to "upload the original", never to "upload nothing". + */ +export type ContractResult = { + /** The file to upload: the conditioned copy, or the input when nothing was needed. */ + path: string; + /** True when `path` differs from the input. */ + changed: boolean; + /** Human-readable contract breaches that triggered the re-encode, for logging/UI. */ + reasons: string[]; + /** + * Set when the file could NOT be brought into the contract and the original is being + * uploaded instead. Never silently empty — the whole point of this gate is that a + * failure is visible. (`importClip` swallows exactly this case today, which is how a + * 4K master can still enter a draft.) + */ + failure?: string; +}; + +/** + * Bring a file into the upload contract before it is uploaded: probe it, and re-encode + * only if it breaches (see {@link decideUploadContract}). + * + * A compliant file is returned untouched — no copy, no re-encode, no quality generation + * spent. That is the intended steady state once the recorder emits 1080p/5 Mbps: this + * gate costs one probe and nothing else. It exists for the case where the recorder's + * format negotiation loses on some device we have not tested, or an import slips a 4K + * master through — outcomes we cannot prevent, only catch. + * + * Failure policy is deliberately "fail open, loudly": an upload that happens at reduced + * quality is better than an upload that does not happen, but it must be reported rather + * than absorbed. + */ +export async function ensureUploadContract(path: string): Promise { + 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: [] }; + } + + 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 }; +} diff --git a/src/utils/import-normalization.ts b/src/utils/import-normalization.ts index bd37635..811a500 100644 --- a/src/utils/import-normalization.ts +++ b/src/utils/import-normalization.ts @@ -38,7 +38,7 @@ export const NORMALIZE_MAX_BITRATE = 8_000_000; /** Video codecs the merge pipeline handles natively (hardware decode on both platforms). */ const NATIVE_VIDEO_CODECS = new Set(['h264', 'hevc']); /** HDR transfer functions: HLG (iPhone camera default) and PQ (HDR10 / Dolby Vision 8.x). */ -const HDR_TRANSFERS = new Set(['arib-std-b67', 'smpte2084']); +export const HDR_TRANSFERS = new Set(['arib-std-b67', 'smpte2084']); export type ImportDecision = | { action: 'passthrough' } @@ -49,17 +49,17 @@ export type ImportDecision = * suffix (yuv420p10le, p010le, ...) — matching the suffix rather than a bare `includes('10')` * keeps 8-bit chroma-subsampling names like `yuv410p` from being misclassified. */ -function is10Bit(pixelFormat: string): boolean { +export function is10Bit(pixelFormat: string): boolean { return /10(le|be)?$/.test(pixelFormat); } /** Effective fps for the decision: average when known (catches VFR), else nominal. */ -function effectiveFps(probe: VideoProbeResult): number { +export function effectiveFps(probe: VideoProbeResult): number { return probe.averageFps > 0 ? probe.averageFps : probe.nominalFps; } /** Display (post-rotation) dimensions: a 90/270 rotation swaps coded width/height. */ -function displaySize(probe: VideoProbeResult): { width: number; height: number } { +export function displaySize(probe: VideoProbeResult): { width: number; height: number } { const swapped = probe.rotation % 180 !== 0; return { width: swapped ? probe.height : probe.width, diff --git a/src/utils/upload-contract.test.ts b/src/utils/upload-contract.test.ts new file mode 100644 index 0000000..3e8ee0f --- /dev/null +++ b/src/utils/upload-contract.test.ts @@ -0,0 +1,201 @@ +import { describe, expect, it } from '@jest/globals'; +import type { VideoProbeResult } from 'react-native-video-trim'; + +import { + decideUploadContract, + effectiveBitrate, + UPLOAD_MAX_LONG_EDGE, + UPLOAD_TARGET_BITRATE, + UPLOAD_TARGET_FPS, +} from './upload-contract'; + +/** A clip that already satisfies the contract; override one field to test one rule. */ +function probe(overrides: Partial = {}): VideoProbeResult { + return { + hasVideo: true, + videoCodec: 'h264', + width: 1920, + height: 1080, + rotation: 0, + nominalFps: 30, + averageFps: 30, + bitrate: 5_000_000, + pixelFormat: 'yuv420p', + colorTransfer: 'bt709', + hasAudio: true, + audioCodec: 'aac', + audioSampleRate: 48000, + audioChannels: 2, + duration: 8000, + fileSize: 5_000_000, + ...overrides, + }; +} + +/** + * The two PulseCam uploads that were actually sitting on the dev box, measured with + * ffprobe. These are the files that would not play on a phone — the regression test + * for this whole gate is that both of them get normalized. + */ +const REAL_UPLOADS: Record = { + '669b7a78 (343MB, 110s)': probe({ + videoCodec: 'hevc', + width: 3840, + height: 2160, + bitrate: 24_761_167, + duration: 110_588, + fileSize: 343_727_447, + }), + 'a9dd0919 (579MB, 166s)': probe({ + videoCodec: 'hevc', + width: 3840, + height: 2160, + bitrate: 27_831_557, + duration: 165_821, + fileSize: 579_118_404, + }), +}; + +describe('decideUploadContract', () => { + describe('the real 4K HEVC uploads that broke phone playback', () => { + for (const [name, p] of Object.entries(REAL_UPLOADS)) { + it(`normalizes ${name} on every count`, () => { + const decision = decideUploadContract(p); + expect(decision.action).toBe('normalize'); + if (decision.action !== 'normalize') return; + + expect(decision.reasons).toEqual( + expect.arrayContaining([ + expect.stringContaining('video codec hevc'), + expect.stringContaining('3840x2160'), + expect.stringContaining('Mbps'), + ]), + ); + expect(decision.options).toMatchObject({ + codec: 'h264', + bitrate: UPLOAD_TARGET_BITRATE, + frameRate: UPLOAD_TARGET_FPS, + width: UPLOAD_MAX_LONG_EDGE, + }); + // Landscape source: the long edge is pinned via width, height follows the aspect ratio. + expect(decision.options.height).toBeUndefined(); + }); + } + }); + + it('passes a compliant clip through untouched — the whole point of the recorder pin', () => { + expect(decideUploadContract(probe())).toEqual({ action: 'passthrough' }); + }); + + it('converts HEVC that is otherwise perfect (this is where it diverges from imports)', () => { + const decision = decideUploadContract(probe({ videoCodec: 'hevc' })); + expect(decision.action).toBe('normalize'); + if (decision.action !== 'normalize') return; + expect(decision.reasons).toEqual(['video codec hevc']); + // Nothing is wrong with the geometry, so no scaling is requested. + expect(decision.options.width).toBeUndefined(); + expect(decision.options.height).toBeUndefined(); + expect(decision.options.codec).toBe('h264'); + }); + + it('pins the long edge by HEIGHT for a rotated (portrait) 4K clip, not width', () => { + // 3840x2160 coded + 90deg rotation displays as 2160x3840 — portrait. + const decision = decideUploadContract( + probe({ videoCodec: 'hevc', width: 3840, height: 2160, rotation: 90, bitrate: 24_000_000 }), + ); + expect(decision.action).toBe('normalize'); + if (decision.action !== 'normalize') return; + expect(decision.options.height).toBe(UPLOAD_MAX_LONG_EDGE); + expect(decision.options.width).toBeUndefined(); + expect(decision.reasons).toEqual( + expect.arrayContaining([expect.stringContaining('2160x3840')]), + ); + }); + + it('accepts a clip sitting exactly on the long-edge cap, and rejects one pixel over', () => { + expect(decideUploadContract(probe({ width: 1920, height: 1080 })).action).toBe('passthrough'); + expect(decideUploadContract(probe({ width: 1921, height: 1080 })).action).toBe('normalize'); + }); + + it('leaves a slightly-over-target bitrate alone rather than burning a generation on 7%', () => { + expect(decideUploadContract(probe({ bitrate: 5_400_000 })).action).toBe('passthrough'); + expect(decideUploadContract(probe({ bitrate: 8_000_000 })).action).toBe('normalize'); + }); + + it('caps high frame rates', () => { + expect(decideUploadContract(probe({ nominalFps: 60, averageFps: 60 })).action).toBe( + 'normalize', + ); + // 29.97 NTSC must pass untouched. + expect(decideUploadContract(probe({ nominalFps: 29.97, averageFps: 29.97 })).action).toBe( + 'passthrough', + ); + }); + + it('normalizes 10-bit and HDR sources', () => { + expect(decideUploadContract(probe({ pixelFormat: 'yuv420p10le' })).action).toBe('normalize'); + expect(decideUploadContract(probe({ colorTransfer: 'arib-std-b67' })).action).toBe('normalize'); + expect(decideUploadContract(probe({ colorTransfer: 'smpte2084' })).action).toBe('normalize'); + // 8-bit format whose name merely contains "10". + expect(decideUploadContract(probe({ pixelFormat: 'yuv410p' })).action).toBe('passthrough'); + }); + + it('conforms audio only, copying the video, when just the audio codec is wrong', () => { + const decision = decideUploadContract(probe({ audioCodec: 'opus' })); + expect(decision.action).toBe('normalize'); + if (decision.action !== 'normalize') return; + expect(decision.options).toEqual({ copyVideo: true }); + expect(decision.reasons).toEqual(['audio codec opus']); + }); + + it('does a full re-encode (not a video copy) when audio AND video are both wrong', () => { + const decision = decideUploadContract(probe({ videoCodec: 'hevc', audioCodec: 'opus' })); + expect(decision.action).toBe('normalize'); + if (decision.action !== 'normalize') return; + expect(decision.options.copyVideo).toBeUndefined(); + expect(decision.options.codec).toBe('h264'); + expect(decision.reasons).toEqual(['video codec hevc', 'audio codec opus']); + }); + + it('ignores audio entirely on a silent clip', () => { + expect(decideUploadContract(probe({ hasAudio: false, audioCodec: '' })).action).toBe( + 'passthrough', + ); + }); + + it('passes through anything with no video stream', () => { + expect(decideUploadContract(probe({ hasVideo: false })).action).toBe('passthrough'); + }); +}); + +describe('effectiveBitrate', () => { + it('prefers the declared stream bitrate', () => { + expect(effectiveBitrate(probe({ bitrate: 4_000_000 }))).toBe(4_000_000); + }); + + it('derives from size and duration when the container declares nothing', () => { + // 10 MB over 10 s = 8 Mbps. + expect( + effectiveBitrate(probe({ bitrate: -1, fileSize: 10_000_000, duration: 10_000 })), + ).toBeCloseTo(8_000_000); + }); + + it('reports unknown rather than guessing when neither source is usable', () => { + expect(effectiveBitrate(probe({ bitrate: -1, fileSize: 0, duration: 0 }))).toBe(-1); + }); + + it('does not treat an undeclared bitrate as a reason to re-encode', () => { + expect(decideUploadContract(probe({ bitrate: -1, fileSize: 0, duration: 0 }))).toEqual({ + action: 'passthrough', + }); + }); + + it('catches a 4K master whose container declares no bitrate, via size/duration', () => { + const decision = decideUploadContract( + probe({ bitrate: -1, fileSize: 343_727_447, duration: 110_588, videoCodec: 'h264' }), + ); + expect(decision.action).toBe('normalize'); + if (decision.action !== 'normalize') return; + expect(decision.reasons).toEqual(expect.arrayContaining([expect.stringContaining('Mbps')])); + }); +}); diff --git a/src/utils/upload-contract.ts b/src/utils/upload-contract.ts new file mode 100644 index 0000000..3fd36c8 --- /dev/null +++ b/src/utils/upload-contract.ts @@ -0,0 +1,178 @@ +import type { CompressOptions, VideoProbeResult } from 'react-native-video-trim'; + +import { displaySize, effectiveFps, HDR_TRANSFERS, is10Bit } from './import-normalization'; + +/** + * The upload contract (§ playback). + * + * 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 + * (`useVideoOutput` in use-recorder.ts) and what PulseClip's own exports already + * produce. The difference is that this module *enforces* it. + * + * Why enforcement is needed at all, measured on real recordings from two builds: + * + * 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 — VisionCamera 5.2.0 rescored the format vote that the + * recorder's `targetResolution` participates in, so 1080p now wins it. Everything else + * still breaches: the bitrate lands 65% over a pin that has been set since 07-20, the + * codec is HEVC (fine on iOS, undecodable in Android Chrome), and the index is written + * at the end of the file. + * + * That is the case for a gate rather than a set of settings. `targetResolution` was + * silently ignored for weeks; `targetBitRate` and `fileType` still are. Each is a + * request to a subsystem that may or may not honour it, on hardware we have not tested. + * A probe-and-transcode gate does not ask. + * + * So: the recorder pin makes the common case FREE (a compliant file passes through + * untouched), and this gate makes every case CORRECT. Keep both. Neither replaces + * the other. + * + * How this differs from {@link decideImport}, which enforces a similar-looking policy + * at the Photos-import boundary: + * + * - **HEVC is not acceptable here.** Imports may keep it (iOS decodes it natively and + * the merge engine's fast path likes format-uniform clips), but an upload is watched + * in a browser, and Android Chrome will not decode HEVC at any size. Uploads convert. + * - **The bitrate ceiling is tighter** — an upload is streamed over a phone network, + * not read off local flash. + * + * Faststart is deliberately absent from the decision below: `moov` placement is not + * visible in a `probeVideo()` result. It is guaranteed on the writing side instead — + * the export/merge/compress paths emit it — because a file that is otherwise compliant + * should not be re-encoded just to move its index. + */ + +/** Long-edge cap. A 1080p long edge is 4x fewer pixels than 4K — the single biggest win. */ +export const UPLOAD_MAX_LONG_EDGE = 1920; +/** Re-encode target when a clip breaches the contract. Matches the recorder's own pin. */ +export const UPLOAD_TARGET_BITRATE = 5_000_000; +/** + * Re-encode trigger, deliberately above {@link UPLOAD_TARGET_BITRATE}. A clip that is + * already close to target is left alone: re-encoding 5.4 Mbps down to 5.0 costs a full + * transcode and a generation of quality to save ~7% of the bytes. Only a real breach + * (a 4K master at 23 Mbps) is worth the pass. + */ +export const UPLOAD_MAX_BITRATE = 6_500_000; +/** Frame-rate ceiling: passes 29.97/30 with margin, catches 60/120 (slo-mo, screen caps). */ +export const UPLOAD_MAX_FPS = 33; +/** Re-encode target frame rate. */ +export const UPLOAD_TARGET_FPS = 30; +/** The one codec that plays everywhere the browser lane cares about, Android Chrome included. */ +export const UPLOAD_VIDEO_CODEC = 'h264'; +/** The one audio codec that is MP4-muxable by stream copy across our paths. */ +export const UPLOAD_AUDIO_CODEC = 'aac'; + +export type UploadContractDecision = + | { action: 'passthrough' } + | { action: 'normalize'; options: Partial; reasons: string[] }; + +/** + * Effective video bitrate in bits per second. + * + * `probe.bitrate` is the *stream* bitrate and is `-1` when the container does not + * declare one — which is exactly the case for some camera-written MP4s, i.e. the files + * this gate exists to catch. Falling back to size/duration slightly overstates the + * video rate (it includes audio and container overhead), but it overstates in the safe + * direction: it can only push a borderline file towards being normalized, never away. + * + * Returns `-1` when neither source is usable, which the caller treats as "unknown" — + * an unknown bitrate is not by itself grounds to re-encode. + */ +export function effectiveBitrate(probe: VideoProbeResult): number { + if (probe.bitrate > 0) return probe.bitrate; + const seconds = probe.duration / 1000; + if (seconds > 0 && probe.fileSize > 0) return (probe.fileSize * 8) / seconds; + return -1; +} + +/** + * Decide how a file must be conditioned before it is uploaded: send the original bytes, + * conform only its audio, or re-encode it into the contract. + * + * Pure — feed it a `probeVideo()` result. The caller (`ensureUploadContract`) owns the + * file I/O; keeping the policy pure is what makes it testable without a device. + */ +export function decideUploadContract(probe: VideoProbeResult): UploadContractDecision { + // No video stream to constrain (audio-only artifacts ride other paths). Nothing to do. + if (!probe.hasVideo) return { action: 'passthrough' }; + + const reasons: string[] = []; + + if (probe.videoCodec !== UPLOAD_VIDEO_CODEC) { + // Includes HEVC, which is the common case: it is what the iPhone records by default, + // it is fine on iOS, and it is undecodable in Android Chrome. + reasons.push(`video codec ${probe.videoCodec || 'unknown'}`); + } + + const display = displaySize(probe); + const longEdge = Math.max(display.width, display.height); + const needsDownscale = longEdge > UPLOAD_MAX_LONG_EDGE; + if (needsDownscale) { + reasons.push(`${display.width}x${display.height} exceeds ${UPLOAD_MAX_LONG_EDGE}`); + } + + const bitrate = effectiveBitrate(probe); + if (bitrate > UPLOAD_MAX_BITRATE) { + reasons.push( + `${(bitrate / 1_000_000).toFixed(1)} Mbps exceeds ${UPLOAD_MAX_BITRATE / 1_000_000}`, + ); + } + + const fps = effectiveFps(probe); + if (fps > UPLOAD_MAX_FPS) { + reasons.push(`${Math.round(fps)} fps exceeds ${UPLOAD_MAX_FPS}`); + } + + // 10-bit / HDR: hardware H.264 encoders reject 10-bit input, and an HDR clip tone-maps + // unpredictably in a browser. Both force the SDR 8-bit re-encode. + if (is10Bit(probe.pixelFormat)) { + reasons.push(`10-bit pixel format ${probe.pixelFormat}`); + } + if (HDR_TRANSFERS.has(probe.colorTransfer)) { + reasons.push(`HDR transfer ${probe.colorTransfer}`); + } + + const audioHostile = probe.hasAudio && probe.audioCodec !== UPLOAD_AUDIO_CODEC; + + if (reasons.length === 0) { + if (audioHostile) { + // Video already satisfies the contract — stream-copy it and pay only for the audio. + return { + action: 'normalize', + options: { copyVideo: true }, + reasons: [`audio codec ${probe.audioCodec}`], + }; + } + return { action: 'passthrough' }; + } + + if (audioHostile) { + reasons.push(`audio codec ${probe.audioCodec}`); + } + + const options: Partial = { + codec: UPLOAD_VIDEO_CODEC, + bitrate: UPLOAD_TARGET_BITRATE, + frameRate: UPLOAD_TARGET_FPS, + }; + if (needsDownscale) { + // FFmpeg auto-rotates before filters run, so the cap is applied against DISPLAY + // orientation: pin the long edge, let the other follow the aspect ratio (-2). Pinning + // width unconditionally would upscale a portrait clip to 1920 wide — 4x the pixels + // the contract is trying to remove. + if (display.width >= display.height) { + options.width = UPLOAD_MAX_LONG_EDGE; + } else { + options.height = UPLOAD_MAX_LONG_EDGE; + } + } + + return { action: 'normalize', options, reasons }; +} From b0bfdc831f9e4a4f9384eaca745f87d6842b1768 Mon Sep 17 00:00:00 2001 From: Jonathan Locala Date: Tue, 4 Aug 2026 12:28:01 -0400 Subject: [PATCH 2/2] fix(upload): apply the contract to segment uploads too The gate only ran in uploadMerged. uploadSegments sent every clip exactly as recorded. That mattered because `uploadUnit` is chosen by the DESTINATION SERVER, not by the app -- the pairing decides whether a draft uploads as one merged video or as separate clips. So a server asking for segments silently turned the contract off: no warning, no log, the protection simply did not run. Same shape as the bug this whole change exists for -- a safeguard that looks present on every path and isn't. Conditioning for segments goes to a stable path (drafts/{id}/upload/{seg}.mp4) rather than a cache temp name, because segment uploads resume byte-wise via TUS HEAD + PATCH: a resumed run has to send the bytes it began with, and re-encoding on resume would splice a second, subtly different encode into a half-finished transfer. A fixed path means a resumed run finds what it already produced. It lives inside the draft dir so deleteDraftDir reclaims it, and beside segments/ rather than in it so it can never be taken for a clip. The merged path solves the same problem by persisting its conditioned path in the draft row, which segments have no column for. --- src/features/upload/upload-manager.ts | 17 +++++++++-- src/utils/ensure-upload-contract.ts | 42 +++++++++++++++++++++++++++ src/utils/file-store.ts | 25 ++++++++++++++-- 3 files changed, 80 insertions(+), 4 deletions(-) diff --git a/src/features/upload/upload-manager.ts b/src/features/upload/upload-manager.ts index 606e13a..bdbd465 100644 --- a/src/features/upload/upload-manager.ts +++ b/src/features/upload/upload-manager.ts @@ -20,7 +20,7 @@ import { getDraftToken } from '@/db/secure-token'; import { getDraftTranscriptRow } from '@/db/transcripts'; import { linesToVtt } from '@/features/transcription/vtt'; import { parseTranscriptLines } from '@/features/transcription/whisper'; -import { ensureUploadContract } from '@/utils/ensure-upload-contract'; +import { ensureUploadContract, ensureUploadContractCached } from '@/utils/ensure-upload-contract'; import { absolutize, toFileUri } from '@/utils/file-store'; import { effFile } from '@/utils/segment-window'; import { generateThumbnailFile } from '@/utils/video'; @@ -766,7 +766,20 @@ class BackgroundUploadManager { for (const [index, segment] of segments.entries()) { reportClip(index + 1, index); - const file = new File(absolutize(effFile(segment))); + // Same contract as the merged unit. `uploadUnit` is chosen by the DESTINATION SERVER, not + // by us, so leaving this path ungated would silently disable the contract for any server + // that asks for segments — the protection would look present and not run. + const contract = await ensureUploadContractCached( + absolutize(effFile(segment)), + draftId, + segment.id, + ); + if (contract.failure) { + console.warn( + `[contract] uploading unconditioned clip ${segment.id} for ${draftId}: ${contract.failure}`, + ); + } + const file = new File(toFileUri(contract.path)); const checksum = await md5Checksum(file); const videoKey = `${segment.id}:video` as const; diff --git a/src/utils/ensure-upload-contract.ts b/src/utils/ensure-upload-contract.ts index 125c041..d38fae4 100644 --- a/src/utils/ensure-upload-contract.ts +++ b/src/utils/ensure-upload-contract.ts @@ -1,5 +1,7 @@ +import { File } from 'expo-file-system'; import { compress, probeVideo } from 'react-native-video-trim'; +import { toFileUri, uploadDest } from './file-store'; import { decideUploadContract } from './upload-contract'; /** @@ -69,3 +71,43 @@ export async function ensureUploadContract(path: string): Promise { + const dest = uploadDest(draftId, segmentId); + if (dest.exists && (dest.size ?? 0) > 0) { + // Already conditioned on an earlier attempt — reuse verbatim. + return { path: dest.uri, changed: true, reasons: [] }; + } + + const result = await ensureUploadContract(sourcePath); + if (!result.changed) return result; + + 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 }; + } catch (e) { + // The conditioned bytes exist but could not be parked. Upload them from where they are + // rather than falling back to the oversized original; a resume may re-encode, which is + // worse than this but still better than uploading 4K. + console.warn('[contract] could not park the conditioned clip; using the cache copy', e); + return result; + } +} diff --git a/src/utils/file-store.ts b/src/utils/file-store.ts index c06297d..2b942ff 100644 --- a/src/utils/file-store.ts +++ b/src/utils/file-store.ts @@ -5,14 +5,14 @@ import { Directory, File, Paths } from 'expo-file-system'; // between launches without invalidating references (§2.2). // drafts/{draftId}/segments/{segmentId}.mp4 — pristine original // drafts/{draftId}/segments/{segmentId}.edited.{rev}.mp4 — re-encoded editor output +// drafts/{draftId}/upload/{segmentId}.mp4 — upload-contract copy (see uploadDir) /** * Normalize a bare filesystem path to a `file://` URI. `merge()` / `getFrameAt` / the camera hand * back bare paths, but expo's `File`, expo-video, whisper, sharing, etc. all want a URI. A value * that already has a scheme is returned unchanged. */ -export const toFileUri = (path: string): string => - path.startsWith('/') ? `file://${path}` : path; +export const toFileUri = (path: string): string => (path.startsWith('/') ? `file://${path}` : path); export function segmentRelPath(draftId: string, segmentId: string): string { return `drafts/${draftId}/segments/${segmentId}.mp4`; @@ -55,6 +55,27 @@ function segmentsDir(draftId: string): Directory { return dir; } +/** + * The draft's conditioned-upload dir, creating it (and any missing parents) if needed. + * + * A STABLE location inside the draft's own directory, not a cache temp name. Segment uploads + * resume byte-wise (TUS HEAD + PATCH), so a resumed run must re-send the exact bytes it started + * with — re-encoding on resume would splice a second encode into a half-finished transfer. A + * fixed path means the conditioned file is found and reused instead. Living under + * `drafts/{draftId}/` means `deleteDraftDir` reclaims it with the draft, and it sits beside + * `segments/` rather than inside it so it can never be mistaken for a clip. + */ +function uploadDir(draftId: string): Directory { + const dir = new Directory(Paths.document, 'drafts', draftId, 'upload'); + dir.create({ intermediates: true, idempotent: true }); + return dir; +} + +/** The on-disk conditioned upload copy for a clip, creating the upload dir if needed. */ +export function uploadDest(draftId: string, segmentId: string): File { + return new File(uploadDir(draftId), `${segmentId}.mp4`); +} + /** The on-disk pristine segment file for a draft, creating the segments dir if needed. */ function segmentDest(draftId: string, segmentId: string): File { return new File(segmentsDir(draftId), `${segmentId}.mp4`);