Skip to content

feat: export caption sidecars and merge editor follow-ups#686

Open
webadderall wants to merge 10 commits into
mainfrom
feat/export-caption-sidecars
Open

feat: export caption sidecars and merge editor follow-ups#686
webadderall wants to merge 10 commits into
mainfrom
feat/export-caption-sidecars

Conversation

@webadderall

@webadderall webadderall commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

Validation

  • ./node_modules/.bin/vitest --run src/components/video-editor/captionEditing.test.ts
  • ./node_modules/.bin/tsc --noEmit --noUnusedLocals false --noUnusedParameters false

Summary by CodeRabbit

Release Notes

  • New Features

    • Added inline caption editing with automatic SRT/VTT sidecar file generation
    • Enhanced project save workflows with modal dialogs and unsaved-changes prompting supporting rename and copy modes
    • Separated webcam width and height controls for independent dimension adjustment
    • Extended vertical padding range for advanced layout customization
    • Improved export bitrate quality settings
  • Improvements

    • Strengthened project file path validation during saves
    • Enhanced webcam crop preview synchronization
    • Refined caption word normalization for edge cases
  • Localization

    • Added webcam dimension translations across 11+ languages

@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds inline caption cue editing in the video preview, independent webcam width/height controls (replacing a single size slider), advanced vertical padding beyond 100%, modal-driven project Save/Copy/Rename flows with unsaved-changes gating, .srt/.vtt caption sidecar generation at export time, increased export bitrate multipliers, and refactored export-pipeline annotation rendering with sceneTransform propagation and zoom-transition interpolation removal.

Changes

Inline Caption Editing and Sidecar Export

Layer / File(s) Summary
Caption cue normalization and edit utilities
src/components/video-editor/captionEditing.ts, src/components/video-editor/captionLayout.ts, src/components/video-editor/captionEditing.test.ts, electron/ipc/register/export.ts
normalizeCaptionWords and splitCaptionWords filter empty/invalid words so sound-effect captions fall back to cue.text. The IPC export module gains cue/payload TypeScript shapes, timestamp formatters, cue normalization, payload parser, SRT/VTT serializers, and .srt/.vtt sidecar file-writing logic.
Inline caption editing UI in VideoPlayback
src/components/video-editor/VideoPlayback.tsx
Adds onEditAutoCaption prop; introduces caption editing session state, textarea ref, and beginCaptionEdit/commitCaptionEdit/cancelCaptionEdit; renders the caption box as an accessible interactive button and swaps in a dynamically-sized textarea overlay when editing is active.
VideoEditor caption wiring and sidecar toggle
src/components/video-editor/VideoEditor.tsx
Adds handleSaveAutoCaptionEdit callback wired through onEditAutoCaption into VideoPlayback; removes forced auto-caption enabling; defaults includeCaptionSidecar to false; updates export menu showCaptionSidecarOption condition.

Independent Webcam Dimensions and Advanced Vertical Padding

Layer / File(s) Summary
WebcamOverlaySettings width/height and overlay dimension helpers
src/components/video-editor/types.ts, src/components/video-editor/webcamOverlay.ts, src/components/video-editor/webcamOverlay.test.ts
Adds width/height to WebcamOverlaySettings and DEFAULT_WEBCAM_OVERLAY; adds ADVANCED_VERTICAL_PADDING_MAX = 250; implements getWebcamOverlayDimensionsPx, updates getWebcamOverlayPosition to accept independent width/height, and adds getCropMatchedWebcamHeightPercent with all new tests.
Persistence normalization and padded layout
src/components/video-editor/projectPersistence.ts, src/components/video-editor/projectPersistence.test.ts, src/components/video-editor/videoPlayback/layoutUtils.ts, src/components/video-editor/videoPlayback/layoutUtils.test.ts
Normalizes webcam.width/webcam.height independently in persistence; clamps vertical padding to ADVANCED_VERTICAL_PADDING_MAX when unlinked; updates computePaddedLayout centering for advanced directional offset. Tests cover both behaviors.
Settings panel webcam sliders and i18n
src/components/video-editor/SettingsPanel.tsx, src/i18n/locales/*/settings.json
Replaces single webcam size slider with separate Width/Height sliders; updates crop handler to call getCropMatchedWebcamHeightPercent with previewFrame; raises top/bottom padding slider max to ADVANCED_VERTICAL_PADDING_MAX; adds webcamWidth/webcamHeight keys in 10 locales.
WebcamCropControl aspect-ratio-free helpers
src/components/video-editor/WebcamCropControl.tsx
Replaces aspect-ratio-parameterized flipCropHorizontally/resizeCrop with normalizeWebcamCropRegion-based implementations; passes previewFrame through all onCropChange call sites.
VideoPlayback webcam bubble and annotation layout
src/components/video-editor/VideoPlayback.tsx
Computes webcamHeight via getCropMatchedWebcamHeightPercent; switches bubble sizing to getWebcamOverlayDimensionsPx; updates DOM styling to explicit width/height/aspectRatio; tracks annotationSceneTransform/annotationRecordingRect state; wraps annotation overlays in transform container; removes zoom transition interpolation.
Export rendering webcam width/height sizing
src/lib/exporter/frameRenderer.ts, src/lib/exporter/modernFrameRenderer.ts, src/lib/exporter/modernVideoExporter.ts, src/lib/exporter/modernVideoExporter.nativeStaticLayout.test.ts
Updates WebcamLayoutCache to width/height fields; recomputes bubble canvas, mask, and shadow from dimensions.width/height; adds unsupported-rectangular-webcam-overlay skip reason; prefers webcam.width for native static-layout sizePercent.

Modal Project Save / Copy / Rename Flow

Layer / File(s) Summary
IPC save-project-file trust hardening and mode
electron/ipc/register/project.ts
Adds NamedProjectSaveMode type; strengthens existingProjectPath acceptance with extension + trust checks; adds early rejection for untrusted paths; save-project-file-named accepts mode, generates new projectId for copy-to-different-path, gates old-project cleanup behind rename+different-path.
VideoEditor dialog state and promise helpers
src/components/video-editor/VideoEditor.tsx
Introduces dialog types; adds React state for save/unsaved-changes dialogs; adds resolver refs and focus effect; implements openProjectSaveDialog/resolveProjectSaveDialog/openUnsavedChangesDialog/resolveUnsavedChangesDialog.
saveProject, saveProjectWithName, and dialog submit
src/components/video-editor/VideoEditor.tsx
saveProject opens the save dialog when saving "as" or without a target; saveProjectWithName accepts and forwards mode; adds handleProjectSaveDialogSubmit for copy-mode; inline rename uses mode="rename".
Unsaved-changes gate and dialog JSX
src/components/video-editor/VideoEditor.tsx
Adds confirmReplaceSourceWithUnsavedChanges; gates project/library open and media import; renders projectSaveDialog and unsavedChangesDialog in all UI branches (main layout, loading, error).

Export Pipeline: Bitrates, Annotations, Zoom Transition Removal, Cursor

Layer / File(s) Summary
Export bitrate multiplier increases
src/lib/exporter/exportBitrate.ts, src/lib/exporter/exportBitrate.test.ts
"quality" multiplier increases from 0.9 → 1; "balanced" from 0.5 → 0.75; all affected test expectations updated.
Annotation sceneTransform propagation and maskRect layout
src/lib/exporter/modernFrameRenderer.ts, src/lib/exporter/frameRenderer.ts, src/lib/exporter/modernFrameRenderer.test.ts
Adds optional sceneTransform to composeBlurAnnotationFrame; positions annotation sprites relative to layoutCache.maskRect; moves annotationContainer under cameraContainer; forwards sampled sceneTransform through temporal blur rendering; new test verifies blur annotations use sampled transform.
Zoom transition interpolation removal and border-radius helper
src/lib/exporter/modernFrameRenderer.ts, src/lib/exporter/frameRenderer.ts, src/lib/exporter/modernVideoExporter.ts
Removes transition-based interpolation from findDominantRegion destructuring and updateAnimationState; switches border-radius scaling to scalePreviewBorderRadius; passes experimentalNvidiaCudaExport flag to nativeStaticLayoutExport.
Cursor renderer configurable minViewportScale
src/components/video-editor/videoPlayback/cursorRenderer.ts, src/components/video-editor/VideoPlayback.tsx
Adds minViewportScale to CursorRenderConfig; updates getCursorViewportScale to accept it; enables autoGenerateMipmaps on cursor texture; wires minViewportScale: 0 in VideoPlayback Pixi cursor overlay init.

Sequence Diagram(s)

sequenceDiagram
  rect rgba(135, 206, 250, 0.5)
    Note over VideoEditor,VideoPlayback: Caption Editing Flow
  end
  participant VideoEditor
  participant VideoPlayback
  participant captionEditing
  participant electronIPC
  VideoPlayback->>VideoEditor: onEditAutoCaption(target, draftText)
  VideoEditor->>captionEditing: updateCaptionCuesForEditedTarget(target, text)
  captionEditing-->>VideoEditor: updated project cues
  VideoEditor->>VideoEditor: showSuccessToast()

  rect rgba(255, 200, 100, 0.5)
    Note over VideoEditor,electronIPC: Project Save / Copy / Rename Flow
  end
  participant User
  User->>VideoEditor: saveProject(saveAs=true)
  VideoEditor->>VideoEditor: openProjectSaveDialog()
  User->>VideoEditor: handleProjectSaveDialogSubmit(name, mode)
  VideoEditor->>electronIPC: saveProjectFileNamed(name, data, mode)
  alt mode="copy" and different path
    electronIPC->>electronIPC: randomUUID() → new projectId
  else mode="rename"
    electronIPC->>electronIPC: ensureProjectDataHasProjectId()
    electronIPC->>electronIPC: cleanup old project file + thumbnail
  end
  electronIPC-->>VideoEditor: {path, projectId}
  VideoEditor->>VideoEditor: resolveProjectSaveDialog(result)
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

  • webadderallorg/Recordly#669: Implements the same caption sidecar export feature in electron/ipc/register/export.ts with matching .srt/.vtt payload parsing and file-writing logic.
  • webadderallorg/Recordly#392: Directly introduces the inline auto-caption editing infrastructure in VideoEditor, VideoPlayback, and the captionEditing/captionLayout cue-word normalization paths that this PR extends.
  • webadderallorg/Recordly#304: Overlaps with the saveProjectFileNamed IPC handler and saveProjectWithName wiring in VideoEditor.tsx that this PR refactors to add mode support.

Suggested labels

Slop

🐇 A bunny hops through the code with glee,
Adding width AND height for webcam to be free!
Captions now editable, right in the view,
Save As or Copy — pick whichever suits you.
Bitrates go up, zoom transitions depart,
This diff is quite large but it's got all my heart! 🎬

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 2.38% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description provides a clear summary section, validation instructions, and references the upstream PRs merged, but lacks structured detail on motivation, change type, testing guide, and completeness per the template. Expand the description to include explicit Type of Change selection, comprehensive Testing Guide with steps/environment setup, and Related Issue(s) references for better clarity.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main deliverables: caption sidecar export feature plus editor follow-ups from upstream merges.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/export-caption-sidecars
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch feat/export-caption-sidecars

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/components/video-editor/VideoEditor.tsx (1)

628-653: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Gate sidecar export on cue data, not burn-in caption visibility.

Line 628 requires autoCaptionSettings.enabled, but generating captions only updates autoCaptions at Lines 2852-2853. In the default generated-caption flow, valid cues can exist while the sidecar option stays hidden and no payload is sent. Use the validated sidecar cue list as the eligibility check instead.

🐛 Proposed fix
-	const hasCaptionsForSidecar = autoCaptionSettings.enabled && autoCaptions.length > 0;
 	const captionSidecarCues = useMemo(
 		() =>
 			autoCaptions
@@
 				})),
 		[autoCaptions],
 	);
+	const hasCaptionsForSidecar = captionSidecarCues.length > 0;
 	const captionSidecarPayload =
-		hasCaptionsForSidecar && captionSidecarCues.length > 0 && includeCaptionSidecar
+		hasCaptionsForSidecar && includeCaptionSidecar
 			? {
 					format: "both" as const,
 					cues: captionSidecarCues,

Also applies to: 2852-2853, 6081-6083

🤖 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 628 - 653, The
hasCaptionsForSidecar variable on line 628 gates the sidecar export based on
autoCaptionSettings.enabled, but this doesn't guarantee valid captions exist.
Instead of checking autoCaptionSettings.enabled and autoCaptions.length, use the
validated captionSidecarCues array as the eligibility check. Replace the
hasCaptionsForSidecar definition to check if captionSidecarCues has items, so
the sidecar payload is only sent when actual valid cue data is present, not just
when the caption setting is enabled.
🧹 Nitpick comments (2)
src/components/video-editor/projectPersistence.test.ts (1)

6-46: ⚡ Quick win

Add coverage for webcam width/height normalization branches.

This suite only asserts padding behavior, but normalizeProjectEditor in this PR also changed webcam.width/webcam.height fallback/clamp logic. Add focused assertions for explicit width/height, legacy size fallback, and clamping.

Proposed test additions
 describe("normalizeProjectEditor", () => {
+	it("normalizes webcam width/height from explicit values and legacy size fallback", () => {
+		const explicit = normalizeProjectEditor({
+			webcam: { width: 88, height: 36, size: 50 } as never,
+		});
+		expect(explicit.webcam.width).toBe(88);
+		expect(explicit.webcam.height).toBe(36);
+
+		const legacy = normalizeProjectEditor({
+			webcam: { size: 55 } as never,
+		});
+		expect(legacy.webcam.width).toBe(55);
+		expect(legacy.webcam.height).toBe(55);
+	});
+
+	it("clamps webcam width/height to allowed range", () => {
+		const editor = normalizeProjectEditor({
+			webcam: { width: 2, height: 1000 } as never,
+		});
+		expect(editor.webcam.width).toBe(10);
+		expect(editor.webcam.height).toBe(100);
+	});
+
 	it("preserves the extended advanced vertical padding range", () => {
🤖 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/projectPersistence.test.ts` around lines 6 - 46,
The test suite for normalizeProjectEditor currently only covers padding
normalization behavior and is missing test coverage for the webcam width/height
normalization logic that was also changed. Add new test cases to the describe
block that verify the webcam width/height handling in normalizeProjectEditor,
including tests for: explicit width/height values being preserved, legacy size
property fallback when width/height are not provided, and proper clamping of
webcam dimensions to their maximum values. These tests should follow the same
pattern as the existing padding tests using expect and toMatchObject assertions.
electron/ipc/register/project.ts (1)

301-305: 💤 Low value

Consider resolving symlinks for defense-in-depth.

The isPathInsideDirectory check uses path normalization but doesn't resolve symlinks. A symlink inside projectsDir pointing outside would pass this check, and fs.writeFile would follow the symlink to write outside the trusted directory.

In a desktop app where users have full filesystem control, this is low-risk (users would be "attacking themselves"). However, for defense-in-depth, consider using fs.realpath before the directory containment check:

🛡️ Optional hardening
+      const resolvedExistingPath = existingProjectPath
+        ? await fs.realpath(existingProjectPath).catch(() => path.resolve(existingProjectPath))
+        : null;
       const trustedExistingProjectPath = existingProjectPath &&
         path.extname(existingProjectPath).toLowerCase() === `.${PROJECT_FILE_EXTENSION}` &&
-        (isTrustedProjectPath(existingProjectPath) || isPathInsideDirectory(existingProjectPath, projectsDir))
-        ? path.resolve(existingProjectPath)
+        (isTrustedProjectPath(existingProjectPath) || isPathInsideDirectory(resolvedExistingPath!, projectsDir))
+        ? resolvedExistingPath
         : null
🤖 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/project.ts` around lines 301 - 305, The
trustedExistingProjectPath assignment does not resolve symlinks before
validating containment with isPathInsideDirectory, which could allow a symlink
inside projectsDir to escape the trusted boundary. To fix this, resolve the
existingProjectPath using fs.realpathSync before passing it to the
isPathInsideDirectory check, ensuring the actual target location of any symlinks
is validated as being within projectsDir rather than the symlink itself.
🤖 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 `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 3925-3927: In the getCropMatchedWebcamHeightPercent function call,
the second argument is incorrectly passing webcamWidth when it should pass
webcamHeight. This causes the height to be recalculated based on width every
time, overwriting user manual height adjustments. Replace the second webcamWidth
argument with webcamHeight to preserve the user's previously set height value
when applying crop updates.

In `@src/components/video-editor/VideoPlayback.tsx`:
- Around line 2390-2404: The AnnotationOverlay component is receiving the
interactionScale prop but not using it, which prevents the Rnd component from
accounting for zoom scale during drag and resize operations. Add
interactionScale as a property to the AnnotationOverlayProps interface, then
destructure interactionScale in the AnnotationOverlay component signature, and
finally pass this interactionScale value to the Rnd component's scale prop to
ensure drag and resize deltas are calculated correctly under zoom conditions.

In `@src/lib/exporter/modernFrameRenderer.ts`:
- Around line 2569-2571: The getCropMatchedWebcamHeightPercent method is
receiving already-cropped dimensions from renderableWebcamSource after
refreshWebcamFrameCache has created a crop-trimmed cache, which double-applies
crop aspect compensation and skews webcam overlay proportions. Instead of
passing renderableWebcamSource.width and renderableWebcamSource.height to
getCropMatchedWebcamHeightPercent, pass the original uncropped source dimensions
from liveSource (liveSourceWidth and liveSourceHeight) to ensure crop-matched
height is calculated correctly without double compensation.

---

Outside diff comments:
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 628-653: The hasCaptionsForSidecar variable on line 628 gates the
sidecar export based on autoCaptionSettings.enabled, but this doesn't guarantee
valid captions exist. Instead of checking autoCaptionSettings.enabled and
autoCaptions.length, use the validated captionSidecarCues array as the
eligibility check. Replace the hasCaptionsForSidecar definition to check if
captionSidecarCues has items, so the sidecar payload is only sent when actual
valid cue data is present, not just when the caption setting is enabled.

---

Nitpick comments:
In `@electron/ipc/register/project.ts`:
- Around line 301-305: The trustedExistingProjectPath assignment does not
resolve symlinks before validating containment with isPathInsideDirectory, which
could allow a symlink inside projectsDir to escape the trusted boundary. To fix
this, resolve the existingProjectPath using fs.realpathSync before passing it to
the isPathInsideDirectory check, ensuring the actual target location of any
symlinks is validated as being within projectsDir rather than the symlink
itself.

In `@src/components/video-editor/projectPersistence.test.ts`:
- Around line 6-46: The test suite for normalizeProjectEditor currently only
covers padding normalization behavior and is missing test coverage for the
webcam width/height normalization logic that was also changed. Add new test
cases to the describe block that verify the webcam width/height handling in
normalizeProjectEditor, including tests for: explicit width/height values being
preserved, legacy size property fallback when width/height are not provided, and
proper clamping of webcam dimensions to their maximum values. These tests should
follow the same pattern as the existing padding tests using expect and
toMatchObject assertions.
🪄 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: bbab6d85-a7c5-48cb-94dd-db98892ba0ba

📥 Commits

Reviewing files that changed from the base of the PR and between 952d63d and 9f6cb6e.

📒 Files selected for processing (34)
  • electron/ipc/register/export.ts
  • electron/ipc/register/project.ts
  • src/components/video-editor/SettingsPanel.tsx
  • src/components/video-editor/VideoEditor.tsx
  • src/components/video-editor/VideoPlayback.tsx
  • src/components/video-editor/WebcamCropControl.tsx
  • src/components/video-editor/captionEditing.test.ts
  • src/components/video-editor/captionEditing.ts
  • src/components/video-editor/captionLayout.ts
  • src/components/video-editor/projectPersistence.test.ts
  • src/components/video-editor/projectPersistence.ts
  • src/components/video-editor/types.ts
  • src/components/video-editor/videoPlayback/cursorRenderer.ts
  • src/components/video-editor/videoPlayback/layoutUtils.test.ts
  • src/components/video-editor/videoPlayback/layoutUtils.ts
  • src/components/video-editor/webcamOverlay.test.ts
  • src/components/video-editor/webcamOverlay.ts
  • src/i18n/locales/en/settings.json
  • src/i18n/locales/es/settings.json
  • src/i18n/locales/fr/settings.json
  • src/i18n/locales/it/settings.json
  • src/i18n/locales/ko/settings.json
  • src/i18n/locales/nl/settings.json
  • src/i18n/locales/pt-BR/settings.json
  • src/i18n/locales/ru/settings.json
  • src/i18n/locales/zh-CN/settings.json
  • src/i18n/locales/zh-TW/settings.json
  • src/lib/exporter/exportBitrate.test.ts
  • src/lib/exporter/exportBitrate.ts
  • src/lib/exporter/frameRenderer.ts
  • src/lib/exporter/modernFrameRenderer.test.ts
  • src/lib/exporter/modernFrameRenderer.ts
  • src/lib/exporter/modernVideoExporter.nativeStaticLayout.test.ts
  • src/lib/exporter/modernVideoExporter.ts

Comment on lines +3925 to +3927
? getCropMatchedWebcamHeightPercent(
webcamWidth,
webcamWidth,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Preserve user-set webcam height when applying crop updates.

Line 3927 passes webcamWidth as both width and height, which forces auto-height recomputation on every crop edit and overwrites manual height choices. Pass the current webcamHeight as the second argument.

Proposed fix
 											height: previewFrame
 												? getCropMatchedWebcamHeightPercent(
 														webcamWidth,
-														webcamWidth,
+														webcamHeight,
 														previewFrame.width,
 														previewFrame.height,
 														cropRegion,
📝 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.

Suggested change
? getCropMatchedWebcamHeightPercent(
webcamWidth,
webcamWidth,
height: previewFrame
? getCropMatchedWebcamHeightPercent(
webcamWidth,
webcamHeight,
previewFrame.width,
previewFrame.height,
cropRegion,
🤖 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/SettingsPanel.tsx` around lines 3925 - 3927, In
the getCropMatchedWebcamHeightPercent function call, the second argument is
incorrectly passing webcamWidth when it should pass webcamHeight. This causes
the height to be recalculated based on width every time, overwriting user manual
height adjustments. Replace the second webcamWidth argument with webcamHeight to
preserve the user's previously set height value when applying crop updates.

Comment on lines +2390 to +2404
setAnnotationSceneTransform((current) => {
if (
Math.abs(current.scale - appliedTransform.scale) < 0.001 &&
Math.abs(current.x - appliedTransform.x) < 0.1 &&
Math.abs(current.y - appliedTransform.y) < 0.1
) {
return current;
}

return {
scale: appliedTransform.scale,
x: appliedTransform.x,
y: appliedTransform.y,
};
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify AnnotationOverlay consumes interactionScale and forwards it to the Rnd scale prop.
annotation_file="$(fd -i '^AnnotationOverlay\.tsx$' . | head -n 1)"
test -n "$annotation_file" || { echo "AnnotationOverlay.tsx not found"; exit 1; }

# Expect: interactionScale is declared in props/destructuring and <Rnd ... scale={interactionScale}> is present.
rg -n -C 4 'interactionScale|sceneTransform|recordingRect|<Rnd|scale=' "$annotation_file"

Repository: webadderallorg/Recordly

Length of output: 238


🏁 Script executed:

annotation_file="$(fd -i '^AnnotationOverlay\.tsx$' . | head -n 1)"
test -n "$annotation_file" || { echo "AnnotationOverlay.tsx not found"; exit 1; }

# Get file stats and show the component signature and full Rnd element
echo "=== File: $annotation_file ==="
wc -l "$annotation_file"
echo ""
echo "=== Component signature (first 100 lines) ==="
head -n 100 "$annotation_file"

Repository: webadderallorg/Recordly

Length of output: 3305


🏁 Script executed:

annotation_file="$(fd -i '^AnnotationOverlay\.tsx$' . | head -n 1)"
# Search for component export and interface/type definitions
echo "=== Component export and props type ==="
rg -n 'export|interface|type.*Props|function AnnotationOverlay' "$annotation_file" | head -n 20

Repository: webadderallorg/Recordly

Length of output: 183


🏁 Script executed:

annotation_file="$(fd -i '^AnnotationOverlay\.tsx$' . | head -n 1)"
# Get the full Rnd element to see all its props
echo "=== Full Rnd element with props ==="
rg -n '<Rnd' -A 50 "$annotation_file" | head -n 80

Repository: webadderallorg/Recordly

Length of output: 2191


🏁 Script executed:

video_file="$(fd -i '^VideoPlayback\.tsx$' . | head -n 1)"
# Search for where AnnotationOverlay is used/imported and see if interactionScale is passed
rg -n 'AnnotationOverlay|interactionScale' "$video_file" | head -n 50

Repository: webadderallorg/Recordly

Length of output: 228


🏁 Script executed:

video_file="$(fd -i '^VideoPlayback\.tsx$' . | head -n 1)"
# Get context around where AnnotationOverlay component is rendered
rg -n -B 5 -A 15 '<AnnotationOverlay' "$video_file"

Repository: webadderallorg/Recordly

Length of output: 806


🏁 Script executed:

video_file="$(fd -i '^VideoPlayback\.tsx$' . | head -n 1)"
# Get full AnnotationOverlay component with all props
rg -n -A 30 '<AnnotationOverlay' "$video_file" | head -n 40

Repository: webadderallorg/Recordly

Length of output: 1353


🏁 Script executed:

# Also check the lines around 2390-2404 in VideoPlayback to see annotationSceneTransform usage
video_file="$(fd -i '^VideoPlayback\.tsx$' . | head -n 1)"
sed -n '2385,2410p' "$video_file"

Repository: webadderallorg/Recordly

Length of output: 680


🏁 Script executed:

# Check full AnnotationOverlayProps interface definition
annotation_file="$(fd -i '^AnnotationOverlay\.tsx$' . | head -n 1)"
sed -n '7,20p' "$annotation_file"

Repository: webadderallorg/Recordly

Length of output: 537


🏁 Script executed:

# Check if sceneTransform and recordingRect are in the props but I missed them
annotation_file="$(fd -i '^AnnotationOverlay\.tsx$' . | head -n 1)"
rg -n 'sceneTransform|recordingRect|interactionScale' "$annotation_file"

Repository: webadderallorg/Recordly

Length of output: 49


🏁 Script executed:

video_file="$(fd -i '^VideoPlayback\.tsx$' . | head -n 1)"
# Search for where annotationSceneTransform is declared/initialized
rg -n 'annotationSceneTransform|setAnnotationSceneTransform' "$video_file" | head -n 20

Repository: webadderallorg/Recordly

Length of output: 427


🏁 Script executed:

# Check the <Rnd> props in Rnd documentation or react-rnd to understand what scale prop does
# Also check if there are other components using scale prop with Rnd
rg -n 'scale=' | grep -i rnd | head -n 10

Repository: webadderallorg/Recordly

Length of output: 49


🏁 Script executed:

# Check the exact signature/destructuring of AnnotationOverlay function to see if interactionScale is there
annotation_file="$(fd -i '^AnnotationOverlay\.tsx$' . | head -n 1)"
sed -n '19,35p' "$annotation_file"

Repository: webadderallorg/Recordly

Length of output: 551


🏁 Script executed:

# Double check the complete props interface and what's actually being passed from VideoPlayback
annotation_file="$(fd -i '^AnnotationOverlay\.tsx$' . | head -n 1)"
cat "$annotation_file" | head -n 50

Repository: webadderallorg/Recordly

Length of output: 1667


🏁 Script executed:

# Verify lines 3322-3399 in VideoPlayback to understand "Also applies to"
video_file="$(fd -i '^VideoPlayback\.tsx$' . | head -n 1)"
sed -n '3320,3330p' "$video_file"

Repository: webadderallorg/Recordly

Length of output: 425


Add interactionScale prop to AnnotationOverlay and pass it to Rnd's scale prop.

The overlay is CSS-transformed with annotationSceneTransform, but interactionScale is being passed to AnnotationOverlay (line 3398 in VideoPlayback) without being consumed. The AnnotationOverlayProps interface and component destructuring don't include it, and the <Rnd> component doesn't receive it as a scale prop. Without this, drag/resize deltas will be calculated in screen pixels rather than accounting for the zoom scale, causing persisted coordinates to be incorrect under zoom.

🤖 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/VideoPlayback.tsx` around lines 2390 - 2404, The
AnnotationOverlay component is receiving the interactionScale prop but not using
it, which prevents the Rnd component from accounting for zoom scale during drag
and resize operations. Add interactionScale as a property to the
AnnotationOverlayProps interface, then destructure interactionScale in the
AnnotationOverlay component signature, and finally pass this interactionScale
value to the Rnd component's scale prop to ensure drag and resize deltas are
calculated correctly under zoom conditions.

Comment on lines 2569 to 2571
if (needsCacheBackedSource) {
this.refreshWebcamFrameCache(
liveSource,
liveSourceWidth,
liveSourceHeight,
);
this.refreshWebcamFrameCache(liveSource, liveSourceWidth, liveSourceHeight);
const cachedSource = this.getCachedWebcamRenderSource();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use uncropped source dimensions for crop-matched height calculation.

At Line 2923 and Line 2924, getCropMatchedWebcamHeightPercent receives renderableWebcamSource.width/height, but Line 2570 may already have produced a crop-trimmed cache. That double-applies crop aspect compensation and can skew webcam overlay proportions for cropped webcams.

💡 Suggested fix
 		const widthPercent = webcam.width ?? webcam.size ?? 50;
+		const aspectSourceWidth =
+			liveSourceDimensions.width > 0
+				? liveSourceDimensions.width
+				: renderableWebcamSource.width;
+		const aspectSourceHeight =
+			liveSourceDimensions.height > 0
+				? liveSourceDimensions.height
+				: renderableWebcamSource.height;
 		const heightPercent = getCropMatchedWebcamHeightPercent(
 			widthPercent,
 			webcam.height ?? webcam.size ?? 50,
-			renderableWebcamSource.width,
-			renderableWebcamSource.height,
+			aspectSourceWidth,
+			aspectSourceHeight,
 			webcam.cropRegion,
 		);

Also applies to: 2920-2926

🤖 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/lib/exporter/modernFrameRenderer.ts` around lines 2569 - 2571, The
getCropMatchedWebcamHeightPercent method is receiving already-cropped dimensions
from renderableWebcamSource after refreshWebcamFrameCache has created a
crop-trimmed cache, which double-applies crop aspect compensation and skews
webcam overlay proportions. Instead of passing renderableWebcamSource.width and
renderableWebcamSource.height to getCropMatchedWebcamHeightPercent, pass the
original uncropped source dimensions from liveSource (liveSourceWidth and
liveSourceHeight) to ensure crop-matched height is calculated correctly without
double compensation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants