Repository navigation
fix(cli): media-use sends a picture on a machine with no ffmpeg - #7112
Conversation
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 blocker · 0 important · 1 nit · head 18f2d1f
The no-ffmpeg path works, and the fails without the fix: test is named and valid. One regression for machines that have ffmpeg but no usable sharp, inline.
- nit:
shrunkrunsneedFileon every picture in the request, including aphotothat the avatar route ignores when--avataris set. A stale--photopath next to--avatarnow fails with "no file", where before it was ignored.
Scope
Read the full diff (2 files). The image media() calls are references (image and video routes), firstFrame and photo (generate.ts:196,224,227,281), and picturesOf covers all of them, so pictures.get(path)! cannot miss. assetsOf never uploads a picture. The runOnce fake-clock loop ends when the run settles. CI was still running at review time.
| async function shrunk(paths: string[], work: string): Promise<Map<string, string>> { | ||
| const out = new Map<string, string>(); | ||
| if (!paths.length) return out; | ||
| const sharp = await loadSharp("media-use pictures"); |
There was a problem hiding this comment.
blocker: if sharp can't be loaded, every picture request fails, even when ffmpeg is installed. sharp is an optionalDependency, so npm skips it silently on platforms without a prebuilt binary and under --omit=optional. loadSharp then tries a runtime install, which rejects offline, on a failed install, or right away when HYPERFRAMES_NO_OPTIONAL_INSTALL is set (optionalPackages.ts:206). On main those machines sent pictures through ffmpeg. Now generate throws before building the body, which contradicts the body's "a machine with ffmpeg keeps every format it had". The fix is to treat a rejected loadSharp like an unreadable picture and use the existing ffmpeg resize for every path. Do the same in aspectOf (:543): if sharp is missing there, fall back to probeVideo on the prepared JPEG. A test that stubs loadSharp to reject while ffmpeg is present would pin this.
There was a problem hiding this comment.
Verdict: REQUEST CHANGES — 2 resolved, 0 open, 1 new · 18f2d1f → 174bdef. Both fixes are correct: sharpOrNone falls back to ffmpeg in shrunk and to probeVideo in aspectOf, and picturesOf now reads the route's own pure body. New: the Comments check fails on this head. generate.ts comment share went from 6.5% (49/750) to 6.6% (51/775), 2 lines over. Cut 2 comment lines, for example the new one in shrunk, and this is an approve.
What
media-use generatesends a picture (a reference, a first frame or an avatar photo) on a machine with no ffmpeg. Before, every picture went through ffmpeg to fit it in 2048 px, so on such a machine the step stopped with "ffmpeg is not installed" before anything was sent.Why
A picture needs no video tool. Agents on fresh Windows machines run
media-use generatewith a reference picture and hit the ffmpeg error as their first failure. Sound and motion clips still need ffmpeg and still ask for it.Related work
None.
How
generateprepares the pictures before building the body: sharp turns each upright (EXIF orientation), fits it in 2048 px and writes a JPEG. sharp is the CLI's optional dependency, loaded throughloadSharp, which installs it once on first use.--avatarwith a stale--photostill ignores the photo.HYPERFRAMES_NO_OPTIONAL_INSTALL), every picture goes through the same ffmpeg resize as before, and so does a picture sharp cannot read (its own build has no HEIC).aspectOfreads the first frame's size from the prepared picture: with sharp, or with ffprobe when sharp is missing.No visible change
CLI behavior only, no UI.
Test plan
generate.test.ts› "sends a picture with no ffmpeg on the machine, shrunk to 2048 px":HYPERFRAMES_FFMPEG_PATHpoints at no file, a 3000x1000 PNG reference goes out as a 2048x683 JPEG. fails without the fix: on main it throws "ffmpeg is not installed".generate.test.ts› "sends a picture sharp cannot read through ffmpeg": a BMP reference goes out as a JPEG of its own size.generate.test.ts› "sends pictures through ffmpeg when sharp cannot be loaded":loadSharprejects, a first frame still goes out and the result is cropped to its shape (probed with ffprobe). Fails at 18f2d1f.generate.test.ts› "prepares only the pictures the route sends: --avatar leaves a stale --photo alone". Fails at 18f2d1f ("no file at …").runOncein the pickVoice re-run tests now moves the fake clock until the run ends instead of in one 31-minute jump: the avatar's photo is now read asynchronously before the first poll, so a single jump landed before polling started and the test timed out.generate.test.ts54 of 54, three runs in a row; with main'sgenerate.ts, only the no-ffmpeg test fails.generate.tsadds no comment lines over its base share (the Comments check).Size
Two files, about 100 lines with the tests.