Fix video thumbnailing (the .jpeg extension is required)

desktop/src/main/ipc.ts

@@ -161,8 +161,9 @@ export const attachIPCHandlers = () => {
             _,
             command: string[],
             dataOrPath: Uint8Array | string,
+            outputFileExtension: string,
             timeoutMS: number,
-        ) => ffmpegExec(command, dataOrPath, timeoutMS),
+        ) => ffmpegExec(command, dataOrPath, outputFileExtension, timeoutMS),
     );
 
     // - ML

desktop/src/main/services/ffmpeg.ts

@@ -40,6 +40,7 @@ const outputPathPlaceholder = "OUTPUT";
 export const ffmpegExec = async (
     command: string[],
     dataOrPath: Uint8Array | string,
+    outputFileExtension: string,
     timeoutMS: number,
 ): Promise<Uint8Array> => {
     // TODO (MR): This currently copies files for both input and output. This
@@ -56,7 +57,7 @@ export const ffmpegExec = async (
         isInputFileTemporary = false;
     }
 
-    const outputFilePath = await makeTempFilePath();
+    const outputFilePath = await makeTempFilePath(outputFileExtension);
     try {
         if (dataOrPath instanceof Uint8Array)
             await fs.writeFile(inputFilePath, dataOrPath);

desktop/src/main/services/image.ts

@@ -9,7 +9,7 @@ import { deleteTempFile, makeTempFilePath } from "../utils-temp";
 
 export const convertToJPEG = async (imageData: Uint8Array) => {
     const inputFilePath = await makeTempFilePath();
-    const outputFilePath = await makeTempFilePath(".jpeg");
+    const outputFilePath = await makeTempFilePath("jpeg");
 
     // Construct the command first, it may throw NotAvailable on win32.
     const command = convertToJPEGCommand(inputFilePath, outputFilePath);
@@ -77,7 +77,7 @@ export const generateImageThumbnail = async (
         isInputFileTemporary = false;
     }
 
-    const outputFilePath = await makeTempFilePath(".jpeg");
+    const outputFilePath = await makeTempFilePath("jpeg");
 
     // Construct the command first, it may throw `NotAvailable` on win32.
     let quality = 70;

desktop/src/main/utils-temp.ts

@@ -29,17 +29,18 @@ const randomPrefix = () => {
  *
  * The function returns the path to a file in the system temp directory (in an
  * Ente specific folder therin) with a random prefix and an (optional)
- * {@link suffix}.
+ * {@link extension}.
  *
- * It ensures that there is no existing file with the same name already.
+ * It ensures that there is no existing item with the same name already.
  *
  * Use {@link deleteTempFile} to remove this file when you're done.
  */
-export const makeTempFilePath = async (suffix?: string) => {
+export const makeTempFilePath = async (extension?: string) => {
     const tempDir = await enteTempDirPath();
+    const suffix = extension ? "." + extension : "";
     let result: string;
     do {
-        result = path.join(tempDir, `${randomPrefix()}${suffix ?? ""}`);
+        result = path.join(tempDir, randomPrefix() + suffix);
     } while (existsSync(result));
     return result;
 };

desktop/src/preload.ts

@@ -142,9 +142,16 @@ const generateImageThumbnail = (
 const ffmpegExec = (
     command: string[],
     dataOrPath: Uint8Array | string,
+    outputFileExtension: string,
     timeoutMS: number,
 ): Promise<Uint8Array> =>
-    ipcRenderer.invoke("ffmpegExec", command, dataOrPath, timeoutMS);
+    ipcRenderer.invoke(
+        "ffmpegExec",
+        command,
+        dataOrPath,
+        outputFileExtension,
+        timeoutMS,
+    );
 
 // - ML
 

web/apps/photos/src/services/ffmpeg.ts

@@ -25,10 +25,14 @@ import { type DedicatedFFmpegWorker } from "worker/ffmpeg.worker";
  *
  * See also {@link generateVideoThumbnailNative}.
  */
-export const generateVideoThumbnailWeb = async (blob: Blob) => {
-    const thumbnailAtTime = (seekTime: number) =>
-        ffmpegExecWeb(commandForThumbnailAtTime(seekTime), blob, 0);
+export const generateVideoThumbnailWeb = async (blob: Blob) =>
+    generateVideoThumbnail((seekTime: number) =>
+        ffmpegExecWeb(genThumbnailCommand(seekTime), blob, "jpeg", 0),
+    );
 
+const generateVideoThumbnail = async (
+    thumbnailAtTime: (seekTime: number) => Promise<Uint8Array>,
+) => {
     try {
         // Try generating thumbnail at seekTime 1 second.
         return await thumbnailAtTime(1);
@@ -56,21 +60,17 @@ export const generateVideoThumbnailWeb = async (blob: Blob) => {
 export const generateVideoThumbnailNative = async (
     electron: Electron,
     dataOrPath: Uint8Array | string,
-) => {
-    const thumbnailAtTime = (seekTime: number) =>
-        electron.ffmpegExec(commandForThumbnailAtTime(seekTime), dataOrPath, 0);
+) =>
+    generateVideoThumbnail((seekTime: number) =>
+        electron.ffmpegExec(
+            genThumbnailCommand(seekTime),
+            dataOrPath,
+            "jpeg",
+            0,
+        ),
+    );
 
-    try {
-        // Try generating thumbnail at seekTime 1 second.
-        return await thumbnailAtTime(1);
-    } catch (e) {
-        // If that fails, try again at the beginning. If even this throws, let
-        // it fail.
-        return await thumbnailAtTime(0);
-    }
-};
-
-const commandForThumbnailAtTime = (seekTime: number) => [
+const genThumbnailCommand = (seekTime: number) => [
     ffmpegPathPlaceholder,
     "-i",
     inputPathPlaceholder,
@@ -103,7 +103,7 @@ export async function extractVideoMetadata(file: File | ElectronFile) {
             outputPathPlaceholder,
         ],
         file,
-        `metadata.txt`,
+        "txt",
     );
     return parseFFmpegExtractedMetadata(metadata);
 }
@@ -184,7 +184,7 @@ export async function convertToMP4(file: File) {
             outputPathPlaceholder,
         ],
         file,
-        "output.mp4",
+        "mp4",
         30 * 1000,
     );
 }
@@ -198,10 +198,11 @@ export async function convertToMP4(file: File) {
 const ffmpegExecWeb = async (
     command: string[],
     blob: Blob,
+    outputFileExtension: string,
     timeoutMs: number,
 ) => {
     const worker = await workerFactory.lazy();
-    return await worker.exec(command, blob, timeoutMs);
+    return await worker.exec(command, blob, outputFileExtension, timeoutMs);
 };
 
 /**
@@ -232,7 +233,7 @@ const ffmpegExecNative = async (
 const ffmpegExec2 = async (
     command: string[],
     inputFile: File | ElectronFile,
-    outputFileName: string,
+    outputFileExtension: string,
     timeoutMS: number = 0,
 ) => {
     const electron = globalThis.electron;
@@ -247,7 +248,12 @@ const ffmpegExec2 = async (
         // );
     } else {
         /* TODO(MR): ElectronFile changes */
-        return ffmpegExecWeb(command, inputFile as File, timeoutMS);
+        return ffmpegExecWeb(
+            command,
+            inputFile as File,
+            outputFileExtension,
+            timeoutMS,
+        );
     }
 };
 

web/apps/photos/src/worker/ffmpeg.worker.ts

@@ -26,10 +26,16 @@ export class DedicatedFFmpegWorker {
      * This is a sibling of {@link ffmpegExec} exposed by the desktop app in
      * `ipc.ts`. See [Note: FFmpeg in Electron].
      */
-    async exec(command: string[], blob: Blob, timeoutMs): Promise<Uint8Array> {
+    async exec(
+        command: string[],
+        blob: Blob,
+        outputFileExtension: string,
+        timeoutMs,
+    ): Promise<Uint8Array> {
         if (!this.ffmpeg.isLoaded()) await this.ffmpeg.load();
 
-        const go = () => ffmpegExec(this.ffmpeg, command, blob);
+        const go = () =>
+            ffmpegExec(this.ffmpeg, command, outputFileExtension, blob);
 
         const request = this.ffmpegTaskQueue.queueUpRequest(() =>
             timeoutMs ? withTimeout(go(), timeoutMs) : go(),
@@ -41,9 +47,15 @@ export class DedicatedFFmpegWorker {
 
 expose(DedicatedFFmpegWorker, self);
 
-const ffmpegExec = async (ffmpeg: FFmpeg, command: string[], blob: Blob) => {
-    const inputPath = `${randomPrefix()}.in`;
-    const outputPath = `${randomPrefix()}.out`;
+const ffmpegExec = async (
+    ffmpeg: FFmpeg,
+    command: string[],
+    outputFileExtension: string,
+    blob: Blob,
+) => {
+    const inputPath = randomPrefix();
+    const outputSuffix = outputFileExtension ? "." + outputFileExtension : "";
+    const outputPath = randomPrefix() + outputSuffix;
 
     const cmd = substitutePlaceholders(command, inputPath, outputPath);
 

web/packages/next/types/ipc.ts

@@ -254,6 +254,12 @@ export interface Electron {
      * a temporary file, and then that path gets substituted in the FFmpeg
      * {@link command} in lieu of {@link inputPathPlaceholder}.
      *
+     * @param outputFileExtension The extension (without the dot, e.g. "jpeg")
+     * to use for the output file that we ask FFmpeg to create in
+     * {@param command}. While this file will eventually get deleted, and we'll
+     * just return its contents, for some FFmpeg command the extension matters
+     * (e.g. conversion to a JPEG fails if the extension is arbitrary).
+     *
      * @param timeoutMS If non-zero, then abort and throw a timeout error if the
      * ffmpeg command takes more than the given number of milliseconds.
      *
@@ -263,6 +269,7 @@ export interface Electron {
     ffmpegExec: (
         command: string[],
         dataOrPath: Uint8Array | string,
+        outputFileExtension: string,
         timeoutMS: number,
     ) => Promise<Uint8Array>;