Add caption sidecar export option#669
Open
webadderall wants to merge 1 commit into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds optional caption sidecar (SRT/VTT) file generation when exporting MP4 videos. Type definitions span the Electron API, preload layer, and React types. The Electron IPC handlers validate, serialize, and write caption files alongside exported videos. Export settings thread the ChangesCaption Sidecar Export Feature
Sequence Diagram(s)sequenceDiagram
participant UI as Export UI
participant VE as VideoEditor
participant Settings as resolveExportStartSettings
participant Save as saveBlobExport
participant Electron as electronAPI
participant IPC as IPC Handler
UI->>VE: toggle includeCaptionSidecar
VE->>VE: compute captionSidecarPayload
VE->>Settings: includeCaptionSidecar flag
Settings->>Settings: MP4: keep, else: false
VE->>Save: sidecarForThisExport
Save->>Electron: finalizeExportedVideo + captionSidecar
Electron->>IPC: invoke with captionSidecar payload
IPC->>IPC: parse & serialize cues
IPC->>IPC: write .srt/.vtt files
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The PR spans multiple layers (types, Electron handlers, preload, React state, UI) with moderate complexity. The Electron sidecar implementation (serialization, file writing) and VideoEditor integration (state computation, multi-path wiring) require careful review, but changes are cohesive and follow a clear contract. Export settings resolution and UI toggle are straightforward. Test updates are minor. Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@electron/ipc/register/export.ts`:
- Around line 989-990: The handler currently awaits writeCaptionSidecars(...)
after the video file is already persisted and, on sidecar failure, returns an
overall failure even though the video succeeded; change the flow in the
save-exported-video and write-exported-video-to-path handlers (and the code
paths invoking writeCaptionSidecars) so that the video write/move (the
fs.writeFile or video persistence logic) determines success first, and any
errors from writeCaptionSidecars are caught and handled separately: log the
sidecar error (with context) and return a partial-success/result that indicates
the video was saved while sidecar writing failed (or still return success for
the video write but include sidecar failure detail), rather than treating
sidecar failure as a full export failure. Ensure references to
writeCaptionSidecars, save-exported-video, and write-exported-video-to-path are
updated accordingly so retries won’t re-do already-persisted video writes.
- Around line 347-362: The writeCaptionSidecars function currently writes
sidecar files for any videoPath when payload is present; update it to enforce
MP4-only sidecar writes by checking the parsed extension (from videoPath) and
returning early unless parsed.ext === '.mp4' (or lowercased '.mp4') before
constructing basePath and calling fs.writeFile for serializeSrt/serializeVtt;
keep the existing payload.format branches intact so sidecars are only written
for MP4 outputs.
🪄 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: bcd5caac-c3c1-494a-97d5-2e84fb8f4c49
📒 Files selected for processing (8)
electron/electron-env.d.tselectron/ipc/register/export.tselectron/preload.tssrc/components/video-editor/ExportSettingsMenu.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/exportStartSettings.test.tssrc/components/video-editor/exportStartSettings.tssrc/lib/exporter/types.ts
Comment on lines
+347
to
+362
| async function writeCaptionSidecars(videoPath: string, payload: CaptionSidecarPayload | null) { | ||
| if (!payload) { | ||
| return; | ||
| } | ||
|
|
||
| const parsed = path.parse(videoPath); | ||
| const basePath = path.join(parsed.dir, parsed.name); | ||
|
|
||
| if (payload.format === "srt" || payload.format === "both") { | ||
| await fs.writeFile(`${basePath}.srt`, serializeSrt(payload.cues), "utf8"); | ||
| } | ||
|
|
||
| if (payload.format === "vtt" || payload.format === "both") { | ||
| await fs.writeFile(`${basePath}.vtt`, serializeVtt(payload.cues), "utf8"); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Enforce MP4-only sidecar writes in the main process.
Line 355 currently writes sidecars whenever a payload is present, regardless of output extension. This allows .gif (or any other extension) exports to receive sidecars if an upstream caller sends the payload, which breaks the MP4-only contract.
Suggested fix
async function writeCaptionSidecars(videoPath: string, payload: CaptionSidecarPayload | null) {
if (!payload) {
return;
}
+ if (path.extname(videoPath).toLowerCase() !== ".mp4") {
+ return;
+ }
const parsed = path.parse(videoPath);
const basePath = path.join(parsed.dir, parsed.name);🤖 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/register/export.ts` around lines 347 - 362, The
writeCaptionSidecars function currently writes sidecar files for any videoPath
when payload is present; update it to enforce MP4-only sidecar writes by
checking the parsed extension (from videoPath) and returning early unless
parsed.ext === '.mp4' (or lowercased '.mp4') before constructing basePath and
calling fs.writeFile for serializeSrt/serializeVtt; keep the existing
payload.format branches intact so sidecars are only written for MP4 outputs.
Comment on lines
989
to
+990
| await fs.writeFile(result.filePath, Buffer.from(videoData)); | ||
| await writeCaptionSidecars(result.filePath, sidecarPayload); |
Contributor
There was a problem hiding this comment.
Don’t report full export failure after video persistence succeeds.
In these paths, writeCaptionSidecars(...) is awaited after the video file is already written/moved. If sidecar writing fails, the handler returns failure even though the video is already saved, causing partial-success misreporting and brittle retry behavior.
Suggested direction
- await moveExportedTempFile(tempPath, resolvedPath);
- await writeCaptionSidecars(resolvedPath, sidecarPayload);
- releaseOwnedExportPath(tempPath);
- approveUserPath(resolvedPath);
- return { success: true, path: resolvedPath, canceled: false, message: "Video exported successfully" };
+ await moveExportedTempFile(tempPath, resolvedPath);
+ releaseOwnedExportPath(tempPath);
+ approveUserPath(resolvedPath);
+
+ let sidecarError: string | undefined;
+ try {
+ await writeCaptionSidecars(resolvedPath, sidecarPayload);
+ } catch (error) {
+ sidecarError = error instanceof Error ? error.message : String(error);
+ console.warn("[export] Video saved but caption sidecar write failed:", sidecarError);
+ }
+
+ return {
+ success: true,
+ path: resolvedPath,
+ canceled: false,
+ message: sidecarError
+ ? "Video exported, but caption sidecar files could not be written."
+ : "Video exported successfully",
+ error: sidecarError,
+ };Apply the same pattern to save-exported-video and write-exported-video-to-path so all three flows behave consistently.
Also applies to: 1031-1032, 1089-1130
🤖 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/register/export.ts` around lines 989 - 990, The handler
currently awaits writeCaptionSidecars(...) after the video file is already
persisted and, on sidecar failure, returns an overall failure even though the
video succeeded; change the flow in the save-exported-video and
write-exported-video-to-path handlers (and the code paths invoking
writeCaptionSidecars) so that the video write/move (the fs.writeFile or video
persistence logic) determines success first, and any errors from
writeCaptionSidecars are caught and handled separately: log the sidecar error
(with context) and return a partial-success/result that indicates the video was
saved while sidecar writing failed (or still return success for the video write
but include sidecar failure detail), rather than treating sidecar failure as a
full export failure. Ensure references to writeCaptionSidecars,
save-exported-video, and write-exported-video-to-path are updated accordingly so
retries won’t re-do already-persisted video writes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Credit
Testing
Summary by CodeRabbit
Release Notes