Desktop side

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

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

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;
-    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,
-            inputFilePath,
-            outputFileName,
-            dontTimeout,
-        );
-        return new File([outputFileData], outputFileName);
-    } finally {
-        if (createdTempInputFile) {
-            await deleteTempFile(inputFilePath);
-        }
+    timeoutMS: number,
+): Promise<Uint8Array> => {
+    // 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);
     }
-}
 
-export async function runFFmpegCmd_(
-    cmd: string[],
-    inputFilePath: string,
-    outputFileName: string,
-    dontTimeout = false,
-) {
-    let tempOutputFilePath: string;
+    let outputFilePath: string | undefined;
     try {
-        tempOutputFilePath = await makeTempFilePath(outputFileName);
+        outputFilePath = 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;
-            }
-        });
+        const cmd = substitutePlaceholders(
+            command,
+            inputFilePath,
+            outputFilePath,
+        );
 
-        if (dontTimeout) await execAsync(cmd);
-        else await withTimeout(execAsync(cmd), 30 * 1000);
+        if (timeoutMS) await withTimeout(execAsync(cmd), 30 * 1000);
+        else await execAsync(cmd);
 
-        if (!existsSync(tempOutputFilePath)) {
-            throw new Error("ffmpeg output file not found");
-        }
-        const outputFile = await fs.readFile(tempOutputFilePath);
-        return new Uint8Array(outputFile);
-    } catch (e) {
-        log.error("FFMPEG command failed", e);
-        throw e;
+        return fs.readFile(outputFilePath);
     } finally {
-        await deleteTempFile(tempOutputFilePath);
+        if (isInputFileTemporary) await deleteTempFile(inputFilePath);
+        if (outputFilePath) await deleteTempFile(outputFilePath);
     }
-}
+};
+
+const substitutePlaceholders = (
+    command: string[],
+    inputFilePath: string,
+    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;
+        }
+    });
 
 /**
  * Return the path to the `ffmpeg` binary.

desktop/src/preload.ts

@@ -122,8 +122,6 @@ const fsWriteFile = (path: string, contents: string): Promise<void> =>
 const fsIsDir = (dirPath: string): Promise<boolean> =>
     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<File> =>
+    timeoutMS: number,
+): Promise<Uint8Array> =>
     ipcRenderer.invoke(
-        "runFFmpegCmd",
-        cmd,
-        inputFile,
+        "ffmpegExec",
+        command,
+        inputDataOrPath,
         outputFileName,
-        dontTimeout,
+        timeoutMS,
     );
 
 // - ML
@@ -255,6 +253,7 @@ const setPendingUploadFiles = (
 ): Promise<void> =>
     ipcRenderer.invoke("setPendingUploadFiles", type, filePaths);
 
+// - TODO: AUDIT below this
 // -
 
 const getElectronFilesFromGoogleZip = (
@@ -341,7 +340,7 @@ contextBridge.exposeInMainWorld("electron", {
 
     convertToJPEG,
     generateImageThumbnail,
-    runFFmpegCmd,
+    ffmpegExec,
 
     // - ML
 

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[],

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);

web/packages/next/types/ipc.ts

@@ -237,12 +237,12 @@ export interface Electron {
     ) => Promise<Uint8Array>;
 
     /**
-     * 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<File>;
+    ) => Promise<Uint8Array>;
 
     // - ML