feat: 集成 SenseVoice 字幕引擎，支持多模型选择#775
Conversation
- 模型从 ggml-small (466MB) 升级到 ggml-large-v3 (3.1GB)，中文识别准确率大幅提升 - 下载源从 huggingface.co 改为 hf-mirror.com，解决国内无法下载的问题 - 更新 UI toast 消息
此次提交实现了完整的字幕生成系统重构： 1. 添加SenseVoice语音识别引擎，基于sherpa-onnx实现 2. 重构模型管理系统，支持多模型切换、下载、删除和状态追踪 3. 新增中文本地化翻译与模型选择UI界面 4. 优化ffmpeg二进制文件可执行性检查，适配macOS安全机制 5. 更新构建配置与.gitignore，新增模型打包与忽略规则 6. 新增开发文档索引与模型下载脚本 7. 修复多处代码规范与路径处理问题
- 新增模型注册表 support/models.ts，支持 SenseVoice(通义) + Whisper 多模型 - 集成 sherpa-onnx 运行 SenseVoice ONNX 模型，本地离线推理 - 按时间间隙(≥1s)+字符数(15字)自动切分长句为标准字幕长度 - 内置 SenseVoice int8 模型(239MB)，postinstall 自动下载 - 修复 macOS 16 ffmpeg 权限兼容、伴音文件查找、路径解析 - 字幕边距增加最小值保护，视觉优化
|
|
📝 WalkthroughWalkthroughThis change replaces the Whisper-small-only caption workflow with a multi-model system, adds bundled SenseVoice support, updates caption IPC and editor controls, persists model selection, refreshes macOS binaries, and adds developer documentation and packaging configuration. ChangesCaption model workflow
Native macOS artifact refresh
Documentation and build configuration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant VideoEditor
participant electronAPI
participant registerCaptionHandlers
participant downloadModel
participant SenseVoiceEngine
VideoEditor->>electronAPI: downloadModel(modelId)
electronAPI->>registerCaptionHandlers: invoke download-model
registerCaptionHandlers->>downloadModel: download model files
downloadModel-->>VideoEditor: model-download-progress
VideoEditor->>electronAPI: generateAutoCaptions(modelId)
electronAPI->>registerCaptionHandlers: invoke generate-auto-captions
registerCaptionHandlers->>SenseVoiceEngine: generate captions from extracted audio
SenseVoiceEngine-->>VideoEditor: caption cues and success
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)package.jsonTraceback (most recent call last): src/i18n/locales/en/settings.jsonTraceback (most recent call last): src/i18n/locales/zh-CN/settings.jsonTraceback (most recent call last): Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/video-editor/VideoEditor.tsx (2)
2901-2980: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
handleGenerateAutoCaptionsis missingselectedModelIdfrom its dependency array, and the guard text is misleading for non-Whisper models.
selectedModelIdis read at line 2948 (modelId: selectedModelId) but is not listed in theuseCallbackdeps (2971-2980). Whenever a render changesselectedModelIdwithout also changing one of the other listed deps (e.g.whisperModelPathstaying the same for some edge cases), this memoized callback keeps referencing the previousselectedModelIdand will send a stalemodelIdtogenerateAutoCaptions, causing generation to run with the wrong engine/model. This is exactly the kind of contract this PR is built around (Whisper vs. SenseVoice dispatch keyed offmodelId, perelectron/ipc/captions/generate.ts), so it deserves a hard dependency rather than relying onwhisperModelPathas an implicit proxy.Separately, line 2938's guard text ("Select a Whisper model or download the model first") is now inaccurate since the default/selected model can be SenseVoice — this will confuse users who have SenseVoice selected.
🔧 Suggested fixes
- toast.error("Select a Whisper model or download the model first"); + toast.error("Select a model or download it first"); return; } ... }, [ autoCaptionSettings.language, isGeneratingCaptions, webcam.sourcePath, syncActiveVideoSource, videoPath, videoSourcePath, whisperExecutablePath, whisperModelPath, + selectedModelId, ]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/video-editor/VideoEditor.tsx` around lines 2901 - 2980, Add selectedModelId to the dependency array of handleGenerateAutoCaptions so generateAutoCaptions always receives the current model identifier. Update the missing whisperModelPath guard message to refer to selecting or downloading the selected caption model, accurately covering SenseVoice and other supported models.
2752-2810: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winStale
selectedModelIdclosure in this effect (mount-only deps).The effect has
[]as its dependency array, but both theonModelDownloadProgresslistener and the async model-status loop compare againstselectedModelId(lines 2765, 2799). Since the effect only runs once on mount, these comparisons always use theselectedModelIdvalue captured at first render — not the current selection. If the user switches models viahandleSelectModeland amodel-download-progress/status event later arrives for the newly selected model, this code will silently skip updatingwhisperModelPathbecause it's still comparing against the original (e.g. default"sensevoice-small") id.Use a ref updated on every render (or add
selectedModelIdto deps and accept re-subscription) so the comparison always sees the latest selection.🔧 Suggested fix using a ref
+ const selectedModelIdRef = useRef(selectedModelId); + useEffect(() => { + selectedModelIdRef.current = selectedModelId; + }, [selectedModelId]); + useEffect(() => { const unsubscribe = window.electronAPI.onModelDownloadProgress((state) => { setModelDownloadProgress((prev) => ({ ...prev, [state.modelId]: { status: state.status, progress: state.progress }, })); if (state.status === "downloaded" && state.path) { setModelStatuses((prev) => ({ ...prev, [state.modelId]: { exists: true, path: state.path }, })); - if (state.modelId === selectedModelId) { + if (state.modelId === selectedModelIdRef.current) { setWhisperModelPath(state.path); } } ... if (result.success) { setModelStatuses((prev) => ({ ...prev, [model.id]: { exists: result.exists, path: result.path }, })); - if (result.exists && result.path && model.id === selectedModelId) { + if (result.exists && result.path && model.id === selectedModelIdRef.current) { setWhisperModelPath(result.path); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/video-editor/VideoEditor.tsx` around lines 2752 - 2810, Fix the stale selectedModelId capture in the mount-only model-loading effect by maintaining a ref that is updated with the latest selection on every render. Use that ref in both the onModelDownloadProgress callback and the async model-status loop when deciding whether to update whisperModelPath, while keeping the effect’s single subscription behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dev_docs/INDEX.md`:
- Around line 4-7: Add blank lines immediately before and after the Markdown
table under the “文章” heading in INDEX.md, preserving the table contents and
formatting while satisfying MD058.
In `@electron-builder.json5`:
- Around line 57-76: Remove the duplicate mac configuration keys and merge the
entries into one mac block: retain the entitlements settings once, and combine
the dmg and zip targets for x64/arm64 with the dir target for arm64 in a single
target array.
In `@electron/ipc/captions/generate.ts`:
- Around line 210-236: Wrap the SenseVoice extraction and generation flow in a
try/finally block so wavPath is removed for both expected failures and
unexpected throws, while preserving the existing result handling and return
values. Also update the wavPath filename construction to include a random
suffix, matching the whisper path and preventing concurrent-run collisions.
In `@electron/ipc/captions/sensevoice.ts`:
- Around line 44-66: Update tokensToWords so each word starts at its own token
timestamp, with the first word starting at timestamps[0] rather than an
inter-token midpoint. Preserve each finalized word’s midpoint-based endMs, but
set the next word’s startMs from the current token’s timestamp so SenseVoice
gapToPrev can detect pauses.
- Around line 92-125: Wrap the recognizer lifecycle in an outer try/finally so
recognizer.free() always runs, including when sherpa.readWave(audioPath) fails.
Keep stream.free() in its existing inner cleanup block, and ensure wave loading
occurs inside the outer guarded scope around the recognizer usage.
In `@electron/ipc/captions/whisper.ts`:
- Around line 166-176: Update the auxiliary-file loop around
model.auxiliaryFiles to remove the unconditional duplicate download and use the
same atomic .download temporary-file, download, and rename workflow as the
primary model. Check readability only on the final auxPath, download to the
temporary path when absent, and rename it to auxPath only after a successful
download so interrupted transfers cannot leave a seemingly valid partial file.
- Around line 71-74: Update the redirect handling in request to resolve
response.headers.location against currentUrl before recursively calling request,
using URL resolution that supports both absolute and relative Location values.
Pass the resolved URL to the next request while preserving redirectCount,
resolve, and reject behavior.
In `@electron/ipc/constants.ts`:
- Line 22: Update the Whisper model constants in constants.ts: remove unused
Whisper exports, or rename WHISPER_SMALL_MODEL_PATH to accurately represent the
ggml-large-v3.bin model and update all references accordingly. Ensure no
exported symbol retains a misleading “SMALL” name for the large-v3 model.
In `@package.json`:
- Around line 54-55: Remove the sherpa-onnx-darwin-arm64 entry from dependencies
in package.json, leaving sherpa-onnx as the sole direct dependency so its
built-in optional dependency mechanism selects the correct platform binary.
In `@scripts/download-bundled-models.mjs`:
- Around line 129-137: Update the download flow around downloadFile to download
each file only once to tempDest, then atomically rename tempDest to dest using
rename from node:fs/promises. Remove the rm, writeFile, and second downloadFile
operations while preserving the existing error handling and cleanup behavior.
---
Outside diff comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 2901-2980: Add selectedModelId to the dependency array of
handleGenerateAutoCaptions so generateAutoCaptions always receives the current
model identifier. Update the missing whisperModelPath guard message to refer to
selecting or downloading the selected caption model, accurately covering
SenseVoice and other supported models.
- Around line 2752-2810: Fix the stale selectedModelId capture in the mount-only
model-loading effect by maintaining a ref that is updated with the latest
selection on every render. Use that ref in both the onModelDownloadProgress
callback and the async model-status loop when deciding whether to update
whisperModelPath, while keeping the effect’s single subscription behavior
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 142722ac-2dd1-4f64-9ef9-fabe23fcfc50
⛔ Files ignored due to path filters (2)
dev_docs/journal.dbis excluded by!**/*.dbpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (29)
.gitignoredev_docs/INDEX.mdelectron-builder.json5electron/electron-env.d.tselectron/ipc/captions/engine.tselectron/ipc/captions/generate.tselectron/ipc/captions/models.tselectron/ipc/captions/sensevoice.tselectron/ipc/captions/sherpa-onnx.d.tselectron/ipc/captions/whisper.tselectron/ipc/constants.tselectron/ipc/ffmpeg/binary.tselectron/ipc/register/captions.tselectron/native/bin/darwin-arm64/recordly-native-cursor-monitorelectron/native/bin/darwin-arm64/recordly-system-cursorselectron/native/bin/darwin-arm64/recordly-window-listelectron/native/bin/darwin-x64/recordly-native-cursor-monitorelectron/native/bin/darwin-x64/recordly-system-cursorselectron/native/bin/darwin-x64/recordly-window-listelectron/preload.tspackage.jsonscripts/download-bundled-models.mjssrc/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/captionStyle.tssrc/components/video-editor/editorPreferences.tssrc/i18n/locales/en/settings.jsonsrc/i18n/locales/zh-CN/settings.jsonvite.config.ts
| ## 文章 | ||
| | 日期 | 标题 | 类型 | | ||
| |------|------|------| | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Surround the table with blank lines.
Add a blank line before and after the table to satisfy Markdown linting rule MD058.
Proposed fix
## 文章
+
| 日期 | 标题 | 类型 |
|------|------|------|
+
## 命令📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## 文章 | |
| | 日期 | 标题 | 类型 | | |
| |------|------|------| | |
| ## 文章 | |
| | 日期 | 标题 | 类型 | | |
| |------|------|------| | |
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 5-5: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@dev_docs/INDEX.md` around lines 4 - 7, Add blank lines immediately before and
after the Markdown table under the “文章” heading in INDEX.md, preserving the
table contents and formatting while satisfying MD058.
Source: Linters/SAST tools
| "entitlements": "build/entitlements.mac.plist", | ||
| "entitlementsInherit": "build/entitlements.mac.inherit.plist", | ||
| "target": [ | ||
| { | ||
| "target": "dmg", | ||
| "arch": ["x64", "arm64"] | ||
| }, | ||
| { | ||
| "target": "zip", | ||
| "arch": ["x64", "arm64"] | ||
| } | ||
| { | ||
| "target": "dmg", | ||
| "arch": ["x64", "arm64"] | ||
| }, | ||
| { | ||
| "target": "zip", | ||
| "arch": ["x64", "arm64"] | ||
| } | ||
| ], | ||
| "entitlements": "build/entitlements.mac.plist", | ||
| "entitlementsInherit": "build/entitlements.mac.inherit.plist", | ||
| "target": [ | ||
| { | ||
| "target": "dir", | ||
| "arch": ["arm64"] | ||
| } | ||
| ], |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Duplicate mac keys clobber the dmg/zip targets.
entitlements, entitlementsInherit, and target are each declared twice in the mac block. With last-key-wins JSON parsing, the effective target becomes only [{ "target": "dir", "arch": ["arm64"] }], silently dropping the dmg/zip (x64 + arm64) distributables from release builds. Merge into a single target array.
🛠️ Proposed fix
"mac": {
"hardenedRuntime": true,
"entitlements": "build/entitlements.mac.plist",
"entitlementsInherit": "build/entitlements.mac.inherit.plist",
"target": [
- {
- "target": "dmg",
- "arch": ["x64", "arm64"]
- },
- {
- "target": "zip",
- "arch": ["x64", "arm64"]
- }
- ],
- "entitlements": "build/entitlements.mac.plist",
- "entitlementsInherit": "build/entitlements.mac.inherit.plist",
- "target": [
- {
- "target": "dir",
- "arch": ["arm64"]
- }
- ],
+ { "target": "dmg", "arch": ["x64", "arm64"] },
+ { "target": "zip", "arch": ["x64", "arm64"] },
+ { "target": "dir", "arch": ["arm64"] }
+ ],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "entitlements": "build/entitlements.mac.plist", | |
| "entitlementsInherit": "build/entitlements.mac.inherit.plist", | |
| "target": [ | |
| { | |
| "target": "dmg", | |
| "arch": ["x64", "arm64"] | |
| }, | |
| { | |
| "target": "zip", | |
| "arch": ["x64", "arm64"] | |
| } | |
| { | |
| "target": "dmg", | |
| "arch": ["x64", "arm64"] | |
| }, | |
| { | |
| "target": "zip", | |
| "arch": ["x64", "arm64"] | |
| } | |
| ], | |
| "entitlements": "build/entitlements.mac.plist", | |
| "entitlementsInherit": "build/entitlements.mac.inherit.plist", | |
| "target": [ | |
| { | |
| "target": "dir", | |
| "arch": ["arm64"] | |
| } | |
| ], | |
| "entitlements": "build/entitlements.mac.plist", | |
| "entitlementsInherit": "build/entitlements.mac.inherit.plist", | |
| "target": [ | |
| { "target": "dmg", "arch": ["x64", "arm64"] }, | |
| { "target": "zip", "arch": ["x64", "arm64"] }, | |
| { "target": "dir", "arch": ["arm64"] } | |
| ], |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron-builder.json5` around lines 57 - 76, Remove the duplicate mac
configuration keys and merge the entries into one mac block: retain the
entitlements settings once, and combine the dmg and zip targets for x64/arm64
with the dir target for arm64 in a single target array.
| const ffmpegPath = getFfmpegBinaryPath(); | ||
| const tempDir = app.getPath("temp"); | ||
| const wavPath = path.join(tempDir, `sensevoice-${Date.now()}.wav`); | ||
|
|
||
| // Use the same audio extraction as whisper path | ||
| const audioSource = await extractCaptionAudioSource({ | ||
| videoPath: options.videoPath, | ||
| ffmpegPath, | ||
| wavPath, | ||
| }); | ||
|
|
||
| const result = await engine.generate({ | ||
| audioPath: wavPath, | ||
| modelPath: options.whisperModelPath, | ||
| model, | ||
| language: options.language, | ||
| tempDir, | ||
| }); | ||
|
|
||
| if (!result.success) { | ||
| await fs.rm(wavPath, { force: true }).catch(() => undefined); | ||
| throw new Error(result.error || "SenseVoice caption generation failed."); | ||
| } | ||
|
|
||
| await fs.rm(wavPath, { force: true }).catch(() => undefined); | ||
|
|
||
| return { success: true, cues: result.cues, audioSourceLabel: audioSource.label }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
SenseVoice temp WAV can leak on unexpected throw.
Cleanup of wavPath only runs after a successful/failed result, but if extractCaptionAudioSource or engine.generate throws, the temp WAV is never removed. The whisper path guards this with finally; wrap the SenseVoice extraction/generation in try/finally for parity. Minor: the sensevoice-${Date.now()}.wav name (line 212) can collide under concurrent runs — the whisper path appends a random suffix.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile, spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron/ipc/captions/generate.ts` around lines 210 - 236, Wrap the
SenseVoice extraction and generation flow in a try/finally block so wavPath is
removed for both expected failures and unexpected throws, while preserving the
existing result handling and return values. Also update the wavPath filename
construction to include a random suffix, matching the whisper path and
preventing concurrent-run collisions.
| const nextTsMs = | ||
| i + 1 < tokens.length ? Math.round(timestamps[i + 1] * 1000) : tsMs + 200; | ||
| const endMs = Math.round((tsMs + nextTsMs) / 2); | ||
|
|
||
| // Finalize previous word | ||
| if (buffer) { | ||
| words.push({ text: buffer, startMs, endMs }); | ||
| } | ||
|
|
||
| // Start new word | ||
| if (tok.startsWith(" ")) { | ||
| buffer = tok.slice(1); | ||
| } else { | ||
| buffer = tok; | ||
| } | ||
| startMs = endMs; | ||
| } | ||
|
|
||
| // Finalize last word | ||
| if (buffer) { | ||
| const lastTs = Math.round(timestamps[tokens.length - 1] * 1000); | ||
| words.push({ text: buffer, startMs, endMs: lastTs + 200 }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file list =="
git ls-files | rg '(^|/)electron/ipc/captions/sensevoice\.ts$|(^|/)electron/.+captions|(^|/)sensevoice'
echo
echo "== outline sensevoice.ts =="
ast-grep outline electron/ipc/captions/sensevoice.ts --view expanded || true
echo
echo "== relevant lines from sensevoice.ts =="
nl -ba electron/ipc/captions/sensevoice.ts | sed -n '1,220p'
echo
echo "== search downstream cue builder / gapToPrev =="
rg -n "gapToPrev|split at pauses|SenseVoice|tokensToWords|recognizer\.free|free\(\)" electron/ipc/captions -SRepository: webadderallorg/Recordly
Length of output: 1107
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== sensevoice.ts (1-220) =="
sed -n '1,220p' electron/ipc/captions/sensevoice.ts
echo
echo "== generate.ts (1-260) =="
sed -n '1,260p' electron/ipc/captions/generate.ts
echo
echo "== engine.ts (1-220) =="
sed -n '1,220p' electron/ipc/captions/engine.tsRepository: webadderallorg/Recordly
Length of output: 15019
Word timings are collapsed together, so pause-based splits never trigger.
tokensToWords sets each new word’s startMs to the previous word’s endMs, so gapToPrev in the SenseVoice cue builder is always 0. That means the ≥1s pause split never fires and captions only break on the length cap. The first word also starts at the first inter-token midpoint, not the first token timestamp. Preserve the next word’s actual start time instead of chaining starts from the prior end.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron/ipc/captions/sensevoice.ts` around lines 44 - 66, Update
tokensToWords so each word starts at its own token timestamp, with the first
word starting at timestamps[0] rather than an inter-token midpoint. Preserve
each finalized word’s midpoint-based endMs, but set the next word’s startMs from
the current token’s timestamp so SenseVoice gapToPrev can detect pauses.
| const recognizer = sherpa.createOfflineRecognizer({ | ||
| featConfig: { sampleRate: 16000, featureDim: 80 }, | ||
| modelConfig: { | ||
| senseVoice: { | ||
| model: modelPath, | ||
| language: svLanguage, | ||
| useInverseTextNormalization: 1, | ||
| }, | ||
| tokens: tokensPath, | ||
| }, | ||
| decodingMethod: "greedy_search", | ||
| }); | ||
|
|
||
| const wave = sherpa.readWave(audioPath); | ||
|
|
||
| let resultText = ""; | ||
| let resultTokens: string[] = []; | ||
| let resultTimestamps: number[] = []; | ||
| let recognizerError: string | null = null; | ||
|
|
||
| const stream = recognizer.createStream(); | ||
| try { | ||
| stream.acceptWaveform(wave.sampleRate, wave.samples); | ||
| recognizer.decode(stream); | ||
| const raw = recognizer.getResult(stream) as Record<string, unknown>; | ||
| resultText = (raw.text as string) ?? ""; | ||
| resultTokens = (raw.tokens as string[]) ?? []; | ||
| resultTimestamps = (raw.timestamps as number[]) ?? []; | ||
| } catch (error) { | ||
| recognizerError = error instanceof Error ? error.message : String(error); | ||
| } finally { | ||
| stream.free(); | ||
| } | ||
| recognizer.free(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="electron/ipc/captions/sensevoice.ts"
echo '--- file outline ---'
ast-grep outline "$file" --view expanded || true
echo
echo '--- relevant lines ---'
nl -ba "$file" | sed -n '1,220p'
echo
echo '--- search for readWave / free usage ---'
rg -n "readWave|free\(\)|createOfflineRecognizer|createStream\(" "$file" "electron/ipc/captions" || trueRepository: webadderallorg/Recordly
Length of output: 652
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="electron/ipc/captions/sensevoice.ts"
echo '--- relevant lines 70-170 ---'
sed -n '70,170p' "$file"
echo
echo '--- all free/readWave occurrences in repository ---'
rg -n "readWave|free\(\)|createOfflineRecognizer|createStream\(" electron/ipc/captions || trueRepository: webadderallorg/Recordly
Length of output: 4031
Wrap the recognizer in an outer cleanup block. sherpa.readWave(audioPath) runs before try/finally, so a malformed WAV can skip recognizer.free() and leak the native resource. Move wave loading inside the guarded scope or add an outer try/finally.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron/ipc/captions/sensevoice.ts` around lines 92 - 125, Wrap the
recognizer lifecycle in an outer try/finally so recognizer.free() always runs,
including when sherpa.readWave(audioPath) fails. Keep stream.free() in its
existing inner cleanup block, and ensure wave loading occurs inside the outer
guarded scope around the recognizer usage.
| if (statusCode >= 300 && statusCode < 400 && response.headers.location) { | ||
| response.resume(); | ||
| if (redirectCount >= 5) { | ||
| reject(new Error("Too many redirects while downloading Whisper model.")); | ||
| return; | ||
| } | ||
|
|
||
| const nextUrl = new URL(location, currentUrl).toString(); | ||
| void request(nextUrl, redirectCount + 1) | ||
| .then(resolve) | ||
| .catch(reject); | ||
| return; | ||
| return request(response.headers.location, redirectCount + 1).then(resolve, reject); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Resolve relative HTTP redirects to prevent invalid URL crashes.
While many CDNs return absolute URLs for redirects, HTTP spec permits relative URLs (e.g., /downloads/file.bin) in the Location header. Passing a relative URL directly to https.get will throw a synchronous ERR_INVALID_URL crash.
Parse the redirect URL relative to the currentUrl before making the next request.
🛠️ Proposed fix
if (statusCode >= 300 && statusCode < 400 && response.headers.location) {
response.resume();
- return request(response.headers.location, redirectCount + 1).then(resolve, reject);
+ const nextUrl = new URL(response.headers.location, currentUrl).toString();
+ return request(nextUrl, redirectCount + 1).then(resolve, reject);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (statusCode >= 300 && statusCode < 400 && response.headers.location) { | |
| response.resume(); | |
| if (redirectCount >= 5) { | |
| reject(new Error("Too many redirects while downloading Whisper model.")); | |
| return; | |
| } | |
| const nextUrl = new URL(location, currentUrl).toString(); | |
| void request(nextUrl, redirectCount + 1) | |
| .then(resolve) | |
| .catch(reject); | |
| return; | |
| return request(response.headers.location, redirectCount + 1).then(resolve, reject); | |
| } | |
| if (statusCode >= 300 && statusCode < 400 && response.headers.location) { | |
| response.resume(); | |
| const nextUrl = new URL(response.headers.location, currentUrl).toString(); | |
| return request(nextUrl, redirectCount + 1).then(resolve, reject); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron/ipc/captions/whisper.ts` around lines 71 - 74, Update the redirect
handling in request to resolve response.headers.location against currentUrl
before recursively calling request, using URL resolution that supports both
absolute and relative Location values. Pass the resolved URL to the next request
while preserving redirectCount, resolve, and reject behavior.
| for (const aux of model.auxiliaryFiles) { | ||
| const auxPath = path.join(storageDir, aux.fileName); | ||
| try { | ||
| await fs.access(auxPath, fsConstants.R_OK); | ||
| continue; // Already exists | ||
| } catch { | ||
| await downloadFileWithProgress(aux.url, auxPath, () => undefined); | ||
| } | ||
| await downloadFileWithProgress(aux.url, auxPath, () => {}); | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Fix duplicate download logic and prevent partial file corruption for auxiliary files.
This block contains two distinct critical issues:
- Duplicate redundant download: There is an unconditional
await downloadFileWithProgress(aux.url, auxPath, () => {});immediately after thetry/catchblock. If thefs.accesscheck fails, the file is downloaded inside thecatchblock, and then mistakenly downloaded a second time. - Permanent partial-file corruption: Auxiliary files are downloaded directly to their final destination path (
auxPath). If a network error or interruption occurs mid-download, a truncated file is left behind. On the next launch,fs.access(auxPath, fsConstants.R_OK)will erroneously detect the file as fully valid, skip the download, and pass a corrupted model to the transcription engine, breaking caption generation.
Auxiliary files must use the same atomic .download temp-file + rename mechanism as the primary model.
🛡️ Proposed fix
if (model.auxiliaryFiles) {
for (const aux of model.auxiliaryFiles) {
const auxPath = path.join(storageDir, aux.fileName);
+ const tempAuxPath = `${auxPath}.download`;
try {
await fs.access(auxPath, fsConstants.R_OK);
continue; // Already exists
} catch {
- await downloadFileWithProgress(aux.url, auxPath, () => undefined);
+ await fs.rm(tempAuxPath, { force: true }).catch(() => undefined);
+ await downloadFileWithProgress(aux.url, tempAuxPath, () => undefined);
+ await fs.rename(tempAuxPath, auxPath);
}
- await downloadFileWithProgress(aux.url, auxPath, () => {});
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const aux of model.auxiliaryFiles) { | |
| const auxPath = path.join(storageDir, aux.fileName); | |
| try { | |
| await fs.access(auxPath, fsConstants.R_OK); | |
| continue; // Already exists | |
| } catch { | |
| await downloadFileWithProgress(aux.url, auxPath, () => undefined); | |
| } | |
| await downloadFileWithProgress(aux.url, auxPath, () => {}); | |
| } | |
| } | |
| if (model.auxiliaryFiles) { | |
| for (const aux of model.auxiliaryFiles) { | |
| const auxPath = path.join(storageDir, aux.fileName); | |
| const tempAuxPath = `${auxPath}.download`; | |
| try { | |
| await fs.access(auxPath, fsConstants.R_OK); | |
| continue; // Already exists | |
| } catch { | |
| await fs.rm(tempAuxPath, { force: true }).catch(() => undefined); | |
| await downloadFileWithProgress(aux.url, tempAuxPath, () => undefined); | |
| await fs.rename(tempAuxPath, auxPath); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron/ipc/captions/whisper.ts` around lines 166 - 176, Update the
auxiliary-file loop around model.auxiliaryFiles to remove the unconditional
duplicate download and use the same atomic .download temporary-file, download,
and rename workflow as the primary model. Check readability only on the final
auxPath, download to the temporary path when absent, and rename it to auxPath
only after a successful download so interrupted transfers cannot leave a
seemingly valid partial file.
| "https://hf-mirror.com/ggerganov/whisper.cpp/resolve/main/ggml-large-v3.bin"; | ||
| export const WHISPER_MODEL_DIR = path.join(USER_DATA_PATH, "whisper"); | ||
| export const WHISPER_SMALL_MODEL_PATH = path.join(WHISPER_MODEL_DIR, "ggml-small.bin"); | ||
| export const WHISPER_SMALL_MODEL_PATH = path.join(WHISPER_MODEL_DIR, "ggml-large-v3.bin"); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP '\bWHISPER_(SMALL_MODEL_PATH|MODEL_DOWNLOAD_URL|MODEL_DIR)\b' -g '!**/constants.ts'Repository: webadderallorg/Recordly
Length of output: 161
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## files\n'
git ls-files 'electron/ipc/constants.ts' 'electron/ipc/captions/models.ts' 'electron/ipc/**' 'src/**' | sed -n '1,200p'
printf '\n## constants.ts\n'
cat -n electron/ipc/constants.ts
printf '\n## models.ts\n'
cat -n electron/ipc/captions/models.ts
printf '\n## references to WHISPER constants\n'
rg -n '\bWHISPER_(SMALL_MODEL_PATH|MODEL_DOWNLOAD_URL|MODEL_DIR)\b' .
printf '\n## references to ggml-large-v3.bin\n'
rg -n 'ggml-large-v3\.bin|small model|WHISPER_SMALL_MODEL_PATH' electron src .Repository: webadderallorg/Recordly
Length of output: 20219
Drop the stale Whisper exports. WHISPER_SMALL_MODEL_PATH points to ggml-large-v3.bin; either rename it to match the large-v3 model or remove the unused Whisper constants.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@electron/ipc/constants.ts` at line 22, Update the Whisper model constants in
constants.ts: remove unused Whisper exports, or rename WHISPER_SMALL_MODEL_PATH
to accurately represent the ggml-large-v3.bin model and update all references
accordingly. Ensure no exported symbol retains a misleading “SMALL” name for the
large-v3 model.
| "sherpa-onnx": "^1.13.4", | ||
| "sherpa-onnx-darwin-arm64": "^1.13.4", |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Remove platform-specific binding from dependencies to fix cross-platform installation.
Adding sherpa-onnx-darwin-arm64 directly to dependencies will cause a hard installation failure (npm ERR! code EBADPLATFORM) when running npm install on Windows, Linux, or Intel Macs, because that package enforces "os": ["darwin"] and "cpu": ["arm64"] constraints in its own package.json.
sherpa-onnx already handles platform-specific binaries securely via its own optionalDependencies mechanism, which automatically selects and installs the correct binary for the user's OS and architecture.
🐛 Proposed fix
"ffprobe-static": "^3.1.0",
"sherpa-onnx": "^1.13.4",
- "sherpa-onnx-darwin-arm64": "^1.13.4",
"uiohook-napi": "^1.5.4"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "sherpa-onnx": "^1.13.4", | |
| "sherpa-onnx-darwin-arm64": "^1.13.4", | |
| "sherpa-onnx": "^1.13.4", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` around lines 54 - 55, Remove the sherpa-onnx-darwin-arm64 entry
from dependencies in package.json, leaving sherpa-onnx as the sole direct
dependency so its built-in optional dependency mechanism selects the correct
platform binary.
| const tempDest = `${dest}.download`; | ||
| try { | ||
| await downloadFile(file.url, tempDest); | ||
| // Rename temp -> final | ||
| await rm(tempDest, { force: true }); | ||
| await writeFile(tempDest, ""); // touch | ||
| await rm(tempDest); | ||
| // Re-download to proper path | ||
| await downloadFile(file.url, dest); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Model is downloaded twice; the temp file is discarded instead of renamed.
The comment says "Rename temp -> final", but the code downloads to tempDest, deletes it, touches/deletes again, then re-downloads the same URL to dest. Every bundled file (including the ~239 MB model.int8.onnx) is fetched twice. Download once to the temp path and atomically rename it into place.
♻️ Proposed fix
const tempDest = `${dest}.download`;
try {
await downloadFile(file.url, tempDest);
- // Rename temp -> final
- await rm(tempDest, { force: true });
- await writeFile(tempDest, ""); // touch
- await rm(tempDest);
- // Re-download to proper path
- await downloadFile(file.url, dest);
+ // Atomically move temp -> final
+ await rename(tempDest, dest);
} catch (error) {Add rename to the node:fs/promises import on line 16.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/download-bundled-models.mjs` around lines 129 - 137, Update the
download flow around downloadFile to download each file only once to tempDest,
then atomically rename tempDest to dest using rename from node:fs/promises.
Remove the rm, writeFile, and second downloadFile operations while preserving
the existing error handling and cleanup behavior.
1 participant
解决的问题
1. 字幕引擎扩展（非硬编码）
原 Recordly 只支持 whisper.cpp，模型和引擎都硬编码。本次新增模型注册表 + 引擎抽象，支持多模型自由切换。
electron/ipc/captions/models.ts定义模型注册表，每个模型标注 engine 类型（whisper / sensevoice）electron/ipc/captions/engine.ts引擎抽象接口electron/ipc/captions/sensevoice.tsSenseVoice 引擎实现（基于 sherpa-onnx）2. 中文语音识别显著提升
集成阿里通义 SenseVoice 模型，中文识别效果远超 Whisper Large V3。
npm run download:models或 postinstall 自动下载3. 字幕时间轴对齐
SenseVoice 返回逐词时间戳，按时间间隙 + 字长自动切分：
4. 伴音文件路径修复
resolveCaptionAudioCandidates之前只扫描视频文件和摄像头。新增从视频同目录查找*.mic.wav/*.system.wav伴音文件，优先作为音频源。5. macOS 16 兼容性
ffmpeg-static 在 macOS 16 因未签名二进制被系统拦截。
getFfmpegBinaryPath新增 spawnSync 验证，自动 fallback 到系统 /opt/homebrew/bin/ffmpeg。6. 字幕边距优化
字幕框 padding 从纯 fontSize 比例改为
Math.max(比例, 最小值)，避免字体缩小时边距消失。变更文件
electron/ipc/captions/models.ts— 模型注册表（新增）electron/ipc/captions/engine.ts— 引擎接口（新增）electron/ipc/captions/sensevoice.ts— SenseVoice 引擎（新增）electron/ipc/captions/sherpa-onnx.d.ts— sherpa-onnx 类型声明（新增）electron/ipc/captions/generate.ts— 引擎分发、音频候选electron/ipc/captions/whisper.ts— 多模型下载/状态管理electron/ipc/register/captions.ts— 新增多模型 IPC handlerselectron/preload.ts— 多模型 API 桥接electron/electron-env.d.ts— 类型定义electron/ipc/ffmpeg/binary.ts— macOS 16 执行性验证electron-builder.json5— 内置模型打包scripts/download-bundled-models.mjs— 模型下载脚本（新增）src/components/video-editor/VideoEditor.tsx— 模型选择状态src/components/video-editor/SettingsPanel.tsx— 模型选择 UIsrc/components/video-editor/editorPreferences.ts— 持久化选中模型src/components/video-editor/captionStyle.ts— 边距最小值保护vite.config.ts— sherpa-onnx externalspackage.json— sherpa-onnx 依赖依赖变更
sherpa-onnx,sherpa-onnx-darwin-arm64onnxruntime-nodeSummary by CodeRabbit
New Features
Bug Fixes