Skip to content

fix: derive timelapse duration from compiled video #192

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.

Sign up for GitHub

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

Merged
ascpixi merged 3 commits into from
Apr 3, 2026
Merged

fix: derive timelapse duration from compiled video #192

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6438e21
fix: derive timelapse duration from compiled video
samdev-7 Apr 3, 2026
dc8e311
fix: derive timelapse duration from compiled video
samdev-7 Apr 3, 2026
257d682
fix: clarify realTimeDuration JSDoc, admin description, and add worke…
samdev-7 Apr 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions apps/server/src/job.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Queue, QueueEvents } from "bullmq";
import type { EditListEntry } from "@hackclub/lapse-api";
import { TIMELAPSE_FACTOR, type EditListEntry } from "@hackclub/lapse-api";
import { REALIZE_JOB_QUEUE_NAME, RealizeJobOutputsSchema, type RealizeJobInputs, type RealizeJobOutputs } from "@hackclub/lapse-jobs";

import { logError, logInfo, logWarning } from "@/logging.js";
Expand Down Expand Up @@ -39,9 +39,9 @@ realizeEvents.waitUntilReady()
.then(() => {
realizeEvents.on("completed", async ({ jobId, returnvalue }) => {
const result = RealizeJobOutputsSchema.parse(typeof returnvalue === "object" ? returnvalue : JSON.parse(returnvalue));
const { videoKey, thumbnailKey, timelapseId } = result;
const { videoKey, thumbnailKey, timelapseId, videoDuration } = result;

logInfo(`Timelapse ${timelapseId} finished processing! job=${jobId}`, { videoKey, thumbnailKey });
logInfo(`Timelapse ${timelapseId} finished processing! job=${jobId}`, { videoKey, thumbnailKey, videoDuration });

const draft = await database().draftTimelapse.findFirst({
where: {
Expand Down Expand Up @@ -83,7 +83,8 @@ realizeEvents.waitUntilReady()
data: {
associatedJobId: null,
s3Key: videoKey,
thumbnailS3Key: thumbnailKey
thumbnailS3Key: thumbnailKey,
...(videoDuration != null && { duration: videoDuration * TIMELAPSE_FACTOR })
},
Comment on lines 83 to 88
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

Duration is derived using a runtime TIMELAPSE_FACTOR import. Since the server and worker ship as separate images, a version-skew deploy could result in the worker encoding with one factor while the server multiplies by another, permanently storing the wrong duration. To make this robust, consider having the worker return the factor used (or the already-multiplied real-time duration) and use that value for the DB update.

Copilot uses AI. Check for mistakes.
include: { owner: true }
});
Expand Down
5 changes: 4 additions & 1 deletion apps/server/src/routers/admin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,13 +516,16 @@ export default os.router({
.use(requiredAuth("ADMIN"))
.use(requiredScopes("elevated"))
.handler(async () => {
// Only recalculate durations for timelapses that haven't been realized yet.
// Realized timelapses have their duration set from the compiled video, which is the source of truth.
const PAGE_SIZE = 100;
let updated = 0;
let cursor: string | undefined;

while (true) {
const batch = await database().timelapse.findMany({
select: { id: true, snapshots: true },
where: { s3Key: null },
Comment on lines +519 to +528
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

This endpoint now explicitly skips realized timelapses (where: { s3Key: null }), but the public admin contract/docs still describe it as “recalculates the duration of every timelapse from its snapshots”. Please update the API contract description (and/or rename/add params) so callers aren’t misled about what will be updated.

Copilot uses AI. Check for mistakes.
take: PAGE_SIZE,
orderBy: { id: "asc" },
...(cursor ? { skip: 1, cursor: { id: cursor } } : {})
Expand All @@ -549,7 +552,7 @@ export default os.router({
}
}

logInfo(`Recalculated durations for ${updated} timelapses.`);
logInfo(`Recalculated durations for ${updated} unrealized timelapses (skipped realized timelapses).`);

return apiOk({ updated });
}),
Expand Down
6 changes: 4 additions & 2 deletions apps/worker/src/workers/realize.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,8 @@ export const realizeJobWorker = new Worker<RealizeJobInputs, RealizeJobOutputs>(
]);

// Thumbnail generation - we opt for a simple approach where we just get the frame in the middle of the video.
const thumbnailTimestamp = (await measureVideoDuration(outputPath)) / 2;
const videoDuration = await measureVideoDuration(outputPath);
const thumbnailTimestamp = videoDuration / 2;

// Arguments to generate thumbnails regardless of output format
let thumbnailContentType = "image/avif";
Expand Down Expand Up @@ -344,7 +345,8 @@ export const realizeJobWorker = new Worker<RealizeJobInputs, RealizeJobOutputs>(
return {
timelapseId,
videoKey,
thumbnailKey
thumbnailKey,
videoDuration
};
}
finally {
Expand Down
8 changes: 7 additions & 1 deletion packages/jobs/src/realize.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,13 @@ export const RealizeJobOutputsSchema = z.object({
/**
* The S3 key for the thumbnail, stored in the public S3 bucket, shared by both the server and the worker.
*/
thumbnailKey: z.string()
thumbnailKey: z.string(),

/**
* The duration of the compiled output video in seconds, as measured by ffprobe.
* Optional for backwards compatibility with in-flight jobs that predate this field.
*/
videoDuration: z.number().nonnegative().optional()
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

videoDuration is validated as nonnegative(), which allows 0, but measureVideoDuration() throws for duration <= 0. Consider using positive() (or finite().positive()) here so the schema matches the actual invariant and rejects zero/NaN values.

Suggested change
videoDuration: z.number().nonnegative().optional()
videoDuration: z.number().finite().positive().optional()

Copilot uses AI. Check for mistakes.
});

/**
Expand Down
Loading