diff --git a/sidecar/index.ts b/sidecar/index.ts index cada431..df7f87e 100644 --- a/sidecar/index.ts +++ b/sidecar/index.ts @@ -6,6 +6,7 @@ import { ffprobe } from "./probe/ffprobe"; import { runSharp, type SharpRequest } from "./sharp/run"; import { runFfmpeg, type FfmpegRequest } from "./ffmpeg/run"; import { safeInputPath } from "./paths"; +import { needsConversion } from "./sharp/decodable"; import sharp from "sharp"; import { createSidecarConnector } from "@@/sidecar/connect"; import { API_URL } from "@@/officer-url.mjs"; @@ -254,6 +255,15 @@ async function route(request: Request): Promise { hasAlpha: meta.hasAlpha ?? false, pages: meta.pages ?? 1, orientation: meta.orientation ?? null, + /* + Reported, because it is a cost a caller can plan around rather than merely absorb. + + True means every operation on this file starts with an ImageMagick conversion — 8.5 s against + 1.1 s for the resize and encode after it. A caller making one derivative should ignore this. A + caller making seven should decode once itself and work from that, which is the difference + between 67 seconds and 16 for one phone photograph. + */ + needsConversion: needsConversion(meta.format ?? ""), }); } diff --git a/sidecar/sharp/decodable.ts b/sidecar/sharp/decodable.ts index 8b62250..b1a4cb3 100644 --- a/sidecar/sharp/decodable.ts +++ b/sidecar/sharp/decodable.ts @@ -24,6 +24,20 @@ import { unprocessable } from "../errors"; /** Formats sharp reports but cannot decode here. Extend only with evidence, not suspicion. */ const NEEDS_CONVERSION = new Set(["heif", "heic", "avif-sequence"]); +/** + * Does this format have to go through ImageMagick before sharp can read its pixels? + * + * Exported because it is expensive enough to plan around, not merely to handle. A caller building + * several derivatives of one file pays this conversion PER REQUEST — measured at 8.5 s against 1.1 s + * for the resize and encode that follow it — so a seven-rung ladder spends most of its time producing + * the same intermediate seven times. Knowing in advance lets that caller decode once itself. + * + * It is a function here rather than a copy of the set over there on purpose: which formats need this + * changes when the build changes, and a consumer holding its own list would keep paying for a + * conversion that had stopped being necessary. + */ +export const needsConversion = (format: string): boolean => NEEDS_CONVERSION.has(format.toLowerCase()); + export type Decodable = { path: string; /** Call when finished. Removes the intermediate, if there was one. */ diff --git a/sidecar/sharp/run.ts b/sidecar/sharp/run.ts index edc3d81..0f35fec 100644 --- a/sidecar/sharp/run.ts +++ b/sidecar/sharp/run.ts @@ -147,6 +147,28 @@ export async function runSharp(body: SharpRequest): Promise { throw invalid("output must be an object", "output"); } + /* + Unknown keys are refused, because the shape of this object is guessable and the wrong guess is + SILENT. + + `output: { webp: { quality: 82 } }` reads perfectly and mirrors how `pipeline` names an operation, + so it is what a caller writes from memory. Accepted quietly it meant: format fell through to the + "webp" default — right answer, wrong reason — and the options were never read, so the quality asked + for was not the quality encoded. Ask for `{ png: … }` the same way and you get a WebP with a .png + on the end of it. + + Every other part of this API rejects a name it does not know. This one did not, and the failure it + let through was the kind nobody finds by looking at the output. + */ + const known = new Set(["format", "options"]); + const stray = Object.keys(output).filter((key) => !known.has(key)); + if (stray.length) { + throw invalid( + `output has no ${stray.map((key) => JSON.stringify(key)).join(", ")}. It takes { "format": "webp", "options": { … } } — the format is a NAME, not a key.`, + "output", + ); + } + const format = output.format ?? "webp"; if (typeof format !== "string" || !OUTPUT_FORMATS[format]) { throw invalid(