From 94aa8dae9e6a7989f2060d4b576393b690b81001 Mon Sep 17 00:00:00 2001 From: vineyardbovines Date: Thu, 25 Jun 2026 18:10:03 -0400 Subject: [PATCH] Address review feedback on video upload telemetry - Restore minimumFileSizeForCompress: 0 on the rnc call so unacceptable-format files still get force-transcoded regardless of size; the acceptable-format early-return above keeps the new metrics signal intact - Extract the skip-reason union into VideoCompressSkipReason in video/types.ts so the analytics event type and the telemetry helper share one source of truth - Rename the compressSkipped payload field reason -> skipReason - Rename the skip reason 'below-threshold' -> 'below-byte-threshold' to leave room for future below-duration / below-bitrate skips - Replace the hand-rolled uploadId generator with nanoid/non-secure, matching the rest of the codebase --- src/analytics/metrics/types.ts | 7 ++----- src/lib/media/video/compress.ts | 9 ++++++++- src/lib/media/video/compress.web.ts | 2 +- src/lib/media/video/telemetry.ts | 24 ++++++------------------ src/lib/media/video/types.ts | 19 ++++++++++++------- src/view/com/composer/state/video.ts | 2 +- 6 files changed, 30 insertions(+), 33 deletions(-) diff --git a/src/analytics/metrics/types.ts b/src/analytics/metrics/types.ts index bdd23d6fba..e34feae3a5 100644 --- a/src/analytics/metrics/types.ts +++ b/src/analytics/metrics/types.ts @@ -5,6 +5,7 @@ import {type Platform} from 'react-native' import {type NotificationReason} from '#/lib/hooks/useNotificationHandler' +import {type VideoCompressSkipReason} from '#/lib/media/video/types' import {type NotificationType} from '#/state/queries/notifications/types' import {type FeedDescriptor} from '#/state/queries/post-feed' import {type LiveEventFeedMetricContext} from '#/features/liveEvents/types' @@ -1362,11 +1363,7 @@ export type Events = { 'video:upload:compressSkipped': { uploadId: string engine: string - reason: - | 'gif' - | 'below-threshold' - | 'no-webcodecs' - | 'compress-error-fallback' + skipReason: VideoCompressSkipReason bytes: number mimeType: string elapsedMs: number diff --git a/src/lib/media/video/compress.ts b/src/lib/media/video/compress.ts index 2b05940983..dfb2e17b4b 100644 --- a/src/lib/media/video/compress.ts +++ b/src/lib/media/video/compress.ts @@ -43,7 +43,7 @@ export async function compressVideo( uri: file.uri, size: file.fileSize, mimeType: file.mimeType ?? 'video/mp4', - passthroughReason: 'below-threshold', + passthroughReason: 'below-byte-threshold', } } @@ -53,6 +53,13 @@ export async function compressVideo( compressionMethod: 'manual', bitrate: 3_000_000, // 3mbps maxSize: 1920, + // Force a transcode for unacceptable-format files regardless of size. + // rnc's default minimumFileSizeForCompress would otherwise pass small + // unacceptable-format files through unchanged and the server would + // reject them. Acceptable formats are already short-circuited above so + // they never reach this call. + // WARNING: this ONE SPECIFIC ARG is in MB -sfn + minimumFileSizeForCompress: 0, getCancellationId: id => { if (signal) { signal.addEventListener('abort', () => { diff --git a/src/lib/media/video/compress.web.ts b/src/lib/media/video/compress.web.ts index 83d8fc4e5b..62e8469351 100644 --- a/src/lib/media/video/compress.web.ts +++ b/src/lib/media/video/compress.web.ts @@ -65,7 +65,7 @@ export async function compressVideo( } else if (!hasCodecs) { fallbackReason = 'no-webcodecs' } else if (blob.size < COMPRESSION_MIN_SIZE_BYTES) { - fallbackReason = 'below-threshold' + fallbackReason = 'below-byte-threshold' } else { try { return await doCompression(blob, asset.uri, {onProgress, signal}) diff --git a/src/lib/media/video/telemetry.ts b/src/lib/media/video/telemetry.ts index 068800ec83..7ef5a874ef 100644 --- a/src/lib/media/video/telemetry.ts +++ b/src/lib/media/video/telemetry.ts @@ -1,6 +1,8 @@ import {Platform} from 'react-native' import {type ImagePickerAsset} from 'expo-image-picker' +import {nanoid} from 'nanoid/non-secure' +import {type VideoCompressSkipReason} from '#/lib/media/video/types' import {Sentry} from '#/logger/sentry/lib' import {type Metrics} from '#/analytics/metrics' @@ -16,20 +18,6 @@ const COMPRESS_ENGINE = type Phase = 'compress' | 'upload' | 'processing' -type SkipReason = - | 'gif' - | 'below-threshold' - | 'no-webcodecs' - | 'compress-error-fallback' - -function makeUploadId(): string { - const c = (globalThis as {crypto?: {randomUUID?: () => string}}).crypto - if (c?.randomUUID) return c.randomUUID() - return `up_${Date.now().toString(36)}_${Math.random() - .toString(36) - .slice(2, 10)}` -} - function errorClass(e: unknown): string { if (e instanceof Error) return e.name || 'Error' return 'Unknown' @@ -43,7 +31,7 @@ export type VideoTelemetry = { compressSkipped: (video: { size: number mimeType: string - reason: SkipReason + skipReason: VideoCompressSkipReason }) => void compressCompleted: (video: {size: number; mimeType: string}) => void compressFailed: (e: unknown) => void @@ -65,7 +53,7 @@ export function createVideoTelemetry({ signal: AbortSignal metric: MetricFn }): VideoTelemetry { - const uploadId = makeUploadId() + const uploadId = nanoid() const engine = COMPRESS_ENGINE const startedAt = Date.now() @@ -167,11 +155,11 @@ export function createVideoTelemetry({ }) }, - compressSkipped({size, mimeType, reason}) { + compressSkipped({size, mimeType, skipReason}) { metric('video:upload:compressSkipped', { uploadId, engine, - reason, + skipReason, bytes: size, mimeType, elapsedMs: Date.now() - phaseStartedAt, diff --git a/src/lib/media/video/types.ts b/src/lib/media/video/types.ts index 69175ea48c..d22832794a 100644 --- a/src/lib/media/video/types.ts +++ b/src/lib/media/video/types.ts @@ -1,3 +1,13 @@ +// Why the compress engine returned the input unchanged. Used both as the +// reason on `CompressedVideo.passthroughReason` and as the `skipReason` field +// on the `video:upload:compressSkipped` analytics event, so the two stay in +// sync. +export type VideoCompressSkipReason = + | 'gif' + | 'below-byte-threshold' + | 'no-webcodecs' + | 'compress-error-fallback' + export type CompressedVideo = { uri: string mimeType: string @@ -5,11 +15,6 @@ export type CompressedVideo = { // web only, can fall back to uri if missing bytes?: ArrayBuffer // Set when the engine returned the input unchanged. Undefined means the - // bytes were actually re-encoded. Used by telemetry to split - // compressCompleted vs compressSkipped, and to label the skip reason. - passthroughReason?: - | 'gif' - | 'below-threshold' - | 'no-webcodecs' - | 'compress-error-fallback' + // bytes were actually re-encoded. + passthroughReason?: VideoCompressSkipReason } diff --git a/src/view/com/composer/state/video.ts b/src/view/com/composer/state/video.ts index c719864178..6737d1f628 100644 --- a/src/view/com/composer/state/video.ts +++ b/src/view/com/composer/state/video.ts @@ -305,7 +305,7 @@ export async function processVideo( telemetry.compressSkipped({ size: video.size, mimeType: video.mimeType, - reason: video.passthroughReason, + skipReason: video.passthroughReason, }) } else { telemetry.compressCompleted({size: video.size, mimeType: video.mimeType})