Skip to content

Conversation

@mattdawkins
Copy link
Member

No description provided.

@mattdawkins mattdawkins requested a review from BryonLewis January 8, 2026 00:04
Comment on lines +188 to +197
async function processBatches(
batches: number[][],
videoPath: string,
fps: number,
): Promise<void> {
if (batches.length === 0) return;
const [batch, ...remaining] = batches;
await Promise.all(batch.map((frame) => extractFrame(videoPath, frame, fps).catch(() => null)));
await processBatches(remaining, videoPath, fps);
}
Copy link
Collaborator

@BryonLewis BryonLewis Jan 8, 2026

Choose a reason for hiding this comment

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

I'd have to look into how this specific ffmpeg comand works (in extractFrame) but I'm guessing you probably wouldn't want to be spawning a new process for each frame because of the overhead. There should be an option to be able to a range of frames in a single process.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have a follow-up to address this with an optional setting, but it depends on code in the other branch landing first (the multi-camera option)

@BryonLewis BryonLewis marked this pull request as draft January 8, 2026 00:34
@mattdawkins mattdawkins force-pushed the add-native-option branch 2 times, most recently from e727497 to 73a7e8d Compare January 8, 2026 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants