From a22e96c56a250d293319ec19dedb301118aaebf9 Mon Sep 17 00:00:00 2001 From: Samuel Newman Date: Mon, 7 Sep 2026 14:56:33 +0300 Subject: [PATCH] bound image compression memory usage --- .../dialogs/lists/CreateOrEditListDialog.tsx | 8 + src/lib/api/index.ts | 60 +++- src/lib/async/map-with-serial-retry.test.ts | 64 ++++- src/lib/async/map-with-serial-retry.ts | 78 +++++- src/lib/media/image-manipulator.ts | 47 ++++ src/lib/media/manip.web.ts | 13 +- src/lib/media/util.ts | 27 ++ src/screens/Onboarding/StepFinished/index.tsx | 63 +++-- src/screens/Onboarding/StepProfile/index.tsx | 17 +- .../Profile/Header/EditProfileDialog.tsx | 14 + src/state/gallery.ts | 104 +++++-- src/view/com/composer/Composer.tsx | 31 ++- src/view/com/composer/SelectMediaButton.tsx | 257 ++++++++++-------- src/view/com/composer/drafts/DraftItem.tsx | 45 ++- src/view/com/composer/photos/Gallery.tsx | 7 +- 15 files changed, 602 insertions(+), 233 deletions(-) diff --git a/src/components/dialogs/lists/CreateOrEditListDialog.tsx b/src/components/dialogs/lists/CreateOrEditListDialog.tsx index 64f7421699..7beee22375 100644 --- a/src/components/dialogs/lists/CreateOrEditListDialog.tsx +++ b/src/components/dialogs/lists/CreateOrEditListDialog.tsx @@ -5,6 +5,7 @@ import {msg} from '@lingui/core/macro' import {useLingui} from '@lingui/react' import {Plural, Trans} from '@lingui/react/macro' +import {revokeObjectUrl} from '#/lib/media/image-manipulator' import {cleanError} from '#/lib/strings/errors' import {isOverMaxGraphemeCount} from '#/lib/strings/helpers' import {richTextToString} from '#/lib/strings/rich-text-helpers' @@ -187,6 +188,13 @@ function DialogInner({ ImageMeta | undefined | null >() + useEffect( + () => () => { + revokeObjectUrl(newListAvatar?.path) + }, + [newListAvatar], + ) + // When creating with pre-filled values (from Starter Pack), consider dirty // immediately so the Save button is enabled const hasInitialValuesForCreate = !list && initialValues != null diff --git a/src/lib/api/index.ts b/src/lib/api/index.ts index dcbbe0e3e9..fec5834f7d 100644 --- a/src/lib/api/index.ts +++ b/src/lib/api/index.ts @@ -1,3 +1,4 @@ +import * as Device from 'expo-device' import {TID} from '@atproto/common-web' import {type $Typed, type Client} from '@atproto/lex' import { @@ -12,6 +13,8 @@ import {type QueryClient} from '@tanstack/react-query' import {type LinkResolvers} from '#/lib/api/resolve' import {mapWithSerialRetry} from '#/lib/async/map-with-serial-retry' import {IMAGE_SIZE_CONFIG_POSTS} from '#/lib/constants' +import {revokeObjectUrl} from '#/lib/media/image-manipulator' +import {getImageCompressionConcurrency} from '#/lib/media/util' import {isNetworkError} from '#/lib/strings/errors' import {shortenLinks, stripInvalidMentions} from '#/lib/strings/rich-text-manip' import {logger} from '#/logger' @@ -29,6 +32,7 @@ import { type PostDraft, type ThreadDraft, } from '#/view/com/composer/state/composer' +import {IS_ANDROID} from '#/env' import {app, chat, com} from '#/lexicons' import * as bsky from '#/types/bsky' import {createGIFDescription} from '../gif-alt-text' @@ -340,11 +344,15 @@ async function resolveMedia( compressedImages.map(async (compressedImage, i) => { const {path, width, height, mime} = compressedImage logger.debug(`Uploading image #${i}`) - const res = await uploadBlob(pdsClient, path, mime) - return { - image: res.blob, - alt: imagesDraft[i].alt, - aspectRatio: {width, height}, + try { + const res = await uploadBlob(pdsClient, path, mime) + return { + image: res.blob, + alt: imagesDraft[i].alt, + aspectRatio: {width, height}, + } + } finally { + revokeObjectUrl(path) } }), ) @@ -364,12 +372,16 @@ async function resolveMedia( compressedImages.map(async (compressedImage, i) => { const {path, width, height, mime} = compressedImage logger.debug(`Uploading image #${i}`) - const res = await uploadBlob(pdsClient, path, mime) - return { - $type: 'app.bsky.embed.gallery#image' as const, - image: res.blob, - alt: imagesDraft[i].alt, - aspectRatio: {width, height}, + try { + const res = await uploadBlob(pdsClient, path, mime) + return { + $type: 'app.bsky.embed.gallery#image' as const, + image: res.blob, + alt: imagesDraft[i].alt, + aspectRatio: {width, height}, + } + } finally { + revokeObjectUrl(path) } }), ) @@ -490,15 +502,35 @@ type CompressedImage = Awaited> async function compressImages( images: ComposerImage[], ): Promise { + const totalMemoryBytes = Device.totalMemory + const concurrency = getImageCompressionConcurrency({ + isAndroid: IS_ANDROID, + totalMemoryBytes, + }) + logger.debug('Compressing images', { + concurrency, + totalMemoryBytes, + }) + return mapWithSerialRetry( images, (image, i) => { logger.debug(`Compressing image #${i}`) return compressImage(image, IMAGE_SIZE_CONFIG_POSTS) }, - isBitmapLoadingError, - (_error, i) => { - logger.info(`Retrying image compression serially`, {index: i}) + { + concurrency, + shouldRetry: isBitmapLoadingError, + onRetry(_error, i) { + logger.info(`Retrying image compression serially`, { + index: i, + concurrency, + totalMemoryBytes, + }) + }, + onDiscard(image) { + revokeObjectUrl(image.path) + }, }, ) } diff --git a/src/lib/async/map-with-serial-retry.test.ts b/src/lib/async/map-with-serial-retry.test.ts index bc50febc31..42183a8d17 100644 --- a/src/lib/async/map-with-serial-retry.test.ts +++ b/src/lib/async/map-with-serial-retry.test.ts @@ -25,7 +25,10 @@ describe('mapWithSerialRetry', () => { return value * 2 }, - error => String(error).includes('retryable'), + { + concurrency: 3, + shouldRetry: error => String(error).includes('retryable'), + }, ) expect(result).toEqual([2, 4, 6]) @@ -43,10 +46,63 @@ describe('mapWithSerialRetry', () => { const mapper = jest.fn((value: string) => Promise.reject(new Error(value))) await expect( - mapWithSerialRetry(['retryable', 'permanent'], mapper, error => - String(error).includes('retryable'), - ), + mapWithSerialRetry(['retryable', 'permanent'], mapper, { + concurrency: 2, + shouldRetry: error => String(error).includes('retryable'), + }), ).rejects.toThrow('permanent') expect(mapper).toHaveBeenCalledTimes(2) }) + + it('limits the concurrent attempts', async () => { + let inFlight = 0 + let maxInFlight = 0 + let release: (() => void) | undefined + const blocked = new Promise(resolve => { + release = resolve + }) + + const resultPromise = mapWithSerialRetry( + [1, 2, 3, 4], + async value => { + inFlight++ + maxInFlight = Math.max(maxInFlight, inFlight) + await blocked + inFlight-- + return value + }, + { + concurrency: 2, + shouldRetry: () => false, + }, + ) + + await Promise.resolve() + expect(maxInFlight).toBe(2) + release?.() + await expect(resultPromise).resolves.toEqual([1, 2, 3, 4]) + }) + + it('discards successful results when the batch fails', async () => { + const onDiscard = jest.fn() + + await expect( + mapWithSerialRetry( + [1, 2, 3], + value => + value === 2 + ? Promise.reject(new Error('permanent')) + : Promise.resolve(value * 2), + { + concurrency: 2, + shouldRetry: () => false, + onDiscard, + }, + ), + ).rejects.toThrow('permanent') + expect(onDiscard.mock.calls).toEqual([ + [2, 0], + [6, 2], + ]) + }) }) diff --git a/src/lib/async/map-with-serial-retry.ts b/src/lib/async/map-with-serial-retry.ts index 7dbfd2e3ae..6d59d81fff 100644 --- a/src/lib/async/map-with-serial-retry.ts +++ b/src/lib/async/map-with-serial-retry.ts @@ -1,31 +1,83 @@ /** - * Maps all values concurrently, then retries eligible failures one at a time. - * Successful results from the concurrent attempt are reused. + * Maps values with bounded concurrency, then retries eligible failures one at + * a time. Successful results from the concurrent attempt are reused. */ export async function mapWithSerialRetry( values: T[], mapper: (value: T, index: number) => Promise, - shouldRetry: (error: unknown) => boolean, - onRetry?: (error: unknown, index: number) => void, + { + concurrency, + shouldRetry, + onRetry, + onDiscard, + }: { + concurrency: number + shouldRetry: (error: unknown) => boolean + onRetry?: (error: unknown, index: number) => void + onDiscard?: (value: U, index: number) => void + }, ): Promise { - const settled = await Promise.allSettled(values.map(mapper)) + const settled = new Array>(values.length) + let nextIndex = 0 + + async function worker() { + while (nextIndex < values.length) { + const index = nextIndex++ + try { + settled[index] = { + status: 'fulfilled', + value: await mapper(values[index], index), + } + } catch (reason) { + settled[index] = {status: 'rejected', reason} + } + } + } + + const workerCount = Math.min( + values.length, + Math.max(1, Math.floor(concurrency)), + ) + await Promise.all(Array.from({length: workerCount}, worker)) + + const completed: {value: U; index: number}[] = [] + for (let i = 0; i < settled.length; i++) { + const result = settled[i] + if (result.status === 'fulfilled') { + completed.push({value: result.value, index: i}) + } + } + + const discardCompleted = () => { + for (const result of completed) { + onDiscard?.(result.value, result.index) + } + } const nonRetryableFailure = settled.find( result => result.status === 'rejected' && !shouldRetry(result.reason), ) if (nonRetryableFailure?.status === 'rejected') { + discardCompleted() throw nonRetryableFailure.reason } - const results: U[] = [] - for (let i = 0; i < settled.length; i++) { - const result = settled[i] - if (result.status === 'fulfilled') { - results.push(result.value) - } else { - onRetry?.(result.reason, i) - results.push(await mapper(values[i], i)) + const results = new Array(values.length) + try { + for (let i = 0; i < settled.length; i++) { + const result = settled[i] + if (result.status === 'fulfilled') { + results[i] = result.value + } else { + onRetry?.(result.reason, i) + const value = await mapper(values[i], i) + results[i] = value + completed.push({value, index: i}) + } } + } catch (error) { + discardCompleted() + throw error } return results diff --git a/src/lib/media/image-manipulator.ts b/src/lib/media/image-manipulator.ts index 41fde37c81..da319f5ee2 100644 --- a/src/lib/media/image-manipulator.ts +++ b/src/lib/media/image-manipulator.ts @@ -5,6 +5,8 @@ import { type SaveOptions, } from 'expo-image-manipulator' +import {IS_WEB} from '#/env' + export async function renderImage( source: string, manipulate?: (context: ImageManipulatorContext) => void, @@ -19,9 +21,54 @@ export async function renderImage( try { return await image.saveAsync(saveOptions) } finally { + releaseWebImageResources(image) image.release() } } finally { + await releaseWebContextResources(context) context.release() } } + +export function revokeObjectUrl(uri: string | undefined): void { + if (IS_WEB && uri?.startsWith('blob:')) { + URL.revokeObjectURL(uri) + } +} + +function releaseWebImageResources(image: unknown): void { + if (!IS_WEB) { + return + } + + const webImage = image as { + uri?: string + canvas?: {width: number; height: number} + } + revokeObjectUrl(webImage.uri) + if (webImage.canvas) { + webImage.canvas.width = 0 + webImage.canvas.height = 0 + } +} + +async function releaseWebContextResources(context: unknown): Promise { + if (!IS_WEB) { + return + } + + try { + const currentTask = ( + context as { + currentTask?: Promise<{width: number; height: number}> + } + ).currentTask + const canvas = await currentTask + if (canvas) { + canvas.width = 0 + canvas.height = 0 + } + } catch { + // The original render error is more useful than a cleanup error. + } +} diff --git a/src/lib/media/manip.web.ts b/src/lib/media/manip.web.ts index 690dc32023..901f546d84 100644 --- a/src/lib/media/manip.web.ts +++ b/src/lib/media/manip.web.ts @@ -1,3 +1,4 @@ +import {revokeObjectUrl} from './image-manipulator' import {type PickerImage} from './picker.shared' import {type Dimensions} from './types' import { @@ -14,10 +15,14 @@ export async function compressIfNeeded( if (img.size < maxSize) { return img } - return await doResize(img.path, { - maxDimension, - maxSize, - }) + try { + return await doResize(img.path, { + maxDimension, + maxSize, + }) + } finally { + revokeObjectUrl(img.path) + } } export interface DownloadAndResizeOpts { diff --git a/src/lib/media/util.ts b/src/lib/media/util.ts index 8db418f045..0835a9f8aa 100644 --- a/src/lib/media/util.ts +++ b/src/lib/media/util.ts @@ -27,6 +27,33 @@ export function getResizedDimensions( } } +/** + * Select a compression limit that balances throughput with decoded bitmap + * memory. Android benefits from more parallel work on higher-memory devices; + * web and iOS plateau at two concurrent images. + */ +export function getImageCompressionConcurrency({ + isAndroid, + totalMemoryBytes, +}: { + isAndroid: boolean + totalMemoryBytes: number | null +}): number { + if (!isAndroid) { + return 2 + } + if (totalMemoryBytes === null) { + return 1 + } + if (totalMemoryBytes >= 6_000_000_000) { + return 5 + } + if (totalMemoryBytes >= 3_000_000_000) { + return 3 + } + return 1 +} + // Fairly accurate estimate that is more performant // than decoding and checking length of URI export function getDataUriSize(uri: string): number { diff --git a/src/screens/Onboarding/StepFinished/index.tsx b/src/screens/Onboarding/StepFinished/index.tsx index f6f96bde9e..657c24dc72 100644 --- a/src/screens/Onboarding/StepFinished/index.tsx +++ b/src/screens/Onboarding/StepFinished/index.tsx @@ -16,6 +16,7 @@ import { TIMELINE_SAVED_FEED, VIDEO_SAVED_FEED, } from '#/lib/constants' +import {revokeObjectUrl} from '#/lib/media/image-manipulator' import {useRequestNotificationsPermission} from '#/lib/notifications/notifications' import {logger} from '#/logger' import {useSetHasCheckedForStarterPack} from '#/state/preferences/used-starter-packs' @@ -148,43 +149,47 @@ export function StepFinished() { })(), (async () => { const {imageUri, imageMime} = profileStepResults - const blobPromise = - imageUri && imageMime - ? uploadBlob(pdsClient, imageUri, imageMime) - : undefined + try { + const blobPromise = + imageUri && imageMime + ? uploadBlob(pdsClient, imageUri, imageMime) + : undefined - await pdsClient.call(upsertProfile, async existing => { - let next: Un$Typed = existing ?? {} + await pdsClient.call(upsertProfile, async existing => { + let next: Un$Typed = existing ?? {} - if (blobPromise) { - const res = await blobPromise - if (res.blob) { - next.avatar = res.blob + if (blobPromise) { + const res = await blobPromise + if (res.blob) { + next.avatar = res.blob + } } - } - if (starterPack) { - next.joinedViaStarterPack = { - uri: starterPack.uri, - cid: starterPack.cid, + if (starterPack) { + next.joinedViaStarterPack = { + uri: starterPack.uri, + cid: starterPack.cid, + } } - } - next.displayName = '' + next.displayName = '' - if (!next.createdAt) { - next.createdAt = toDatetimeString(new Date()) - } - return next - }) + if (!next.createdAt) { + next.createdAt = toDatetimeString(new Date()) + } + return next + }) - ax.metric('onboarding:finished:avatarResult', { - avatarResult: profileStepResults.isCreatedAvatar - ? 'created' - : profileStepResults.image - ? 'uploaded' - : 'default', - }) + ax.metric('onboarding:finished:avatarResult', { + avatarResult: profileStepResults.isCreatedAvatar + ? 'created' + : profileStepResults.image + ? 'uploaded' + : 'default', + }) + } finally { + revokeObjectUrl(imageUri) + } })(), requestNotificationsPermission('AfterOnboarding'), ]) diff --git a/src/screens/Onboarding/StepProfile/index.tsx b/src/screens/Onboarding/StepProfile/index.tsx index f3dc1f848a..a2de0d91ab 100644 --- a/src/screens/Onboarding/StepProfile/index.tsx +++ b/src/screens/Onboarding/StepProfile/index.tsx @@ -9,7 +9,7 @@ import { } from 'react' import {View} from 'react-native' import {Image as ExpoImage} from 'expo-image' -import {ImageManipulator, SaveFormat} from 'expo-image-manipulator' +import {SaveFormat} from 'expo-image-manipulator' import { type ImagePickerOptions, launchImageLibraryAsync, @@ -20,6 +20,7 @@ import {Trans} from '@lingui/react/macro' import {IMAGE_SIZE_CONFIG_2K_1MB} from '#/lib/constants' import {usePhotoLibraryPermission} from '#/lib/hooks/usePermissions' +import {renderImage, revokeObjectUrl} from '#/lib/media/image-manipulator' import {compressIfNeeded} from '#/lib/media/manip' import {openCropper} from '#/lib/media/picker' import {getUriSize} from '#/lib/media/uriSize' @@ -120,18 +121,17 @@ export function StepProfile() { const asset = (response.assets ?? [])[0] if (!asset) return [] + let result: Awaited> | undefined try { - const context = ImageManipulator.manipulate(asset.uri) - const rendered = await context.renderAsync() - const result = await rendered.saveAsync({ + result = await renderImage(asset.uri, undefined, { format: SaveFormat.JPEG, - compress: 1.0, + compress: 1, }) return [ { mime: 'image/jpeg', - height: rendered.height, - width: rendered.width, + height: result.height, + width: result.width, path: result.uri, size: await getUriSize(result.uri), }, @@ -140,7 +140,10 @@ export function StepProfile() { setError( l`This image could not be used. Try a different format like .jpg or .png.`, ) + revokeObjectUrl(result?.uri) return [] + } finally { + revokeObjectUrl(asset.uri) } }, [l, setError, sheetWrapper], diff --git a/src/screens/Profile/Header/EditProfileDialog.tsx b/src/screens/Profile/Header/EditProfileDialog.tsx index d110753143..9164ca9eb2 100644 --- a/src/screens/Profile/Header/EditProfileDialog.tsx +++ b/src/screens/Profile/Header/EditProfileDialog.tsx @@ -5,6 +5,7 @@ import {useLingui} from '@lingui/react' import {Plural, Trans} from '@lingui/react/macro' import {MAX_DESCRIPTION, MAX_DISPLAY_NAME, urls} from '#/lib/constants' +import {revokeObjectUrl} from '#/lib/media/image-manipulator' import {cleanError} from '#/lib/strings/errors' import {isOverMaxGraphemeCount} from '#/lib/strings/helpers' import {logger} from '#/logger' @@ -124,6 +125,19 @@ function DialogInner({ ImageMeta | undefined | null >() + useEffect( + () => () => { + revokeObjectUrl(newUserBanner?.path) + }, + [newUserBanner], + ) + useEffect( + () => () => { + revokeObjectUrl(newUserAvatar?.path) + }, + [newUserAvatar], + ) + const dirty = displayName !== initialDisplayName || description !== initialDescription || diff --git a/src/state/gallery.ts b/src/state/gallery.ts index 3a0a2d9544..0d9bac7ce2 100644 --- a/src/state/gallery.ts +++ b/src/state/gallery.ts @@ -8,7 +8,7 @@ import { import {type ImageManipulatorContext, SaveFormat} from 'expo-image-manipulator' import {nanoid} from 'nanoid/non-secure' -import {renderImage} from '#/lib/media/image-manipulator' +import {renderImage, revokeObjectUrl} from '#/lib/media/image-manipulator' import {getImageDim} from '#/lib/media/manip' import {openCropper} from '#/lib/media/picker' import {type PickerImage} from '#/lib/media/picker.shared' @@ -162,6 +162,7 @@ export async function manipulateImage( return img } + revokeObjectUrl(img.transformed.path) return {alt: img.alt, source: img.source} } @@ -169,12 +170,20 @@ export async function manipulateImage( const result = await renderImage(source.path, context => context.crop(crop), { format: SaveFormat.PNG, }) + let path: string + try { + path = await moveIfNecessary(result.uri) + } catch (error) { + await deleteTemporaryImage(result.uri) + throw error + } + await deleteTemporaryImage(img.transformed?.path) return { alt: img.alt, source: img.source, transformed: { - path: await moveIfNecessary(result.uri), + path, width: result.width, height: result.height, mime: 'image/png', @@ -187,12 +196,19 @@ export function resetImageManipulation( img: ComposerImage, ): ComposerImageWithoutTransformation { if (img.transformed !== undefined) { + revokeObjectUrl(img.transformed.path) return {alt: img.alt, source: img.source} } return img } +/** Release browser resources owned by an image removed from the composer. */ +export function releaseComposerImage(img: ComposerImage): void { + revokeObjectUrl(img.source.path) + revokeObjectUrl(img.transformed?.path) +} + export async function compressImage( img: ComposerImage, {maxDimension, maxSize}: {maxDimension: number; maxSize: number}, @@ -207,7 +223,7 @@ export async function compressImage( let minQualityPercentage = 0 let maxQualityPercentage = 101 // exclusive - let newDataUri + let newDataUri: PickerImage | undefined while (maxQualityPercentage - minQualityPercentage > 1) { if (attempts >= 4) break @@ -237,30 +253,43 @@ export async function compressImage( continue } - const res = await renderImage( - source.path, - context => context.resize({width: w, height: h}), - { - // Requesting base64 makes Android encode the bitmap a second time and - // retain the encoded bytes and string in memory. Measure the URI instead. - base64: false, - compress: qualityPercentage / 100, - format: SaveFormat.JPEG, - }, - ) + let res: Awaited> | undefined + try { + res = await renderImage( + source.path, + context => context.resize({width: w, height: h}), + { + /* + * Leave this off in production. On Android, requesting base64 encodes + * the bitmap a second time and retains the bytes and string in memory. + */ + base64: false, + compress: qualityPercentage / 100, + format: SaveFormat.JPEG, + }, + ) - const size = await getUriSize(res.uri) - if (size <= maxBytes) { - minQualityPercentage = qualityPercentage - newDataUri = { - path: await moveIfNecessary(res.uri), - width: res.width, - height: res.height, - mime: 'image/jpeg', - size, + const size = await getUriSize(res.uri) + if (size <= maxBytes) { + minQualityPercentage = qualityPercentage + const previousPath = newDataUri?.path + const path = await moveIfNecessary(res.uri) + await deleteTemporaryImage(previousPath) + newDataUri = { + path, + width: res.width, + height: res.height, + mime: 'image/jpeg', + size, + } + } else { + await deleteTemporaryImage(res.uri) + maxQualityPercentage = qualityPercentage } - } else { - maxQualityPercentage = qualityPercentage + } catch (error) { + await deleteTemporaryImage(res?.uri) + await deleteTemporaryImage(newDataUri?.path) + throw error } } @@ -271,6 +300,27 @@ export async function compressImage( throw new Error(`Unable to compress image`) } +async function deleteTemporaryImage(path: string | undefined): Promise { + if (!path) { + return + } + if (IS_WEB) { + revokeObjectUrl(path) + return + } + if (!IS_NATIVE) { + return + } + + try { + await deleteAsync(path, {idempotent: true}) + } catch (error) { + logger.info('Failed to delete temporary compressed image', { + safeMessage: error, + }) + } +} + async function moveIfNecessary(from: string) { const cacheDir = IS_NATIVE && getImageCacheDirectory() @@ -305,7 +355,9 @@ async function copyToCache(from: string): Promise { try { const response = await fetch(from) const blob = await response.blob() - return await blobToDataUri(blob) + const dataUri = await blobToDataUri(blob) + revokeObjectUrl(from) + return dataUri } catch (e) { // Blob URL was likely revoked, return as-is for downstream error handling return from diff --git a/src/view/com/composer/Composer.tsx b/src/view/com/composer/Composer.tsx index a1f1a4daf9..a5f528f456 100644 --- a/src/view/com/composer/Composer.tsx +++ b/src/view/com/composer/Composer.tsx @@ -65,6 +65,7 @@ import { VIDEO_MAX_DURATION_MS, } from '#/lib/constants' import {useNonReactiveCallback} from '#/lib/hooks/useNonReactiveCallback' +import {revokeObjectUrl} from '#/lib/media/image-manipulator' import {createVideoTelemetry} from '#/lib/media/video/telemetry' import {mimeToExt} from '#/lib/media/video/util' import {useCallOnce} from '#/lib/once' @@ -79,6 +80,7 @@ import { type ComposerImage, createComposerImage, pasteImage, + releaseComposerImage, } from '#/state/gallery' import {useRequireAltTextEnabled} from '#/state/preferences' import { @@ -157,7 +159,7 @@ import { useSaveDraftMutation, } from './drafts/state/queries' import {type DraftSummary} from './drafts/state/schema' -import {revokeAllMediaUrls} from './drafts/state/storage' +import {revokeAllMediaUrls, revokeMediaUrl} from './drafts/state/storage' import {PostLanguageSelect} from './select-language/PostLanguageSelect' import { type AssetType, @@ -189,6 +191,14 @@ type CancelRef = { onPressCancel: () => void } +function releaseEmbedMedia(media: EmbedDraft['media']): void { + if (media?.type === 'images' || media?.type === 'gallery') { + media.images.forEach(releaseComposerImage) + } else if (media?.type === 'video') { + revokeObjectUrl(media.video.asset?.uri) + } +} + function applyGalleryCap( currentCount: number, incoming: ComposerImage[], @@ -503,6 +513,10 @@ export const ComposePost = ({ const clearVideo = useCallback( (postId: string) => { + const post = thread.posts.find(item => item.id === postId) + if (post?.embed.media?.type === 'video') { + revokeObjectUrl(post.embed.media.video.asset?.uri) + } composerDispatch({ type: 'update_post', postId: postId, @@ -511,7 +525,7 @@ export const ComposePost = ({ }, }) }, - [composerDispatch], + [composerDispatch, thread.posts], ) const restoreVideo = useCallback( @@ -527,9 +541,10 @@ export const ComposePost = ({ let asset: ImagePickerAsset if (IS_WEB) { - // Web: Convert blob URL to a File, then get video metadata (returns data URL) + // Convert the saved draft URL to a File and release the old URL. const response = await fetch(videoInfo.uri) const blob = await response.blob() + revokeMediaUrl(videoInfo.uri) const file = new File([blob], 'restored-video', { type: videoInfo.mimeType, }) @@ -683,6 +698,7 @@ export const ComposePost = ({ ) // Dispatch restore action (this also sets draftId in state) + thread.posts.forEach(post => releaseEmbedMedia(post.embed.media)) composerDispatch({ type: 'restore_from_draft', draftId: draftSummary.id, @@ -715,7 +731,7 @@ export const ComposePost = ({ void restoreVideo(postId, videoInfo) } }, - [composerDispatch, restoreVideo, ax], + [composerDispatch, restoreVideo, ax, thread.posts], ) const [publishOnUpload, setPublishOnUpload] = useState(false) @@ -726,10 +742,11 @@ export const ComposePost = ({ if (IS_ANDROID) { Keyboard.dismiss() } + thread.posts.forEach(post => releaseEmbedMedia(post.embed.media)) closeComposer() clearThumbnailCache(queryClient) revokeAllMediaUrls() - }, [closeComposer, queryClient]) + }, [closeComposer, queryClient, thread.posts]) const getDraftSaveError = useCallback( (e: unknown): string => { @@ -866,11 +883,12 @@ export const ComposePost = ({ // Clear the composer (discard current content) const handleClearComposer = useCallback(() => { + thread.posts.forEach(post => releaseEmbedMedia(post.embed.media)) composerDispatch({ type: 'clear', initInteractionSettings: preferences?.postInteractionSettings, }) - }, [composerDispatch, preferences?.postInteractionSettings]) + }, [composerDispatch, preferences?.postInteractionSettings, thread.posts]) const insets = useSafeAreaInsets() const viewStyles = useMemo( @@ -1762,6 +1780,7 @@ let ComposerPost = memo(function ComposerPost({ title={l`Discard post?`} description={l`Are you sure you'd like to discard this post?`} onConfirm={() => { + releaseEmbedMedia(post.embed.media) dispatch({ type: 'remove_post', postId: post.id, diff --git a/src/view/com/composer/SelectMediaButton.tsx b/src/view/com/composer/SelectMediaButton.tsx index 83c371d336..5686f2afee 100644 --- a/src/view/com/composer/SelectMediaButton.tsx +++ b/src/view/com/composer/SelectMediaButton.tsx @@ -14,6 +14,7 @@ import { usePhotoLibraryPermission, useVideoLibraryPermission, } from '#/lib/hooks/usePermissions' +import {revokeObjectUrl} from '#/lib/media/image-manipulator' import {openUnifiedPicker} from '#/lib/media/picker' import {blobToDataUri, extractDataUriMime} from '#/lib/media/util' import {MAX_GALLERY_IMAGES} from '#/view/com/composer/state/composer' @@ -260,133 +261,151 @@ async function processImagePickerAssets( */ let supportedAssets: ValidatedImagePickerAsset[] = [] - for (const asset of assets) { - const {success, type, mimeType} = await classifyImagePickerAsset(asset) - - if (!success) { - errors.add(SelectedAssetError.Unsupported) - continue - } - - /* - * If we have an `allowedAssetTypes` prop, constrain to that. Otherwise, - * set this to the first valid asset type we see, and then use that to - * constrain all remaining selected assets. - */ - selectableAssetType = allowedAssetTypes || selectableAssetType || type - - // ignore mixed types - if (type !== selectableAssetType) { - errors.add(SelectedAssetError.MixedTypes) - continue - } - - if (type === 'video') { - /** - * We don't care too much about mimeType at this point on native, - * since the `processVideo` step later on will convert to `.mp4`. - */ - if (IS_WEB && !isSupportedVideoMimeType(mimeType)) { - errors.add(SelectedAssetError.Unsupported) - continue - } - - /* - * Filesize appears to be stable across all platforms, so we can use it - * to filter out large files on web. On native, we compress these anyway, - * so we only check on web. On web, we can reject early if the browser - * doesn't support WebCodecs. - */ - if ( - IS_WEB && - !hasWebCodecs() && - asset.fileSize && - asset.fileSize > VIDEO_MAX_SIZE - ) { - errors.add(SelectedAssetError.FileTooBig) - continue - } - } - - if (type === 'image') { - if (!isSupportedImageMimeType(mimeType)) { - errors.add(SelectedAssetError.Unsupported) - continue - } - } - - if (type === 'gif') { - /* - * Filesize appears to be stable across all platforms, so we can use it - * to filter out large files. We can't compress GIFs on either platform. - */ - if (IS_WEB && asset.fileSize && asset.fileSize > VIDEO_MAX_SIZE) { - errors.add(SelectedAssetError.FileTooBig) - continue - } - } - - /* - * All validations passed, we have an asset! - */ - let uri = asset.uri - if (IS_WEB && type === 'image' && asset.file) { - uri = await blobToDataUri(asset.file) - } - - supportedAssets.push({ - mimeType, - ...asset, - /* - * In `expo-image-picker` >= v17, `uri` is now a `blob:` URL, not a - * data-uri. Our handling elsewhere in the app (for web) relies on the - * data-uri, so read images only after their type has been validated. - * Videos retain their File/blob URL and avoid an expensive base64 read. - */ - uri, - }) + const pendingAssetUrls = new Set(assets.map(asset => asset.uri)) + const limitSupportedAssets = (limit: number) => { + supportedAssets.slice(limit).forEach(asset => revokeObjectUrl(asset.uri)) + supportedAssets = supportedAssets.slice(0, limit) } - if (supportedAssets.length > 0) { - if (selectableAssetType === 'image') { - if (supportedAssets.length > selectionCountRemaining) { - errors.add(SelectedAssetError.MaxImages) - supportedAssets = supportedAssets.slice(0, selectionCountRemaining) - } - } else if (selectableAssetType === 'video') { - if (supportedAssets.length > 1) { - errors.add(SelectedAssetError.MaxVideos) - supportedAssets = supportedAssets.slice(0, 1) - } + try { + for (const asset of assets) { + const {success, type, mimeType} = await classifyImagePickerAsset(asset) - if (supportedAssets[0].duration) { - if (IS_WEB) { - /* - * Web reports duration as seconds - */ - supportedAssets[0].duration = supportedAssets[0].duration * 1000 - } - - if (supportedAssets[0].duration > videoMaxDurationMs) { - errors.add(SelectedAssetError.VideoTooLong) - supportedAssets = [] - } - } else { + if (!success) { errors.add(SelectedAssetError.Unsupported) - supportedAssets = [] + continue } - } else if (selectableAssetType === 'gif') { - if (supportedAssets.length > 1) { - errors.add(SelectedAssetError.MaxGIFs) - supportedAssets = supportedAssets.slice(0, 1) + + /* + * If we have an `allowedAssetTypes` prop, constrain to that. Otherwise, + * set this to the first valid asset type we see, and then use that to + * constrain all remaining selected assets. + */ + selectableAssetType = allowedAssetTypes || selectableAssetType || type + + // ignore mixed types + if (type !== selectableAssetType) { + errors.add(SelectedAssetError.MixedTypes) + continue + } + + if (type === 'video') { + /** + * We don't care too much about mimeType at this point on native, + * since the `processVideo` step later on will convert to `.mp4`. + */ + if (IS_WEB && !isSupportedVideoMimeType(mimeType)) { + errors.add(SelectedAssetError.Unsupported) + continue + } + + /* + * Filesize appears to be stable across all platforms, so we can use it + * to filter out large files on web. On native, we compress these anyway, + * so we only check on web. On web, we can reject early if the browser + * doesn't support WebCodecs. + */ + if ( + IS_WEB && + !hasWebCodecs() && + asset.fileSize && + asset.fileSize > VIDEO_MAX_SIZE + ) { + errors.add(SelectedAssetError.FileTooBig) + continue + } + } + + if (type === 'image') { + if (!isSupportedImageMimeType(mimeType)) { + errors.add(SelectedAssetError.Unsupported) + continue + } + } + + if (type === 'gif') { + /* + * Filesize appears to be stable across all platforms, so we can use it + * to filter out large files. We can't compress GIFs on either platform. + */ + if (IS_WEB && asset.fileSize && asset.fileSize > VIDEO_MAX_SIZE) { + errors.add(SelectedAssetError.FileTooBig) + continue + } + } + + /* + * All validations passed, we have an asset! + */ + let uri = asset.uri + if (IS_WEB && type === 'image' && asset.file) { + try { + uri = await blobToDataUri(asset.file) + } finally { + revokeObjectUrl(asset.uri) + } + } + + supportedAssets.push({ + mimeType, + ...asset, + /* + * In `expo-image-picker` >= v17, `uri` is now a `blob:` URL, not a + * data-uri. Our handling elsewhere in the app (for web) relies on the + * data-uri, so read images only after their type has been validated. + * Videos retain their File/blob URL and avoid an expensive base64 read. + */ + uri, + }) + pendingAssetUrls.delete(asset.uri) + } + + if (supportedAssets.length > 0) { + if (selectableAssetType === 'image') { + if (supportedAssets.length > selectionCountRemaining) { + errors.add(SelectedAssetError.MaxImages) + limitSupportedAssets(selectionCountRemaining) + } + } else if (selectableAssetType === 'video') { + if (supportedAssets.length > 1) { + errors.add(SelectedAssetError.MaxVideos) + limitSupportedAssets(1) + } + + if (supportedAssets[0].duration) { + if (IS_WEB) { + /* + * Web reports duration as seconds + */ + supportedAssets[0].duration = supportedAssets[0].duration * 1000 + } + + if (supportedAssets[0].duration > videoMaxDurationMs) { + errors.add(SelectedAssetError.VideoTooLong) + limitSupportedAssets(0) + } + } else { + errors.add(SelectedAssetError.Unsupported) + limitSupportedAssets(0) + } + } else if (selectableAssetType === 'gif') { + if (supportedAssets.length > 1) { + errors.add(SelectedAssetError.MaxGIFs) + limitSupportedAssets(1) + } } } - } - return { - type: selectableAssetType!, // set above - assets: supportedAssets, - errors, + return { + type: selectableAssetType!, // set above + assets: supportedAssets, + errors, + } + } catch (error) { + supportedAssets.forEach(asset => revokeObjectUrl(asset.uri)) + throw error + } finally { + pendingAssetUrls.forEach(revokeObjectUrl) } } diff --git a/src/view/com/composer/drafts/DraftItem.tsx b/src/view/com/composer/drafts/DraftItem.tsx index e741d2a6bd..614e0780bb 100644 --- a/src/view/com/composer/drafts/DraftItem.tsx +++ b/src/view/com/composer/drafts/DraftItem.tsx @@ -283,6 +283,18 @@ function DraftMediaPreview({post}: {post: DraftPostDisplay}) { const [videoThumbnail, setVideoThumbnail] = useState() useEffect(() => { + let cancelled = false + const ownedUrls = new Set() + + const ownUrl = (url: string): boolean => { + if (cancelled) { + storage.revokeMediaUrl(url) + return false + } + ownedUrls.add(url) + return true + } + async function loadMedia() { if (post.images && post.images.length > 0) { const loaded: LoadedImage[] = [] @@ -290,36 +302,49 @@ function DraftMediaPreview({post}: {post: DraftPostDisplay}) { const alt = image.altText || '' try { const url = await storage.loadMediaFromLocal(image.localPath) - loaded.push({url, alt}) + if (ownUrl(url)) { + loaded.push({url, alt}) + } } catch (e) { // Image doesn't exist locally, skip it } } - setLoadedImages(loaded) + if (!cancelled) { + setLoadedImages(loaded) + } } if (post.video?.exists && post.video.localPath) { - try { - const url = await storage.loadMediaFromLocal(post.video.localPath) - if (IS_WEB) { - // can't generate thumbnails on web + if (IS_WEB) { + if (!cancelled) { + // We cannot generate draft video thumbnails on web. setVideoThumbnail("yep, there's a video") - } else { + } + } else { + try { + const url = await storage.loadMediaFromLocal(post.video.localPath) + if (!ownUrl(url)) return logger.debug('generating thumbnail of ', {url}) const thumbnail = await VideoThumbnails.getThumbnailAsync(url, { time: 0, quality: 0.2, }) logger.debug('thumbnail generated', {thumbnail}) - setVideoThumbnail(thumbnail.uri) + if (!cancelled) { + setVideoThumbnail(thumbnail.uri) + } + } catch (e) { + // Video doesn't exist locally } - } catch (e) { - // Video doesn't exist locally } } } void loadMedia() + return () => { + cancelled = true + ownedUrls.forEach(storage.revokeMediaUrl) + } }, [post.images, post.video]) // Nothing to show diff --git a/src/view/com/composer/photos/Gallery.tsx b/src/view/com/composer/photos/Gallery.tsx index 096c197716..79292c2cca 100644 --- a/src/view/com/composer/photos/Gallery.tsx +++ b/src/view/com/composer/photos/Gallery.tsx @@ -18,7 +18,11 @@ import {Trans} from '@lingui/react/macro' import {useWebMediaQueries} from '#/lib/hooks/useWebMediaQueries' import {type Dimensions} from '#/lib/media/types' import {colors} from '#/lib/styles' -import {type ComposerImage, cropImage} from '#/state/gallery' +import { + type ComposerImage, + cropImage, + releaseComposerImage, +} from '#/state/gallery' import {atoms as a, tokens, useTheme} from '#/alf' import {Admonition} from '#/components/Admonition' import * as Dialog from '#/components/Dialog' @@ -119,6 +123,7 @@ const GalleryInner = ({images, containerInfo, dispatch}: GalleryInnerProps) => { dispatch({type: 'embed_update_image', image: next}) }} onRemove={() => { + releaseComposerImage(image) dispatch({type: 'embed_remove_image', image}) }} />