diff --git a/packages/upload-media/CHANGELOG.md b/packages/upload-media/CHANGELOG.md index efedf1fffa2bc3..618b8d29d9273b 100644 --- a/packages/upload-media/CHANGELOG.md +++ b/packages/upload-media/CHANGELOG.md @@ -9,6 +9,11 @@ ### Enhancements - Honor the `image_strip_meta` and `image_max_bit_depth` filters for client-side processed images via the new `imageStripMeta` and `imageMaxBitDepth` settings, carried in the REST API root index ([#80216](https://github.com/WordPress/gutenberg/issues/80216)). +- Animated GIF to video conversion is now abandoned after a timeout (default 30 seconds) and skipped entirely for GIFs whose total decoded pixels (width × height × frame count) exceed a budget. In both cases the original GIF upload is kept and no error is surfaced; a `SCRIPT_DEBUG` diagnostic explains the skip. Both knobs are configurable via the `TranscodeGif` operation args ([#80376](https://github.com/WordPress/gutenberg/issues/80376)). + +### Bug Fixes + +- `cancelItem` no longer awaits the best-effort worker cancellation calls. A busy vips worker is synchronously blocked inside a wasm call and cannot answer the cancellation RPC until every operation already queued in the worker finishes, which for a large animated GIF left cancelled items stuck in the queue - and the parent attachment unfinalized - for minutes ([#80376](https://github.com/WordPress/gutenberg/issues/80376)). ## 0.36.0 (2026-07-14) diff --git a/packages/upload-media/src/store/actions.ts b/packages/upload-media/src/store/actions.ts index 8513a081a77f54..6a5126d36cfb9d 100644 --- a/packages/upload-media/src/store/actions.ts +++ b/packages/upload-media/src/store/actions.ts @@ -206,15 +206,23 @@ export function cancelItem( id: QueueItemId, error: Error, silent = false ) { * best-effort cleanup: when the worker has crashed, the cancel call * itself rejects, and that must not abort the cancellation flow — * onError, item removal, and worker teardown below still need to run. + * + * Deliberately NOT awaited: a busy worker is synchronously blocked + * inside a wasm call and cannot answer the cancellation RPC until + * the current operation — and every operation already queued behind + * it — finishes. For a large animated GIF that is minutes, and + * awaiting here would leave the cancelled item in the queue (gating + * the parent's finalization) that whole time. Nothing below depends + * on the result. */ - await vipsCancelOperations( id ).catch( () => {} ); + vipsCancelOperations( id ).catch( () => {} ); /* * Cancel any ongoing GIF-to-video conversion for this item so a * cancelled upload does not leave the encoder running off-thread. - * Best-effort for the same reason as above. + * Best-effort and fire-and-forget for the same reasons as above. */ - await cancelGifToVideoOperations( id ).catch( () => {} ); + cancelGifToVideoOperations( id ).catch( () => {} ); if ( ! silent ) { const { onError } = item; diff --git a/packages/upload-media/src/store/private-actions.ts b/packages/upload-media/src/store/private-actions.ts index 93a4ebd3737819..f4826d788d81cd 100644 --- a/packages/upload-media/src/store/private-actions.ts +++ b/packages/upload-media/src/store/private-actions.ts @@ -30,7 +30,7 @@ import { getImageDimensions } from '../get-image-dimensions'; import { CLIENT_SIDE_SUPPORTED_MIME_TYPES, HEIC_MIME_TYPES } from './constants'; import { StubFile } from '../stub-file'; import { ErrorCode, UploadError } from '../upload-error'; -import { measure } from './utils/debug-logger'; +import { debug, measure } from './utils/debug-logger'; import { vipsResizeImage, vipsRotateImage, @@ -42,6 +42,8 @@ import { } from './utils'; import { convertGifToVideo, + isConversionTimeoutError, + isSizeLimitConversionError, isUnsupportedConversionError, terminateVideoConversionWorker, } from './utils/video-conversion'; @@ -442,6 +444,10 @@ export function processItem( id: QueueItemId ) { operation, } ); + debug( + `Starting operation ${ operation } for ${ item.file.name } (item ${ item.id })` + ); + switch ( operation ) { case OperationType.Prepare: dispatch.prepareItem( item.id ); @@ -1402,7 +1408,11 @@ export function transcodeGifItem( const file = await convertGifToVideo( item.id, gifFile, - outputMimeType + outputMimeType, + { + timeout: args?.timeout, + maxTotalPixels: args?.maxTotalPixels, + } ); // Hand the transcoded video to the next Upload op as the @@ -1450,6 +1460,19 @@ export function transcodeGifItem( // create an `animated_video` meta entry pointing at the GIF // itself — meaningless. if ( isUnsupportedConversionError( error ) ) { + /* + * A GIF skipped for exceeding the total-pixel budget is a + * graceful outcome like any other unsupported conversion, + * but worth a SCRIPT_DEBUG diagnostic so developers testing + * large GIFs understand why no companion video was produced. + */ + if ( isSizeLimitConversionError( error ) ) { + debug( + `Skipping GIF to video conversion: ${ + error instanceof Error ? error.message : error + }` + ); + } dispatch.cancelItem( id, new Error( 'Animated GIF conversion unsupported' ), @@ -1457,6 +1480,25 @@ export function transcodeGifItem( ); return; } + /* + * The conversion ran past the allowed time and was abandoned: + * the GIF attachment stands alone, which is exactly what the + * user uploaded. SCRIPT_DEBUG diagnostic only, no user-facing + * error. + */ + if ( isConversionTimeoutError( error ) ) { + debug( + `GIF to video conversion timed out; keeping the original GIF only: ${ + error instanceof Error ? error.message : error + }` + ); + dispatch.cancelItem( + id, + new Error( 'Animated GIF conversion timed out' ), + true + ); + return; + } // Real engine failure. The parent GIF attachment is fine — // the user just won't get a companion video. Log the cause // for debuggability and silently cancel the sideload; we do diff --git a/packages/upload-media/src/store/test/actions.ts b/packages/upload-media/src/store/test/actions.ts index f08d225afdce72..3eac392567ca5b 100644 --- a/packages/upload-media/src/store/test/actions.ts +++ b/packages/upload-media/src/store/test/actions.ts @@ -751,6 +751,45 @@ describe( 'actions', () => { ).toHaveLength( 0 ); } ); + it( 'removes the item even when the workers never answer the cancel calls', async () => { + /* + * A busy vips worker is synchronously blocked inside a wasm call + * and cannot answer the cancellation RPC until the current + * operation - and every operation already queued behind it - + * finishes, which for a large animated GIF takes minutes. The + * worker cancels are best-effort cleanup; cancelItem must not + * block on them, or the cancelled item lingers in the queue and + * gates the parent's finalization the whole time. + */ + ( vipsCancelOperations as jest.Mock ).mockImplementationOnce( + () => new Promise( () => {} ) + ); + ( cancelGifToVideoOperations as jest.Mock ).mockImplementationOnce( + () => new Promise( () => {} ) + ); + + const onError = jest.fn(); + unlock( registry.dispatch( uploadStore ) ).addItem( { + file: jpegFile, + onError, + } ); + const item = unlock( + registry.select( uploadStore ) + ).getAllItems()[ 0 ]; + + await registry + .dispatch( uploadStore ) + .cancelItem( item.id, new Error( 'Test error' ) ); + + expect( vipsCancelOperations ).toHaveBeenCalledWith( item.id ); + expect( onError ).toHaveBeenCalledWith( + expect.objectContaining( { message: 'Test error' } ) + ); + expect( + unlock( registry.select( uploadStore ) ).getAllItems() + ).toHaveLength( 0 ); + } ); + describe( 'parent cancellation when child sideload fails', () => { // Helpers used by every scenario below. Set up a parent that // has finished its primary upload (so it has an attachment.id), diff --git a/packages/upload-media/src/store/test/private-actions.js b/packages/upload-media/src/store/test/private-actions.js index ba5cc43741bf64..256f949c4afe04 100644 --- a/packages/upload-media/src/store/test/private-actions.js +++ b/packages/upload-media/src/store/test/private-actions.js @@ -66,6 +66,8 @@ jest.mock( '../utils/video-conversion', () => { cancelGifToVideoOperations: jest.fn(), terminateVideoConversionWorker: jest.fn(), isUnsupportedConversionError: actual.isUnsupportedConversionError, + isSizeLimitConversionError: actual.isSizeLimitConversionError, + isConversionTimeoutError: actual.isConversionTimeoutError, }; } ); @@ -723,6 +725,7 @@ describe( 'private actions', () => { } let consoleError; + let consoleDebug; beforeEach( () => { convertGifToVideo.mockReset(); @@ -730,10 +733,14 @@ describe( 'private actions', () => { consoleError = jest .spyOn( console, 'error' ) .mockImplementation( () => {} ); + consoleDebug = jest + .spyOn( console, 'debug' ) + .mockImplementation( () => {} ); } ); afterEach( () => { consoleError.mockRestore(); + consoleDebug.mockRestore(); } ); it( 'hands the transcoded video to the next Upload via finishOperation', async () => { @@ -751,7 +758,8 @@ describe( 'private actions', () => { expect( convertGifToVideo ).toHaveBeenCalledWith( 'gif-1', gifFile, - 'video/mp4' + 'video/mp4', + { timeout: undefined, maxTotalPixels: undefined } ); // Sideload context: no CacheBlobUrl, no attachment URL update - // the parent GIF attachment already owns the block's URL. @@ -811,7 +819,30 @@ describe( 'private actions', () => { expect( convertGifToVideo ).toHaveBeenCalledWith( 'gif-1', gifFile, - 'video/mp4' + 'video/mp4', + { timeout: undefined, maxTotalPixels: undefined } + ); + } ); + + it( 'passes timeout and maxTotalPixels operation args through', async () => { + convertGifToVideo.mockResolvedValue( + new File( [ 'webm' ], 'animation.webm', { + type: 'video/webm', + } ) + ); + const { select, dispatch } = buildArgs(); + + await transcodeGifItem( 'gif-1', { + outputFormat: 'webm', + timeout: 5000, + maxTotalPixels: 1_000_000, + } )( { select, dispatch } ); + + expect( convertGifToVideo ).toHaveBeenCalledWith( + 'gif-1', + gifFile, + 'video/webm', + { timeout: 5000, maxTotalPixels: 1_000_000 } ); } ); @@ -838,6 +869,58 @@ describe( 'private actions', () => { expect( dispatch.addSideloadItem ).not.toHaveBeenCalled(); } ); + it( 'logs a SCRIPT_DEBUG diagnostic and silently cancels when the GIF exceeds the conversion size limit', async () => { + // An over-budget GIF is a graceful skip like any Unsupported + // outcome, but the skip is logged (under SCRIPT_DEBUG, true in + // this test env) so developers testing large GIFs understand + // why no companion video was produced. + convertGifToVideo.mockRejectedValue( + new Error( + 'Unsupported: GIF exceeds maximum conversion size (5000x5000 x 100 frames = 2500000000 pixels; limit is 300000000)' + ) + ); + const { select, dispatch } = buildArgs(); + + await transcodeGifItem( 'gif-1' )( { select, dispatch } ); + + expect( dispatch.finishOperation ).not.toHaveBeenCalled(); + expect( dispatch.cancelItem ).toHaveBeenCalledTimes( 1 ); + const [ cancelledId, , silent ] = + dispatch.cancelItem.mock.calls[ 0 ]; + expect( cancelledId ).toBe( 'gif-1' ); + expect( silent ).toBe( true ); + expect( consoleDebug ).toHaveBeenCalledWith( + expect.stringContaining( 'exceeds maximum conversion size' ) + ); + expect( consoleError ).not.toHaveBeenCalled(); + expect( dispatch.addSideloadItem ).not.toHaveBeenCalled(); + } ); + + it( 'logs a SCRIPT_DEBUG diagnostic and silently cancels when the conversion times out', async () => { + // The conversion was abandoned after the timeout; the GIF + // attachment stands alone. No user-facing error, but the + // timeout is logged (under SCRIPT_DEBUG) for debuggability. + convertGifToVideo.mockRejectedValue( + new Error( 'GIF to video conversion timed out after 30000ms' ) + ); + const { select, dispatch } = buildArgs(); + + await transcodeGifItem( 'gif-1' )( { select, dispatch } ); + + expect( dispatch.finishOperation ).not.toHaveBeenCalled(); + expect( dispatch.cancelItem ).toHaveBeenCalledTimes( 1 ); + const [ cancelledId, error, silent ] = + dispatch.cancelItem.mock.calls[ 0 ]; + expect( cancelledId ).toBe( 'gif-1' ); + expect( error.message ).toMatch( /timed out/i ); + expect( silent ).toBe( true ); + expect( consoleDebug ).toHaveBeenCalledWith( + expect.stringContaining( 'timed out' ) + ); + expect( consoleError ).not.toHaveBeenCalled(); + expect( dispatch.addSideloadItem ).not.toHaveBeenCalled(); + } ); + it( 'silently cancels the sideload on a hard transcoding failure', async () => { // A real engine failure is also not user-actionable on what the // user thinks of as an image upload: log the cause, cancel diff --git a/packages/upload-media/src/store/types.ts b/packages/upload-media/src/store/types.ts index 209a7dd8b474a5..ef8cf01748d224 100644 --- a/packages/upload-media/src/store/types.ts +++ b/packages/upload-media/src/store/types.ts @@ -378,6 +378,18 @@ export interface OperationArgs { [ OperationType.TranscodeGif ]: { /** Video output format: 'mp4' or 'webm'. */ outputFormat: 'mp4' | 'webm'; + /** + * Time in milliseconds before the conversion is abandoned and only + * the original GIF is kept. `0` disables the timeout. Defaults to + * 30 seconds. + */ + timeout?: number; + /** + * Budget for total decoded pixels (width × height × frame count) + * beyond which conversion is not attempted. `0` disables the check. + * Defaults to the `@wordpress/video-conversion` package default. + */ + maxTotalPixels?: number; }; } diff --git a/packages/upload-media/src/store/utils/test/video-conversion.ts b/packages/upload-media/src/store/utils/test/video-conversion.ts index 853c474a8797e8..65a88067624c60 100644 --- a/packages/upload-media/src/store/utils/test/video-conversion.ts +++ b/packages/upload-media/src/store/utils/test/video-conversion.ts @@ -1,7 +1,11 @@ /** * Internal dependencies */ -import { isUnsupportedConversionError } from '../video-conversion'; +import { + isUnsupportedConversionError, + isSizeLimitConversionError, + isConversionTimeoutError, +} from '../video-conversion'; describe( 'isUnsupportedConversionError', () => { // These are the exact messages thrown by @wordpress/video-conversion's @@ -13,6 +17,7 @@ describe( 'isUnsupportedConversionError', () => { it.each( [ 'Unsupported: WebCodecs unavailable', 'Unsupported: encoder codec not supported', + 'Unsupported: GIF exceeds maximum conversion size (5000x5000 x 100 frames = 2500000000 pixels; limit is 300000000)', ] )( 'recognizes graceful outcome: %s', ( message ) => { expect( isUnsupportedConversionError( new Error( message ) ) ).toBe( true @@ -33,6 +38,48 @@ describe( 'isUnsupportedConversionError', () => { } ); } ); +describe( 'isSizeLimitConversionError', () => { + it( 'recognizes the size-limit message thrown by the worker (contract)', () => { + // Exact wording thrown by @wordpress/video-conversion when the + // total-pixel budget is exceeded; duplicated here because the worker + // boundary only carries the message string. + const error = new Error( + 'Unsupported: GIF exceeds maximum conversion size (5000x5000 x 100 frames = 2500000000 pixels; limit is 300000000)' + ); + expect( isSizeLimitConversionError( error ) ).toBe( true ); + // A size-limit skip is also a graceful "unsupported" outcome. + expect( isUnsupportedConversionError( error ) ).toBe( true ); + } ); + + it( 'does not match other unsupported outcomes', () => { + expect( + isSizeLimitConversionError( + new Error( 'Unsupported: WebCodecs unavailable' ) + ) + ).toBe( false ); + expect( isSizeLimitConversionError( undefined ) ).toBe( false ); + } ); +} ); + +describe( 'isConversionTimeoutError', () => { + it( 'recognizes the timeout error thrown by convertGifToVideo', () => { + expect( + isConversionTimeoutError( + new Error( 'GIF to video conversion timed out after 30000ms' ) + ) + ).toBe( true ); + } ); + + it( 'does not match other errors', () => { + expect( + isConversionTimeoutError( + new Error( 'Unsupported: WebCodecs unavailable' ) + ) + ).toBe( false ); + expect( isConversionTimeoutError( 'timed out' ) ).toBe( false ); + } ); +} ); + /* * The remaining functions wrap the lazily-imported * @wordpress/video-conversion/worker module (mapped to a jest stub via @@ -58,12 +105,9 @@ describe( 'convertGifToVideo', () => { } ); - const result = await convertGifToVideo( - 'item-1', - gif, - 'video/mp4', - 720 - ); + const result = await convertGifToVideo( 'item-1', gif, 'video/mp4', { + maxDimensions: 720, + } ); /* * The original File (not an ArrayBuffer) is passed straight through so @@ -73,7 +117,8 @@ describe( 'convertGifToVideo', () => { 'item-1', gif, 'video/mp4', - 720 + 720, + undefined ); expect( result ).toBeInstanceOf( File ); expect( result.name ).toBe( 'my-anim.mp4' ); @@ -95,6 +140,7 @@ describe( 'convertGifToVideo', () => { 'item-2', gif, 'video/webm', + undefined, undefined ); expect( result.name ).toBe( 'clip.webm' ); @@ -120,6 +166,108 @@ describe( 'convertGifToVideo', () => { } ); } ); +describe( 'convertGifToVideo timeout', () => { + beforeEach( () => { + jest.resetModules(); + jest.useFakeTimers(); + } ); + + afterEach( () => { + jest.useRealTimers(); + } ); + + function makeGif() { + return new File( [ new Uint8Array( [ 0 ] ) ], 'anim.gif', { + type: 'image/gif', + } ); + } + + it( 'abandons a conversion that exceeds the default 30s timeout and cancels the worker', async () => { + const worker = require( '@wordpress/video-conversion/worker' ); + // A conversion that never settles, like a huge GIF churning away. + worker.convertGifToVideo.mockReturnValue( new Promise( () => {} ) ); + worker.cancelGifToVideoOperations.mockResolvedValue( true ); + + const { convertGifToVideo } = require( '../video-conversion' ); + + const promise = convertGifToVideo( 'item-1', makeGif(), 'video/mp4' ); + // Attach a handler before advancing time so the rejection is never + // reported as unhandled. + promise.catch( () => {} ); + + await jest.advanceTimersByTimeAsync( 30_000 ); + + // The exact message is the isConversionTimeoutError contract. + await expect( promise ).rejects.toThrow( + 'GIF to video conversion timed out after 30000ms' + ); + // The worker-side operation was cancelled so it stops churning at + // its next async boundary. + expect( worker.cancelGifToVideoOperations ).toHaveBeenCalledWith( + 'item-1' + ); + } ); + + it( 'does not time out a conversion that finishes in time', async () => { + const worker = require( '@wordpress/video-conversion/worker' ); + worker.convertGifToVideo.mockResolvedValue( new ArrayBuffer( 4 ) ); + + const { convertGifToVideo } = require( '../video-conversion' ); + + const promise = convertGifToVideo( 'item-2', makeGif(), 'video/mp4' ); + await jest.advanceTimersByTimeAsync( 0 ); + const result = await promise; + + expect( result ).toBeInstanceOf( File ); + expect( worker.cancelGifToVideoOperations ).not.toHaveBeenCalled(); + // The timeout timer was cleared; nothing left pending. + expect( jest.getTimerCount() ).toBe( 0 ); + } ); + + it( 'honors a custom timeout', async () => { + const worker = require( '@wordpress/video-conversion/worker' ); + worker.convertGifToVideo.mockReturnValue( new Promise( () => {} ) ); + worker.cancelGifToVideoOperations.mockResolvedValue( true ); + + const { convertGifToVideo } = require( '../video-conversion' ); + + const promise = convertGifToVideo( 'item-3', makeGif(), 'video/mp4', { + timeout: 5_000, + } ); + promise.catch( () => {} ); + + await jest.advanceTimersByTimeAsync( 5_000 ); + + await expect( promise ).rejects.toThrow( /timed out/i ); + } ); + + it( 'treats a timeout of 0 as disabled', async () => { + const worker = require( '@wordpress/video-conversion/worker' ); + let resolveConversion: ( buffer: ArrayBuffer ) => void = () => {}; + worker.convertGifToVideo.mockReturnValue( + new Promise( ( resolve ) => { + resolveConversion = resolve; + } ) + ); + + const { convertGifToVideo } = require( '../video-conversion' ); + + const promise = convertGifToVideo( 'item-4', makeGif(), 'video/mp4', { + timeout: 0, + } ); + + // Way past the default timeout: nothing should reject because no + // timer was ever set. + await jest.advanceTimersByTimeAsync( 120_000 ); + expect( jest.getTimerCount() ).toBe( 0 ); + + resolveConversion( new ArrayBuffer( 4 ) ); + const result = await promise; + expect( result ).toBeInstanceOf( File ); + expect( worker.cancelGifToVideoOperations ).not.toHaveBeenCalled(); + } ); +} ); + describe( 'cancelGifToVideoOperations', () => { beforeEach( () => { jest.resetModules(); diff --git a/packages/upload-media/src/store/utils/video-conversion.ts b/packages/upload-media/src/store/utils/video-conversion.ts index af1e542692e7b3..4e7c6323a6345b 100644 --- a/packages/upload-media/src/store/utils/video-conversion.ts +++ b/packages/upload-media/src/store/utils/video-conversion.ts @@ -19,6 +19,30 @@ import type { QueueItemId } from '../types'; */ const UNSUPPORTED_ERROR_PREFIX = 'Unsupported'; +/** + * Message prefix used by @wordpress/video-conversion for GIFs skipped + * because they exceed the total-pixel budget. This MUST mirror the + * package's exported `SIZE_LIMIT_ERROR_PREFIX`; it is duplicated here for + * the same bundle-size reason as UNSUPPORTED_ERROR_PREFIX above. + */ +const SIZE_LIMIT_ERROR_PREFIX = `${ UNSUPPORTED_ERROR_PREFIX }: GIF exceeds maximum conversion size`; + +/** + * Message prefix for conversions abandoned by the timeout in + * `convertGifToVideo` below. Thrown and detected on the main thread, but + * kept as a message-prefix contract for consistency with the other + * conversion outcomes. + */ +const CONVERSION_TIMEOUT_ERROR_PREFIX = 'GIF to video conversion timed out'; + +/** + * Default time in milliseconds a GIF-to-video conversion may run before it + * is abandoned and only the original GIF is kept. + * + * Beyond ~30 seconds the companion video stops being worth the CPU churn: + */ +export const DEFAULT_CONVERSION_TIMEOUT = 30_000; + /** * Whether an error from GIF-to-video conversion represents an * unsupported-but-graceful outcome (caller should fall back to uploading the @@ -34,6 +58,36 @@ export function isUnsupportedConversionError( error: unknown ): boolean { ); } +/** + * Whether an error from GIF-to-video conversion means the GIF was skipped + * for exceeding the total-pixel budget. Such errors are also "unsupported" + * (graceful) outcomes; this narrower check exists so callers can log why + * no companion video was produced. + * + * @param error Error thrown by `convertGifToVideo`. + * @return Whether the error is a size-limit skip. + */ +export function isSizeLimitConversionError( error: unknown ): boolean { + return ( + error instanceof Error && + error.message.startsWith( SIZE_LIMIT_ERROR_PREFIX ) + ); +} + +/** + * Whether an error from GIF-to-video conversion means the conversion was + * abandoned because it exceeded the allowed time. + * + * @param error Error thrown by `convertGifToVideo`. + * @return Whether the error is a conversion timeout. + */ +export function isConversionTimeoutError( error: unknown ): boolean { + return ( + error instanceof Error && + error.message.startsWith( CONVERSION_TIMEOUT_ERROR_PREFIX ) + ); +} + /** * Cached dynamic import promise for @wordpress/video-conversion/worker. * @@ -80,25 +134,96 @@ function loadVideoConversionModule(): Promise< return videoConversionModulePromise; } +interface ConvertGifToVideoOptions { + /** Maximum dimension for downscaling. */ + maxDimensions?: number; + /** + * Time in milliseconds before the conversion is abandoned and only the + * original GIF is kept. `0` disables the timeout. + */ + timeout?: number; + /** + * Budget for total decoded pixels (width × height × frame count) beyond + * which the conversion is not attempted. Defaults to the + * `@wordpress/video-conversion` package default; `0` disables the check. + */ + maxTotalPixels?: number; +} + /** * Converts an animated GIF to a video file using the video conversion worker. * - * @param id Queue item ID. - * @param file GIF file object. - * @param outputMimeType Output MIME type ('video/mp4' or 'video/webm'). - * @param maxDimensions Optional maximum dimension for downscaling. + * The timeout only covers the conversion itself, not the initial lazy load + * of the worker module (which has its own retry handling). On timeout the + * worker-side operation is cancelled so it stops churning, and the returned + * promise rejects with an error recognized by `isConversionTimeoutError`. + * + * @param id Queue item ID. + * @param file GIF file object. + * @param outputMimeType Output MIME type ('video/mp4' or 'video/webm'). + * @param options Conversion options. + * @param options.maxDimensions Maximum dimension for downscaling. + * @param options.timeout Milliseconds before the conversion is + * abandoned. `0` disables the timeout. + * @param options.maxTotalPixels Budget for total decoded pixels + * (width × height × frame count). `0` disables. * @return Converted video file. */ export async function convertGifToVideo( id: QueueItemId, file: File, outputMimeType: string, - maxDimensions?: number + { + maxDimensions, + timeout = DEFAULT_CONVERSION_TIMEOUT, + maxTotalPixels, + }: ConvertGifToVideoOptions = {} ) { - const { convertGifToVideo: convert } = await loadVideoConversionModule(); + const mod = await loadVideoConversionModule(); // Pass the File straight through: the worker reads its bytes once, off // the main thread, instead of materializing an ArrayBuffer here. - const buffer = await convert( id, file, outputMimeType, maxDimensions ); + const conversion = mod.convertGifToVideo( + id, + file, + outputMimeType, + maxDimensions, + maxTotalPixels + ); + + let buffer: ArrayBuffer; + if ( timeout > 0 ) { + let timer: ReturnType< typeof setTimeout > | undefined; + try { + buffer = await Promise.race( [ + conversion, + new Promise< never >( ( _, reject ) => { + timer = setTimeout( () => { + reject( + new Error( + `${ CONVERSION_TIMEOUT_ERROR_PREFIX } after ${ timeout }ms` + ) + ); + }, timeout ); + } ), + ] ); + } catch ( error ) { + if ( isConversionTimeoutError( error ) ) { + /* + * The race already settled; swallow the orphaned conversion + * promise's eventual rejection ("Operation cancelled") so it + * is not reported as unhandled. + */ + conversion.catch( () => {} ); + // Stop the worker loop at its next async boundary. + mod.cancelGifToVideoOperations( id ).catch( () => {} ); + } + throw error; + } finally { + clearTimeout( timer ); + } + } else { + buffer = await conversion; + } const ext = outputMimeType === 'video/webm' ? 'webm' : 'mp4'; const fileName = `${ getFileBasename( file.name ) }.${ ext }`; diff --git a/packages/video-conversion/CHANGELOG.md b/packages/video-conversion/CHANGELOG.md index 4319be50ac8774..96edd66b872b47 100644 --- a/packages/video-conversion/CHANGELOG.md +++ b/packages/video-conversion/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### New Features + +- `convertGifToVideo` now enforces a budget on total decoded pixels (width × height × frame count, default 300 megapixels, exported as `DEFAULT_MAX_TOTAL_PIXELS`). Over-budget GIFs are rejected up front with the new `SIZE_LIMIT_ERROR_PREFIX` message prefix so callers can skip the conversion gracefully instead of churning the CPU for minutes ([#80376](https://github.com/WordPress/gutenberg/issues/80376)). + ### Enhancements - Use a 10-second key frame interval when converting GIFs to video, roughly halving the output file size for long animations at no quality or encode-time cost ([#80266](https://github.com/WordPress/gutenberg/issues/80266)). diff --git a/packages/video-conversion/README.md b/packages/video-conversion/README.md index fa461fd82d1304..9fc538ee2ee0ff 100644 --- a/packages/video-conversion/README.md +++ b/packages/video-conversion/README.md @@ -49,11 +49,24 @@ _Parameters_ - _gifSource_ `ArrayBuffer | Blob`: GIF file as a Blob/File or ArrayBuffer. - _outputMimeType_ `string`: Output MIME type ('video/mp4' or 'video/webm'). - _maxDimensions_ `number`: Optional maximum dimension for downscaling. +- _maxTotalPixels_ `number`: Optional budget for total decoded pixels (width × height × frame count) beyond which the conversion is rejected with SIZE_LIMIT_ERROR_PREFIX. Defaults to DEFAULT_MAX_TOTAL_PIXELS; `0` disables. _Returns_ - `Promise< ArrayBuffer >`: Encoded video buffer. +### DEFAULT_MAX_TOTAL_PIXELS + +Default budget for total decoded pixels (width × height × frame count) beyond which conversion is not attempted. + +Conversion cost is roughly proportional to the total number of decoded pixels. 300 megapixels approximates what a mid-range machine converts within the ~30s the caller is willing to wait (e.g. a 1920x1080 GIF at ~145 frames); anything larger would likely be abandoned anyway, so it is cheaper to not start. Pass `0` to disable the check. + +### SIZE_LIMIT_ERROR_PREFIX + +Message prefix for GIFs skipped because they exceed the total-pixel budget. + +Starts with UNSUPPORTED_ERROR_PREFIX so existing consumers treat the skip as a graceful fallback (keep the uploaded GIF, no companion video); the longer prefix lets consumers distinguish it, e.g. to log a warning. Like UNSUPPORTED_ERROR_PREFIX, the contract is the message prefix because only the message string survives the worker boundary. + ### UNSUPPORTED_ERROR_PREFIX Message prefix for "unsupported but graceful" outcomes (no WebCodecs, unsupported codec). Consumers detect this prefix and fall back to uploading the original GIF instead of surfacing a hard error. diff --git a/packages/video-conversion/src/index.ts b/packages/video-conversion/src/index.ts index 131ec1badfb89f..3fb5ad8acc298d 100644 --- a/packages/video-conversion/src/index.ts +++ b/packages/video-conversion/src/index.ts @@ -39,6 +39,29 @@ const GIF_DEFAULT_FRAME_DURATION_US = 100_000; */ export const UNSUPPORTED_ERROR_PREFIX = 'Unsupported'; +/** + * Message prefix for GIFs skipped because they exceed the total-pixel budget. + * + * Starts with UNSUPPORTED_ERROR_PREFIX so existing consumers treat the skip + * as a graceful fallback (keep the uploaded GIF, no companion video); the + * longer prefix lets consumers distinguish it, e.g. to log a warning. Like + * UNSUPPORTED_ERROR_PREFIX, the contract is the message prefix because only + * the message string survives the worker boundary. + */ +export const SIZE_LIMIT_ERROR_PREFIX = `${ UNSUPPORTED_ERROR_PREFIX }: GIF exceeds maximum conversion size`; + +/** + * Default budget for total decoded pixels (width × height × frame count) + * beyond which conversion is not attempted. + * + * Conversion cost is roughly proportional to the total number of decoded + * pixels. 300 megapixels approximates what a mid-range machine converts + * within the ~30s the caller is willing to wait (e.g. a 1920x1080 GIF at + * ~145 frames); anything larger would likely be abandoned anyway, so it is + * cheaper to not start. Pass `0` to disable the check. + */ +export const DEFAULT_MAX_TOTAL_PIXELS = 300_000_000; + /** * Serializes encoder access. The upload-media concurrency limit already caps * this at 1, but the lock guards direct callers too. @@ -82,13 +105,18 @@ function padToEven( value: number ): number { * @param gifSource GIF file as a Blob/File or ArrayBuffer. * @param outputMimeType Output MIME type ('video/mp4' or 'video/webm'). * @param maxDimensions Optional maximum dimension for downscaling. + * @param maxTotalPixels Optional budget for total decoded pixels + * (width × height × frame count) beyond which the + * conversion is rejected with SIZE_LIMIT_ERROR_PREFIX. + * Defaults to DEFAULT_MAX_TOTAL_PIXELS; `0` disables. * @return Encoded video buffer. */ export async function convertGifToVideo( id: ItemId, gifSource: ArrayBuffer | Blob, outputMimeType: string, - maxDimensions?: number + maxDimensions?: number, + maxTotalPixels?: number ): Promise< ArrayBuffer > { inProgressOperations.add( id ); @@ -161,6 +189,33 @@ export async function convertGifToVideo( throw new Error( 'GIF contains no decodable frames' ); } + /* + * Enforce the total-pixel budget before any encoding work: an + * over-budget GIF would churn the CPU for minutes only to be + * abandoned. Track metadata does not expose dimensions, so decode + * the first frame (cheap) to learn them. + */ + const pixelBudget = maxTotalPixels ?? DEFAULT_MAX_TOTAL_PIXELS; + if ( pixelBudget > 0 ) { + const { image: probe } = await decoder.decode( { + frameIndex: 0, + } ); + const probeWidth = probe.displayWidth; + const probeHeight = probe.displayHeight; + probe.close(); + + if ( ! inProgressOperations.has( id ) ) { + throw new Error( 'Operation cancelled' ); + } + + const totalPixels = probeWidth * probeHeight * frameCount; + if ( totalPixels > pixelBudget ) { + throw new Error( + `${ SIZE_LIMIT_ERROR_PREFIX } (${ probeWidth }x${ probeHeight } x ${ frameCount } frames = ${ totalPixels } pixels; limit is ${ pixelBudget })` + ); + } + } + const source = new VideoSampleSource( { codec, bitrate: QUALITY_HIGH, diff --git a/packages/video-conversion/src/test/index.ts b/packages/video-conversion/src/test/index.ts index 1959e6867c35c2..b8b0f1a0c7f49c 100644 --- a/packages/video-conversion/src/test/index.ts +++ b/packages/video-conversion/src/test/index.ts @@ -5,6 +5,8 @@ import { convertGifToVideo, cancelOperations, UNSUPPORTED_ERROR_PREFIX, + SIZE_LIMIT_ERROR_PREFIX, + DEFAULT_MAX_TOTAL_PIXELS, } from '../index'; // Configurable decoded-frame dimensions; the default (10x10) is even so the @@ -268,10 +270,13 @@ describe( 'convertGifToVideo', () => { convertGifToVideo( 'item-add-throw', GIF_BUFFER, 'video/mp4' ) ).rejects.toThrow( 'add failed' ); - // The first frame was decoded; it must have been closed despite the - // add() rejection (Issue 1: frame leak on source.add() throw). - expect( mockDecodedFrames ).toHaveLength( 1 ); - expect( mockDecodedFrames[ 0 ].closed ).toBe( true ); + // Two decodes: the budget probe, then the first loop frame. Both + // must have been closed despite the add() rejection (Issue 1: + // frame leak on source.add() throw). + expect( mockDecodedFrames ).toHaveLength( 2 ); + for ( const frame of mockDecodedFrames ) { + expect( frame.closed ).toBe( true ); + } } ); it( 'reads track metadata after tracks.ready, not decoder.completed', async () => { @@ -313,8 +318,9 @@ describe( 'convertGifToVideo', () => { expect( canvas.width ).toBe( 600 ); expect( canvas.height ).toBe( 442 ); } - // The original odd-sized frames are closed once redrawn. - expect( mockDecodedFrames ).toHaveLength( 3 ); + // The budget probe plus the three loop frames, all closed (the + // odd-sized originals are closed once redrawn). + expect( mockDecodedFrames ).toHaveLength( 4 ); for ( const frame of mockDecodedFrames ) { expect( frame.closed ).toBe( true ); } @@ -355,4 +361,91 @@ describe( 'convertGifToVideo', () => { expect( mockVideoSamples ).toHaveLength( 1 ); expect( mockVideoSamples[ 0 ].closed ).toBe( true ); } ); + + describe( 'total pixel budget', () => { + it( 'rejects an over-budget GIF before any encoding work', async () => { + // 10x10 x 3 frames = 300 total pixels; a budget of 200 must + // reject before a single sample is encoded. + const promise = convertGifToVideo( + 'item-over-budget', + GIF_BUFFER, + 'video/mp4', + undefined, + 200 + ); + + await expect( promise ).rejects.toThrow( SIZE_LIMIT_ERROR_PREFIX ); + expect( mockAddedSamples ).toHaveLength( 0 ); + // Only the first frame was decoded (to learn the dimensions), + // and it was closed. + expect( mockDecodedFrames ).toHaveLength( 1 ); + expect( mockDecodedFrames[ 0 ].closed ).toBe( true ); + } ); + + it( 'prefixes the size-limit error with UNSUPPORTED_ERROR_PREFIX (consumer contract)', async () => { + // Consumers must treat an over-budget GIF as a graceful skip + // (keep the uploaded GIF, no companion video), which they detect + // by the Unsupported message prefix. + await expect( + convertGifToVideo( + 'item-budget-prefix', + GIF_BUFFER, + 'video/mp4', + undefined, + 200 + ) + ).rejects.toThrow( + new RegExp( `^${ UNSUPPORTED_ERROR_PREFIX }:` ) + ); + } ); + + it( 'applies the default budget when none is given', async () => { + // 5000x5000 x 100 frames = 2.5e9 pixels, far above any sane + // default. + mockFrameDims = { width: 5000, height: 5000 }; + mockFrameCount = 100; + expect( 5000 * 5000 * 100 ).toBeGreaterThan( + DEFAULT_MAX_TOTAL_PIXELS + ); + + await expect( + convertGifToVideo( + 'item-default-budget', + GIF_BUFFER, + 'video/mp4' + ) + ).rejects.toThrow( SIZE_LIMIT_ERROR_PREFIX ); + expect( mockAddedSamples ).toHaveLength( 0 ); + } ); + + it( 'converts a GIF exactly at the budget', async () => { + // 10x10 x 3 frames = 300 = the budget: not over, so convert. + const result = await convertGifToVideo( + 'item-at-budget', + GIF_BUFFER, + 'video/mp4', + undefined, + 300 + ); + + expect( result ).toBeInstanceOf( ArrayBuffer ); + expect( mockAddedSamples ).toHaveLength( 3 ); + } ); + + it( 'treats a budget of 0 as disabled', async () => { + mockFrameDims = { width: 5000, height: 5000 }; + mockFrameCount = 10; + + const result = await convertGifToVideo( + 'item-budget-disabled', + GIF_BUFFER, + 'video/mp4', + undefined, + 0 + ); + + expect( result ).toBeInstanceOf( ArrayBuffer ); + expect( mockAddedSamples ).toHaveLength( 10 ); + } ); + } ); } ); diff --git a/packages/video-conversion/src/video-conversion-worker.ts b/packages/video-conversion/src/video-conversion-worker.ts index 3b96140ed3381b..4fbb9762d7887e 100644 --- a/packages/video-conversion/src/video-conversion-worker.ts +++ b/packages/video-conversion/src/video-conversion-worker.ts @@ -56,20 +56,24 @@ function getWorkerAPI(): Remote< WorkerAPI > { * @param gifSource GIF file as a Blob/File or ArrayBuffer. * @param outputMimeType Output MIME type ('video/mp4' or 'video/webm'). * @param maxDimensions Optional maximum dimension for downscaling. + * @param maxTotalPixels Optional budget for total decoded pixels + * (width × height × frame count); `0` disables. * @return Video file buffer. */ export async function convertGifToVideo( id: ItemId, gifSource: ArrayBuffer | Blob, outputMimeType: string, - maxDimensions?: number + maxDimensions?: number, + maxTotalPixels?: number ): Promise< ArrayBuffer > { const api = getWorkerAPI(); return api.convertGifToVideo( id, gifSource, outputMimeType, - maxDimensions + maxDimensions, + maxTotalPixels ); }