timeline fix: loadrange in imavid (#4857)

* add dark orange indicator for buffering status * shouldn't play if timeline not initialized * fix edge cases in buffer manager * add onAnimationStutter * add onAnimationStutter * loadrange for imavid * remove log * use 30 as default frame rate / multiplier * add speed text on hover * fix effect dep bug * resolve promise when all samples addressed
voxel51 · Sep 27, 2024 · f5e8a5a · f5e8a5a
1 parent 81037fa
commit f5e8a5a
Show file tree

Hide file tree

Showing 9 changed files with 190 additions and 80 deletions.
diff --git a/app/packages/core/src/components/Modal/ImaVidLooker.tsx b/app/packages/core/src/components/Modal/ImaVidLooker.tsx
@@ -1,6 +1,5 @@
 import { useTheme } from "@fiftyone/components";
 import { AbstractLooker, ImaVidLooker } from "@fiftyone/looker";
-import { BUFFERING_PAUSE_TIMEOUT } from "@fiftyone/looker/src/lookers/imavid/constants";
 import { BaseState } from "@fiftyone/looker/src/state";
 import { FoTimelineConfig, useCreateTimeline } from "@fiftyone/playback";
 import { useDefaultTimelineNameImperative } from "@fiftyone/playback/src/lib/use-default-timeline-name";
@@ -54,6 +53,8 @@ export const ImaVidLookerReact = React.memo(
     });
 
     const { activeLookerRef, setActiveLookerRef } = useModalContext();
+    const imaVidLookerRef =
+      activeLookerRef as unknown as React.MutableRefObject<ImaVidLooker>;
 
     const looker = React.useMemo(
       () => createLooker.current(sampleDataWithExtraParams),
@@ -152,19 +153,59 @@ export const ImaVidLookerReact = React.memo(
       );
     }, [ref]);
 
-    const loadRange = React.useCallback(async (range: BufferRange) => {
-      // todo: implement
-      return new Promise<void>((resolve) => {
-        setTimeout(() => {
-          resolve();
-        }, BUFFERING_PAUSE_TIMEOUT);
-      });
-    }, []);
+    const loadRange = React.useCallback(
+      async (range: Readonly<BufferRange>) => {
+        const storeBufferManager =
+          imaVidLookerRef.current.frameStoreController.storeBufferManager;
+        const fetchBufferManager =
+          imaVidLookerRef.current.frameStoreController.fetchBufferManager;
+
+        if (storeBufferManager.containsRange(range)) {
+          return;
+        }
+
+        const unprocessedStoreBufferRange =
+          storeBufferManager.getUnprocessedBufferRange(range);
+        const unprocessedBufferRange =
+          fetchBufferManager.getUnprocessedBufferRange(
+            unprocessedStoreBufferRange
+          );
+
+        if (!unprocessedBufferRange) {
+          return;
+        }
+
+        imaVidLookerRef.current.frameStoreController.enqueueFetch(
+          unprocessedBufferRange
+        );
+
+        return new Promise<void>((resolve) => {
+          const fetchMoreListener = (e: CustomEvent) => {
+            if (
+              e.detail.id === imaVidLookerRef.current.frameStoreController.key
+            ) {
+              if (storeBufferManager.containsRange(unprocessedBufferRange)) {
+                resolve();
+                window.removeEventListener(
+                  "fetchMore",
+                  fetchMoreListener as EventListener
+                );
+              }
+            }
+          };
+
+          window.addEventListener(
+            "fetchMore",
+            fetchMoreListener as EventListener,
+            { once: true }
+          );
+        });
+      },
+      []
+    );
 
     const renderFrame = React.useCallback((frameNumber: number) => {
-      (
-        activeLookerRef.current as unknown as ImaVidLooker
-      )?.element.drawFrameNoAnimation(frameNumber);
+      imaVidLookerRef.current?.element.drawFrameNoAnimation(frameNumber);
     }, []);
 
     const { getName } = useDefaultTimelineNameImperative();
@@ -189,7 +230,7 @@ export const ImaVidLookerReact = React.memo(
 
     const readyWhen = useCallback(async () => {
       return new Promise<void>((resolve) => {
-        // wait for total frame count to be resolved
+        // hack: wait for total frame count to be resolved
         let intervalId;
         intervalId = setInterval(() => {
           if (totalFrameCountRef.current) {
@@ -200,6 +241,10 @@ export const ImaVidLookerReact = React.memo(
       });
     }, []);
 
+    const onAnimationStutter = useCallback(() => {
+      imaVidLookerRef.current?.element.checkFetchBufferManager();
+    }, []);
+
     const {
       isTimelineInitialized,
       registerOnPauseCallback,
@@ -210,6 +255,10 @@ export const ImaVidLookerReact = React.memo(
       name: timelineName,
       config: timelineCreationConfig,
       waitUntilInitialized: readyWhen,
+      // using this mechanism to resume fetch if it was paused
+      // ideally we have control of fetch in this component but can't do that yet
+      // since imavid is part of the grid too
+      onAnimationStutter,
     });
 
     /**
@@ -224,35 +273,27 @@ export const ImaVidLookerReact = React.memo(
         });
 
         registerOnPlayCallback(() => {
-          (activeLookerRef.current as unknown as ImaVidLooker).element.update(
-            () => ({
-              playing: true,
-            })
-          );
+          imaVidLookerRef.current?.element?.update(() => ({
+            playing: true,
+          }));
         });
 
         registerOnPauseCallback(() => {
-          (activeLookerRef.current as unknown as ImaVidLooker).element.update(
-            () => ({
-              playing: false,
-            })
-          );
+          imaVidLookerRef.current?.element?.update(() => ({
+            playing: false,
+          }));
         });
 
         registerOnSeekCallbacks({
           start: () => {
-            (activeLookerRef.current as unknown as ImaVidLooker).element.update(
-              () => ({
-                seeking: true,
-              })
-            );
+            imaVidLookerRef.current?.element?.update(() => ({
+              seeking: true,
+            }));
           },
           end: () => {
-            (activeLookerRef.current as unknown as ImaVidLooker).element.update(
-              () => ({
-                seeking: false,
-              })
-            );
+            imaVidLookerRef.current?.element?.update(() => ({
+              seeking: false,
+            }));
           },
         });
       }
@@ -265,9 +306,8 @@ export const ImaVidLookerReact = React.memo(
       // hack: poll every 10ms for total frame count
       // replace with event listener or callback
       let intervalId = setInterval(() => {
-        const totalFrameCount = (
-          activeLookerRef.current as unknown as ImaVidLooker
-        ).frameStoreController.totalFrameCount;
+        const totalFrameCount =
+          imaVidLookerRef.current.frameStoreController.totalFrameCount;
         if (totalFrameCount) {
           setTotalFrameCount(totalFrameCount);
           clearInterval(intervalId);

diff --git a/app/packages/looker/src/elements/imavid/index.ts b/app/packages/looker/src/elements/imavid/index.ts
@@ -362,6 +362,8 @@ export class ImaVidElement extends BaseElement<ImaVidState, HTMLImageElement> {
   /**
    * Queue up frames to be fetched if necessary.
    * This method is not blocking, it merely enqueues a fetch job.
+   *
+   * This is for legacy imavid, which is used for thumbnail imavid.
    */
   private ensureBuffers(state: Readonly<ImaVidState>) {
     if (!this.framesController.totalFrameCount) {
@@ -407,6 +409,19 @@ export class ImaVidElement extends BaseElement<ImaVidState, HTMLImageElement> {
     }
   }
 
+  /**
+   * Starts fetch if there are buffers in the fetch buffer manager
+   */
+  public checkFetchBufferManager() {
+    if (!this.framesController.totalFrameCount) {
+      return;
+    }
+
+    if (this.framesController.fetchBufferManager.buffers.length > 0) {
+      this.framesController.resumeFetch();
+    }
+  }
+
   renderSelf(state: Readonly<ImaVidState>) {
     const {
       options: { playbackRate, loop },
@@ -443,7 +458,11 @@ export class ImaVidElement extends BaseElement<ImaVidState, HTMLImageElement> {
       this.framesController.destroy();
     }
 
-    this.ensureBuffers(state);
+    if (this.isThumbnail) {
+      this.ensureBuffers(state);
+    } else {
+      this.checkFetchBufferManager();
+    }
 
     if (!playing && this.isAnimationActive) {
       // this flag will be picked up in `drawFrame`, that in turn will call `pause`

diff --git a/app/packages/looker/src/lookers/imavid/controller.ts b/app/packages/looker/src/lookers/imavid/controller.ts
@@ -123,8 +123,8 @@ export class ImaVidFramesController {
         BUFFER_METADATA_FETCHING
       );
 
-      // subtract by two because 1) cursor is one based and 2) cursor here translates to "after" the cursor
-      return this.fetchMore(range[0] - 2, range[1] - range[0] || 2).finally(
+      // subtract/add by two because 1) cursor is one based and 2) cursor here translates to "after" the cursor
+      return this.fetchMore(range[0] - 2, range[1] - range[0] + 2).finally(
         () => {
           this.fetchBufferManager.removeMetadataFromBufferRange(index);
         }
@@ -165,7 +165,7 @@ export class ImaVidFramesController {
     return this.config.page;
   }
 
-  private get key() {
+  public get key() {
     return this.config.key;
   }
 
@@ -257,23 +257,35 @@ export class ImaVidFramesController {
             const frameIndices = imageFetchPromisesMap.keys();
             const imageFetchPromises = imageFetchPromisesMap.values();
 
-            Promise.all(imageFetchPromises).then((sampleIds) => {
-              for (let i = 0; i < sampleIds.length; i++) {
-                const frameIndex = frameIndices.next().value;
-                const sampleId = sampleIds[i];
-                this.store.frameIndex.set(frameIndex, sampleId);
-                this.store.reverseFrameIndex.set(sampleId, frameIndex);
-
-                this.storeBufferManager.addNewRange([
+            Promise.all(imageFetchPromises)
+              .then((sampleIds) => {
+                for (let i = 0; i < sampleIds.length; i++) {
+                  const frameIndex = frameIndices.next().value;
+                  const sampleId = sampleIds[i];
+                  this.store.frameIndex.set(frameIndex, sampleId);
+                  this.store.reverseFrameIndex.set(sampleId, frameIndex);
+                }
+                resolve();
+              })
+              .then(() => {
+                const newRange = [
                   Number(data.samples.edges[0].cursor) + 1,
                   Number(
                     data.samples.edges[data.samples.edges.length - 1].cursor
                   ) + 1,
-                ]);
-
-                resolve();
-              }
-            });
+                ] as BufferRange;
+
+                this.storeBufferManager.addNewRange(newRange);
+
+                window.dispatchEvent(
+                  new CustomEvent("fetchMore", {
+                    detail: {
+                      id: this.key,
+                    },
+                    bubbles: false,
+                  })
+                );
+              });
           }
         },
       });

diff --git a/app/packages/playback/src/lib/constants.ts b/app/packages/playback/src/lib/constants.ts
@@ -1,7 +1,7 @@
 export const DEFAULT_FRAME_NUMBER = 1;
 export const DEFAULT_LOOP = false;
 export const DEFAULT_SPEED = 1;
-export const DEFAULT_TARGET_FRAME_RATE = 29.97;
+export const DEFAULT_TARGET_FRAME_RATE = 30;
 export const DEFAULT_USE_TIME_INDICATOR = false;
 export const GLOBAL_TIMELINE_ID = "fo-timeline-global";
 export const LOAD_RANGE_SIZE = 250;

diff --git a/app/packages/playback/src/lib/state.ts b/app/packages/playback/src/lib/state.ts
@@ -133,6 +133,11 @@ export type CreateFoTimeline = {
    * If true, the creator will be responsible for managing the animation loop.
    */
   optOutOfAnimation?: boolean;
+
+  /**
+   * Callback to be called when the animation stutters.
+   */
+  onAnimationStutter?: () => void;
 };
 
 const _frameNumbers = atomFamily((_timelineName: TimelineName) =>
@@ -339,7 +344,7 @@ export const setFrameNumberAtom = atom(
       set(_currentBufferingRange(name), newLoadRange);
 
       try {
-        await Promise.all(rangeLoadPromises);
+        await Promise.allSettled(rangeLoadPromises);
         bufferManager.addNewRange(newLoadRange);
       } catch (e) {
         // todo: handle error better, maybe retry
@@ -358,9 +363,8 @@ export const setFrameNumberAtom = atom(
       renderPromises.push(subscriber.renderFrame(newFrameNumber));
     });
 
-    Promise.all(renderPromises).then(() => {
-      set(_frameNumbers(name), newFrameNumber);
-    });
+    await Promise.allSettled(renderPromises);
+    set(_frameNumbers(name), newFrameNumber);
   }
 );