fix timeline api bugs

app/packages/playback/src/lib/state.ts

@@ -222,15 +222,25 @@ export const addTimelineAtom = atom(
 export const addSubscriberAtom = atom(
   null,
   (
-    _get,
+    get,
     set,
     {
       name,
       subscription,
     }: { name: TimelineName; subscription: SequenceTimelineSubscription }
   ) => {
+    // warn if subscription with this id already exists
+    if (get(_subscribers(name)).has(subscription.id)) {
+      console.warn(
+        `Subscription with ${subscription.id} already exists for timeline ${name}. Replacing old subscription. Make sure this is an intentional behavior.`
+      );
+    }
+
+    const bufferManager = get(_dataLoadedBuffers(name));
+
     set(_subscribers(name), (prev) => {
       prev.set(subscription.id, subscription);
+      bufferManager.reset();
       return prev;
     });
   }
@@ -354,6 +364,15 @@ export const getPlayheadStateAtom = atomFamily((_timelineName: TimelineName) =>
   atom((get) => get(_playHeadStates(_timelineName)))
 );
 
+export const getIsTimelineInitializedAtom = atomFamily(
+  (_timelineName: TimelineName) =>
+    atom((get) => {
+      return Boolean(
+        get(_timelineConfigs(_timelineName)).__internal_IsTimelineInitialized
+      );
+    })
+);
+
 export const getTimelineConfigAtom = atomFamily((_timelineName: TimelineName) =>
   atom((get) => get(_timelineConfigs(_timelineName)))
 );

app/packages/playback/src/lib/use-create-timeline.ts

@@ -1,7 +1,7 @@
 import { Optional, useEventHandler } from "@fiftyone/state";
 import { useAtomValue, useSetAtom } from "jotai";
 import { useAtomCallback } from "jotai/utils";
-import { useCallback, useEffect, useMemo, useRef, useState } from "react";
+import { useCallback, useEffect, useMemo, useRef } from "react";
 import {
   _INTERNAL_timelineConfigsLruCache,
   addSubscriberAtom,
@@ -39,9 +39,8 @@ export const useCreateTimeline = (
     [mayBeTimelineName, getName]
   );
 
-  const [isTimelineInitialized, setIsTimelineInitialized] = useState(false);
-
-  const config = useAtomValue(getTimelineConfigAtom(timelineName));
+  const { __internal_IsTimelineInitialized: isTimelineInitialized, ...config } =
+    useAtomValue(getTimelineConfigAtom(timelineName));
   const frameNumber = useAtomValue(getFrameNumberAtom(timelineName));
   const playHeadState = useAtomValue(getPlayheadStateAtom(timelineName));
   const updateFreq = useAtomValue(getTimelineUpdateFreqAtom(timelineName));
@@ -51,65 +50,6 @@ export const useCreateTimeline = (
   const setFrameNumber = useSetAtom(setFrameNumberAtom);
   const setPlayHeadState = useSetAtom(updatePlayheadStateAtom);
 
-  /**
-   * this effect creates the timeline
-   */
-  useEffect(() => {
-    addTimeline({ name: timelineName, config: newTimelineConfig.config });
-
-    // this is so that this timeline is brought to the front of the cache
-    _INTERNAL_timelineConfigsLruCache.get(timelineName);
-
-    setIsTimelineInitialized(true);
-
-    return () => {
-      // when component using this hook unmounts, pause animation
-      // pause();
-    };
-    // note: we're not using newTimelineConfig.config as a dependency
-    // because it's not guaranteed to be referentially stable.
-    // that would require caller to memoize the passed config object.
-    // using just the timelineName as a dependency is fine.
-  }, [addTimeline, timelineName]);
-
-  /**
-   * this effect starts or stops the animation
-   * based on the playhead state
-   */
-  useEffect(() => {
-    if (playHeadState === "playing") {
-      startAnimation();
-    }
-
-    if (playHeadState === "paused") {
-      cancelAnimation();
-    }
-
-    playHeadStateRef.current = playHeadState;
-  }, [playHeadState]);
-
-  /**
-   * this effect establishes a binding with externally
-   * updated frame number. Note that for this effect to have
-   * the required effect, the external setter needs to have disabled animation first
-   * by dispatching a pause event
-   */
-  useEffect(() => {
-    if (!isAnimationActiveRef.current) {
-      frameNumberRef.current = frameNumber;
-    }
-  }, [frameNumber]);
-
-  /**
-   * the following effects are used to keep the refs up to date
-   */
-  useEffect(() => {
-    configRef.current = config;
-  }, [config]);
-  useEffect(() => {
-    updateFreqRef.current = updateFreq;
-  }, [updateFreq]);
-
   const animationId = useRef(-1);
   const configRef = useRef(config);
   const isAnimationActiveRef = useRef(false);
@@ -270,6 +210,68 @@ export const useCreateTimeline = (
     )
   );
 
+  /**
+   * this effect creates the timeline
+   */
+  useEffect(() => {
+    addTimeline({ name: timelineName, config: newTimelineConfig.config });
+
+    // this is so that this timeline is brought to the front of the cache
+    _INTERNAL_timelineConfigsLruCache.get(timelineName);
+
+    return () => {
+      // when component using this hook unmounts, pause animation
+      pause();
+
+      // timeline cleanup is handled by `_INTERNAL_timelineConfigsLruCache::dispose()`
+    };
+    // note: we're not using newTimelineConfig.config as a dependency
+    // because it's not guaranteed to be referentially stable.
+    // that would require caller to memoize the passed config object.
+    // using just the timelineName as a dependency is fine.
+  }, [addTimeline, timelineName]);
+
+  /**
+   * this effect starts or stops the animation
+   * based on the playhead state
+   */
+  useEffect(() => {
+    if (playHeadState === "playing") {
+      startAnimation();
+    }
+
+    if (playHeadState === "paused") {
+      cancelAnimation();
+    }
+
+    playHeadStateRef.current = playHeadState;
+  }, [playHeadState]);
+
+  /**
+   * this effect establishes a binding with externally
+   * updated frame number. Note that for this effect to have
+   * the required effect, the external setter needs to have disabled animation first
+   * by dispatching a pause event
+   */
+  useEffect(() => {
+    if (!isAnimationActiveRef.current) {
+      frameNumberRef.current = frameNumber;
+    }
+  }, [frameNumber]);
+
+  /**
+   * The following effects are used to keep the refs up to date
+   */
+  useEffect(() => {
+    configRef.current = config;
+  }, [config]);
+  useEffect(() => {
+    updateFreqRef.current = updateFreq;
+  }, [updateFreq]);
+
+  /**
+   * This effect is used to refresh the timeline when it's initialized.
+   */
   useEffect(() => {
     if (!isTimelineInitialized) {
       return;

app/packages/playback/src/views/Timeline.tsx

@@ -22,8 +22,8 @@ interface TimelineProps {
 /**
  * Renders a "classic" FO timeline with a seekbar, playhead, speed control, and status indicator.
  */
-export const Timeline = React.forwardRef<HTMLDivElement, TimelineProps>(
-  ({ name, style }, ref) => {
+export const Timeline = React.memo(
+  React.forwardRef<HTMLDivElement, TimelineProps>(({ name, style }, ref) => {
     const { playHeadState, config, play, pause } = useTimeline(name);
     const frameNumber = useFrameNumber(name);
 
@@ -73,5 +73,5 @@ export const Timeline = React.forwardRef<HTMLDivElement, TimelineProps>(
         </FoTimelineControlsContainer>
       </FoTimelineContainer>
     );
-  }
+  })
 );