From ea5ab993993280b7c9fc03c25be658f11369df4d Mon Sep 17 00:00:00 2001 From: Samuel Newman Date: Thu, 29 Aug 2024 17:00:12 +0100 Subject: [PATCH] [Video] Make compress/upload cancelable (#4996) * add abort controller to video upload system * rm log * rm log 2 --- src/lib/async/cancelable.ts | 20 ++++++ src/lib/media/video/compress.ts | 12 +++- src/lib/media/video/compress.web.ts | 5 +- src/state/queries/video/compress-video.ts | 17 +++-- src/state/queries/video/video-upload.ts | 8 ++- src/state/queries/video/video-upload.web.ts | 8 ++- src/state/queries/video/video.ts | 65 +++++++++++-------- src/view/com/composer/Composer.tsx | 10 +-- .../com/composer/ExternalEmbedRemoveBtn.tsx | 4 +- .../videos/VideoTranscodeProgress.tsx | 13 ++-- 10 files changed, 104 insertions(+), 58 deletions(-) create mode 100644 src/lib/async/cancelable.ts diff --git a/src/lib/async/cancelable.ts b/src/lib/async/cancelable.ts new file mode 100644 index 00000000..50fbcc63 --- /dev/null +++ b/src/lib/async/cancelable.ts @@ -0,0 +1,20 @@ +export function cancelable( + f: (args: A) => Promise, + signal: AbortSignal, +) { + return (args: A) => { + return new Promise((resolve, reject) => { + signal.addEventListener('abort', () => { + reject(new AbortError()) + }) + f(args).then(resolve, reject) + }) + } +} + +export class AbortError extends Error { + constructor() { + super('Aborted') + this.name = 'AbortError' + } +} diff --git a/src/lib/media/video/compress.ts b/src/lib/media/video/compress.ts index 60e5e94a..95761759 100644 --- a/src/lib/media/video/compress.ts +++ b/src/lib/media/video/compress.ts @@ -8,19 +8,25 @@ export type CompressedVideo = { export async function compressVideo( file: string, opts?: { - getCancellationId?: (id: string) => void + signal?: AbortSignal onProgress?: (progress: number) => void }, ): Promise { - const {onProgress, getCancellationId} = opts || {} + const {onProgress, signal} = opts || {} const compressed = await Video.compress( file, { - getCancellationId, compressionMethod: 'manual', bitrate: 3_000_000, // 3mbps maxSize: 1920, + getCancellationId: id => { + if (signal) { + signal.addEventListener('abort', () => { + Video.cancelCompression(id) + }) + } + }, }, onProgress, ) diff --git a/src/lib/media/video/compress.web.ts b/src/lib/media/video/compress.web.ts index 968f2b15..11ccb510 100644 --- a/src/lib/media/video/compress.web.ts +++ b/src/lib/media/video/compress.web.ts @@ -10,8 +10,9 @@ export type CompressedVideo = { // doesn't actually compress, but throws if >100MB export async function compressVideo( file: string, - _callbacks?: { - onProgress: (progress: number) => void + _opts?: { + signal?: AbortSignal + onProgress?: (progress: number) => void }, ): Promise { const blob = await fetch(file).then(res => res.blob()) diff --git a/src/state/queries/video/compress-video.ts b/src/state/queries/video/compress-video.ts index a2c739cf..a4c17eac 100644 --- a/src/state/queries/video/compress-video.ts +++ b/src/state/queries/video/compress-video.ts @@ -1,23 +1,30 @@ import {ImagePickerAsset} from 'expo-image-picker' import {useMutation} from '@tanstack/react-query' +import {cancelable} from '#/lib/async/cancelable' import {CompressedVideo, compressVideo} from 'lib/media/video/compress' export function useCompressVideoMutation({ onProgress, onSuccess, onError, + signal, }: { onProgress: (progress: number) => void onError: (e: any) => void onSuccess: (video: CompressedVideo) => void + signal: AbortSignal }) { return useMutation({ - mutationFn: async (asset: ImagePickerAsset) => { - return await compressVideo(asset.uri, { - onProgress: num => onProgress(trunc2dp(num)), - }) - }, + mutationKey: ['video', 'compress'], + mutationFn: cancelable( + (asset: ImagePickerAsset) => + compressVideo(asset.uri, { + onProgress: num => onProgress(trunc2dp(num)), + signal, + }), + signal, + ), onError, onSuccess, onMutate: () => { diff --git a/src/state/queries/video/video-upload.ts b/src/state/queries/video/video-upload.ts index d806249c..a41d4dd1 100644 --- a/src/state/queries/video/video-upload.ts +++ b/src/state/queries/video/video-upload.ts @@ -3,6 +3,7 @@ import {AppBskyVideoDefs} from '@atproto/api' import {useMutation} from '@tanstack/react-query' import {nanoid} from 'nanoid/non-secure' +import {cancelable} from '#/lib/async/cancelable' import {CompressedVideo} from '#/lib/media/video/compress' import {createVideoEndpointUrl} from '#/state/queries/video/util' import {useAgent, useSession} from '#/state/session' @@ -11,16 +12,19 @@ export const useUploadVideoMutation = ({ onSuccess, onError, setProgress, + signal, }: { onSuccess: (response: AppBskyVideoDefs.JobStatus) => void onError: (e: any) => void setProgress: (progress: number) => void + signal: AbortSignal }) => { const {currentAccount} = useSession() const agent = useAgent() return useMutation({ - mutationFn: async (video: CompressedVideo) => { + mutationKey: ['video', 'upload'], + mutationFn: cancelable(async (video: CompressedVideo) => { const uri = createVideoEndpointUrl('/xrpc/app.bsky.video.uploadVideo', { did: currentAccount!.did, name: `${nanoid(12)}.mp4`, // @TODO what are we limiting this to? @@ -59,7 +63,7 @@ export const useUploadVideoMutation = ({ const responseBody = JSON.parse(res.body) as AppBskyVideoDefs.JobStatus return responseBody - }, + }, signal), onError, onSuccess, }) diff --git a/src/state/queries/video/video-upload.web.ts b/src/state/queries/video/video-upload.web.ts index 09d10742..85e07c4e 100644 --- a/src/state/queries/video/video-upload.web.ts +++ b/src/state/queries/video/video-upload.web.ts @@ -2,6 +2,7 @@ import {AppBskyVideoDefs} from '@atproto/api' import {useMutation} from '@tanstack/react-query' import {nanoid} from 'nanoid/non-secure' +import {cancelable} from '#/lib/async/cancelable' import {CompressedVideo} from '#/lib/media/video/compress' import {createVideoEndpointUrl} from '#/state/queries/video/util' import {useAgent, useSession} from '#/state/session' @@ -10,16 +11,19 @@ export const useUploadVideoMutation = ({ onSuccess, onError, setProgress, + signal, }: { onSuccess: (response: AppBskyVideoDefs.JobStatus) => void onError: (e: any) => void setProgress: (progress: number) => void + signal: AbortSignal }) => { const {currentAccount} = useSession() const agent = useAgent() return useMutation({ - mutationFn: async (video: CompressedVideo) => { + mutationKey: ['video', 'upload'], + mutationFn: cancelable(async (video: CompressedVideo) => { const uri = createVideoEndpointUrl('/xrpc/app.bsky.video.uploadVideo', { did: currentAccount!.did, name: `${nanoid(12)}.mp4`, // @TODO: make sure it's always mp4' @@ -70,7 +74,7 @@ export const useUploadVideoMutation = ({ ) return res - }, + }, signal), onError, onSuccess, }) diff --git a/src/state/queries/video/video.ts b/src/state/queries/video/video.ts index 64390801..035dc508 100644 --- a/src/state/queries/video/video.ts +++ b/src/state/queries/video/video.ts @@ -3,7 +3,7 @@ import {ImagePickerAsset} from 'expo-image-picker' import {AppBskyVideoDefs, BlobRef} from '@atproto/api' import {msg} from '@lingui/macro' import {useLingui} from '@lingui/react' -import {useQuery} from '@tanstack/react-query' +import {QueryClient, useQuery, useQueryClient} from '@tanstack/react-query' import {logger} from '#/logger' import {CompressedVideo} from 'lib/media/video/compress' @@ -32,33 +32,41 @@ export interface State { jobStatus?: AppBskyVideoDefs.JobStatus blobRef?: BlobRef error?: string + abortController: AbortController } -function reducer(state: State, action: Action): State { - let updatedState = state - if (action.type === 'SetStatus') { - updatedState = {...state, status: action.status} - } else if (action.type === 'SetProgress') { - updatedState = {...state, progress: action.progress} - } else if (action.type === 'SetError') { - updatedState = {...state, error: action.error} - } else if (action.type === 'Reset') { - updatedState = { - status: 'idle', - progress: 0, - video: null, - blobRef: undefined, +function reducer(queryClient: QueryClient) { + return (state: State, action: Action): State => { + let updatedState = state + if (action.type === 'SetStatus') { + updatedState = {...state, status: action.status} + } else if (action.type === 'SetProgress') { + updatedState = {...state, progress: action.progress} + } else if (action.type === 'SetError') { + updatedState = {...state, error: action.error} + } else if (action.type === 'Reset') { + state.abortController.abort() + queryClient.cancelQueries({ + queryKey: ['video'], + }) + updatedState = { + status: 'idle', + progress: 0, + video: null, + blobRef: undefined, + abortController: new AbortController(), + } + } else if (action.type === 'SetAsset') { + updatedState = {...state, asset: action.asset} + } else if (action.type === 'SetVideo') { + updatedState = {...state, video: action.video} + } else if (action.type === 'SetJobStatus') { + updatedState = {...state, jobStatus: action.jobStatus} + } else if (action.type === 'SetBlobRef') { + updatedState = {...state, blobRef: action.blobRef} } - } else if (action.type === 'SetAsset') { - updatedState = {...state, asset: action.asset} - } else if (action.type === 'SetVideo') { - updatedState = {...state, video: action.video} - } else if (action.type === 'SetJobStatus') { - updatedState = {...state, jobStatus: action.jobStatus} - } else if (action.type === 'SetBlobRef') { - updatedState = {...state, blobRef: action.blobRef} + return updatedState } - return updatedState } export function useUploadVideo({ @@ -69,10 +77,12 @@ export function useUploadVideo({ onSuccess: () => void }) { const {_} = useLingui() - const [state, dispatch] = React.useReducer(reducer, { + const queryClient = useQueryClient() + const [state, dispatch] = React.useReducer(reducer(queryClient), { status: 'idle', progress: 0, video: null, + abortController: new AbortController(), }) const {setJobId} = useUploadStatusQuery({ @@ -116,6 +126,7 @@ export function useUploadVideo({ setProgress: p => { dispatch({type: 'SetProgress', progress: p}) }, + signal: state.abortController.signal, }) const {mutate: onSelectVideo} = useCompressVideoMutation({ @@ -148,6 +159,7 @@ export function useUploadVideo({ }) onVideoCompressed(video) }, + signal: state.abortController.signal, }) const selectVideo = (asset: ImagePickerAsset) => { @@ -163,7 +175,6 @@ export function useUploadVideo({ } const clearVideo = () => { - // @TODO cancel any running jobs dispatch({type: 'Reset'}) } @@ -187,7 +198,7 @@ const useUploadStatusQuery = ({ const [jobId, setJobId] = React.useState() const {isLoading, isError} = useQuery({ - queryKey: ['video-upload', jobId], + queryKey: ['video', 'upload status', jobId], queryFn: async () => { if (!jobId) return // this won't happen, can ignore diff --git a/src/view/com/composer/Composer.tsx b/src/view/com/composer/Composer.tsx index c726d307..7c11f0a9 100644 --- a/src/view/com/composer/Composer.tsx +++ b/src/view/com/composer/Composer.tsx @@ -1,5 +1,4 @@ import React, { - Suspense, useCallback, useEffect, useImperativeHandle, @@ -700,15 +699,10 @@ export const ComposePost = observer(function ComposePost({ ) : videoUploadState.video ? ( - // remove suspense when we get rid of lazy - - - + ) : null} diff --git a/src/view/com/composer/ExternalEmbedRemoveBtn.tsx b/src/view/com/composer/ExternalEmbedRemoveBtn.tsx index 7742900a..57ccc294 100644 --- a/src/view/com/composer/ExternalEmbedRemoveBtn.tsx +++ b/src/view/com/composer/ExternalEmbedRemoveBtn.tsx @@ -25,8 +25,8 @@ export function ExternalEmbedRemoveBtn({onRemove}: {onRemove: () => void}) { }} onPress={onRemove} accessibilityRole="button" - accessibilityLabel={_(msg`Remove image preview`)} - accessibilityHint={_(msg`Removes the image preview`)} + accessibilityLabel={_(msg`Remove attachment`)} + accessibilityHint={_(msg`Removes the attachment`)} onAccessibilityEscape={onRemove}> diff --git a/src/view/com/composer/videos/VideoTranscodeProgress.tsx b/src/view/com/composer/videos/VideoTranscodeProgress.tsx index a44b633c..8a79492d 100644 --- a/src/view/com/composer/videos/VideoTranscodeProgress.tsx +++ b/src/view/com/composer/videos/VideoTranscodeProgress.tsx @@ -3,18 +3,19 @@ import {View} from 'react-native' // @ts-expect-error no type definition import ProgressPie from 'react-native-progress/Pie' import {ImagePickerAsset} from 'expo-image-picker' -import {Trans} from '@lingui/macro' import {atoms as a, useTheme} from '#/alf' -import {Text} from '#/components/Typography' +import {ExternalEmbedRemoveBtn} from '../ExternalEmbedRemoveBtn' import {VideoTranscodeBackdrop} from './VideoTranscodeBackdrop' export function VideoTranscodeProgress({ asset, progress, + clear, }: { asset: ImagePickerAsset progress: number + clear: () => void }) { const t = useTheme() @@ -41,16 +42,14 @@ export function VideoTranscodeProgress({ a.inset_0, ]}> - - Compressing... - + ) }