diff --git a/web/packages/gallery/components/viewer/data-source.ts b/web/packages/gallery/components/viewer/data-source.ts index 933e66d5bc..125a47686f 100644 --- a/web/packages/gallery/components/viewer/data-source.ts +++ b/web/packages/gallery/components/viewer/data-source.ts @@ -145,6 +145,11 @@ export type ItemData = PhotoSwipeSlideData & { * original file could not be fetched because of an network error). */ fetchFailed?: boolean; + /** + * This will be `true` if the item data for this particular file is + * considered transient, and should not be cached. + */ + isTransient?: boolean; }; /** @@ -251,7 +256,7 @@ export const fileViewerDidClose = () => { resetState(); } else { // Selectively clear. - forgetFailedItems(); + forgetFailedOrTransientItems(); forgetExif(); } }; @@ -357,13 +362,40 @@ export const forgetItemDataForFileID = (fileID: number) => { }; /** - * Forget item data for the given {@link file} if its fetch had failed. + * Forget item data for the given {@link file} if its fetch had failed, or if it + * is caching something that is transient in nature. * - * This is called when the user moves away from a slide so that we attempt a - * full retry when they come back the next time. + * It is called when the user moves away from a slide. In particular, this way + * we can reset failures, if any, for a slide so that the fetch is tried again + * when we come back to it. + * + * [Note: File viewer preloading and contentDeactivate] + * + * Note that because of preloading, this will only have a user visible effect if + * the user moves out of the preload range. Let's take an example: + * + * - User opens slide at index `i`. Then the adjacent slides, `i - 1` and `i + + * 1`, also get preloaded. + * + * - User moves away from `i` (in either direction, but let us take the example + * if they move `i - 1`). + * + * - This function will get called for `i` and will clear any failed or + * transient state. + * + * - But since PhotoSwipe already has the slides ready for `i`, if the user then + * moves back to `i` then PhotoSwipe will not attempt to recreate the slide + * and so will not even try to get "itemData". So this will only have an + * effect if they move out of preload range (e.g. `i - 1`, `i - 2`), and then + * back (`i - 1`, `i`). + * + * See: [Note: File viewer error handling] + * + * See: [Note: Caching HLS playlist data] */ -export const forgetFailedItemDataForFileID = (fileID: number) => { - if (_state.itemDataByFileID.get(fileID)?.fetchFailed) +export const forgetItemDataForFileIDIfNeeded = (fileID: number) => { + const itemData = _state.itemDataByFileID.get(fileID); + if (itemData?.fetchFailed || itemData?.isTransient) forgetItemDataForFileID(fileID); }; @@ -381,13 +413,17 @@ export const updateItemDataAlt = (updatedFile: EnteFile) => { }; /** - * Forget item data for the all files whose fetch had failed. + * Forget item data for the all files whose fetch had failed, or if the + * corresponding item data is transient and shouldn't be cached. * - * This is called when the user closes the file viewer so that we attempt a full - * retry when they reopen the viewer the next time. + * This is called when the user closes the file viewer; in particular, this way + * we attempt a full retry for previously failed files when the user reopens the + * viewer the next time. */ -const forgetFailedItems = () => - [..._state.itemDataByFileID.keys()].forEach(forgetFailedItemDataForFileID); +const forgetFailedOrTransientItems = () => + [..._state.itemDataByFileID.keys()].forEach( + forgetItemDataForFileIDIfNeeded, + ); const enqueueUpdates = async ( file: EnteFile, @@ -431,7 +467,22 @@ const enqueueUpdates = async ( createHLSPlaylistItemDataValidity(), ); } else { - update(videoURLD); + if (shouldUsePlayerV2()) { + // See: [Note: Caching HLS playlist data] + // + // TODO(HLS): As an optimization, we can handle the logged in vs + // public albums case separately once we have the status-diff + // state, we don't need to mark status-diff case as transient. + // + // Note that setting the transient flag is not too expensive, + // since the underlying videoURL is still cached by the download + // manager. So effectively, under normal circumstance, it just + // adds one API call (to recheck if an HLS playlist now exists + // for the given file). + update({ ...videoURLD, isTransient: true }); + } else { + update(videoURLD); + } } }; diff --git a/web/packages/gallery/components/viewer/photoswipe.ts b/web/packages/gallery/components/viewer/photoswipe.ts index 72d3fc90e3..2f953e7e88 100644 --- a/web/packages/gallery/components/viewer/photoswipe.ts +++ b/web/packages/gallery/components/viewer/photoswipe.ts @@ -1,5 +1,6 @@ import { pt } from "ente-base/i18n"; import log from "ente-base/log"; +import { albumsAppOrigin } from "ente-base/origins"; import type { EnteFile } from "ente-media/file"; import { FileType } from "ente-media/file-type"; import { settingsSnapshot } from "ente-new/photos/services/settings"; @@ -14,8 +15,8 @@ import { fileViewerDidClose, fileViewerWillOpen, forgetExifForItemData, - forgetFailedItemDataForFileID, forgetItemDataForFileID, + forgetItemDataForFileIDIfNeeded, itemDataForFile, updateFileInfoExifIfNeeded, type ItemData, @@ -166,7 +167,15 @@ export const moreMenuID = "ente-pswp-more-menu"; // TODO(HLS): let _shouldUsePlayerV2: boolean | undefined; export const shouldUsePlayerV2 = () => - (_shouldUsePlayerV2 ??= settingsSnapshot().isInternalUser); + // Enable for internal users and public albums + (_shouldUsePlayerV2 ??= + settingsSnapshot().isInternalUser || isPublicAlbumApp()); + +const isPublicAlbumApp = () => { + const currentURL = new URL(window.location.href); + const albumsURL = new URL(albumsAppOrigin()); + return currentURL.host == albumsURL.host; +}; /** * A wrapper over {@link PhotoSwipe} to tailor its interface for use by our file @@ -711,16 +720,13 @@ export class FileViewerPhotoSwipe { element?.querySelector("video, hls-video"); pswp.on("contentDeactivate", (e) => { - // Reset failures, if any, for this file so that the fetch is tried - // again when we come back to it^. + // PhotoSwipe invokes this event when moving away from a slide. // - // ^ Note that because of how the preloading works, this will have - // an effect (i.e. the retry will happen) only if the user moves - // more than 2 slides and then back, or if they reopen the viewer. - // - // See: [Note: File viewer error handling] + // However it might not have an effect until we move out of preload + // range. See: [Note: File viewer preloading and contentDeactivate]. + const fileID = asItemData(e.content.data).fileID; - if (fileID) forgetFailedItemDataForFileID(fileID); + if (fileID) forgetItemDataForFileIDIfNeeded(fileID); // Pause the video element, if any, when we move away from the // slide. diff --git a/web/packages/gallery/services/video.ts b/web/packages/gallery/services/video.ts index 55a2f74996..ec1cb4d419 100644 --- a/web/packages/gallery/services/video.ts +++ b/web/packages/gallery/services/video.ts @@ -84,6 +84,42 @@ export interface HLSPlaylistData { * the given file. * * See: [Note: Video playlist and preview] + * + * --- + * + * [Note: Caching HLS playlist data] + * + * The playlist data can be cached in an asymmetric manner. + * + * - If a file has a corresponding HLS playlist, then currently there is no + * scenario (apart from file deletion, where the playlist also gets deleted) + * where the playlist is updated or deleted after being created. There is a + * limit to the validity of the presigned chunk URLs within the playlist we + * create which we do handle (`createHLSPlaylistItemDataValidity`), but the + * original playlist itself does not change. In particular, a positive result + * ("this file has a playlist") can be cached indefinitely. + * + * - If a file does not have a HLS playlist, and it is eligible for being + * streamed (e.g. it is not too small where the streaming overhead is not + * required), then a client (this one, or a different one) can process it at + * any arbitrary time. So the negative result ("this file does not have a + * playlist") cannot be cached. + * + * So while we can easily cache the first case ("this file has a playlist"), we + * need to deal with the second case ("this file does not have a playlist") a + * bit more intricately: + * + * - If running in the context of a logged in user (e.g. photos app), we can use + * the "/files/data/status-diff" API to be notified of any modifications to + * the second case for the user's own files. This status-diff happens during + * the regular "sync", and we can use that as a cue to selectively prune cache + * entries for the second case (but can otherwise indefinitely cache it). + * + * - If the file is a shared file, the status-diff will not return it. And if + * we're not running in the context of a logged in user (e.g. the public + * albums app), then there is no status-diff to do. For these two scenarios, + * we thus mark the cached values as "transient" and always recheck for a + * playlist when opening the slide. */ export const hlsPlaylistDataForFile = async ( file: EnteFile,