From 63c49e56608b642198cce0e8fe57d058ed80c265 Mon Sep 17 00:00:00 2001 From: Sachin Sharma Date: Sun, 27 Sep 2026 01:53:26 +0530 Subject: [PATCH] feat(video): honour videoOptions end to end, timestamp keyframes, transcribe speech Every knob under `videoOptions` was unreachable. `NeuroLink.generate()` now forwards it into `TextGenerationOptions` (#1720), but `core/modules/MessageBuilder` rebuilds the same options twice more, and `baseProvider`'s real-stream to fake-stream fallback once, and all three dropped the field. So `--video-frames 2` on a four-second clip produced the tier default of four frames, `--video-quality` and `--video-format` never reached the encoder, and `--transcribe-audio` reached no code at all. The plumbing at the far end has been correct since #478 -- the message builder hands videoOptions to the detector, which hands it to the processor. Nothing ever arrived. The failure is invisible from a passing generation, because the model answers either way; it shows up only if you count the frames. `videoOptions` is now forwarded at those three sites. Its inline declarations on `GenerateOptions`, `TextGenerationOptions` and `StreamOptions` collapse onto `VideoProcessorOptions`, which is what they were already assignable to -- they had begun to drift in what they documented, and one still described transcription as unimplemented. Keyframes now carry the moment they were sampled at (#460). Extraction pairs each frame with its timestamp as the frame is kept rather than deriving the schedule afterwards, because an individual frame whose encode fails is skipped and a reconstructed schedule mislabels every frame after it. The timestamps reach the model twice: as a list in the video's text block, and as per-frame alt text, which the message builder folds into the prompt. The text block's old "extracted every ~Ns" line was also wrong whenever a caller passed `frames`: it printed the duration tier's interval while extraction had used duration/budget. A 3s clip asked for 16 frames was described as "every ~1s" with the frames 0.19s apart. Timestamps are formatted as explicit units, not a clock, for the reason mediaDuration's header already gives -- and this is not hypothetical: an earlier draft labelled frames "0:03" and the model reported the green frame as appearing at "3:00". `extractAndTranscribeAudio` (#433) is implemented rather than stubbed. The issue asked for a stub on the grounds that no transcription backend existed; one does -- AudioProcessor has shipped Whisper transcription for standalone audio for some time -- so a method logging "not yet implemented" would be dead code beside a working implementation of the same thing. It matters for the frame-extraction providers specifically: they cannot hear a clip at all, so a recorded standup arrives as four screenshots. Off by default, and best-effort throughout: no audio track, no ffmpeg, no key, an oversized track or a failed call each return a distinct reason and leave the rest of the pipeline intact. The Whisper call is reproduced rather than shared with AudioProcessor: that file is being edited concurrently, and a merge conflict in the audio pipeline is worse than fifty duplicated lines. Worth removing once both land. Verified live on a committed 14KB clip carrying a spoken word: `--video-frames 1` now yields one frame where the default yields four; OpenAI, which cannot hear video, reports the spoken word with --transcribe-audio and NO_AUDIO without it; and with no transcription backend the skip is reported by name rather than presenting as a silent clip. Failure injection on three assertions confirmed each reports a failure and exits non-zero, not a skip. `multimodalOptionsBuilder`, used only by the Amazon Bedrock provider, had the same drop as the other four sites: it whitelisted `csvOptions`, `pdfOptions` and `imageOptions` but omitted `videoOptions`, so `--video-frames`/`--video-quality`/`--video-format`/`--transcribe-audio` never reached `VideoProcessor` on the Bedrock branch even though the video file itself did. Added `videoOptions: options.videoOptions` to that whitelist so the Bedrock branch carries the same options as the other four sites. Covered by a new test, "the frame budget reaches the processor on the Bedrock branch too", in test/continuous-test-suite-video-frames.ts, which asserts an explicit `--video-frames 1` budget reaches the processor on the Bedrock branch by counting extracted frames from the debug log. The suite's per-test budget is 540 s, so its longest test (two sequential 240 s CLI calls) is never cut short and reported as a skip. The missing-backend transcription test also asserts the CLI exited 0, since transcription is additive and the request itself must succeed. Keyframe timestamps now come from ffmpeg's actual per-frame sample times instead of the idealized `duration / frames` schedule used to build the request: `runFfmpegFrameExtraction` adds a `showinfo` stage to the same filter chain and reads each selected frame's real `pts_time` from stderr, pairing every keyframe with its true sample time and falling back to the idealized value only when ffmpeg reports fewer real timestamps than frames -- the `-vf select` filter only guarantees a minimum gap since the last frame it actually selected, so a source fps lower than the requested density otherwise mislabels every later frame by a compounding amount, up to roughly half the clip's length on a low-fps source asked for a dense schedule. Covered by the new "keyframe timestamps reflect ffmpeg's real sample times, not the ideal schedule" test, which forces the mismatch with a 2fps fixture asked for 16 frames. The no-transcription frame test now also asserts the CLI exited zero and that the answer matches the requested `NO_AUDIO` reply exactly, rather than only checking the spoken word was absent, which a crashed or empty generation satisfied just as well; `answerOnly` strips the `Debug Information` footer's own un-prefixed header line (logged as an embedded newline rather than a new timestamped line) so it can no longer survive into that comparison. `buildTextContent`'s JSDoc now documents its real parameters instead of a removed one. docs/features/multimodal.md described `--transcribe-audio` as an accepted no-op waiting on this change; it now describes the Whisper path, what it needs, and the `No transcript for` warning that names why a transcript was not produced. Closes #433, #460 --- docs-site/static/search-index.json | 4 +- docs/api/README.md | 1 + docs/api/type-aliases/FileProcessingResult.md | 9 +- docs/api/type-aliases/GenerateOptions.md | 29 +- docs/api/type-aliases/ProcessedVideo.md | 38 ++ docs/api/type-aliases/StreamOptions.md | 44 +- .../api/type-aliases/TextGenerationOptions.md | 48 +- docs/api/type-aliases/VideoKeyframe.md | 30 ++ .../api/type-aliases/VideoProcessorOptions.md | 14 + docs/features/multimodal.md | 37 +- package.json | 3 +- src/cli/factories/commandFactory.ts | 4 +- src/lib/core/baseProvider.ts | 1 + src/lib/core/modules/MessageBuilder.ts | 8 + src/lib/processors/media/VideoProcessor.ts | 358 +++++++++++++- src/lib/types/file.ts | 23 +- src/lib/types/generate.ts | 48 +- src/lib/types/processor.ts | 44 ++ src/lib/types/stream.ts | 24 +- src/lib/utils/fileDetector.ts | 54 ++- src/lib/utils/mediaDuration.ts | 24 + src/lib/utils/messageBuilder.ts | 25 +- src/lib/utils/multimodalOptionsBuilder.ts | 7 + test/continuous-test-suite-video-frames.ts | 459 ++++++++++++++++++ 24 files changed, 1133 insertions(+), 203 deletions(-) create mode 100644 docs/api/type-aliases/VideoKeyframe.md create mode 100644 test/continuous-test-suite-video-frames.ts diff --git a/docs-site/static/search-index.json b/docs-site/static/search-index.json index 20cf4d6483..0c5ec499cc 100644 --- a/docs-site/static/search-index.json +++ b/docs-site/static/search-index.json @@ -3899,10 +3899,10 @@ {"objectID":"2c4c2bb10b6fea62d177994e6df1345d05606b65f8122b3c969cf214df3363f1","title":"CLI usage","url":"/docs/features/multimodal#cli-usage","content":"`bash","hierarchy":{"lvl0":"Features","lvl1":"Multimodal Capabilities Guide","lvl2":"CLI usage","lvl3":""}}, {"objectID":"915a9e9701a9e6b02890e744550ed66754549288fa044c90bd2dd873e6b48c2a","title":"Auto-detect","url":"/docs/features/multimodal#auto-detect","content":"neurolink generate \"Describe this video\" --file demo.mp4 --provider google-ai","hierarchy":{"lvl0":"Features","lvl1":"Multimodal Capabilities Guide","lvl2":"Auto-detect","lvl3":""}}, {"objectID":"12ce623ba884b34886674b24daff6ce9e58465c68dc25f4f66c01229c6855513","title":"Explicit flag","url":"/docs/features/multimodal#explicit-flag","content":"neurolink generate \"Summarise this\" --video standup.mp4 --provider vertex","hierarchy":{"lvl0":"Features","lvl1":"Multimodal Capabilities Guide","lvl2":"Explicit flag","lvl3":""}}, -{"objectID":"08774b3c1d8fd456cb7d64ad98281449b9271854310a9a713048ae2112572d5c","title":"Frame-extraction knobs","url":"/docs/features/multimodal#frame-extraction-knobs","content":"neurolink generate \"Describe each scene\" --file demo.mp4 \\\n --provider openai --video-frames 16 --video-quality 90 --video-format jpeg\n--transcribe-audiovideoOptions.transcribeAudiotranscribeAudio`.","hierarchy":{"lvl0":"Features","lvl1":"Multimodal Capabilities Guide","lvl2":"Frame-extraction knobs","lvl3":""}}, +{"objectID":"08774b3c1d8fd456cb7d64ad98281449b9271854310a9a713048ae2112572d5c","title":"Frame-extraction knobs","url":"/docs/features/multimodal#frame-extraction-knobs","content":"neurolink generate \"Describe each scene\" --file demo.mp4 \\\n --provider openai --video-frames 16 --video-quality 90 --video-format jpeg\n--transcribe-audiovideoOptions.transcribeAudiowhisper-1OPENAIAPIKEYOPENAIBASEURLSpoken Audio (transcribed)OPENAIAPIKEYNo transcript for : --transcribe-audiotranscribeAudio`.","hierarchy":{"lvl0":"Features","lvl1":"Multimodal Capabilities Guide","lvl2":"Frame-extraction knobs","lvl3":""}}, {"objectID":"d0faa964a225271bc41d6c279e3e925829ac8e8828518387c39ba60d97cd363f","title":"Asking which mechanism applies","url":"/docs/features/multimodal#asking-which-mechanism-applies","content":"The capability table is exported, so a caller can ask before sending — useful\nfor choosing a provider, or for budgeting.","hierarchy":{"lvl0":"Features","lvl1":"Multimodal Capabilities Guide","lvl2":"Asking which mechanism applies","lvl3":""}}, {"objectID":"10a8f5bf0ea5129e242eb09d5db285a4d91aac091c171050471ebd7eb528cb72","title":"Best practices","url":"/docs/features/multimodal#best-practices","content":"Ask an audio question of a Gemini provider only. Nothing on the frame\n path can hear the clip, so \"what was said\" has no answer there.\nKeep clips under 14 MB when you want native delivery. Re-encoding a\n screen recording at a lower bitrate usually gets a long clip under the\n ceiling without losing anything a model would read.\nRaise rather than for detail over time. Frames cost\n roughly 250 tokens each regardless of quality; more of them is what buys\n coverage of a clip where things change.\nLower for local runtimes. Ollama and llama.cpp hold every\n frame in the same memory as the model; four is the practical default.","hierarchy":{"lvl0":"Features","lvl1":"Multimodal Capabilities Guide","lvl2":"Best practices","lvl3":""}}, -{"objectID":"7bd6ebb59bff22b19f26c8c358eb7012f8a73d4f90eed575ca136f632c6f9bc6","title":"Troubleshooting","url":"/docs/features/multimodal#troubleshooting","content":"| Symptom | Cause | Fix |\n| --------------------------------------------------- | ----------------------------------------------------------------------- | ----------------------------------------------------------------------- |\n| Model describes the file but not its content | No keyframes extracted — ffmpeg missing — and the provider takes frames | Install ffmpeg, or use a Gemini provider, which needs none |\n| in the logs | The clip failed the inline gate; the reason is on the same line | Shorten or re-encode it, or accept the frames |\n| Model cannot hear speech on a Gemini provider | The clip exceeded the inline ceiling and fell back to frames | Get the source under 14 MB |\n| produced no transcript | Not implemented yet (#433); the flag is a no-op that only warns | None yet — track #1757 |\n| Only the first seconds of a long clip are described | The frame budget was hit before the end | Raise ; the interval then spreads evenly across the whole clip |","hierarchy":{"lvl0":"Features","lvl1":"Multimodal Capabilities Guide","lvl2":"Troubleshooting","lvl3":""}}, +{"objectID":"7bd6ebb59bff22b19f26c8c358eb7012f8a73d4f90eed575ca136f632c6f9bc6","title":"Troubleshooting","url":"/docs/features/multimodal#troubleshooting","content":"| Symptom | Cause | Fix |\n| --------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------- |\n| Model describes the file but not its content | No keyframes extracted — ffmpeg missing — and the provider takes frames | Install ffmpeg, or use a Gemini provider, which needs none |\n| in the logs | The clip failed the inline gate; the reason is on the same line | Shorten or re-encode it, or accept the frames |\n| Model cannot hear speech on a Gemini provider | The clip exceeded the inline ceiling and fell back to frames | Get the source under 14 MB, or pass |\n| produced no transcript | The warning names the reason: no audio track, unset, or audio over Whisper's 25 MB limit | Set , or split the recording |\n| Only the first seconds of a long clip are described | The frame budget was hit before the end | Raise ; the interval then spreads evenly across the whole clip |","hierarchy":{"lvl0":"Features","lvl1":"Multimodal Capabilities Guide","lvl2":"Troubleshooting","lvl3":""}}, {"objectID":"79318378a7df7cc816465eb447fa4bd2ba9e6344f17b0a9cf6661377bb03e7e2","title":"Combining Multiple Input Types","url":"/docs/features/multimodal#combining-multiple-input-types","content":"NeuroLink excels at combining different media types in a single request.","hierarchy":{"lvl0":"Features","lvl1":"Multimodal Capabilities Guide","lvl2":"Combining Multiple Input Types","lvl3":""}}, {"objectID":"412f09080a5801a35250df393e2e0694602d5312e2b3d1b017fe469bf9dc0474","title":"Batch with Multimodal (CLI)","url":"/docs/features/multimodal#batch-with-multimodal-cli","content":"The command supports , , , and . The file(s) are attached identically to every prompt in the batch (a one-line notice is printed to stderr, unconditionally — it is not suppressed by , so it never corrupts output written to stdout):\n\n(auto-detect) is not available in , because it collides with the positional (the prompts-list path). Use the explicit / / / flags instead.","hierarchy":{"lvl0":"Features","lvl1":"Multimodal Capabilities Guide","lvl2":"Batch with Multimodal (CLI)","lvl3":""}}, {"objectID":"66d907236327d676dbe6e05193c2793f7c5990dce2ad113cf078eb1ff4c73216","title":"File validation & troubleshooting (CLI)","url":"/docs/features/multimodal#file-validation-troubleshooting-cli","content":"Before any provider call, the CLI validates local file inputs across , , and :\nA path that points at a directory, doesn't exist, or can't be read (e.g. a permissions error) is rejected up front with a clear error and troubleshooting hints — no cryptic / deep in processing. This also applies to the prompts-list positional itself.\nA large file (images > 10 MB, CSV/PDF > 50 MB) prints a non-blocking warning to stderr. Like the batch attachment notice, this is unconditional — visible without and regardless of — and never mixes into stdout, so output stays valid JSON even when large-file warnings fire.\n\nThis runs even under and without API keys configured.","hierarchy":{"lvl0":"Features","lvl1":"Multimodal Capabilities Guide","lvl2":"File validation & troubleshooting (CLI)","lvl3":""}}, diff --git a/docs/api/README.md b/docs/api/README.md index 4801f95d9a..7ccceb97a5 100644 --- a/docs/api/README.md +++ b/docs/api/README.md @@ -1935,6 +1935,7 @@ console.log(result.content); - [JsonTypeGuard](type-aliases/JsonTypeGuard.md) - [ProcessorRegistration](type-aliases/ProcessorRegistration.md) - [ProcessedAudio](type-aliases/ProcessedAudio.md) +- [VideoKeyframe](type-aliases/VideoKeyframe.md) - [ProcessedVideo](type-aliases/ProcessedVideo.md) - [ArchiveFormat](type-aliases/ArchiveFormat.md) - [ArchiveDecompressionResult](type-aliases/ArchiveDecompressionResult.md) diff --git a/docs/api/type-aliases/FileProcessingResult.md b/docs/api/type-aliases/FileProcessingResult.md index 7834fd8d0f..2930075e24 100644 --- a/docs/api/type-aliases/FileProcessingResult.md +++ b/docs/api/type-aliases/FileProcessingResult.md @@ -32,9 +32,14 @@ File processing result after detection and conversion ### images? -> `optional` **images?**: (`Buffer` \| `string`)[] +> `optional` **images?**: (`Buffer` \| `string` \| [`ImageWithAltText`](ImageWithAltText.md))[] -Additional images extracted from the file (e.g., video keyframes, audio cover art) +Additional images extracted from the file (e.g., video keyframes, audio +cover art). + +An entry may carry alt text, which is how a video keyframe states the +timestamp it was sampled at. Bare bytes still work — the field was +widened, not changed. --- diff --git a/docs/api/type-aliases/GenerateOptions.md b/docs/api/type-aliases/GenerateOptions.md index 9e08d67676..b463f8ff0e 100644 --- a/docs/api/type-aliases/GenerateOptions.md +++ b/docs/api/type-aliases/GenerateOptions.md @@ -249,31 +249,14 @@ module's own default, unchanged unless a caller opts in here. ### videoOptions? -> `optional` **videoOptions?**: `object` +> `optional` **videoOptions?**: [`VideoProcessorOptions`](VideoProcessorOptions.md) -#### frames? +Video processing options — keyframe budget, encoder settings, and +whether to transcribe the clip's spoken audio. -> `optional` **frames?**: `number` - -Frames to extract. Unset lets VideoProcessor pick from the clip's duration; clamped to 100. - -#### quality? - -> `optional` **quality?**: `number` - -Frame encoder quality, clamped to 1-100. Default 80. - -#### format? - -> `optional` **format?**: `"jpeg"` \| `"png"` - -Frame encoding. Default jpeg. - -#### transcribeAudio? - -> `optional` **transcribeAudio?**: `boolean` - -Not implemented yet (#433) — warns rather than silently doing nothing. +The canonical shape, rather than a third structurally-identical copy of +it: `StreamOptions` and `TextGenerationOptions` declare the same field, +and the inline copies had already drifted apart in what they documented. --- diff --git a/docs/api/type-aliases/ProcessedVideo.md b/docs/api/type-aliases/ProcessedVideo.md index 359419502c..a151fc667e 100644 --- a/docs/api/type-aliases/ProcessedVideo.md +++ b/docs/api/type-aliases/ProcessedVideo.md @@ -23,6 +23,21 @@ suitable for sending to an LLM. > **keyframes**: `Buffer`[] +### keyframeTimestampsSec + +> **keyframeTimestampsSec**: `number`[] + +When each entry of `keyframes` was sampled, in seconds from the start of +the clip. Same length, same order, ascending. + +A parallel array rather than a richer `keyframes` element type: that +field is read by `fileReferenceRegistry` and by the detector as a plain +`Buffer[]`, and reshaping it would break them for a value they do not +need. Extraction can also drop an individual frame when its encode +fails, so the timestamps are recorded as frames are kept — never +reconstructed from an interval, which would silently mislabel every +frame after a dropped one. + ### metadata > **metadata**: `object` @@ -79,6 +94,29 @@ suitable for sending to an LLM. > `optional` **subtitleText?**: `string` +### transcript? + +> `optional` **transcript?**: `string` + +Speech transcribed from the clip's audio track, when +`VideoProcessorOptions.transcribeAudio` asked for it and it worked. + +Distinct from `subtitleText`, which is an embedded subtitle stream the +file already carried. A recording can have one, both or neither. + +### hasTranscript + +> **hasTranscript**: `boolean` + +### transcriptionSkippedReason? + +> `optional` **transcriptionSkippedReason?**: `string` + +Why no transcript was produced. Follows `ProcessedAudio`'s precedent: +"nobody asked", "there is no audio track", "no API key", "the call +failed" and "nobody was speaking" are five different situations that +otherwise all present as an absent transcript. + ### hasKeyframes > **hasKeyframes**: `boolean` diff --git a/docs/api/type-aliases/StreamOptions.md b/docs/api/type-aliases/StreamOptions.md index b1c2998088..dfa51c4a5d 100644 --- a/docs/api/type-aliases/StreamOptions.md +++ b/docs/api/type-aliases/StreamOptions.md @@ -138,6 +138,20 @@ images: [ --- +### videoOptions? + +> `optional` **videoOptions?**: [`VideoProcessorOptions`](VideoProcessorOptions.md) + +Video processing options (#478, #433). + +Reconstructed option objects have to carry this the same way they carry +`csvOptions` and `pdfOptions`: the message builder hands it to the +detector, which hands it to `VideoProcessor`, and anything that rebuilds +the options along the way and omits it restores the defaults without +saying so. + +--- + ### pdfOptions? > `optional` **pdfOptions?**: `object` @@ -190,36 +204,6 @@ module's own default, unchanged unless a caller opts in here. --- -### videoOptions? - -> `optional` **videoOptions?**: `object` - -#### frames? - -> `optional` **frames?**: `number` - -Frames to extract. Unset lets VideoProcessor pick from the clip's duration; clamped to 100. - -#### quality? - -> `optional` **quality?**: `number` - -Frame encoder quality, clamped to 1-100. Default 80. - -#### format? - -> `optional` **format?**: `"jpeg"` \| `"png"` - -Frame encoding. Default jpeg. - -#### transcribeAudio? - -> `optional` **transcribeAudio?**: `boolean` - -Not implemented yet (#433) — warns rather than silently doing nothing. - ---- - ### tts? > `optional` **tts?**: [`TTSOptions`](TTSOptions.md) diff --git a/docs/api/type-aliases/TextGenerationOptions.md b/docs/api/type-aliases/TextGenerationOptions.md index a9d5efabcf..8d79b2ac78 100644 --- a/docs/api/type-aliases/TextGenerationOptions.md +++ b/docs/api/type-aliases/TextGenerationOptions.md @@ -605,6 +605,20 @@ const result = await neurolink.generate({ --- +### videoOptions? + +> `optional` **videoOptions?**: [`VideoProcessorOptions`](VideoProcessorOptions.md) + +Video processing options (#478, #433). + +Reconstructed option objects have to carry this the same way they carry +`csvOptions` and `pdfOptions`: the message builder hands it to the +detector, which hands it to `VideoProcessor`, and anything that rebuilds +the options along the way and omits it restores the defaults without +saying so. + +--- + ### pdfOptions? > `optional` **pdfOptions?**: `object` @@ -656,40 +670,6 @@ module's own default, unchanged unless a caller opts in here. --- -### videoOptions? - -> `optional` **videoOptions?**: `object` - -Video processing options (mirrors `GenerateOptions.videoOptions`). Never -declared here before, so `buildGenerateTextOptions` had nowhere to -forward the caller's setting even once the allowlist itself named it. - -#### frames? - -> `optional` **frames?**: `number` - -Frames to extract. Unset lets VideoProcessor pick from the clip's duration; clamped to 100. - -#### quality? - -> `optional` **quality?**: `number` - -Frame encoder quality, clamped to 1-100. Default 80. - -#### format? - -> `optional` **format?**: `"jpeg"` \| `"png"` - -Frame encoding. Default jpeg. - -#### transcribeAudio? - -> `optional` **transcribeAudio?**: `boolean` - -Not implemented yet (#433) — warns rather than silently doing nothing. - ---- - ### enableSummarization? > `optional` **enableSummarization?**: `boolean` diff --git a/docs/api/type-aliases/VideoKeyframe.md b/docs/api/type-aliases/VideoKeyframe.md new file mode 100644 index 0000000000..79bc499d3c --- /dev/null +++ b/docs/api/type-aliases/VideoKeyframe.md @@ -0,0 +1,30 @@ +[**NeuroLink API Reference**](../README.md) + +--- + +[NeuroLink API Reference](../README.md) / VideoKeyframe + +# Type Alias: VideoKeyframe + +> **VideoKeyframe** = `object` + +One extracted keyframe and the moment it came from. + +Internal to extraction: `ProcessedVideo` splits this back into the parallel +`keyframes` / `keyframeTimestampsSec` arrays its existing consumers expect. +Keeping the pair together while frames are being read and encoded is what +makes a dropped frame impossible to mislabel — the alternative is +reconstructing timestamps from an interval after the fact, which is wrong +for every frame following a failed encode. + +## Properties + +### buffer + +> `readonly` **buffer**: `Buffer` + +--- + +### timestampSec + +> `readonly` **timestampSec**: `number` diff --git a/docs/api/type-aliases/VideoProcessorOptions.md b/docs/api/type-aliases/VideoProcessorOptions.md index 53b9bec4c0..d9da4b8187 100644 --- a/docs/api/type-aliases/VideoProcessorOptions.md +++ b/docs/api/type-aliases/VideoProcessorOptions.md @@ -37,3 +37,17 @@ Encoder quality 1-100 for the extracted frames. > `optional` **format?**: `"jpeg"` \| `"png"` Frame encoding. Defaults to jpeg. + +--- + +### transcribeAudio? + +> `optional` **transcribeAudio?**: `boolean` + +Transcribe the clip's spoken audio (#433). Off by default: it costs an +ffmpeg pass plus a Whisper call, and most attached video is silent +screen capture. + +Best-effort — a clip with no audio track, no `OPENAI_API_KEY`, or a +failed call still processes, and the reason lands in +`ProcessedVideo.transcriptionSkippedReason`. diff --git a/docs/features/multimodal.md b/docs/features/multimodal.md index 0b3d7e33d3..7783b59ff7 100644 --- a/docs/features/multimodal.md +++ b/docs/features/multimodal.md @@ -478,8 +478,8 @@ const result = await neurolink.generate({ frames: 16, // Keyframe budget. Clamped to the processor ceiling of 100. quality: 90, // Encoder quality 1-100. Default 80. format: "jpeg", // "jpeg" | "png". Default jpeg. - // Not implemented yet (#433) — accepted but a no-op, warns once per - // request instead of silently doing nothing. #1757 implements it. + // Transcribe the clip's speech with OpenAI Whisper and include it as + // text. Needs ffmpeg and OPENAI_API_KEY; see below. transcribeAudio: true, }, }); @@ -503,13 +503,18 @@ neurolink generate "Describe each scene" --file demo.mp4 \ --provider openai --video-frames 16 --video-quality 90 --video-format jpeg ``` -`--transcribe-audio` is accepted on the CLI and by `videoOptions.transcribeAudio` -for frame-path providers, but transcription itself is **not implemented yet** -(tracked as #433, implemented in [#1757](https://github.com/juspay/neurolink/pull/1757)). -Passing it today logs a warning and is otherwise a no-op — keyframes and any -embedded subtitle tracks still extract normally, but no spoken-audio transcript -is produced. It would not help Gemini anyway: native delivery already includes -the clip's audio track. +`--transcribe-audio` (or `videoOptions.transcribeAudio`) gives a frame-path +provider the clip's speech, which keyframes cannot carry. The audio track is +extracted with ffmpeg, transcribed by OpenAI Whisper (`whisper-1`, using +`OPENAI_API_KEY`, and `OPENAI_BASE_URL` when set), and sent alongside the +keyframes under a `Spoken Audio (transcribed)` heading. Transcription is +additive: when it cannot run — the clip has no audio track, `OPENAI_API_KEY` is +unset, the extracted audio is over Whisper's 25 MB limit, or extraction or the +upload fails — the request still succeeds with the keyframes and metadata, and a +`No transcript for : ` warning says which. A Gemini provider does +not need it while the clip is delivered natively, since that includes the audio +track; a clip that falls back to keyframes (over the inline ceiling) loses its +speech unless `--transcribe-audio` is set. Embedded **subtitle tracks** are extracted separately and included whenever they exist, with or without `transcribeAudio`. @@ -550,13 +555,13 @@ estimateVideoTokens({ provider: "openai", durationSec: 60, frameCount: 8 }); // ### Troubleshooting -| Symptom | Cause | Fix | -| --------------------------------------------------- | ----------------------------------------------------------------------- | ----------------------------------------------------------------------- | -| Model describes the file but not its content | No keyframes extracted — ffmpeg missing — and the provider takes frames | Install ffmpeg, or use a Gemini provider, which needs none | -| `Sending keyframes instead of the clip` in the logs | The clip failed the inline gate; the reason is on the same line | Shorten or re-encode it, or accept the frames | -| Model cannot hear speech on a Gemini provider | The clip exceeded the inline ceiling and fell back to frames | Get the source under 14 MB | -| `--transcribe-audio` produced no transcript | Not implemented yet (#433); the flag is a no-op that only warns | None yet — track [#1757](https://github.com/juspay/neurolink/pull/1757) | -| Only the first seconds of a long clip are described | The frame budget was hit before the end | Raise `frames`; the interval then spreads evenly across the whole clip | +| Symptom | Cause | Fix | +| --------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------- | +| Model describes the file but not its content | No keyframes extracted — ffmpeg missing — and the provider takes frames | Install ffmpeg, or use a Gemini provider, which needs none | +| `Sending keyframes instead of the clip` in the logs | The clip failed the inline gate; the reason is on the same line | Shorten or re-encode it, or accept the frames | +| Model cannot hear speech on a Gemini provider | The clip exceeded the inline ceiling and fell back to frames | Get the source under 14 MB, or pass `--transcribe-audio` | +| `--transcribe-audio` produced no transcript | The `No transcript for` warning names the reason: no audio track, `OPENAI_API_KEY` unset, or audio over Whisper's 25 MB limit | Set `OPENAI_API_KEY`, or split the recording | +| Only the first seconds of a long clip are described | The frame budget was hit before the end | Raise `frames`; the interval then spreads evenly across the whole clip | --- diff --git a/package.json b/package.json index 8d883a1324..cc3c435f5c 100644 --- a/package.json +++ b/package.json @@ -248,8 +248,9 @@ "pre-push": "pnpm run check:deps && pnpm run build && pnpm run test:provider-structure && pnpm run test:model-manifests", "check:all": "pnpm run lint && pnpm run format --check && pnpm run validate && pnpm run validate:commit", "test:file-formats": "pnpm exec tsx test/continuous-test-suite-file-formats.ts", - "test:multimodal": "pnpm run test:file-formats && pnpm run test:multimodal:sdk && pnpm run test:video-native", + "test:multimodal": "pnpm run test:file-formats && pnpm run test:multimodal:sdk && pnpm run test:video-native && pnpm run test:video-frames", "test:multimodal:sdk": "pnpm exec tsx test/continuous-test-suite-multimodal-sdk.ts", + "test:video-frames": "pnpm exec tsx test/continuous-test-suite-video-frames.ts", "test:video-native": "pnpm exec tsx test/continuous-test-suite-video-native.ts", "test:mcp-result-cache": "pnpm exec tsx test/continuous-test-suite-mcp-result-cache.ts", "test:mcp-breaker-resolved-errors": "pnpm exec tsx test/continuous-test-suite-mcp-breaker-resolved-errors.ts", diff --git a/src/cli/factories/commandFactory.ts b/src/cli/factories/commandFactory.ts index 534f3bc8e7..09fa511ec2 100644 --- a/src/cli/factories/commandFactory.ts +++ b/src/cli/factories/commandFactory.ts @@ -221,7 +221,9 @@ export class CLICommandFactory { "transcribe-audio": { type: "boolean" as const, default: false, - description: "Extract and transcribe audio from video", + description: + "Transcribe the video's spoken audio (needs ffmpeg and OPENAI_API_KEY). " + + "Unnecessary for Gemini, which hears the clip directly", }, file: { type: "string" as const, diff --git a/src/lib/core/baseProvider.ts b/src/lib/core/baseProvider.ts index d2682195d0..bfc8404e2f 100644 --- a/src/lib/core/baseProvider.ts +++ b/src/lib/core/baseProvider.ts @@ -1248,6 +1248,7 @@ export abstract class BaseProvider implements AIProvider { context: options.context as Record | undefined, csvOptions: options.csvOptions, pdfOptions: options.pdfOptions, + videoOptions: options.videoOptions, // Forward abort, tool filtering, and timeout options to prevent // silent bypass when falling back from real streaming to fake streaming abortSignal: options.abortSignal, diff --git a/src/lib/core/modules/MessageBuilder.ts b/src/lib/core/modules/MessageBuilder.ts index bcd417cb52..3a75b2cb82 100644 --- a/src/lib/core/modules/MessageBuilder.ts +++ b/src/lib/core/modules/MessageBuilder.ts @@ -131,6 +131,10 @@ export class MessageBuilder { csvOptions: options.csvOptions, pdfOptions: options.pdfOptions, imageOptions: options.imageOptions, + // Same reason as csvOptions: the keyframe budget and the + // transcription flag are read downstream by the detector, and + // a reconstruction that omits them silently restores defaults. + videoOptions: options.videoOptions, provider: options.provider, model: options.model, temperature: options.temperature, @@ -291,6 +295,10 @@ export class MessageBuilder { csvOptions: options.csvOptions, pdfOptions: options.pdfOptions, imageOptions: options.imageOptions, + // Same reason as csvOptions: the keyframe budget and the + // transcription flag are read downstream by the detector, and + // a reconstruction that omits them silently restores defaults. + videoOptions: options.videoOptions, provider: options.provider, model: options.model, temperature: options.temperature, diff --git a/src/lib/processors/media/VideoProcessor.ts b/src/lib/processors/media/VideoProcessor.ts index d2e1bb6c6f..dc9b7323a7 100644 --- a/src/lib/processors/media/VideoProcessor.ts +++ b/src/lib/processors/media/VideoProcessor.ts @@ -52,7 +52,10 @@ import { Readable } from "stream"; import { pipeline } from "stream/promises"; import { BaseFileProcessor } from "../base/BaseFileProcessor.js"; -import { formatMediaDuration } from "../../utils/mediaDuration.js"; +import { + formatKeyframeTimestamp, + formatMediaDuration, +} from "../../utils/mediaDuration.js"; import type { FfprobeData, FfprobeStream, @@ -60,6 +63,7 @@ import type { ProcessedVideo, ProcessorFileProcessingResult, ProcessOptions, + VideoKeyframe, VideoProcessorOptions, } from "../../types/index.js"; import { SIZE_LIMITS_MB } from "../config/index.js"; @@ -71,6 +75,8 @@ import { FileErrorCode } from "../errors/index.js"; import { tracers, ATTR, withSpan } from "../../telemetry/index.js"; import { logger } from "../../utils/logger.js"; import { tryImport } from "../../utils/tryImport.js"; +import { withTimeout } from "../../utils/errorHandling.js"; +import { runFfmpeg } from "../../adapters/video/ffmpegAdapter.js"; /** * Narrow a loaded `fluent-ffmpeg` export to the shape this file actually uses: @@ -243,6 +249,21 @@ const VIDEO_CONFIG = { FFMPEG_TIMEOUT_MS: 120_000, /** Timeout for ffprobe metadata extraction in milliseconds */ FFPROBE_TIMEOUT_MS: 10_000, + /** + * Timeout for demuxing the audio track out of a video (#433). + * + * Generous next to frame extraction: this reads the whole file rather than + * seeking to a handful of offsets, and a meeting recording is long. + */ + AUDIO_EXTRACT_TIMEOUT_MS: 180_000, + /** Timeout for one Whisper transcription request in milliseconds */ + TRANSCRIPTION_TIMEOUT_MS: 120_000, + /** + * Whisper's upload ceiling in MB. The extracted track is mono 16 kHz MP3, + * so this is roughly six hours of speech — a clip that breaches it is + * unusual enough to be worth saying so rather than silently truncating. + */ + WHISPER_MAX_SIZE_MB: 25, } as const; /** @@ -339,6 +360,8 @@ export class VideoProcessor extends BaseFileProcessor { filename: this.getFilename(fileInfo), textContent: "", keyframes: [], + keyframeTimestampsSec: [], + hasTranscript: false, metadata: { duration: 0, durationFormatted: formatMediaDuration(0), @@ -537,7 +560,7 @@ export class VideoProcessor extends BaseFileProcessor { ); // Step 5: Extract keyframes - let keyframes: Buffer[] = []; + let keyframes: VideoKeyframe[] = []; try { keyframes = await this.extractKeyframes( tempVideoPath, @@ -555,6 +578,17 @@ export class VideoProcessor extends BaseFileProcessor { span.setAttribute(ATTR.VIDEO_KEYFRAMES_EXTRACTED, keyframes.length); + // Debug-only: the real per-frame sample times, so a caller running + // with --debug can see what the model was actually told without + // re-deriving it from the request body. + if (logger.shouldLog("debug") && keyframes.length > 0) { + logger.debug( + `[NEUROLINK] Video keyframe timestamps: ${keyframes + .map((frame) => frame.timestampSec.toFixed(3)) + .join(",")}`, + ); + } + // Step 6: Extract subtitles let subtitleText: string | undefined; if (metadata.subtitleTracks > 0) { @@ -568,12 +602,22 @@ export class VideoProcessor extends BaseFileProcessor { } } - // Step 7: Build textContent for LLM + // Step 7: Transcribe spoken audio, if asked + const transcription = await this.extractAndTranscribeAudio( + tempVideoPath, + tempDir, + metadata, + filename, + options, + ); + + // Step 8: Build textContent for LLM const textContent = this.buildTextContent( metadata, - keyframes.length, + keyframes.map((frame) => frame.timestampSec), subtitleText, this.getFilename(fileInfo), + transcription.transcript, ); span.setAttribute(ATTR.VIDEO_TEXT_CONTENT_LENGTH, textContent.length); @@ -585,7 +629,7 @@ export class VideoProcessor extends BaseFileProcessor { `(${metadata.durationFormatted}, ${metadata.width}x${metadata.height}, ${metadata.codec})`, ); - // Step 8: Return structured result + // Step 9: Return structured result return { success: true, data: { @@ -594,9 +638,17 @@ export class VideoProcessor extends BaseFileProcessor { size: fileInfo.size, filename: this.getFilename(fileInfo), textContent, - keyframes, + keyframes: keyframes.map((frame) => frame.buffer), + keyframeTimestampsSec: keyframes.map( + (frame) => frame.timestampSec, + ), metadata, subtitleText, + transcript: transcription.transcript, + hasTranscript: !!transcription.transcript, + ...(transcription.skippedReason + ? { transcriptionSkippedReason: transcription.skippedReason } + : {}), hasKeyframes: keyframes.length > 0, frameCount: keyframes.length, }, @@ -620,7 +672,7 @@ export class VideoProcessor extends BaseFileProcessor { ), }; } finally { - // Step 8: Clean up temp files + // Step 10: Clean up temp files if (tempCreated) { await fs.rm(tempDir, { recursive: true, force: true }).catch(() => { // Ignore cleanup errors - temp files will be cleaned by OS eventually @@ -805,6 +857,196 @@ export class VideoProcessor extends BaseFileProcessor { return Math.min(100, Math.max(1, Math.round(quality))); } + /** + * Extract the clip's audio track and transcribe the speech in it (#433). + * + * ## Why this exists at all, now that Gemini hears video directly + * + * It does not help Gemini — a native provider receives the clip and its + * audio together. It is for everyone else. A provider on the frame path + * gets stills and nothing else, so a recorded standup, a support call or a + * narrated demo arrives with its entire spoken content missing, and + * "summarise this meeting" is answered from four screenshots. + * + * ## Why it is not a stub + * + * The issue asked for one, on the grounds that no transcription backend + * existed yet. One does: `AudioProcessor` has shipped Whisper transcription + * for standalone audio files for some time. A method that logged "not yet + * implemented" and returned undefined would be dead code sitting next to a + * working implementation of the same thing, and `--transcribe-audio` would + * still do nothing. + * + * The Whisper call is reproduced here rather than shared with + * `AudioProcessor`: extracting it into a common module is the better + * long-term shape, but that file is being edited concurrently, and a + * merge conflict in the audio pipeline is a worse outcome than fifty + * duplicated lines. The duplication is worth removing once both land. + * + * ## Failure behaviour + * + * Best-effort throughout, and never throws: a clip with no audio track, a + * machine without ffmpeg, a missing key, an oversized track or a failed + * request all return a *reason* and leave the rest of the pipeline intact. + * Each reason is distinct, because from the outside every one of them + * looks the same — no transcript — while the remedies differ completely. + * + * @param videoPath - Temp path to the video, already written by the caller + * @param tempDir - Caller-owned temp directory; removed in its `finally` + * @param metadata - Probed metadata, consulted for an audio track + * @param filename - Display name, for logs + * @param options - Caller settings; transcription runs only when asked + */ + private async extractAndTranscribeAudio( + videoPath: string, + tempDir: string, + metadata: ProcessedVideo["metadata"], + filename: string, + options?: VideoProcessorOptions, + ): Promise<{ transcript?: string; skippedReason?: string }> { + if (!options?.transcribeAudio) { + // Not a skip worth reporting: nobody asked. Reporting it would put a + // "no transcript because..." line in every video result. + return {}; + } + + const skipped = (reason: string): { skippedReason: string } => { + logger.warn( + `[VideoProcessor] No transcript for ${filename}: ${reason}. ` + + `The model will receive the keyframes and metadata only.`, + ); + return { skippedReason: reason }; + }; + + // Probing reports no audio codec both for a genuinely silent clip and + // for one nothing could open. Either way there is nothing to send, and + // the ffmpeg pass below would only fail more slowly. + if (!metadata.audioCodec) { + return skipped("the video has no audio track"); + } + + const apiKey = process.env.OPENAI_API_KEY; + if (!apiKey) { + return skipped( + "OPENAI_API_KEY is not set, and Whisper is the only transcription backend wired up", + ); + } + + const audioPath = join(tempDir, "audio.mp3"); + try { + await runFfmpeg( + [ + "-y", + "-v", + "error", + "-i", + videoPath, + // Drop the video stream outright. Without -vn ffmpeg tries to carry + // it into an MP3 container as cover art and fails on most inputs. + "-vn", + // Mono at 16 kHz is what Whisper resamples to anyway, and it keeps + // an hour-long recording comfortably under the upload ceiling. + "-ac", + "1", + "-ar", + "16000", + "-c:a", + "libmp3lame", + "-q:a", + "4", + audioPath, + ], + { timeoutMs: VIDEO_CONFIG.AUDIO_EXTRACT_TIMEOUT_MS }, + ); + } catch (error) { + const detail = + error instanceof Error ? error.message.split("\n")[0] : String(error); + return skipped(`the audio track could not be extracted — ${detail}`); + } + + let audioBuffer: Buffer; + try { + audioBuffer = await fs.readFile(audioPath); + } catch { + return skipped("ffmpeg reported success but wrote no audio file"); + } + if (audioBuffer.length === 0) { + return skipped("the extracted audio track was empty"); + } + + const sizeMB = audioBuffer.length / (1024 * 1024); + if (sizeMB > VIDEO_CONFIG.WHISPER_MAX_SIZE_MB) { + return skipped( + `the extracted audio is ${sizeMB.toFixed(1)}MB, over Whisper's ` + + `${VIDEO_CONFIG.WHISPER_MAX_SIZE_MB}MB limit — split the recording`, + ); + } + + const baseUrl = ( + process.env.OPENAI_BASE_URL ?? "https://api.openai.com/v1" + ).replace(/\/+$/, ""); + const form = new FormData(); + form.append( + "file", + new Blob([new Uint8Array(audioBuffer)], { type: "audio/mpeg" }), + "audio.mp3", + ); + form.append("model", "whisper-1"); + form.append("response_format", "verbose_json"); + + // `withTimeout` only races a promise against a timer — it cannot cancel + // the request. Without the abort, a timed-out upload keeps its socket and + // its in-flight body alive after the caller has already moved on. + const abort = new AbortController(); + const timer = setTimeout( + () => abort.abort(), + VIDEO_CONFIG.TRANSCRIPTION_TIMEOUT_MS, + ); + try { + const response = await withTimeout( + fetch(`${baseUrl}/audio/transcriptions`, { + method: "POST", + headers: { Authorization: `Bearer ${apiKey}` }, + body: form, + signal: abort.signal, + }), + VIDEO_CONFIG.TRANSCRIPTION_TIMEOUT_MS, + ); + + if (!response.ok) { + const detail = await response.text().catch(() => ""); + return skipped( + `the transcription request failed — HTTP ${response.status}` + + `${detail ? `: ${detail.slice(0, 200)}` : ""}`, + ); + } + + const payload: unknown = await response.json(); + const text = + typeof payload === "object" && + payload !== null && + typeof (payload as { text?: unknown }).text === "string" + ? (payload as { text: string }).text.trim() + : ""; + + if (text.length === 0) { + // A successful call returning nothing is a real outcome — silence, + // music, no speech — and not the same as a failure. + return skipped("Whisper returned an empty transcript for this audio"); + } + + logger.debug( + `[VideoProcessor] Transcribed ${filename} via openai-whisper (${text.length} chars)`, + ); + return { transcript: text }; + } catch (error) { + const detail = error instanceof Error ? error.message : String(error); + return skipped(`the transcription request failed — ${detail}`); + } finally { + clearTimeout(timer); + } + } + /** * Extract keyframes from a video at calculated intervals. * @@ -829,14 +1071,14 @@ export class VideoProcessor extends BaseFileProcessor { * @param tempDir - Temp directory for frame output * @param durationSec - Video duration in seconds * @param options - Caller frame budget / encoder settings - * @returns Array of encoded frame buffers (JPEG unless png was requested) + * @returns Each kept frame with the second it was sampled at, ascending */ private async extractKeyframes( videoPath: string, tempDir: string, durationSec: number, options?: VideoProcessorOptions, - ): Promise { + ): Promise { if (durationSec <= 0) { return []; } @@ -885,7 +1127,14 @@ export class VideoProcessor extends BaseFileProcessor { const framesDir = join(tempDir, "frames"); await fs.mkdir(framesDir, { recursive: true }); - await this.runFfmpegFrameExtraction( + // The requested `timestamps` are an idealized, evenly-spaced schedule. + // ffmpeg's `-vf select` only guarantees a MINIMUM gap since the last + // frame it actually selected — on a source whose real frame spacing + // exceeds that gap, the selected frames land later than the schedule + // and the gap compounds across every subsequent frame. `showinfo` + // reports each selected frame's true `pts_time`, which is what gets + // paired with the encoded file below instead of the idealized value. + const realTimestamps = await this.runFfmpegFrameExtraction( videoPath, framesDir, timestamps, @@ -893,7 +1142,7 @@ export class VideoProcessor extends BaseFileProcessor { ); // Read extracted frames and resize with sharp - const keyframes: Buffer[] = []; + const keyframes: VideoKeyframe[] = []; for (let i = 0; i < timestamps.length; i++) { const framePath = join( framesDir, @@ -923,7 +1172,14 @@ export class VideoProcessor extends BaseFileProcessor { : pipeline.jpeg({ quality }) ).toBuffer(); - keyframes.push(resized); + // Paired as the frame is kept, not derived afterwards from the + // interval: the catch below drops a frame whose encode failed, and a + // reconstructed schedule would then mislabel every frame after it. + // Prefer the real PTS ffmpeg reported for this output file; only + // fall back to the idealized schedule value if `showinfo` didn't + // report one (e.g. an older/unexpected ffmpeg build). + const timestampSec = realTimestamps[i] ?? timestamps[i]; + keyframes.push({ buffer: resized, timestampSec }); } catch { // Skip individual frame on resize/encode failure } @@ -935,24 +1191,36 @@ export class VideoProcessor extends BaseFileProcessor { /** * Run ffmpeg to extract frames at specified timestamps. * - * Uses the `-vf select` filter to pick frames at exact timestamps, - * which is more efficient than seeking for each frame individually. + * Uses the `-vf select` filter to pick frames at least `intervalSec` apart, + * which is more efficient than seeking for each frame individually. This is + * a minimum-gap filter, not a seek to an exact time: on a source whose real + * frame spacing exceeds the requested interval, the frame actually selected + * can land well after the ideal schedule. A `showinfo` stage between + * `select` and `scale` reports each selected frame's true `pts_time` to + * stderr so the caller can label frames with when they were really + * sampled, not the idealized request. * * @param videoPath - Path to the video file * @param outputDir - Directory to write frame files * @param timestamps - Array of timestamps in seconds + * @param intervalSec - Minimum gap enforced between selected frames + * @returns The real `pts_time` (seconds) ffmpeg reported for each selected + * frame, in output-file order; may be shorter than `timestamps` if fewer + * real frames were available to select */ private async runFfmpegFrameExtraction( videoPath: string, outputDir: string, timestamps: number[], intervalSec: number, - ): Promise { + ): Promise { const ff = await loadFluentFfmpeg(); return new Promise((resolve, reject) => { // Improved select expression to pick exactly one frame per interval // instead of multiple frames within a 0.5s window. const selectExpr = `isnan(prev_selected_t)+gte(t-prev_selected_t,${intervalSec}-0.001)`; + const PTS_TIME_RE = /pts_time:\s*(-?\d+(?:\.\d+)?)/; + const realTimestamps: number[] = []; const timeoutId = setTimeout(() => { reject( @@ -965,7 +1233,7 @@ export class VideoProcessor extends BaseFileProcessor { ff(videoPath) .outputOptions([ "-vf", - `select='${selectExpr}',scale='min(${VIDEO_CONFIG.FRAME_MAX_DIMENSION}\\,iw):-2'`, + `select='${selectExpr}',showinfo,scale='min(${VIDEO_CONFIG.FRAME_MAX_DIMENSION}\\,iw):-2'`, "-vsync", "vfr", "-q:v", @@ -974,9 +1242,17 @@ export class VideoProcessor extends BaseFileProcessor { String(timestamps.length), ]) .output(join(outputDir, "frame_%04d.jpg")) + .on("stderr", (line: string) => { + // showinfo writes one log line per selected frame, in the same + // ascending order ffmpeg encodes frame_0001, frame_0002, ... + const match = PTS_TIME_RE.exec(line); + if (match) { + realTimestamps.push(Number.parseFloat(match[1])); + } + }) .on("end", () => { clearTimeout(timeoutId); - resolve(); + resolve(realTimestamps); }) .on("error", (err: Error) => { clearTimeout(timeoutId); @@ -1111,16 +1387,18 @@ export class VideoProcessor extends BaseFileProcessor { * - Subtitle text (if available) * * @param metadata - Extracted video metadata - * @param frameCount - Number of keyframes extracted + * @param keyframeTimestampsSec - The real second each kept keyframe was sampled at * @param subtitleText - Extracted subtitle text (if any) * @param filename - Original filename + * @param transcript - Machine transcription of the spoken audio (if requested and available) * @returns Formatted text content for the LLM */ private buildTextContent( metadata: ProcessedVideo["metadata"], - frameCount: number, + keyframeTimestampsSec: number[], subtitleText: string | undefined, filename: string, + transcript: string | undefined, ): string { const lines: string[] = []; @@ -1154,14 +1432,24 @@ export class VideoProcessor extends BaseFileProcessor { ); lines.push(""); - if (frameCount > 0) { - const intervalSec = this.getFrameInterval(metadata.duration); - lines.push( - `## Keyframes (${frameCount} frames extracted every ~${intervalSec}s)`, - ); + if (keyframeTimestampsSec.length > 0) { + // The real sample times, not the duration tier's nominal interval. Those + // two disagree whenever the caller passed `videoOptions.frames`, because + // an explicit budget spreads frames evenly across the clip instead of + // following the tier — so a 3s clip asked for 16 frames used to be + // described as "every ~1s" while the frames were 0.19s apart. Listing + // what was actually sampled cannot drift from what was sent, and it is + // also what lets the model answer a question about a specific moment. + lines.push(`## Keyframes (${keyframeTimestampsSec.length} extracted)`); lines.push( - "The following images are keyframes extracted from the video at regular intervals.", + "The images attached below are keyframes from this video, in order. " + + "Each was sampled at the timestamp listed here:", ); + for (const [index, timestampSec] of keyframeTimestampsSec.entries()) { + lines.push( + `- Frame ${index + 1}: ${formatKeyframeTimestamp(timestampSec)}`, + ); + } } else { lines.push("## Keyframes"); lines.push( @@ -1169,6 +1457,26 @@ export class VideoProcessor extends BaseFileProcessor { ); } + if (transcript) { + lines.push(""); + // Labelled as transcribed speech, not as captions: the two can both be + // present and disagree, and a model told which is which can say so + // rather than averaging them into one confident answer. + lines.push("## Spoken Audio (transcribed)"); + lines.push( + "The following is a machine transcription of the speech in this video:", + ); + const maxTranscriptChars = 20_000; + if (transcript.length > maxTranscriptChars) { + lines.push( + transcript.substring(0, maxTranscriptChars) + + `\n... [truncated, ${transcript.length - maxTranscriptChars} more characters]`, + ); + } else { + lines.push(transcript); + } + } + if (subtitleText) { lines.push(""); lines.push("## Subtitles / Captions"); diff --git a/src/lib/types/file.ts b/src/lib/types/file.ts index 6b5750a75e..80e381d347 100644 --- a/src/lib/types/file.ts +++ b/src/lib/types/file.ts @@ -2,6 +2,8 @@ * File detection and processing types for unified file handling */ +import type { ImageWithAltText } from "./multimodal.js"; + /** * Supported file types for multimodal input */ @@ -310,8 +312,15 @@ export type FileProcessingResult = { type: FileType; content: string | Buffer; mimeType: string; - /** Additional images extracted from the file (e.g., video keyframes, audio cover art) */ - images?: Array; + /** + * Additional images extracted from the file (e.g., video keyframes, audio + * cover art). + * + * An entry may carry alt text, which is how a video keyframe states the + * timestamp it was sampled at. Bare bytes still work — the field was + * widened, not changed. + */ + images?: Array; metadata: { confidence: number; size?: number; @@ -614,6 +623,16 @@ export type VideoProcessorOptions = { quality?: number; /** Frame encoding. Defaults to jpeg. */ format?: "jpeg" | "png"; + /** + * Transcribe the clip's spoken audio (#433). Off by default: it costs an + * ffmpeg pass plus a Whisper call, and most attached video is silent + * screen capture. + * + * Best-effort — a clip with no audio track, no `OPENAI_API_KEY`, or a + * failed call still processes, and the reason lands in + * `ProcessedVideo.transcriptionSkippedReason`. + */ + transcribeAudio?: boolean; }; /** diff --git a/src/lib/types/generate.ts b/src/lib/types/generate.ts index bce725d71d..bdd3a17589 100644 --- a/src/lib/types/generate.ts +++ b/src/lib/types/generate.ts @@ -39,6 +39,7 @@ import type { import type { NeurolinkCredentials } from "./providers.js"; import type { CSVProcessorOptions, + VideoProcessorOptions, FileWithMetadata, MultimodalAudioEntry, MultimodalVideoEntry, @@ -213,17 +214,15 @@ export type GenerateOptions = { outputFormat?: VisionImageOutputFormat; }; - // Video processing options - videoOptions?: { - /** Frames to extract. Unset lets VideoProcessor pick from the clip's duration; clamped to 100. */ - frames?: number; - /** Frame encoder quality, clamped to 1-100. Default 80. */ - quality?: number; - /** Frame encoding. Default jpeg. */ - format?: "jpeg" | "png"; - /** Not implemented yet (#433) — warns rather than silently doing nothing. */ - transcribeAudio?: boolean; - }; + /** + * Video processing options — keyframe budget, encoder settings, and + * whether to transcribe the clip's spoken audio. + * + * The canonical shape, rather than a third structurally-identical copy of + * it: `StreamOptions` and `TextGenerationOptions` declare the same field, + * and the inline copies had already drifted apart in what they documented. + */ + videoOptions?: VideoProcessorOptions; /** * Text-to-Speech (TTS) configuration @@ -1597,6 +1596,17 @@ export type TextGenerationOptions = { // NEW: CSV Processing Options (#379: canonical shape — see above) csvOptions?: CSVProcessorOptions; + /** + * Video processing options (#478, #433). + * + * Reconstructed option objects have to carry this the same way they carry + * `csvOptions` and `pdfOptions`: the message builder hands it to the + * detector, which hands it to `VideoProcessor`, and anything that rebuilds + * the options along the way and omits it restores the defaults without + * saying so. + */ + videoOptions?: VideoProcessorOptions; + /** PDF processing options (#258). */ pdfOptions?: { /** Password for an encrypted PDF (image-conversion fallback path). */ @@ -1624,22 +1634,6 @@ export type TextGenerationOptions = { outputFormat?: VisionImageOutputFormat; }; - /** - * Video processing options (mirrors `GenerateOptions.videoOptions`). Never - * declared here before, so `buildGenerateTextOptions` had nowhere to - * forward the caller's setting even once the allowlist itself named it. - */ - videoOptions?: { - /** Frames to extract. Unset lets VideoProcessor pick from the clip's duration; clamped to 100. */ - frames?: number; - /** Frame encoder quality, clamped to 1-100. Default 80. */ - quality?: number; - /** Frame encoding. Default jpeg. */ - format?: "jpeg" | "png"; - /** Not implemented yet (#433) — warns rather than silently doing nothing. */ - transcribeAudio?: boolean; - }; - enableSummarization?: boolean; // Enable/disable summarization for this specific request /** diff --git a/src/lib/types/processor.ts b/src/lib/types/processor.ts index d3cf8a3ec1..91d5f0dce8 100644 --- a/src/lib/types/processor.ts +++ b/src/lib/types/processor.ts @@ -846,6 +846,21 @@ export type ProcessedAudio = ProcessedFileBase & { coverArt?: Buffer; }; +/** + * One extracted keyframe and the moment it came from. + * + * Internal to extraction: `ProcessedVideo` splits this back into the parallel + * `keyframes` / `keyframeTimestampsSec` arrays its existing consumers expect. + * Keeping the pair together while frames are being read and encoded is what + * makes a dropped frame impossible to mislabel — the alternative is + * reconstructing timestamps from an interval after the fact, which is wrong + * for every frame following a failed encode. + */ +export type VideoKeyframe = { + readonly buffer: Buffer; + readonly timestampSec: number; +}; + /** * Processed video result. * Extends ProcessedFileBase with video-specific fields including metadata, @@ -855,6 +870,19 @@ export type ProcessedAudio = ProcessedFileBase & { export type ProcessedVideo = ProcessedFileBase & { textContent: string; keyframes: Buffer[]; + /** + * When each entry of `keyframes` was sampled, in seconds from the start of + * the clip. Same length, same order, ascending. + * + * A parallel array rather than a richer `keyframes` element type: that + * field is read by `fileReferenceRegistry` and by the detector as a plain + * `Buffer[]`, and reshaping it would break them for a value they do not + * need. Extraction can also drop an individual frame when its encode + * fails, so the timestamps are recorded as frames are kept — never + * reconstructed from an interval, which would silently mislabel every + * frame after a dropped one. + */ + keyframeTimestampsSec: number[]; metadata: { duration: number; durationFormatted: string; @@ -870,6 +898,22 @@ export type ProcessedVideo = ProcessedFileBase & { fileSize: number; }; subtitleText?: string; + /** + * Speech transcribed from the clip's audio track, when + * `VideoProcessorOptions.transcribeAudio` asked for it and it worked. + * + * Distinct from `subtitleText`, which is an embedded subtitle stream the + * file already carried. A recording can have one, both or neither. + */ + transcript?: string; + hasTranscript: boolean; + /** + * Why no transcript was produced. Follows `ProcessedAudio`'s precedent: + * "nobody asked", "there is no audio track", "no API key", "the call + * failed" and "nobody was speaking" are five different situations that + * otherwise all present as an absent transcript. + */ + transcriptionSkippedReason?: string; hasKeyframes: boolean; frameCount: number; }; diff --git a/src/lib/types/stream.ts b/src/lib/types/stream.ts index ee96a31642..279d4c5bab 100644 --- a/src/lib/types/stream.ts +++ b/src/lib/types/stream.ts @@ -42,6 +42,7 @@ import type { StandardRecord, ValidationSchema } from "./aliases.js"; import type { CSVProcessorOptions, FileWithMetadata, + VideoProcessorOptions, VisionImageOutputFormat, } from "./file.js"; import type { WorkflowConfig } from "./workflow.js"; @@ -420,6 +421,17 @@ export type StreamOptions = { // CSV processing options (#379: canonical shape — see CSVProcessorOptions) csvOptions?: CSVProcessorOptions; + /** + * Video processing options (#478, #433). + * + * Reconstructed option objects have to carry this the same way they carry + * `csvOptions` and `pdfOptions`: the message builder hands it to the + * detector, which hands it to `VideoProcessor`, and anything that rebuilds + * the options along the way and omits it restores the defaults without + * saying so. + */ + videoOptions?: VideoProcessorOptions; + /** PDF processing options (#258). */ pdfOptions?: { /** Password for an encrypted PDF (image-conversion fallback path). */ @@ -452,18 +464,6 @@ export type StreamOptions = { outputFormat?: VisionImageOutputFormat; }; - // Video processing options - videoOptions?: { - /** Frames to extract. Unset lets VideoProcessor pick from the clip's duration; clamped to 100. */ - frames?: number; - /** Frame encoder quality, clamped to 1-100. Default 80. */ - quality?: number; - /** Frame encoding. Default jpeg. */ - format?: "jpeg" | "png"; - /** Not implemented yet (#433) — warns rather than silently doing nothing. */ - transcribeAudio?: boolean; - }; - /** * Text-to-Speech (TTS) configuration for streaming * diff --git a/src/lib/utils/fileDetector.ts b/src/lib/utils/fileDetector.ts index 72228c5796..69df7a4da4 100644 --- a/src/lib/utils/fileDetector.ts +++ b/src/lib/utils/fileDetector.ts @@ -70,6 +70,7 @@ import { normalizeMimeHint, } from "./mimeTypeHints.js"; import { PDFProcessor } from "./pdfProcessor.js"; +import { formatKeyframeTimestamp } from "./mediaDuration.js"; /** * Short-TTL cache of URL → Content-Type (#323). A URL is commonly detected more @@ -1044,6 +1045,30 @@ export class FileDetector { /** * SDK-8: Format an informative placeholder when a file processor fails. * Instead of bare "[Video file: name]" strings, include size, format, and + * Describe one keyframe well enough for a model to place it in time. + * + * The clip's total length rides along because the eighth frame of a + * recording means nothing without knowing how long the recording is. + * + * Explicit units rather than a clock — see `formatKeyframeTimestamp`. An + * earlier draft used "0:03" here and the model reported the frame as + * appearing at "3:00". + * + * A timestamp can be missing if extraction and its schedule ever disagree + * in length. That should not happen, but an unlabelled frame is better + * than one labelled "NaN". + */ + private static describeKeyframe( + timestampSec: number | undefined, + durationFormatted: string, + ): string | undefined { + if (typeof timestampSec !== "number" || !Number.isFinite(timestampSec)) { + return undefined; + } + return `Video keyframe at ${formatKeyframeTimestamp(timestampSec)} into a ${durationFormatted} video`; + } + + /** * the reason for failure so the LLM can acknowledge the attachment. */ private static formatInformativePlaceholder( @@ -1463,10 +1488,13 @@ export class FileDetector { videoOptions, ); if (videoResult.success && videoResult.data) { + // Bound to a local because the keyframe map below is a closure, and + // TypeScript drops the `videoResult.data` narrowing across one. + const video = videoResult.data; return { type: "video", content: - videoResult.data.textContent || + video.textContent || FileDetector.formatInformativePlaceholder( "Video", videoFilename, @@ -1474,21 +1502,33 @@ export class FileDetector { detection, ), mimeType: detection.mimeType, + // Each keyframe carries the moment it was sampled at. Alt text is + // the only per-image channel that survives to the model — the + // message builder folds it into the prompt as + // "[Image N: ]" — so without it the frames arrive as an + // unlabelled pile and "what happens at 0:30?" has no answer even + // though the frame at 0:30 is right there. images: - videoResult.data.keyframes && videoResult.data.keyframes.length > 0 - ? videoResult.data.keyframes + video.keyframes && video.keyframes.length > 0 + ? video.keyframes.map((frame, index) => ({ + data: frame, + altText: FileDetector.describeKeyframe( + video.keyframeTimestampsSec[index], + video.metadata.durationFormatted, + ), + })) : undefined, metadata: { ...detection.metadata, - frameCount: videoResult.data.frameCount, - hasKeyframes: videoResult.data.hasKeyframes, + frameCount: video.frameCount, + hasKeyframes: video.hasKeyframes, // Carried so native delivery can apply its duration ceiling // without opening the container a second time. The processor // reports 0 when nothing could probe it; that is "unknown", not a // zero-length clip, so it is left off rather than passed through // as a number a ceiling check would happily accept. - ...(videoResult.data.metadata.duration > 0 - ? { durationSec: videoResult.data.metadata.duration } + ...(video.metadata.duration > 0 + ? { durationSec: video.metadata.duration } : {}), }, }; diff --git a/src/lib/utils/mediaDuration.ts b/src/lib/utils/mediaDuration.ts index 1c9489225a..be8812877c 100644 --- a/src/lib/utils/mediaDuration.ts +++ b/src/lib/utils/mediaDuration.ts @@ -46,3 +46,27 @@ export function formatMediaDuration(seconds: number): string { return parts.join(" "); } + +/** + * Format the moment a video keyframe was sampled at — "3.00s", + * "1234.00s (20m 34s)". + * + * Deliberately not the clock form, for the reason in this module's header: + * the string is read by a language model, and "0:03" is ambiguous. That is + * not theoretical here — an early draft labelled frames "0:03" and the model + * reported the green frame as appearing at "3:00". + * + * Deliberately not `formatMediaDuration` alone either. That rounds to whole + * seconds, and an explicit `videoOptions.frames` spreads frames evenly rather + * than on the duration tier: sixteen frames across a 3.6s clip are 0.225s + * apart, and every one of them would render as "0s". The precise value is + * therefore always present, with the readable form added only past a minute, + * where "1234.00s" stops meaning anything to a reader. + * + * Non-finite and negative inputs clamp to zero rather than emitting "NaNs". + */ +export function formatKeyframeTimestamp(seconds: number): string { + const safe = Number.isFinite(seconds) && seconds > 0 ? seconds : 0; + const precise = `${safe.toFixed(2)}s`; + return safe >= 60 ? `${precise} (${formatMediaDuration(safe)})` : precise; +} diff --git a/src/lib/utils/messageBuilder.ts b/src/lib/utils/messageBuilder.ts index ce2d8b7385..2637d253f7 100644 --- a/src/lib/utils/messageBuilder.ts +++ b/src/lib/utils/messageBuilder.ts @@ -1065,25 +1065,6 @@ export function mergeMediaFileAliases(input: { input.videoFiles = undefined; } -/** - * #478: `transcribeAudio` (CLI `--transcribe-audio`) is accepted by the options - * surface but no video-audio transcription exists yet — VideoProcessor extracts - * keyframes and embedded subtitle tracks only, and the transcription step is - * still open as #433. Say so once per request rather than letting the caller - * believe a transcript was produced and silently omitted. - */ -function warnIfVideoTranscriptionRequested( - videoOptions: GenerateOptions["videoOptions"], -): void { - if (videoOptions?.transcribeAudio) { - logger.warn( - "[NEUROLINK] Video audio transcription was requested but is not implemented yet " + - "(tracked as #433). Keyframes and any embedded subtitle tracks are still extracted; " + - "spoken audio will not be transcribed.", - ); - } -} - /** * Process the unified files array with auto-detection. * Handles lazy file registration, full processing, and preview injection. @@ -1160,6 +1141,10 @@ async function detectFileForUnifiedArray( frames: options.videoOptions.frames, quality: options.videoOptions.quality, format: options.videoOptions.format, + // #433: `--transcribe-audio` stopped here until now — the flag was + // accepted, warned about, and never handed to the processor that + // would have to act on it. + transcribeAudio: options.videoOptions.transcribeAudio, } : undefined, provider: provider, @@ -1209,8 +1194,6 @@ export async function processUnifiedFilesArray( const totalFiles = pending.length; const files = pending; - warnIfVideoTranscriptionRequested(options.videoOptions); - return withSpan( { name: "neurolink.file.process_all", diff --git a/src/lib/utils/multimodalOptionsBuilder.ts b/src/lib/utils/multimodalOptionsBuilder.ts index 1908870bc3..4054c7088f 100644 --- a/src/lib/utils/multimodalOptionsBuilder.ts +++ b/src/lib/utils/multimodalOptionsBuilder.ts @@ -16,6 +16,7 @@ import type { StreamOptions } from "../types/index.js"; * - input.audioFiles: Audio files (Buffer | string paths) * - input.videoFiles: Video files (Buffer | string paths) * - csvOptions: CSV parsing options + * - videoOptions: Video frame/quality/format/transcription options * - systemPrompt: System-level instructions * - conversationMessages: Chat history * - temperature: Model temperature (0-1) @@ -66,6 +67,12 @@ export function buildMultimodalOptions( csvOptions: options.csvOptions, pdfOptions: options.pdfOptions, imageOptions: options.imageOptions, + // Same whitelist hazard as above, one level up: `videoFiles` reaching + // the processor is not enough on its own — without `videoOptions` too, + // the frame/quality/format/transcribeAudio knobs a caller set on + // `StreamOptions` never reach VideoProcessor on the Bedrock branch, and + // it silently falls back to the duration-tier default. + videoOptions: options.videoOptions, systemPrompt: options.systemPrompt, conversationHistory: options.conversationMessages, provider: providerName, diff --git a/test/continuous-test-suite-video-frames.ts b/test/continuous-test-suite-video-frames.ts new file mode 100644 index 0000000000..ed7108a515 --- /dev/null +++ b/test/continuous-test-suite-video-frames.ts @@ -0,0 +1,459 @@ +#!/usr/bin/env tsx +/** + * Continuous Test Suite: video keyframe timestamps and audio transcription. + * + * Covers VIDEO-011 (#433) and VIDEO-018 (#460). + * + * ## What these assertions are really guarding + * + * Every knob under `videoOptions` was unreachable. `NeuroLink.generate()` + * rebuilt its options into `TextGenerationOptions` and carried `csvOptions` + * and `pdfOptions` across but not `videoOptions`, and two more + * reconstructions downstream dropped it the same way. So `--video-frames 2` + * on a four-second clip produced the tier default of four frames, and + * `--transcribe-audio` reached no code that could act on it. The plumbing at + * the far end had been correct since #478; nothing ever arrived. + * + * That failure is invisible from a passing generation — the model answers + * either way — which is why the frame-count assertion below reads the actual + * number extracted rather than whether the request succeeded. + * + * ## The fixture + * + * `test/fixtures/media/sample-clip.mp4` — 3.6s, 160x120, H.264 + AAC, three + * solid colour segments (red, blue, green) and a voice saying "the secret + * word is flamingo". OpenAI is used throughout precisely because it is on + * the frame-extraction path: it cannot hear the clip, so the spoken word is + * a clean probe for whether a transcript was produced and attached. + * + * Frame extraction needs ffmpeg, so most of this suite skips without it. + * That is the right behaviour rather than a gap — ffmpeg is deliberately + * absent in CI, and native delivery (the path that needs none) is covered by + * continuous-test-suite-video-native.ts. + * + * Run: npx tsx test/continuous-test-suite-video-frames.ts + */ + +process.env.NEUROLINK_SKIP_MCP = "true"; + +import "dotenv/config"; +import { existsSync } from "node:fs"; +import * as path from "node:path"; +import { fileURLToPath } from "node:url"; +import { + assert, + defineSuite, + runCLI, + Skip, + tempDir, +} from "./helpers/harness.js"; +import { hasFfmpeg, makeVideoFile } from "./helpers/mediaFixtures.js"; + +// The longest test runs two CLI calls in sequence, each allowed 240 s. A live +// suite reports a per-test timeout as SKIP, so the budget must outlast both +// calls or a slow-but-correct run hides its assertions. +const { test, runSuite } = defineSuite("Video keyframes and transcription", { + perTestTimeoutMs: 540_000, +}); + +const HERE = path.dirname(fileURLToPath(import.meta.url)); +const CLIP = path.join(HERE, "fixtures", "media", "sample-clip.mp4"); + +const FRAME_PROVIDER = process.env.VIDEO_FRAME_PROVIDER ?? "openai"; + +/** `Video processed: → bytes text + keyframes` */ +const PROCESSED = /Video processed:[^\n]*?\+\s*(\d+)\s*keyframes/; + +function requireFixtures(): void { + if (!existsSync(CLIP)) { + throw new Error("the committed video fixture is missing"); + } +} + +async function requireFrameExtraction(): Promise { + requireFixtures(); + if (!(await hasFfmpeg())) { + throw new Skip("ffmpeg is unavailable, so no frames can be extracted"); + } +} + +function requireOpenAI(): void { + if (!process.env.OPENAI_API_KEY) { + throw new Skip("no OpenAI credentials"); + } +} + +/** Frames actually extracted, per the processor's own log line. */ +function extractedFrameCount(output: string): number | null { + const match = PROCESSED.exec(output); + return match ? Number(match[1]) : null; +} + +/** + * The model's reply, with the CLI's own logging removed. + * + * `--debug` is required for the processor's log lines to appear at all, and + * the CLI writes them to **stdout** alongside the answer. Asserting on raw + * stdout therefore matches against several hundred lines of tool + * registration and timing — a digit-matching assertion in particular would + * pass on a model name. Every log line carries an ISO-8601 prefix, so + * dropping those leaves the reply — with one exception: every + * `Debug Information...:` footer header (`commandFactory.ts`) is logged as + * `logger.debug("\n" + chalk.yellow(...))`, and `console.debug(prefix, + * message)` joins its two arguments with a single space, so the embedded + * leading `\n` splits the prefix from the header text onto its own, + * un-prefixed line. That line does not start with a timestamp and would + * otherwise survive into the "reply". Filter it explicitly rather than rely + * on the timestamp prefix alone. + */ +function answerOnly(stdout: string): string { + return stdout + .split("\n") + .filter( + (line) => + !/^\[\d{4}-\d{2}-\d{2}T/.test(line) && !/^Debug Information/.test(line), + ) + .join("\n") + .trim(); +} + +// --------------------------------------------------------------------------- +// videoOptions reaches the processor at all +// --------------------------------------------------------------------------- + +await test("the frame budget from the CLI is honoured", async () => { + await requireFrameExtraction(); + requireOpenAI(); + + const baseline = await runCLI( + [ + "generate", + "Reply OK.", + "--file", + CLIP, + "--provider", + FRAME_PROVIDER, + "--debug", + ], + { env: { NEUROLINK_LOG_LEVEL: "debug" }, timeoutMs: 240_000 }, + ); + const defaultFrames = extractedFrameCount( + `${baseline.stdout}${baseline.stderr}`, + ); + // Precondition: without a measured default there is nothing to compare a + // request against, and "the budget was applied" would be unfalsifiable. + assert( + defaultFrames !== null && defaultFrames > 1, + "precondition: the unconstrained run must extract more than one frame", + ); + + const budgeted = await runCLI( + [ + "generate", + "Reply OK.", + "--file", + CLIP, + "--provider", + FRAME_PROVIDER, + "--video-frames", + "1", + "--debug", + ], + { env: { NEUROLINK_LOG_LEVEL: "debug" }, timeoutMs: 240_000 }, + ); + const requestedFrames = extractedFrameCount( + `${budgeted.stdout}${budgeted.stderr}`, + ); + assert( + requestedFrames === 1, + "an explicit frame budget must reach the processor and be applied", + ); +}); + +await test("keyframes are labelled with the moment they came from", async () => { + await requireFrameExtraction(); + requireOpenAI(); + + const res = await runCLI( + [ + "generate", + "Answer with a number and nothing else: how many seconds into the " + + "video does the first green frame appear? If the frames carry no " + + "timing information at all, answer exactly UNTIMED.", + "--file", + CLIP, + "--provider", + FRAME_PROVIDER, + "--debug", + ], + { env: { NEUROLINK_LOG_LEVEL: "debug" }, timeoutMs: 240_000 }, + ); + const combined = `${res.stdout}${res.stderr}`; + + const frames = extractedFrameCount(combined); + assert( + frames !== null && frames > 1, + "precondition: several keyframes must have been extracted and sent", + ); + const answer = answerOnly(res.stdout); + assert( + !/UNTIMED/i.test(answer), + "the model must be able to place the frames in time", + ); + // The green segment starts at 2.4s, so the first green keyframe is the one + // sampled at 3s. Both readings are accepted: the assertion is that a + // timestamp reached the model at all, not that it rounded a particular way. + assert( + /\b[23](\.\d+)?\b/.test(answer), + "the reported moment must match when the green frames were sampled", + ); +}); + +// --------------------------------------------------------------------------- +// Frame labels must reflect where ffmpeg actually sampled, not the request +// --------------------------------------------------------------------------- + +/** `Video keyframe timestamps: 0.000,0.500,1.000,...` (debug-only log line) */ +const KEYFRAME_TIMESTAMPS = /Video keyframe timestamps: ([0-9.,]+)/; + +function reportedTimestamps(output: string): number[] | null { + const match = KEYFRAME_TIMESTAMPS.exec(output); + if (!match) { + return null; + } + const values = match[1] + .split(",") + .map((s) => Number.parseFloat(s)) + .filter((n) => Number.isFinite(n)); + return values.length > 0 ? values : null; +} + +await test("keyframe timestamps reflect ffmpeg's real sample times, not the ideal schedule", async () => { + if (!(await hasFfmpeg())) { + throw new Skip("ffmpeg is unavailable, so no frames can be extracted"); + } + requireOpenAI(); + + // A source with real frames only every 0.5s (2 fps), asked for a much + // denser schedule (16 frames over ~4s => an ideal 0.25s spacing). ffmpeg's + // `-vf select` only guarantees a MINIMUM gap since the last frame it + // actually selected, so it can never produce more than the 8 real frames + // at {0, 0.5, ..., 3.5}s. If those are labelled with the ideal schedule + // instead of their true sample time, the last kept frame is mislabelled + // at 1.75s instead of its real ~3.5s — about half the video's length. + const dir = tempDir("neurolink-lowfps-"); + const clipPath = await makeVideoFile(dir, "lowfps.mp4", 4, ["-r", "2"]); + + const res = await runCLI( + [ + "generate", + "Reply OK.", + "--file", + clipPath, + "--provider", + FRAME_PROVIDER, + "--video-frames", + "16", + "--debug", + ], + { env: { NEUROLINK_LOG_LEVEL: "debug" }, timeoutMs: 240_000 }, + ); + const combined = `${res.stdout}${res.stderr}`; + + const frames = extractedFrameCount(combined); + assert( + frames !== null && frames > 1, + "precondition: several keyframes must have been extracted from the low-fps clip", + ); + + const timestamps = reportedTimestamps(combined); + assert( + timestamps !== null && timestamps.length > 1, + "precondition: the processor must report its per-frame sample times in debug output", + ); + + const lastTimestamp = timestamps![timestamps!.length - 1]; + // Real sampling reaches the last ~0.5s-spaced frame near 3.5s; labelling + // frames with the idealized 16-way schedule instead would cap the last + // one at 1.75s, so 2.5s cleanly separates the two behaviours. + assert( + lastTimestamp > 2.5, + `the last kept frame must be labelled near its real sample time, not the idealized schedule (got ${lastTimestamp}s)`, + ); +}); + +// --------------------------------------------------------------------------- +// Transcription: the discriminating pair +// --------------------------------------------------------------------------- + +const SPOKEN_WORD = /flamingo/i; + +await test("a frame-path provider cannot hear the clip by default", async () => { + await requireFrameExtraction(); + requireOpenAI(); + + const res = await runCLI( + [ + "generate", + "What is the secret word spoken aloud in this video? If you received " + + "no audio and no transcript, reply exactly NO_AUDIO.", + "--file", + CLIP, + "--provider", + FRAME_PROVIDER, + "--debug", + ], + { env: { NEUROLINK_LOG_LEVEL: "debug" }, timeoutMs: 240_000 }, + ); + const combined = `${res.stdout}${res.stderr}`; + + // Precondition: the video must have been processed, or "it did not know + // the word" is equally true of a run where no file was ever attached. + assert( + extractedFrameCount(combined) !== null, + "precondition: the video must have been processed", + ); + // A crashed/errored generate call also has no "flamingo" in its (empty) + // stdout, which would otherwise let a broken run masquerade as a correct + // default-off result. Require the call to have actually succeeded, and + // the model to have given the exact answer the prompt asked for — not + // merely "didn't say the word", which a failure also satisfies. + assert( + res.exitCode === 0, + `the request must succeed for this to be a meaningful result (exit ${res.exitCode})`, + ); + assert( + answerOnly(res.stdout) === "NO_AUDIO", + "without transcription the model must answer exactly NO_AUDIO, not merely omit the word", + ); +}); + +await test("--transcribe-audio carries the speech to a frame-path provider", async () => { + await requireFrameExtraction(); + requireOpenAI(); + + const res = await runCLI( + [ + "generate", + "The attached material includes a section headed Spoken Audio " + + "(transcribed). Read that section and reply with the secret word it " + + "contains, exactly one word and nothing else.", + "--file", + CLIP, + "--provider", + FRAME_PROVIDER, + "--transcribe-audio", + "--temperature", + "0", + "--debug", + ], + { env: { NEUROLINK_LOG_LEVEL: "debug" }, timeoutMs: 300_000 }, + ); + const combined = `${res.stdout}${res.stderr}`; + + assert( + extractedFrameCount(combined) !== null, + "precondition: the video must have been processed", + ); + assert( + SPOKEN_WORD.test(answerOnly(res.stdout)), + "with transcription the spoken word must reach a frame-path provider", + ); +}); + +// --------------------------------------------------------------------------- +// Bedrock has its own `videoOptions` reconstruction (multimodalOptionsBuilder) +// that the three sites above do not cover — verify it separately. +// --------------------------------------------------------------------------- + +function requireBedrock(): void { + if ( + !process.env.AWS_ACCESS_KEY_ID || + !process.env.AWS_SECRET_ACCESS_KEY || + !process.env.AWS_REGION + ) { + throw new Skip("no AWS Bedrock credentials"); + } +} + +await test("the frame budget reaches the processor on the Bedrock branch too", async () => { + await requireFrameExtraction(); + requireBedrock(); + + // Bedrock builds its own multimodal options object + // (src/lib/utils/multimodalOptionsBuilder.ts) instead of going through the + // three sites the tests above cover. The video is still extracted, and the + // "Video processed" log fires, before Bedrock's own API call — whether or + // not that call itself succeeds is irrelevant to this assertion. + const budgeted = await runCLI( + [ + "generate", + "Reply OK.", + "--file", + CLIP, + "--provider", + "bedrock", + "--video-frames", + "1", + "--debug", + ], + { env: { NEUROLINK_LOG_LEVEL: "debug" }, timeoutMs: 240_000 }, + ); + const combined = `${budgeted.stdout}${budgeted.stderr}`; + const requestedFrames = extractedFrameCount(combined); + assert( + requestedFrames !== null, + "precondition: the video must have been processed on the Bedrock branch", + ); + assert( + requestedFrames === 1, + "an explicit frame budget must reach the processor on the Bedrock branch too", + ); +}); + +await test("a transcript that cannot be produced says why", async () => { + await requireFrameExtraction(); + if (!(process.env.GOOGLE_AI_API_KEY || process.env.GEMINI_API_KEY)) { + throw new Skip("no Google AI credentials for the non-OpenAI request"); + } + + // Asking for a transcript with no transcription backend configured. The + // request must still succeed — transcription is additive — and the reason + // must be stated rather than presenting as a clip with no speech in it. + const res = await runCLI( + [ + "generate", + "Reply OK.", + "--file", + CLIP, + "--provider", + "google-ai", + "--transcribe-audio", + "--debug", + ], + { + env: { NEUROLINK_LOG_LEVEL: "debug", OPENAI_API_KEY: "" }, + timeoutMs: 240_000, + }, + ); + const combined = `${res.stdout}${res.stderr}`; + + assert( + extractedFrameCount(combined) !== null, + "precondition: the video must have been processed", + ); + assert( + res.exitCode === 0, + `the request must still succeed without a transcription backend (exit ${res.exitCode})`, + ); + assert( + /No transcript for/.test(combined), + "a skipped transcription must be reported, not silent", + ); + assert( + /OPENAI_API_KEY is not set/.test(combined), + "the report must name the missing backend rather than the clip", + ); +}); + +await runSuite();