[desktop] Fix large video conversion (#1713)

This was the last pending TODO for getting the new build at par with how the olde one behaved.
ente-io · May 13, 2024 · 8a8d240 · 8a8d240
2 parents 87ed5c1 + 859adea
commit 8a8d240
Show file tree

Hide file tree

Showing 15 changed files with 377 additions and 236 deletions.
diff --git a/desktop/src/main/ipc.ts b/desktop/src/main/ipc.ts
@@ -69,6 +69,7 @@ import {
     watchUpdateIgnoredFiles,
     watchUpdateSyncedFiles,
 } from "./services/watch";
+import { clearConvertToMP4Results } from "./stream";
 
 /**
  * Listen for IPC events sent/invoked by the renderer process, and route them to
@@ -108,6 +109,8 @@ export const attachIPCHandlers = () => {
 
     ipcMain.on("clearStores", () => clearStores());
 
+    ipcMain.on("clearConvertToMP4Results", () => clearConvertToMP4Results());
+
     ipcMain.handle("saveEncryptionKey", (_, encryptionKey: string) =>
         saveEncryptionKey(encryptionKey),
     );
@@ -171,14 +174,7 @@ export const attachIPCHandlers = () => {
             command: string[],
             dataOrPathOrZipItem: Uint8Array | string | ZipItem,
             outputFileExtension: string,
-            timeoutMS: number,
-        ) =>
-            ffmpegExec(
-                command,
-                dataOrPathOrZipItem,
-                outputFileExtension,
-                timeoutMS,
-            ),
+        ) => ffmpegExec(command, dataOrPathOrZipItem, outputFileExtension),
     );
 
     // - ML

diff --git a/desktop/src/main/services/app-update.ts b/desktop/src/main/services/app-update.ts
@@ -140,17 +140,17 @@ const checkForUpdatesAndNotify = async (mainWindow: BrowserWindow) => {
     log.debug(() => "Attempting auto update");
     await autoUpdater.downloadUpdate();
 
-    let timeoutId: ReturnType<typeof setTimeout>;
+    let timeout: ReturnType<typeof setTimeout>;
     const fiveMinutes = 5 * 60 * 1000;
     autoUpdater.on("update-downloaded", () => {
-        timeoutId = setTimeout(
+        timeout = setTimeout(
             () => showUpdateDialog({ autoUpdatable: true, version }),
             fiveMinutes,
         );
     });
 
     autoUpdater.on("error", (error) => {
-        clearTimeout(timeoutId);
+        clearTimeout(timeout);
         log.error("Auto update failed", error);
         showUpdateDialog({ autoUpdatable: false, version });
     });

diff --git a/desktop/src/main/services/ffmpeg.ts b/desktop/src/main/services/ffmpeg.ts
@@ -1,11 +1,10 @@
 import pathToFfmpeg from "ffmpeg-static";
 import fs from "node:fs/promises";
 import type { ZipItem } from "../../types/ipc";
-import log from "../log";
-import { ensure, withTimeout } from "../utils/common";
+import { ensure } from "../utils/common";
 import { execAsync } from "../utils/electron";
 import {
-    deleteTempFile,
+    deleteTempFileIgnoringErrors,
     makeFileForDataOrPathOrZipItem,
     makeTempFilePath,
 } from "../utils/temp";
@@ -46,13 +45,7 @@ export const ffmpegExec = async (
     command: string[],
     dataOrPathOrZipItem: Uint8Array | string | ZipItem,
     outputFileExtension: string,
-    timeoutMS: number,
 ): Promise<Uint8Array> => {
-    // TODO (MR): This currently copies files for both input (when
-    // dataOrPathOrZipItem is data) and output. This needs to be tested
-    // extremely large video files when invoked downstream of `convertToMP4` in
-    // the web code.
-
     const {
         path: inputFilePath,
         isFileTemporary: isInputFileTemporary,
@@ -69,17 +62,13 @@ export const ffmpegExec = async (
             outputFilePath,
         );
 
-        if (timeoutMS) await withTimeout(execAsync(cmd), timeoutMS);
-        else await execAsync(cmd);
+        await execAsync(cmd);
 
         return fs.readFile(outputFilePath);
     } finally {
-        try {
-            if (isInputFileTemporary) await deleteTempFile(inputFilePath);
-            await deleteTempFile(outputFilePath);
-        } catch (e) {
-            log.error("Could not clean up temp files", e);
-        }
+        if (isInputFileTemporary)
+            await deleteTempFileIgnoringErrors(inputFilePath);
+        await deleteTempFileIgnoringErrors(outputFilePath);
     }
 };
 
@@ -112,3 +101,32 @@ const ffmpegBinaryPath = () => {
     // https://github.com/eugeneware/ffmpeg-static/issues/16
     return ensure(pathToFfmpeg).replace("app.asar", "app.asar.unpacked");
 };
+
+/**
+ * A variant of {@link ffmpegExec} adapted to work with streams so that it can
+ * handle the MP4 conversion of large video files.
+ *
+ * See: [Note: Convert to MP4]
+
+ * @param inputFilePath The path to a file on the user's local file system. This
+ * is the video we want to convert.
+ * @param inputFilePath The path to a file on the user's local file system where
+ * we should write the converted MP4 video.
+ */
+export const ffmpegConvertToMP4 = async (
+    inputFilePath: string,
+    outputFilePath: string,
+): Promise<void> => {
+    const command = [
+        ffmpegPathPlaceholder,
+        "-i",
+        inputPathPlaceholder,
+        "-preset",
+        "ultrafast",
+        outputPathPlaceholder,
+    ];
+
+    const cmd = substitutePlaceholders(command, inputFilePath, outputFilePath);
+
+    await execAsync(cmd);
+};
diff --git a/desktop/src/main/services/image.ts b/desktop/src/main/services/image.ts
@@ -6,7 +6,7 @@ import { CustomErrorMessage, type ZipItem } from "../../types/ipc";
 import log from "../log";
 import { execAsync, isDev } from "../utils/electron";
 import {
-    deleteTempFile,
+    deleteTempFileIgnoringErrors,
     makeFileForDataOrPathOrZipItem,
     makeTempFilePath,
 } from "../utils/temp";
@@ -23,12 +23,8 @@ export const convertToJPEG = async (imageData: Uint8Array) => {
         await execAsync(command);
         return new Uint8Array(await fs.readFile(outputFilePath));
     } finally {
-        try {
-            await deleteTempFile(inputFilePath);
-            await deleteTempFile(outputFilePath);
-        } catch (e) {
-            log.error("Could not clean up temp files", e);
-        }
+        await deleteTempFileIgnoringErrors(inputFilePath);
+        await deleteTempFileIgnoringErrors(outputFilePath);
     }
 };
 
@@ -107,12 +103,9 @@ export const generateImageThumbnail = async (
         } while (thumbnail.length > maxSize && quality > 50);
         return thumbnail;
     } finally {
-        try {
-            if (isInputFileTemporary) await deleteTempFile(inputFilePath);
-            await deleteTempFile(outputFilePath);
-        } catch (e) {
-            log.error("Could not clean up temp files", e);
-        }
+        if (isInputFileTemporary)
+            await deleteTempFileIgnoringErrors(inputFilePath);
+        await deleteTempFileIgnoringErrors(outputFilePath);
     }
 };