Skip to content

Fix export menu source resolution detection#621

Open
LILQK wants to merge 1 commit into
webadderallorg:mainfrom
LILQK:codex/export-resolution-clean
Open

Fix export menu source resolution detection#621
LILQK wants to merge 1 commit into
webadderallorg:mainfrom
LILQK:codex/export-resolution-clean

Conversation

@LILQK
Copy link
Copy Markdown

@LILQK LILQK commented May 29, 2026

Summary

  • Track loaded source video dimensions in editor state when metadata loads
  • Use the real source dimensions for export menu MP4/GIF size calculations
  • Avoid falling back to 1920x1080 after loading 2K/4K recordings

Tests

  • npx tsc --noEmit
  • npx vitest run src/components/video-editor/exportDimensions.test.ts src/components/video-editor/exportStartSettings.test.ts

Summary by CodeRabbit

  • Bug Fixes
    • Improved tracking and handling of source video dimensions during editing and export operations
    • Enhanced fallback behavior when video metadata is unavailable, using default dimensions (1920×1080)
    • Refined preview aspect ratio calculations to better reflect actual source video dimensions
    • Improved GIF and MP4 export dimension handling to use actual source video data

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 29, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 75ce2272-295d-486b-a6b0-efc613cd45f0

📥 Commits

Reviewing files that changed from the base of the PR and between 966b63e and 59920ca.

📒 Files selected for processing (2)
  • src/components/video-editor/VideoEditor.tsx
  • src/components/video-editor/VideoPlayback.tsx

📝 Walkthrough

Walkthrough

VideoPlayback now emits source video intrinsic dimensions (width, height, duration) via an optional onVideoMetadataChange callback when metadata loads. VideoEditor subscribes to this callback, tracks the dimensions in state with a 1920×1080 fallback, and synchronizes the state when the video path changes. Export and preview logic now prefer the tracked dimensions for sizing GIF/MP4 outputs and preview aspect ratios.

Changes

Source Video Metadata Tracking

Layer / File(s) Summary
VideoPlayback metadata emission
src/components/video-editor/VideoPlayback.tsx
New optional onVideoMetadataChange callback prop in VideoPlaybackProps receives intrinsic video width, height, and duration. The prop is destructured and invoked in the onLoadedMetadata handler with metadata from the source video element.
VideoEditor state and fallback dimensions
src/components/video-editor/VideoEditor.tsx
State tracks sourceVideoDimensions (width/height or null). Derived value effectiveSourceVideoDimensions provides a 1920×1080 fallback when metadata is unavailable.
VideoEditor metadata synchronization
src/components/video-editor/VideoEditor.tsx
Effect resets sourceVideoDimensions to null on videoPath change. Memoized handleVideoMetadataChange validates and stores metadata updates. Callback is wired to VideoPlayback via the new prop.
Export and render dimension usage
src/components/video-editor/VideoEditor.tsx
GIF/MP4 desired source dimensions, export start settings (sourceWidth/sourceHeight), and preview aspect-ratio calculations now use sourceVideoDimensions (or fallback) instead of reading directly from the preview element. Dependency arrays updated to track dimension changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • webadderallorg/Recordly#567: Refactors export-start settings into resolveExportStartSettings using the sourceVideoDimensions introduced in this PR.
  • webadderallorg/Recordly#462: Also modifies MP4 target dimension derivation logic in VideoEditor; overlaps with dimension-calculation changes in this PR.

Suggested labels

Checked

Poem

🐰 A rabbit hops through video streams, tracking width and height like dreams, ✨
Metadata flows from play to edit's hand,
Export dimensions now brand-new and grand! 🎬

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix export menu source resolution detection' accurately summarizes the main change: fixing how source video resolution is determined for the export menu.
Description check ✅ Passed The description includes key sections (Summary and Tests) covering the purpose, motivation, and testing approach, fulfilling the core requirements of the template.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 unit tests (beta)
  • Create PR with unit tests

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.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant