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
This commit is contained in:
@@ -5,6 +5,7 @@
|
|||||||
import {type Platform} from 'react-native'
|
import {type Platform} from 'react-native'
|
||||||
|
|
||||||
import {type NotificationReason} from '#/lib/hooks/useNotificationHandler'
|
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 NotificationType} from '#/state/queries/notifications/types'
|
||||||
import {type FeedDescriptor} from '#/state/queries/post-feed'
|
import {type FeedDescriptor} from '#/state/queries/post-feed'
|
||||||
import {type LiveEventFeedMetricContext} from '#/features/liveEvents/types'
|
import {type LiveEventFeedMetricContext} from '#/features/liveEvents/types'
|
||||||
@@ -1362,11 +1363,7 @@ export type Events = {
|
|||||||
'video:upload:compressSkipped': {
|
'video:upload:compressSkipped': {
|
||||||
uploadId: string
|
uploadId: string
|
||||||
engine: string
|
engine: string
|
||||||
reason:
|
skipReason: VideoCompressSkipReason
|
||||||
| 'gif'
|
|
||||||
| 'below-threshold'
|
|
||||||
| 'no-webcodecs'
|
|
||||||
| 'compress-error-fallback'
|
|
||||||
bytes: number
|
bytes: number
|
||||||
mimeType: string
|
mimeType: string
|
||||||
elapsedMs: number
|
elapsedMs: number
|
||||||
|
|||||||
@@ -43,7 +43,7 @@ export async function compressVideo(
|
|||||||
uri: file.uri,
|
uri: file.uri,
|
||||||
size: file.fileSize,
|
size: file.fileSize,
|
||||||
mimeType: file.mimeType ?? 'video/mp4',
|
mimeType: file.mimeType ?? 'video/mp4',
|
||||||
passthroughReason: 'below-threshold',
|
passthroughReason: 'below-byte-threshold',
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -53,6 +53,13 @@ export async function compressVideo(
|
|||||||
compressionMethod: 'manual',
|
compressionMethod: 'manual',
|
||||||
bitrate: 3_000_000, // 3mbps
|
bitrate: 3_000_000, // 3mbps
|
||||||
maxSize: 1920,
|
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 => {
|
getCancellationId: id => {
|
||||||
if (signal) {
|
if (signal) {
|
||||||
signal.addEventListener('abort', () => {
|
signal.addEventListener('abort', () => {
|
||||||
|
|||||||
@@ -65,7 +65,7 @@ export async function compressVideo(
|
|||||||
} else if (!hasCodecs) {
|
} else if (!hasCodecs) {
|
||||||
fallbackReason = 'no-webcodecs'
|
fallbackReason = 'no-webcodecs'
|
||||||
} else if (blob.size < COMPRESSION_MIN_SIZE_BYTES) {
|
} else if (blob.size < COMPRESSION_MIN_SIZE_BYTES) {
|
||||||
fallbackReason = 'below-threshold'
|
fallbackReason = 'below-byte-threshold'
|
||||||
} else {
|
} else {
|
||||||
try {
|
try {
|
||||||
return await doCompression(blob, asset.uri, {onProgress, signal})
|
return await doCompression(blob, asset.uri, {onProgress, signal})
|
||||||
|
|||||||
@@ -1,6 +1,8 @@
|
|||||||
import {Platform} from 'react-native'
|
import {Platform} from 'react-native'
|
||||||
import {type ImagePickerAsset} from 'expo-image-picker'
|
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 {Sentry} from '#/logger/sentry/lib'
|
||||||
import {type Metrics} from '#/analytics/metrics'
|
import {type Metrics} from '#/analytics/metrics'
|
||||||
|
|
||||||
@@ -16,20 +18,6 @@ const COMPRESS_ENGINE =
|
|||||||
|
|
||||||
type Phase = 'compress' | 'upload' | 'processing'
|
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 {
|
function errorClass(e: unknown): string {
|
||||||
if (e instanceof Error) return e.name || 'Error'
|
if (e instanceof Error) return e.name || 'Error'
|
||||||
return 'Unknown'
|
return 'Unknown'
|
||||||
@@ -43,7 +31,7 @@ export type VideoTelemetry = {
|
|||||||
compressSkipped: (video: {
|
compressSkipped: (video: {
|
||||||
size: number
|
size: number
|
||||||
mimeType: string
|
mimeType: string
|
||||||
reason: SkipReason
|
skipReason: VideoCompressSkipReason
|
||||||
}) => void
|
}) => void
|
||||||
compressCompleted: (video: {size: number; mimeType: string}) => void
|
compressCompleted: (video: {size: number; mimeType: string}) => void
|
||||||
compressFailed: (e: unknown) => void
|
compressFailed: (e: unknown) => void
|
||||||
@@ -65,7 +53,7 @@ export function createVideoTelemetry({
|
|||||||
signal: AbortSignal
|
signal: AbortSignal
|
||||||
metric: MetricFn
|
metric: MetricFn
|
||||||
}): VideoTelemetry {
|
}): VideoTelemetry {
|
||||||
const uploadId = makeUploadId()
|
const uploadId = nanoid()
|
||||||
const engine = COMPRESS_ENGINE
|
const engine = COMPRESS_ENGINE
|
||||||
const startedAt = Date.now()
|
const startedAt = Date.now()
|
||||||
|
|
||||||
@@ -167,11 +155,11 @@ export function createVideoTelemetry({
|
|||||||
})
|
})
|
||||||
},
|
},
|
||||||
|
|
||||||
compressSkipped({size, mimeType, reason}) {
|
compressSkipped({size, mimeType, skipReason}) {
|
||||||
metric('video:upload:compressSkipped', {
|
metric('video:upload:compressSkipped', {
|
||||||
uploadId,
|
uploadId,
|
||||||
engine,
|
engine,
|
||||||
reason,
|
skipReason,
|
||||||
bytes: size,
|
bytes: size,
|
||||||
mimeType,
|
mimeType,
|
||||||
elapsedMs: Date.now() - phaseStartedAt,
|
elapsedMs: Date.now() - phaseStartedAt,
|
||||||
|
|||||||
@@ -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 = {
|
export type CompressedVideo = {
|
||||||
uri: string
|
uri: string
|
||||||
mimeType: string
|
mimeType: string
|
||||||
@@ -5,11 +15,6 @@ export type CompressedVideo = {
|
|||||||
// web only, can fall back to uri if missing
|
// web only, can fall back to uri if missing
|
||||||
bytes?: ArrayBuffer
|
bytes?: ArrayBuffer
|
||||||
// Set when the engine returned the input unchanged. Undefined means the
|
// Set when the engine returned the input unchanged. Undefined means the
|
||||||
// bytes were actually re-encoded. Used by telemetry to split
|
// bytes were actually re-encoded.
|
||||||
// compressCompleted vs compressSkipped, and to label the skip reason.
|
passthroughReason?: VideoCompressSkipReason
|
||||||
passthroughReason?:
|
|
||||||
| 'gif'
|
|
||||||
| 'below-threshold'
|
|
||||||
| 'no-webcodecs'
|
|
||||||
| 'compress-error-fallback'
|
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -305,7 +305,7 @@ export async function processVideo(
|
|||||||
telemetry.compressSkipped({
|
telemetry.compressSkipped({
|
||||||
size: video.size,
|
size: video.size,
|
||||||
mimeType: video.mimeType,
|
mimeType: video.mimeType,
|
||||||
reason: video.passthroughReason,
|
skipReason: video.passthroughReason,
|
||||||
})
|
})
|
||||||
} else {
|
} else {
|
||||||
telemetry.compressCompleted({size: video.size, mimeType: video.mimeType})
|
telemetry.compressCompleted({size: video.size, mimeType: video.mimeType})
|
||||||
|
|||||||
Reference in New Issue
Block a user