[web] Club into a live photo only if within 5 mins (#2859)

web/apps/photos/src/services/upload/uploadManager.ts

@@ -1,6 +1,5 @@
 import { createComlinkCryptoWorker } from "@/base/crypto";
 import { type CryptoWorker } from "@/base/crypto/worker";
-import { ensureElectron } from "@/base/electron";
 import { lowercaseExtension, nameAndExtension } from "@/base/file";
 import log from "@/base/log";
 import type { Electron } from "@/base/types/ipc";
@@ -35,7 +34,12 @@ import {
     tryParseTakeoutMetadataJSON,
     type ParsedMetadataJSON,
 } from "./takeout";
-import UploadService, { uploadItemFileName, uploader } from "./uploadService";
+import UploadService, {
+    uploadItemCreationDate,
+    uploadItemFileName,
+    uploadItemSize,
+    uploader,
+} from "./uploadService";
 
 export type FileID = number;
 
@@ -424,7 +428,10 @@ class UploadManager {
             }
 
             if (mediaItems.length) {
-                const clusteredMediaItems = await clusterLivePhotos(mediaItems);
+                const clusteredMediaItems = await clusterLivePhotos(
+                    mediaItems,
+                    this.parsedMetadataJSONMap,
+                );
 
                 this.abortIfCancelled();
 
@@ -835,6 +842,7 @@ const markUploaded = async (electron: Electron, item: ClusteredUploadItem) => {
  */
 const clusterLivePhotos = async (
     items: UploadItemWithCollectionIDAndName[],
+    parsedMetadataJSONMap: Map<string, ParsedMetadataJSON>,
 ) => {
     const result: ClusteredUploadItem[] = [];
     items
@@ -862,7 +870,7 @@ const clusterLivePhotos = async (
             collectionID: g.collectionID,
             uploadItem: g.uploadItem,
         };
-        if (await areLivePhotoAssets(fa, ga)) {
+        if (await areLivePhotoAssets(fa, ga, parsedMetadataJSONMap)) {
             const [image, video] =
                 fFileType == FileType.image ? [f, g] : [g, f];
             result.push({
@@ -903,6 +911,7 @@ interface PotentialLivePhotoAsset {
 const areLivePhotoAssets = async (
     f: PotentialLivePhotoAsset,
     g: PotentialLivePhotoAsset,
+    parsedMetadataJSONMap: Map<string, ParsedMetadataJSON>,
 ) => {
     if (f.collectionID != g.collectionID) return false;
 
@@ -949,6 +958,27 @@ const areLivePhotoAssets = async (
         return false;
     }
 
+    // Finally, ensure that the creation times of the image and video are within
+    // some epsilon of each other. This is to avoid clubbing together unrelated
+    // items that coincidentally have the same name (this is not uncommon since,
+    // e.g. many cameras use a deterministic numbering scheme).
+
+    const fDate = await uploadItemCreationDate(
+        f.uploadItem,
+        f.fileType,
+        f.collectionID,
+        parsedMetadataJSONMap,
+    );
+    const gDate = await uploadItemCreationDate(
+        g.uploadItem,
+        g.fileType,
+        g.collectionID,
+        parsedMetadataJSONMap,
+    );
+    if (!fDate || !gDate) return false;
+    const secondDelta = Math.abs(fDate - gDate) / 1e6;
+    if (secondDelta > 5 * 60 /* 5 mins */) return false;
+
     return true;
 };
 
@@ -978,18 +1008,6 @@ const removePotentialLivePhotoSuffix = (name: string, suffix?: string) => {
     return foundSuffix ? name.slice(0, foundSuffix.length * -1) : name;
 };
 
-/**
- * Return the size of the given {@link uploadItem}.
- */
-const uploadItemSize = async (uploadItem: UploadItem): Promise<number> => {
-    if (uploadItem instanceof File) return uploadItem.size;
-    if (typeof uploadItem == "string")
-        return ensureElectron().pathOrZipItemSize(uploadItem);
-    if (Array.isArray(uploadItem))
-        return ensureElectron().pathOrZipItemSize(uploadItem);
-    return uploadItem.file.size;
-};
-
 /**
  * [Note: Memory pressure when uploading video files]
  *

web/apps/photos/src/services/upload/uploadService.ts

@@ -764,6 +764,10 @@ const extractImageOrVideoMetadata = async (
 
     const hash = await computeHash(uploadItem, worker);
 
+    // Some of this logic is duplicated below in `uploadItemCreationDate`.
+    //
+    // See: [Note: Duplicate retrieval of creation date for live photo clubbing]
+
     const parsedMetadataJSON = matchTakeoutMetadata(
         fileName,
         collectionID,
@@ -811,7 +815,7 @@ const extractImageOrVideoMetadata = async (
 
 const tryExtractImageMetadata = async (
     uploadItem: UploadItem,
-    lastModifiedMs: number,
+    lastModifiedMs: number | undefined,
 ): Promise<ParsedMetadata> => {
     let file: File;
     if (typeof uploadItem == "string" || Array.isArray(uploadItem)) {
@@ -820,9 +824,8 @@ const tryExtractImageMetadata = async (
         // reasonable to read the entire stream into memory here.
         const { response } = await readStream(ensureElectron(), uploadItem);
         const path = typeof uploadItem == "string" ? uploadItem : uploadItem[1];
-        file = new File([await response.arrayBuffer()], basename(path), {
-            lastModified: lastModifiedMs,
-        });
+        const opts = lastModifiedMs ? { lastModified: lastModifiedMs } : {};
+        file = new File([await response.arrayBuffer()], basename(path), opts);
     } else if (uploadItem instanceof File) {
         file = uploadItem;
     } else {
@@ -846,6 +849,66 @@ const tryExtractVideoMetadata = async (uploadItem: UploadItem) => {
     }
 };
 
+/**
+ * Return the creation date for the given {@link uploadItem}.
+ *
+ * [Note: Duplicate retrieval of creation date for live photo clubbing]
+ *
+ * This function duplicates some logic of {@link extractImageOrVideoMetadata}.
+ * This duplication, while not good, is currently unavoidable with the way the
+ * code is structured since the live photo clubbing happens at an earlier time
+ * in the pipeline when we don't have the Exif data, but the Exif data is needed
+ * to determine the file's creation time (to ensure that we only club photos and
+ * videos with close by creation times, instead of just relying on file names).
+ *
+ * Note that unlike {@link extractImageOrVideoMetadata}, we don't try to
+ * fallback to the file's modification time. This is because for the purpose of
+ * live photo clubbing, we wish to use the creation date only in cases where we
+ * have it.
+ */
+export const uploadItemCreationDate = async (
+    uploadItem: UploadItem,
+    fileType: FileType,
+    collectionID: number,
+    parsedMetadataJSONMap: Map<string, ParsedMetadataJSON>,
+) => {
+    const fileName = uploadItemFileName(uploadItem);
+
+    const parsedMetadataJSON = matchTakeoutMetadata(
+        fileName,
+        collectionID,
+        parsedMetadataJSONMap,
+    );
+
+    if (parsedMetadataJSON?.creationTime)
+        return parsedMetadataJSON?.creationTime;
+
+    let parsedMetadata: ParsedMetadata;
+    if (fileType == FileType.image) {
+        parsedMetadata = await tryExtractImageMetadata(uploadItem, undefined);
+    } else if (fileType == FileType.video) {
+        parsedMetadata = await tryExtractVideoMetadata(uploadItem);
+    } else {
+        throw new Error(`Unexpected file type ${fileType} for ${uploadItem}`);
+    }
+
+    return parsedMetadata.creationDate?.timestamp;
+};
+
+/**
+ * Return the size of the given {@link uploadItem}.
+ */
+export const uploadItemSize = async (
+    uploadItem: UploadItem,
+): Promise<number> => {
+    if (uploadItem instanceof File) return uploadItem.size;
+    if (typeof uploadItem == "string")
+        return ensureElectron().pathOrZipItemSize(uploadItem);
+    if (Array.isArray(uploadItem))
+        return ensureElectron().pathOrZipItemSize(uploadItem);
+    return uploadItem.file.size;
+};
+
 const computeHash = async (uploadItem: UploadItem, worker: CryptoWorker) => {
     const { stream, chunkCount } = await readUploadItem(uploadItem);
     const hashState = await worker.initChunkHashing();