From 95863ce4173c30e560178e5af192f85cb1a864d4 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Fri, 26 Jul 2024 15:32:08 +0200 Subject: [PATCH] Ensure imported files are transcoded and compressed (#579) --- .../src/blockMediaPanel/importMedia.tsx | 19 +++- packages/ffmpeg/src/@types/global.d.ts | 1 + packages/ffmpeg/src/index.ts | 13 ++- packages/upload-media/README.md | 1 + packages/upload-media/src/store/actions.ts | 107 +++++++++++------- packages/upload-media/src/store/reducer.ts | 5 +- .../upload-media/src/store/test/actions.ts | 5 +- .../upload-media/src/store/test/reducer.ts | 4 +- packages/upload-media/src/store/types.ts | 2 + tests/e2e/specs/videos/importExternal.spec.ts | 7 ++ tsconfig.shared.json | 5 +- 11 files changed, 111 insertions(+), 58 deletions(-) diff --git a/packages/editor/src/blockMediaPanel/importMedia.tsx b/packages/editor/src/blockMediaPanel/importMedia.tsx index 86928d9b..a36856b4 100644 --- a/packages/editor/src/blockMediaPanel/importMedia.tsx +++ b/packages/editor/src/blockMediaPanel/importMedia.tsx @@ -6,7 +6,8 @@ import { import { useDispatch, useSelect } from '@wordpress/data'; import { store as editorStore } from '@wordpress/editor'; import { isBlobURL } from '@wordpress/blob'; -import { __ } from '@wordpress/i18n'; +import { __, sprintf } from '@wordpress/i18n'; +import { store as noticesStore } from '@wordpress/notices'; import type { Attachment } from '@mexp/media-utils'; import { store as uploadStore } from '@mexp/upload-media'; @@ -22,6 +23,7 @@ export function ImportMedia( { url, onChange }: ImportMediaProps ) { const { baseControlProps, controlProps } = useBaseControlProps( {} ); const { addItemFromUrl } = useDispatch( uploadStore ); + const { createErrorNotice } = useDispatch( noticesStore ); const isUploading = useIsUploadingByUrl( url ); const currentPostId = useSelect( ( select ) => select( editorStore ).getCurrentPostId(), @@ -36,6 +38,21 @@ export function ImportMedia( { url, onChange }: ImportMediaProps ) { void addItemFromUrl( { url, onChange: ( [ media ] ) => onChange( media ), + onError: ( err: Error ) => { + void createErrorNotice( + sprintf( + /* translators: %s: error message */ + __( + 'There was an error importing the file: %s', + 'media-experiments' + ), + err.message + ), + { + type: 'snackbar', + } + ); + }, additionalData: { post: currentPostId, mexp_media_source: diff --git a/packages/ffmpeg/src/@types/global.d.ts b/packages/ffmpeg/src/@types/global.d.ts index f4c52fc1..0be49893 100644 --- a/packages/ffmpeg/src/@types/global.d.ts +++ b/packages/ffmpeg/src/@types/global.d.ts @@ -1,5 +1,6 @@ declare global { const FFMPEG_CDN_URL: string; + let SCRIPT_DEBUG: boolean; } export type {}; diff --git a/packages/ffmpeg/src/index.ts b/packages/ffmpeg/src/index.ts index b50e75b3..fc6f6465 100644 --- a/packages/ffmpeg/src/index.ts +++ b/packages/ffmpeg/src/index.ts @@ -49,10 +49,7 @@ const FFMPEG_CONFIG = { const ffmpegCoreUrl = FFMPEG_CDN_URL; -const isDevelopment = - typeof process !== 'undefined' && - process.env && - process.env.NODE_ENV !== 'production'; +const isDevelopment = typeof SCRIPT_DEBUG !== 'undefined' && SCRIPT_DEBUG; function readFile( file: File ): Promise< Uint8Array > { const reader = new FileReader(); @@ -123,14 +120,18 @@ async function runFFmpegWithConfig( // Delete file in MEMFS to free memory. ffmpeg.FS( 'unlink', tempFileName ); + // TODO: Consider using ffmpeg.setLogger() and look for messages such as "Decoder (codec av1) not found for input stream". + // Allows throwing with more detailed error message. + if ( ! data.buffer.byteLength ) { + throw new Error( `File ${ fileName } could not be processed` ); + } + return new File( [ new Blob( [ data.buffer ], { type: mimeType } ) ], fileName, { type: mimeType } ); } catch ( err ) { - // eslint-disable-next-line no-console -- We want to surface this error. - console.error( err ); throw err; } finally { try { diff --git a/packages/upload-media/README.md b/packages/upload-media/README.md index 8ef0bd7b..8c470d4a 100644 --- a/packages/upload-media/README.md +++ b/packages/upload-media/README.md @@ -254,6 +254,7 @@ Optimizes/Compresses an existing video item. _Parameters_ - _id_ `QueueItemId`: Item ID. +- _args_ `[OptimizeVideoItemArgs]`: Additional arguments for the operation. #### pauseQueue diff --git a/packages/upload-media/src/store/actions.ts b/packages/upload-media/src/store/actions.ts index a0f03aaa..689171d0 100644 --- a/packages/upload-media/src/store/actions.ts +++ b/packages/upload-media/src/store/actions.ts @@ -229,12 +229,17 @@ export function addItem( { const itemId = uuidv4(); - const blobUrl = createBlobURL( file ); - dispatch< CacheBlobUrlAction >( { - type: Type.CacheBlobUrl, - id: itemId, - blobUrl, - } ); + let blobUrl; + + // StubFile could be coming from addItemFromUrl(). + if ( ! ( file instanceof StubFile ) ) { + blobUrl = createBlobURL( file ); + dispatch< CacheBlobUrlAction >( { + type: Type.CacheBlobUrl, + id: itemId, + blobUrl, + } ); + } dispatch< AddAction >( { type: Type.Add, @@ -260,11 +265,13 @@ export function addItem( { blurHash, dominantColor, abortController: abortController || new AbortController(), - operations, + operations: Array.isArray( operations ) + ? operations + : [ OperationType.Prepare ], }, } ); - dispatch.prepareItem( itemId ); + dispatch.processItem( itemId ); }; } @@ -349,8 +356,8 @@ export function addItemFromUrl( { sourceUrl: url, operations: [ [ OperationType.FetchRemoteFile, { url, fileName } ], - OperationType.AddPoster, - OperationType.Upload, + // This will add the next steps, such as compression, poster generation, and upload. + OperationType.Prepare, ], } ); }; @@ -402,12 +409,14 @@ export function addSideloadItem( { ...additionalData, }, parentId, - operations, + operations: Array.isArray( operations ) + ? operations + : [ OperationType.Prepare ], abortController: new AbortController(), }, } ); - dispatch.prepareItem( itemId ); + dispatch.processItem( itemId ); }; } @@ -505,7 +514,7 @@ export function muteExistingVideo( { }, } ); - dispatch.prepareItem( itemId ); + dispatch.processItem( itemId ); }; } @@ -576,7 +585,7 @@ export function addSubtitlesForExistingVideo( { }, } ); - dispatch.prepareItem( itemId ); + dispatch.processItem( itemId ); }; } @@ -640,7 +649,7 @@ export function addPosterForExistingVideo( { }, } ); - dispatch.prepareItem( itemId ); + dispatch.processItem( itemId ); }; } @@ -776,7 +785,7 @@ export function optimizeExistingItem( { }, } ); - dispatch.prepareItem( itemId ); + dispatch.processItem( itemId ); }; } @@ -903,6 +912,10 @@ export function processItem( id: QueueItemId ) { } ); switch ( operation ) { + case OperationType.Prepare: + dispatch.prepareItem( item.id ); + break; + case OperationType.ResizeCrop: dispatch.resizeCropItem( item.id, @@ -923,7 +936,10 @@ export function processItem( id: QueueItemId ) { break; case OperationType.TranscodeVideo: - dispatch.optimizeVideoItem( item.id ); + dispatch.optimizeVideoItem( + item.id, + operationArgs as OperationArgs[ OperationType.TranscodeVideo ] + ); break; case OperationType.MuteVideo: @@ -1104,13 +1120,19 @@ export function addPosterForItem( id: QueueItemId ) { try { switch ( mediaType ) { case 'video': - const src = createBlobURL( item.file ); + let src = isBlobURL( item.attachment?.url ) + ? item.attachment?.url + : undefined; - dispatch< CacheBlobUrlAction >( { - type: Type.CacheBlobUrl, - id, - blobUrl: src, - } ); + if ( ! src ) { + src = createBlobURL( item.file ); + + dispatch< CacheBlobUrlAction >( { + type: Type.CacheBlobUrl, + id, + blobUrl: src, + } ); + } const poster = await getPosterFromVideo( src, @@ -1128,6 +1150,7 @@ export function addPosterForItem( id: QueueItemId ) { dispatch.finishOperation( id, { poster, attachment: { + url: item.attachment?.url || src, poster: posterUrl, }, } ); @@ -1222,15 +1245,6 @@ export function prepareItem( id: QueueItemId ) { const { file } = item; - // TODO: Check canTranscode either here, in muteExistingVideo, or in the UI. - - // Transcoding type has already been set, e.g. via muteExistingVideo() or addSideloadItem(). - // Also allow empty arrays, useful for example when sideloading original image. - if ( item.operations !== undefined ) { - dispatch.processItem( id ); - return; - } - const mediaType = getMediaTypeFromMimeType( file.type ); const operations: Operation[] = []; @@ -1320,7 +1334,11 @@ export function prepareItem( id: QueueItemId ) { window.crossOriginIsolated && canProcessWithFFmpeg( file ) ) { - operations.push( OperationType.TranscodeVideo ); + operations.push( [ + OperationType.TranscodeVideo, + // Don't make a fuzz if video cannot be transcoded. + { continueOnError: true }, + ] ); } operations.push( @@ -1365,7 +1383,7 @@ export function prepareItem( id: QueueItemId ) { operations, } ); - dispatch.processItem( id ); + dispatch.finishOperation( id, {} ); }; } @@ -1442,14 +1460,16 @@ export function uploadPoster( id: QueueItemId ) { return; } - // Video block expects such a structure for the poster. - // https://github.com/WordPress/gutenberg/blob/e0a413d213a2a829ece52c6728515b10b0154d8d/packages/block-library/src/video/edit.js#L154 // TODO: Pass poster ID as well so that the video block can update `featured_media` via the REST API. const updatedAttachment = { ...attachment, + // Video block expects such a structure for the poster. + // https://github.com/WordPress/gutenberg/blob/e0a413d213a2a829ece52c6728515b10b0154d8d/packages/block-library/src/video/edit.js#L154 image: { src: posterAttachment.url, }, + // Expected by ImportMedia / addItemFromUrl() + poster: posterAttachment.url, }; // This might be confusing, but the idea is to update the original @@ -1948,12 +1968,18 @@ export function optimizeImageItem( }; } +type OptimizeVideoItemArgs = OperationArgs[ OperationType.TranscodeVideo ]; + /** * Optimizes/Compresses an existing video item. * - * @param id Item ID. + * @param id Item ID. + * @param [args] Additional arguments for the operation. */ -export function optimizeVideoItem( id: QueueItemId ) { +export function optimizeVideoItem( + id: QueueItemId, + args?: OptimizeVideoItemArgs +) { return async ( { select, dispatch, registry }: ThunkArgs ) => { const item = select.getItem( id ) as QueueItem; @@ -1992,6 +2018,11 @@ export function optimizeVideoItem( id: QueueItemId ) { }, } ); } catch ( error ) { + if ( args?.continueOnError ) { + dispatch.finishOperation( id, {} ); + return; + } + dispatch.cancelItem( id, error instanceof Error diff --git a/packages/upload-media/src/store/reducer.ts b/packages/upload-media/src/store/reducer.ts index 372f3028..5ae422c5 100644 --- a/packages/upload-media/src/store/reducer.ts +++ b/packages/upload-media/src/store/reducer.ts @@ -151,8 +151,8 @@ function reducer( return { ...item, operations: [ - ...action.operations, ...( item.operations || [] ), + ...action.operations, ], }; } ), @@ -176,9 +176,6 @@ function reducer( ? { ...item.attachment, ...action.item.attachment, - // TODO: Update to pass this correctly. - // url: action.item?.url, - // mimeType: action.item?.file?.type, } : undefined; diff --git a/packages/upload-media/src/store/test/actions.ts b/packages/upload-media/src/store/test/actions.ts index 4b256a6e..9da3b941 100644 --- a/packages/upload-media/src/store/test/actions.ts +++ b/packages/upload-media/src/store/test/actions.ts @@ -138,7 +138,7 @@ describe( 'actions', () => { sourceFile: expect.any( File ), status: ItemStatus.Processing, attachment: { - url: expect.stringMatching( /^blob:/ ), + url: undefined, }, operations: [ [ @@ -148,8 +148,7 @@ describe( 'actions', () => { fileName: 'example.jpg', }, ], - OperationType.AddPoster, - OperationType.Upload, + OperationType.Prepare, ], } ) ); diff --git a/packages/upload-media/src/store/test/reducer.ts b/packages/upload-media/src/store/test/reducer.ts index 6b362d5b..8d2a34cd 100644 --- a/packages/upload-media/src/store/test/reducer.ts +++ b/packages/upload-media/src/store/test/reducer.ts @@ -246,7 +246,7 @@ describe( 'reducer', () => { } ); describe( `${ Type.AddOperations }`, () => { - it( 'prepends operations to the list', () => { + it( 'appends operations to the list', () => { const initialState: State = { imageSizes: {}, queueStatus: 'active', @@ -282,9 +282,9 @@ describe( 'reducer', () => { id: '1', status: ItemStatus.Processing, operations: [ + OperationType.Upload, OperationType.Compress, OperationType.AddPoster, - OperationType.Upload, ], }, ], diff --git a/packages/upload-media/src/store/types.ts b/packages/upload-media/src/store/types.ts index 6fd5d073..5531d1af 100644 --- a/packages/upload-media/src/store/types.ts +++ b/packages/upload-media/src/store/types.ts @@ -197,6 +197,7 @@ export enum ItemStatus { } export enum OperationType { + Prepare = 'PREPARE', AddPoster = 'ADD_POSTER', UploadPoster = 'UPLOAD_POSTER', UploadOriginal = 'UPLOAD_ORIGINAL', @@ -231,6 +232,7 @@ export type OperationArgs = { interlaced?: boolean; }; [ OperationType.ResizeCrop ]: { resize?: ImageSizeCrop }; + [ OperationType.TranscodeVideo ]: { continueOnError?: true }; }; type OperationWithArgs< T extends keyof OperationArgs = keyof OperationArgs > = diff --git a/tests/e2e/specs/videos/importExternal.spec.ts b/tests/e2e/specs/videos/importExternal.spec.ts index 9a9bdd53..6e2123f9 100644 --- a/tests/e2e/specs/videos/importExternal.spec.ts +++ b/tests/e2e/specs/videos/importExternal.spec.ts @@ -58,6 +58,13 @@ test.describe( 'Videos', () => { expect( src ).toMatch( /\/wp-content\/uploads\// ); + const blockAttributes = await page.evaluate( + () => + window.wp.data.select( 'core/block-editor' ).getSelectedBlock() + ?.attributes ?? {} + ); + await expect( blockAttributes.poster ).toMatch( /-poster\.jpeg$/ ); + // TODO: Check why mime type is not consistent. await expect( settingsPanel ).toHaveText( /Mime type: video\/(mp4|webm)/ diff --git a/tsconfig.shared.json b/tsconfig.shared.json index 9e5b3e7e..8a09a7c3 100644 --- a/tsconfig.shared.json +++ b/tsconfig.shared.json @@ -31,8 +31,5 @@ "typeRoots": [ "./@types", "./node_modules/@types" ], "types": [ "jest", "node" ] }, - "exclude": [ - "packages/*/dist-types/**", - "packages/*/dist/**" - ] + "exclude": [ "packages/*/dist-types/**", "packages/*/dist/**" ] }