From 31a19cb7380a1482d2033c03b72b4bcaec67ba19 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Sun, 21 Apr 2024 11:29:58 +0530 Subject: [PATCH] Desktop side --- desktop/docs/dependencies.md | 4 +- desktop/src/main/ipc.ts | 18 ++- desktop/src/main/services/ffmpeg.ts | 127 +++++++++----------- desktop/src/preload.ts | 23 ++-- web/apps/photos/src/services/ffmpeg.ts | 10 +- web/apps/photos/src/worker/ffmpeg.worker.ts | 4 +- web/packages/next/types/ipc.ts | 18 +-- 7 files changed, 92 insertions(+), 112 deletions(-) diff --git a/desktop/docs/dependencies.md b/desktop/docs/dependencies.md index 62f70e8e4..b159b13eb 100644 --- a/desktop/docs/dependencies.md +++ b/desktop/docs/dependencies.md @@ -94,12 +94,12 @@ Some extra ones specific to the code here are: ### Format conversion -The main tool we use is for arbitrary conversions is FFMPEG. To bundle a +The main tool we use is for arbitrary conversions is ffmpeg. To bundle a (platform specific) static binary of ffmpeg with our app, we use [ffmpeg-static](https://github.com/eugeneware/ffmpeg-static). > There is a significant (~20x) speed difference between using the compiled -> FFMPEG binary and using the WASM one (that our renderer process already has). +> ffmpeg binary and using the wasm one (that our renderer process already has). > Which is why we bundle it to speed up operations on the desktop app. In addition, we also bundle a static Linux binary of imagemagick in our extra diff --git a/desktop/src/main/ipc.ts b/desktop/src/main/ipc.ts index 3a526a01c..9ea4d802f 100644 --- a/desktop/src/main/ipc.ts +++ b/desktop/src/main/ipc.ts @@ -12,7 +12,6 @@ import type { FSWatcher } from "chokidar"; import { ipcMain } from "electron/main"; import type { CollectionMapping, - ElectronFile, FolderWatch, PendingUploads, } from "../types/ipc"; @@ -39,12 +38,9 @@ import { updateAndRestart, updateOnNextRestart, } from "./services/app-update"; -import { runFFmpegCmd } from "./services/ffmpeg"; +import { convertToJPEG, generateImageThumbnail } from "./services/convert"; +import { ffmpegExec } from "./services/ffmpeg"; import { getDirFiles } from "./services/fs"; -import { - convertToJPEG, - generateImageThumbnail, -} from "./services/convert"; import { clipImageEmbedding, clipTextEmbeddingIfAvailable, @@ -156,14 +152,14 @@ export const attachIPCHandlers = () => { ); ipcMain.handle( - "runFFmpegCmd", + "ffmpegExec", ( _, - cmd: string[], - inputFile: File | ElectronFile, + command: string[], + inputDataOrPath: Uint8Array | string, outputFileName: string, - dontTimeout?: boolean, - ) => runFFmpegCmd(cmd, inputFile, outputFileName, dontTimeout), + timeoutMS: number, + ) => ffmpegExec(command, inputDataOrPath, outputFileName, timeoutMS), ); // - ML diff --git a/desktop/src/main/services/ffmpeg.ts b/desktop/src/main/services/ffmpeg.ts index 8142983af..c49ac6700 100644 --- a/desktop/src/main/services/ffmpeg.ts +++ b/desktop/src/main/services/ffmpeg.ts @@ -1,23 +1,19 @@ import pathToFfmpeg from "ffmpeg-static"; -import { existsSync } from "node:fs"; import fs from "node:fs/promises"; -import { ElectronFile } from "../../types/ipc"; -import log from "../log"; -import { writeStream } from "../stream"; import { withTimeout } from "../utils"; import { execAsync } from "../utils-electron"; import { deleteTempFile, makeTempFilePath } from "../utils-temp"; -const INPUT_PATH_PLACEHOLDER = "INPUT"; -const FFMPEG_PLACEHOLDER = "FFMPEG"; -const OUTPUT_PATH_PLACEHOLDER = "OUTPUT"; +const ffmpegPathPlaceholder = "FFMPEG"; +const inputPathPlaceholder = "INPUT"; +const outputPathPlaceholder = "OUTPUT"; /** * Run a ffmpeg command * - * [Note: FFMPEG in Electron] + * [Note: ffmpeg in Electron] * - * There is a wasm build of FFMPEG, but that is currently 10-20 times slower + * There is a wasm build of ffmpeg, but that is currently 10-20 times slower * that the native build. That is slow enough to be unusable for our purposes. * https://ffmpegwasm.netlify.app/docs/performance * @@ -37,76 +33,65 @@ const OUTPUT_PATH_PLACEHOLDER = "OUTPUT"; * $ file ente.app/Contents/Frameworks/Electron\ Framework.framework/Versions/Current/Libraries/libffmpeg.dylib * .../libffmpeg.dylib: Mach-O 64-bit dynamically linked shared library arm64 * - * I'm not sure if our code is supposed to be able to use it, and how. + * But I'm not sure if our code is supposed to be able to use it, and how. */ -export async function runFFmpegCmd( - cmd: string[], - inputFile: File | ElectronFile, +export const ffmpegExec = async ( + command: string[], + inputDataOrPath: Uint8Array | string, outputFileName: string, - dontTimeout?: boolean, -) { - let inputFilePath = null; - let createdTempInputFile = null; + timeoutMS: number, +): Promise => { + // TODO (MR): This currently copies files for both input and output. This + // needs to be tested extremely large video files when invoked downstream of + // `convertToMP4` in the web code. + + let inputFilePath: string; + let isInputFileTemporary: boolean; + if (typeof inputDataOrPath == "string") { + inputFilePath = inputDataOrPath; + isInputFileTemporary = false; + } else { + inputFilePath = await makeTempFilePath("input" /* arbitrary */); + isInputFileTemporary = true; + await fs.writeFile(inputFilePath, inputDataOrPath); + } + + let outputFilePath: string | undefined; try { - if (!existsSync(inputFile.path)) { - const tempFilePath = await makeTempFilePath(inputFile.name); - await writeStream(tempFilePath, await inputFile.stream()); - inputFilePath = tempFilePath; - createdTempInputFile = true; - } else { - inputFilePath = inputFile.path; - } - const outputFileData = await runFFmpegCmd_( - cmd, + outputFilePath = await makeTempFilePath(outputFileName); + + const cmd = substitutePlaceholders( + command, inputFilePath, - outputFileName, - dontTimeout, + outputFilePath, ); - return new File([outputFileData], outputFileName); - } finally { - if (createdTempInputFile) { - await deleteTempFile(inputFilePath); - } - } -} -export async function runFFmpegCmd_( - cmd: string[], + if (timeoutMS) await withTimeout(execAsync(cmd), 30 * 1000); + else await execAsync(cmd); + + return fs.readFile(outputFilePath); + } finally { + if (isInputFileTemporary) await deleteTempFile(inputFilePath); + if (outputFilePath) await deleteTempFile(outputFilePath); + } +}; + +const substitutePlaceholders = ( + command: string[], inputFilePath: string, - outputFileName: string, - dontTimeout = false, -) { - let tempOutputFilePath: string; - try { - tempOutputFilePath = await makeTempFilePath(outputFileName); - - cmd = cmd.map((cmdPart) => { - if (cmdPart === FFMPEG_PLACEHOLDER) { - return ffmpegBinaryPath(); - } else if (cmdPart === INPUT_PATH_PLACEHOLDER) { - return inputFilePath; - } else if (cmdPart === OUTPUT_PATH_PLACEHOLDER) { - return tempOutputFilePath; - } else { - return cmdPart; - } - }); - - if (dontTimeout) await execAsync(cmd); - else await withTimeout(execAsync(cmd), 30 * 1000); - - if (!existsSync(tempOutputFilePath)) { - throw new Error("ffmpeg output file not found"); + outputFilePath: string, +) => + command.map((segment) => { + if (segment == ffmpegPathPlaceholder) { + return ffmpegBinaryPath(); + } else if (segment == inputPathPlaceholder) { + return inputFilePath; + } else if (segment == outputPathPlaceholder) { + return outputFilePath; + } else { + return segment; } - const outputFile = await fs.readFile(tempOutputFilePath); - return new Uint8Array(outputFile); - } catch (e) { - log.error("FFMPEG command failed", e); - throw e; - } finally { - await deleteTempFile(tempOutputFilePath); - } -} + }); /** * Return the path to the `ffmpeg` binary. diff --git a/desktop/src/preload.ts b/desktop/src/preload.ts index 5a7d0eda4..c3f964e17 100644 --- a/desktop/src/preload.ts +++ b/desktop/src/preload.ts @@ -122,8 +122,6 @@ const fsWriteFile = (path: string, contents: string): Promise => const fsIsDir = (dirPath: string): Promise => ipcRenderer.invoke("fsIsDir", dirPath); -// - AUDIT below this - // - Conversion const convertToJPEG = ( @@ -144,18 +142,18 @@ const generateImageThumbnail = ( maxSize, ); -const runFFmpegCmd = ( - cmd: string[], - inputFile: File | ElectronFile, +const ffmpegExec = ( + command: string[], + inputDataOrPath: Uint8Array | string, outputFileName: string, - dontTimeout?: boolean, -): Promise => + timeoutMS: number, +): Promise => ipcRenderer.invoke( - "runFFmpegCmd", - cmd, - inputFile, + "ffmpegExec", + command, + inputDataOrPath, outputFileName, - dontTimeout, + timeoutMS, ); // - ML @@ -255,6 +253,7 @@ const setPendingUploadFiles = ( ): Promise => ipcRenderer.invoke("setPendingUploadFiles", type, filePaths); +// - TODO: AUDIT below this // - const getElectronFilesFromGoogleZip = ( @@ -341,7 +340,7 @@ contextBridge.exposeInMainWorld("electron", { convertToJPEG, generateImageThumbnail, - runFFmpegCmd, + ffmpegExec, // - ML diff --git a/web/apps/photos/src/services/ffmpeg.ts b/web/apps/photos/src/services/ffmpeg.ts index 17833f426..f80227264 100644 --- a/web/apps/photos/src/services/ffmpeg.ts +++ b/web/apps/photos/src/services/ffmpeg.ts @@ -151,14 +151,14 @@ export async function convertToMP4(file: File) { } /** - * Run the given FFMPEG command. + * Run the given ffmpeg command. * - * If we're running in the context of our desktop app, use the FFMPEG binary we + * If we're running in the context of our desktop app, use the ffmpeg binary we * bundle with our desktop app to run the command. Otherwise fallback to using - * the WASM ffmpeg we link to from our web app in a web worker. + * the wasm ffmpeg we link to from our web app in a web worker. * - * As a rough ballpark, the native FFMPEG integration in the desktop app is - * 10-20x faster than the WASM one currently. See: [Note: FFMPEG in Electron]. + * As a rough ballpark, the native ffmpeg integration in the desktop app is + * 10-20x faster than the wasm one currently. See: [Note: ffmpeg in Electron]. */ const ffmpegExec = async ( cmd: string[], diff --git a/web/apps/photos/src/worker/ffmpeg.worker.ts b/web/apps/photos/src/worker/ffmpeg.worker.ts index 2e6045008..8403c3f6c 100644 --- a/web/apps/photos/src/worker/ffmpeg.worker.ts +++ b/web/apps/photos/src/worker/ffmpeg.worker.ts @@ -20,10 +20,10 @@ export class DedicatedFFmpegWorker { } /** - * Execute a FFMPEG {@link command}. + * Execute a ffmpeg {@link command}. * * This is a sibling of {@link ffmpegExec} in ipc.ts exposed by the desktop - * app. See [Note: FFMPEG in Electron]. + * app. See [Note: ffmpeg in Electron]. */ run(cmd, inputFile, outputFileName, timeoutMS) { return this.wasmFFmpeg.run(cmd, inputFile, outputFileName, timeoutMS); diff --git a/web/packages/next/types/ipc.ts b/web/packages/next/types/ipc.ts index 400067153..d87b8e830 100644 --- a/web/packages/next/types/ipc.ts +++ b/web/packages/next/types/ipc.ts @@ -237,12 +237,12 @@ export interface Electron { ) => Promise; /** - * Execute a FFMPEG {@link command}. + * Execute a ffmpeg {@link command}. * - * This executes the command using the FFMPEG executable we bundle with our - * desktop app. There is also a FFMPEG WASM implementation that we use when + * This executes the command using the ffmpeg executable we bundle with our + * desktop app. There is also a ffmpeg wasm implementation that we use when * running on the web, it also has a sibling function with the same - * parameters. See [Note: FFMPEG in Electron]. + * parameters. See [Note: ffmpeg in Electron]. * * @param command An array of strings, each representing one positional * parameter in the command to execute. Placeholders for the input, output @@ -253,14 +253,14 @@ export interface Electron { * @param inputDataOrPath The bytes of the input file, or the path to the * input file on the user's local disk. In both cases, the data gets * serialized to a temporary file, and then that path gets substituted in - * the FFMPEG {@link command} by {@link inputPathPlaceholder}. + * the ffmpeg {@link command} by {@link inputPathPlaceholder}. * - * @param outputFileName The name of the file we instruct FFMPEG to produce + * @param outputFileName The name of the file we instruct ffmpeg to produce * when giving it the given {@link command}. The contents of this file get * returned as the result. * - * @param timeoutMS If non-zero, then throw a timeout error if the FFMPEG - * command takes more than the given number of milliseconds. + * @param timeoutMS If non-zero, then abort and throw a timeout error if the + * ffmpeg command takes more than the given number of milliseconds. * * @returns The contents of the output file produced by the ffmpeg command * at {@link outputFileName}. @@ -270,7 +270,7 @@ export interface Electron { inputDataOrPath: Uint8Array | string, outputFileName: string, timeoutMS: number, - ) => Promise; + ) => Promise; // - ML