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 2 commits
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
7 changes: 4 additions & 3 deletions apps/server/src/job.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
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, realTimeDuration } = result;

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

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,
...(realTimeDuration != null && { duration: realTimeDuration })
},
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,
realTimeDuration: videoDuration * TIMELAPSE_FACTOR
};
Comment on lines 347 to 352
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.

realTimeDuration is being returned as videoDuration * TIMELAPSE_FACTOR (i.e., already converted to the timelapse’s “real-time” duration). Given the PR description talks about returning the measured videoDuration and letting the server multiply by 60, it would be clearer to either (a) return the raw videoDuration and do the conversion server-side, or (b) rename this field to something that makes it obvious it’s already in timelapse-duration seconds (and optionally log/return both values).

Copilot uses AI. Check for mistakes.
}
finally {
Expand Down
2 changes: 1 addition & 1 deletion packages/api/src/contracts/admin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ export const adminRouterContract = {
})),

recalculateDurations: contract("POST", "/admin/recalculateDurations")
.route({ description: "Recalculates the duration of every timelapse from its snapshots. Requires administrator permissions and an `elevated` grant." })
.route({ description: "Recalculates the duration of unrealized timelapses (those still processing, without a compiled video) from their snapshots. Realized timelapses are skipped because their duration is derived from the compiled video. Requires administrator permissions and an `elevated` grant." })
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.

The updated route description says unrealized timelapses are “those still processing”, but the server-side filter is where: { s3Key: null }, which will also include timelapses that are no longer processing (e.g. FAILED_PROCESSING with associatedJobId: null). Consider tweaking the description to match the actual criterion (“no compiled video / s3Key is null”) rather than “still processing”.

Suggested change
.route({ description: "Recalculates the duration of unrealized timelapses (those still processing, without a compiled video) from their snapshots. Realized timelapses are skipped because their duration is derived from the compiled video. Requires administrator permissions and an `elevated` grant." })
.route({ description: "Recalculates the duration of unrealized timelapses (those without a compiled video, where `s3Key` is null) from their snapshots. Realized timelapses are skipped because their duration is derived from the compiled video. Requires administrator permissions and an `elevated` grant." })

Copilot uses AI. Check for mistakes.
.input(NO_INPUT)
.output(apiResult({
updated: z.number().int().nonnegative()
Expand Down
10 changes: 9 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,15 @@ 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 real-time duration of the timelapse in seconds (i.e. `videoDuration * TIMELAPSE_FACTOR`),
* as measured by ffprobe on the compiled output video. This value can be stored directly as the
* timelapse `duration` without further conversion.
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.

The JSDoc for realTimeDuration is a bit contradictory: ffprobe measures the compiled video’s duration (videoDuration), but realTimeDuration is a derived value after multiplying by TIMELAPSE_FACTOR. Consider updating the comment to explicitly say it’s derived from the ffprobe-measured duration (or alternatively rename/expose the raw videoDuration separately) to avoid confusing future consumers about units/source-of-truth.

Suggested change
* The real-time duration of the timelapse in seconds (i.e. `videoDuration * TIMELAPSE_FACTOR`),
* as measured by ffprobe on the compiled output video. This value can be stored directly as the
* timelapse `duration` without further conversion.
* The real-time duration of the timelapse in seconds, derived as
* `videoDuration * TIMELAPSE_FACTOR` from the compiled output video's duration measured by ffprobe.
* This value can be stored directly as the timelapse `duration` without further conversion.

Copilot uses AI. Check for mistakes.
* Optional for backwards compatibility with in-flight jobs that predate this field.
*/
realTimeDuration: z.number().gt(0).finite().optional()
});

/**
Expand Down
Loading