Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

timeline fix: loadrange in imavid #4857

Merged
merged 11 commits into from
Sep 27, 2024
112 changes: 76 additions & 36 deletions app/packages/core/src/components/Modal/ImaVidLooker.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -54,6 +53,8 @@ export const ImaVidLookerReact = React.memo(
});

const { activeLookerRef, setActiveLookerRef } = useModalContext();
const imaVidLookerRef =
activeLookerRef as unknown as React.MutableRefObject<ImaVidLooker>;
sashankaryal marked this conversation as resolved.
Show resolved Hide resolved

const looker = React.useMemo(
() => createLooker.current(sampleDataWithExtraParams),
Expand Down Expand Up @@ -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();
Expand All @@ -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) {
Expand All @@ -200,6 +241,10 @@ export const ImaVidLookerReact = React.memo(
});
}, []);

const onAnimationStutter = useCallback(() => {
imaVidLookerRef.current?.element.checkFetchBufferManager();
}, []);

const {
isTimelineInitialized,
registerOnPauseCallback,
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the grid moving to react? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I definitely haven't thought about that one way or the other. 😅

// since imavid is part of the grid too
onAnimationStutter,
});

/**
Expand All @@ -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,
}));
},
});
}
Expand All @@ -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);
Expand Down
21 changes: 20 additions & 1 deletion app/packages/looker/src/elements/imavid/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 },
Expand Down Expand Up @@ -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`
Expand Down
44 changes: 28 additions & 16 deletions app/packages/looker/src/lookers/imavid/controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -165,7 +165,7 @@ export class ImaVidFramesController {
return this.config.page;
}

private get key() {
public get key() {
return this.config.key;
}

Expand Down Expand Up @@ -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,
})
);
});
}
},
});
Expand Down
2 changes: 1 addition & 1 deletion app/packages/playback/src/lib/constants.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
12 changes: 8 additions & 4 deletions app/packages/playback/src/lib/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) =>
Expand Down Expand Up @@ -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
Expand All @@ -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);
}
);

Expand Down
Loading
Loading