From 1f0c80cabc9b556cce5c3b7e5e0a03887d941c9a Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Tue, 23 Apr 2024 10:21:39 +0530 Subject: [PATCH] Refactor 1 --- desktop/src/main/ipc.ts | 8 +- desktop/src/main/services/ffmpeg.ts | 12 ++- desktop/src/main/services/image.ts | 21 ++-- desktop/src/preload.ts | 4 +- .../photos/src/services/upload/thumbnail.ts | 95 +++++++++---------- .../src/services/upload/uploadService.ts | 30 ++++++ web/apps/photos/src/types/upload/index.ts | 5 + web/packages/next/types/ipc.ts | 6 +- 8 files changed, 111 insertions(+), 70 deletions(-) diff --git a/desktop/src/main/ipc.ts b/desktop/src/main/ipc.ts index 9229d3b38..91efbb09f 100644 --- a/desktop/src/main/ipc.ts +++ b/desktop/src/main/ipc.ts @@ -147,8 +147,12 @@ export const attachIPCHandlers = () => { ipcMain.handle( "generateImageThumbnail", - (_, imageData: Uint8Array, maxDimension: number, maxSize: number) => - generateImageThumbnail(imageData, maxDimension, maxSize), + ( + _, + dataOrPath: Uint8Array | string, + maxDimension: number, + maxSize: number, + ) => generateImageThumbnail(dataOrPath, maxDimension, maxSize), ); ipcMain.handle( diff --git a/desktop/src/main/services/ffmpeg.ts b/desktop/src/main/services/ffmpeg.ts index b1ae9e301..f99f7ef8f 100644 --- a/desktop/src/main/services/ffmpeg.ts +++ b/desktop/src/main/services/ffmpeg.ts @@ -48,17 +48,19 @@ export const ffmpegExec = async ( let inputFilePath: string; let isInputFileTemporary: boolean; - if (typeof dataOrPath == "string") { - inputFilePath = dataOrPath; - isInputFileTemporary = false; - } else { + if (dataOrPath instanceof Uint8Array) { inputFilePath = await makeTempFilePath(); isInputFileTemporary = true; - await fs.writeFile(inputFilePath, dataOrPath); + } else { + inputFilePath = dataOrPath; + isInputFileTemporary = false; } const outputFilePath = await makeTempFilePath(); try { + if (dataOrPath instanceof Uint8Array) + await fs.writeFile(inputFilePath, dataOrPath); + const cmd = substitutePlaceholders( command, inputFilePath, diff --git a/desktop/src/main/services/image.ts b/desktop/src/main/services/image.ts index 47a189244..7fae50757 100644 --- a/desktop/src/main/services/image.ts +++ b/desktop/src/main/services/image.ts @@ -63,14 +63,23 @@ const imageMagickPath = () => path.join(isDev ? "build" : process.resourcesPath, "image-magick"); export const generateImageThumbnail = async ( - imageData: Uint8Array, + dataOrPath: Uint8Array | string, maxDimension: number, maxSize: number, ): Promise => { - const inputFilePath = await makeTempFilePath(); + let inputFilePath: string; + let isInputFileTemporary: boolean; + if (dataOrPath instanceof Uint8Array) { + inputFilePath = await makeTempFilePath(); + isInputFileTemporary = true; + } else { + inputFilePath = dataOrPath; + isInputFileTemporary = false; + } + const outputFilePath = await makeTempFilePath(".jpeg"); - // Construct the command first, it may throw NotAvailable on win32. + // Construct the command first, it may throw `NotAvailable` on win32. let quality = 70; let command = generateImageThumbnailCommand( inputFilePath, @@ -80,8 +89,8 @@ export const generateImageThumbnail = async ( ); try { - if (imageData instanceof Uint8Array) - await fs.writeFile(inputFilePath, imageData); + if (dataOrPath instanceof Uint8Array) + await fs.writeFile(inputFilePath, dataOrPath); let thumbnail: Uint8Array; do { @@ -98,7 +107,7 @@ export const generateImageThumbnail = async ( return thumbnail; } finally { try { - await deleteTempFile(inputFilePath); + if (isInputFileTemporary) await deleteTempFile(inputFilePath); await deleteTempFile(outputFilePath); } catch (e) { log.error("Ignoring error when cleaning up temp files", e); diff --git a/desktop/src/preload.ts b/desktop/src/preload.ts index 0464022b0..728e8d012 100644 --- a/desktop/src/preload.ts +++ b/desktop/src/preload.ts @@ -128,13 +128,13 @@ const convertToJPEG = (imageData: Uint8Array): Promise => ipcRenderer.invoke("convertToJPEG", imageData); const generateImageThumbnail = ( - imageData: Uint8Array, + dataOrPath: Uint8Array | string, maxDimension: number, maxSize: number, ): Promise => ipcRenderer.invoke( "generateImageThumbnail", - imageData, + dataOrPath, maxDimension, maxSize, ); diff --git a/web/apps/photos/src/services/upload/thumbnail.ts b/web/apps/photos/src/services/upload/thumbnail.ts index 6be2ea6bd..aae7521e8 100644 --- a/web/apps/photos/src/services/upload/thumbnail.ts +++ b/web/apps/photos/src/services/upload/thumbnail.ts @@ -1,5 +1,5 @@ import log from "@/next/log"; -import { CustomErrorMessage, type Electron } from "@/next/types/ipc"; +import { type Electron } from "@/next/types/ipc"; import { withTimeout } from "@ente/shared/utils"; import { FILE_TYPE } from "constants/file"; import { BLACK_THUMBNAIL_BASE64 } from "constants/upload"; @@ -13,30 +13,6 @@ const maxThumbnailDimension = 720; /** Maximum size (in bytes) of the generated thumbnail */ const maxThumbnailSize = 100 * 1024; // 100 KB -class ModuleState { - /** - * This will be set to true if we get an error from the Node.js side of our - * desktop app telling us that native JPEG conversion is not available for - * the current OS/arch combination. That way, we can stop pestering it again - * and again (saving an IPC round-trip). - * - * Note the double negative when it is used. - */ - isNativeThumbnailCreationNotAvailable = false; -} - -const moduleState = new ModuleState(); - -interface GeneratedThumbnail { - /** The JPEG data of the generated thumbnail */ - thumbnail: Uint8Array; - /** - * `true` if this is a fallback (all black) thumbnail we're returning since - * thumbnail generation failed for some reason. - */ - hasStaticThumbnail: boolean; -} - /** * Generate a JPEG thumbnail for the given image or video data. * @@ -46,19 +22,18 @@ interface GeneratedThumbnail { * itself that might not yet have support for more exotic formats. * * @param blob The data (blob) of the file whose thumbnail we want to generate. - * @param fileTypeInfo The type of the file whose {@link blob} we were given. + * @param fileTypeInfo The type information for the file. * - * @return {@link GeneratedThumbnail}, a thin wrapper for the raw JPEG bytes of - * the generated thumbnail. + * @return The JPEG data of the generated thumbnail. */ export const generateThumbnail = async ( blob: Blob, fileTypeInfo: FileTypeInfo, -): Promise => { +): Promise => { try { const thumbnail = fileTypeInfo.fileType === FILE_TYPE.IMAGE - ? await generateImageThumbnail(blob, fileTypeInfo) + ? await generateImageThumbnailUsingCanvas(blob, fileTypeInfo) : await generateVideoThumbnail(blob); if (thumbnail.length == 0) throw new Error("Empty thumbnail"); @@ -73,38 +48,54 @@ export const generateThumbnail = async ( * A fallback, black, thumbnail for use in cases where thumbnail generation * fails. */ -const fallbackThumbnail = () => +export const fallbackThumbnail = () => Uint8Array.from(atob(BLACK_THUMBNAIL_BASE64), (c) => c.charCodeAt(0)); -const generateImageThumbnail = async ( - blob: Blob, +/** + * Generate a JPEG thumbnail for the given file using native tools. + * + * This function only works when we're running in the context of our desktop + * app, and this dependency is enforced by the need to pass the {@link electron} + * object which we use to perform IPC with the Node.js side of our desktop app. + * + * @param fileOrPath Either the image or video File, or the path to the image or + * video file on the user's local filesystem, whose thumbnail we want to + * generate. + * + * @param fileTypeInfo The type information for the file. + * + * @return The JPEG data of the generated thumbnail. + * + * @see {@link generateThumbnail}. + */ +export const generateThumbnailNative = async ( + electron: Electron, + fileOrPath: File | string, fileTypeInfo: FileTypeInfo, -) => { - const electron = globalThis.electron; - const available = !moduleState.isNativeThumbnailCreationNotAvailable; - if (electron && available) { - try { - return await generateImageThumbnailInElectron(electron, blob); - } catch (e) { - if (e.message == CustomErrorMessage.NotAvailable) { - moduleState.isNativeThumbnailCreationNotAvailable = true; - } else { - log.error("Native thumbnail creation failed", e); - } - } - } +): Promise => { + try { + const thumbnail = + fileTypeInfo.fileType === FILE_TYPE.IMAGE + ? await generateImageThumbnailNative(electron, fileOrPath) + : await generateVideoThumbnail(blob); - return await generateImageThumbnailUsingCanvas(blob, fileTypeInfo); + if (thumbnail.length == 0) throw new Error("Empty thumbnail"); + return { thumbnail, hasStaticThumbnail: false }; + } catch (e) { + log.error(`Failed to generate ${fileTypeInfo.exactType} thumbnail`, e); + return { thumbnail: fallbackThumbnail(), hasStaticThumbnail: true }; + } }; -const generateImageThumbnailInElectron = async ( +const generateImageThumbnailNative = async ( electron: Electron, - blob: Blob, + fileOrPath: File | string, ): Promise => { const startTime = Date.now(); - const data = new Uint8Array(await blob.arrayBuffer()); const jpegData = await electron.generateImageThumbnail( - data, + fileOrPath instanceof File + ? new Uint8Array(await fileOrPath.arrayBuffer()) + : fileOrPath, maxThumbnailDimension, maxThumbnailSize, ); diff --git a/web/apps/photos/src/services/upload/uploadService.ts b/web/apps/photos/src/services/upload/uploadService.ts index f2be02ec0..2d16aab70 100644 --- a/web/apps/photos/src/services/upload/uploadService.ts +++ b/web/apps/photos/src/services/upload/uploadService.ts @@ -359,6 +359,22 @@ const readAsset = async ( : await readFile(fileTypeInfo, file); }; +// TODO(MR): Merge with the uploader +class ModuleState { + /** + * This will be set to true if we get an error from the Node.js side of our + * desktop app telling us that native JPEG conversion is not available for + * the current OS/arch combination. That way, we can stop pestering it again + * and again (saving an IPC round-trip). + * + * Note the double negative when it is used. + */ + isNativeThumbnailCreationNotAvailable = false; +} + +const moduleState = new ModuleState(); + + async function readFile( fileTypeInfo: FileTypeInfo, rawFile: File | ElectronFile, @@ -380,6 +396,20 @@ async function readFile( filedata = await getUint8ArrayView(rawFile); } + const electron = globalThis.electron; + const available = !moduleState.isNativeThumbnailCreationNotAvailable; + if (electron && available) { + try { + return await generateImageThumbnailInElectron(electron, blob); + } catch (e) { + if (e.message == CustomErrorMessage.NotAvailable) { + moduleState.isNativeThumbnailCreationNotAvailable = true; + } else { + log.error("Native thumbnail creation failed", e); + } + } + } + if (filedata instanceof Uint8Array) { } else { diff --git a/web/apps/photos/src/types/upload/index.ts b/web/apps/photos/src/types/upload/index.ts index 175af6824..f9d744dee 100644 --- a/web/apps/photos/src/types/upload/index.ts +++ b/web/apps/photos/src/types/upload/index.ts @@ -114,7 +114,12 @@ export interface UploadURL { export interface FileInMemory { filedata: Uint8Array | DataStream; + /** The JPEG data of the generated thumbnail */ thumbnail: Uint8Array; + /** + * `true` if this is a fallback (all black) thumbnail we're returning since + * thumbnail generation failed for some reason. + */ hasStaticThumbnail: boolean; } diff --git a/web/packages/next/types/ipc.ts b/web/packages/next/types/ipc.ts index 6ba5c7832..d392b6f3b 100644 --- a/web/packages/next/types/ipc.ts +++ b/web/packages/next/types/ipc.ts @@ -221,8 +221,8 @@ export interface Electron { * not yet possible, this function will throw an error with the * {@link CustomErrorMessage.NotAvailable} message. * - * @param imageData The raw image data (the contents of the image file) - * whose thumbnail we want to generate. + * @param dataOrPath The raw image data (the contents of the image file), or + * the path to the image file, whose thumbnail we want to generate. * @param maxDimension The maximum width or height of the generated * thumbnail. * @param maxSize Maximum size (in bytes) of the generated thumbnail. @@ -230,7 +230,7 @@ export interface Electron { * @returns JPEG data of the generated thumbnail. */ generateImageThumbnail: ( - imageData: Uint8Array, + dataOrPath: Uint8Array | string, maxDimension: number, maxSize: number, ) => Promise;