Fix the 5 min studio export limit being enforced incorrectly #1117
Conversation
oscartbeaumont
changed the title
WalkthroughAdds a SilentError and brief delay to suppress a spurious global error during export/upload when sharing is disallowed. Removes the Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Dialog as ExportDialog
participant Uploader
User->>Dialog: Click "Share/Upload"
Dialog->>Dialog: Check sharing allowed?
alt Not allowed (requires upgrade)
Dialog->>Dialog: wait 1s
Dialog-->>Uploader: throw SilentError (control-flow interrupt)
else Allowed
Dialog->>Uploader: start upload
Uploader-->>Dialog: success / error
end
rect lightgrey
Note over Dialog: onError handler
alt error is SilentError
Dialog->>Dialog: Suppress global error dialog
else other error
Dialog->>Dialog: Show global error dialog
end
end
sequenceDiagram
autonumber
participant Caller as Rust API
participant FFmpeg as ffmpeg::format
Caller->>FFmpeg: ffmpeg::init() (best-effort / guarded)
Caller->>FFmpeg: format::input(path)
alt input ok (has video stream)
FFmpeg-->>Caller: Input context (duration, streams)
Caller->>Caller: duration = input.duration() / AV_TIME_BASE
Caller-->>Caller: return valid + duration
else input err or no video
FFmpeg-->>Caller: error / no video
Caller-->>Caller: return invalid / error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (1)
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/routes/editor/ExportDialog.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: Do not manually import icons in the desktop app; rely on auto-imported icons
In the desktop app, use @tanstack/solid-query for server state management
Files:
apps/desktop/src/routes/editor/ExportDialog.tsx
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/desktop/src/routes/editor/ExportDialog.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/desktop/src/routes/editor/ExportDialog.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/routes/editor/ExportDialog.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (2)
apps/desktop/src/routes/editor/ExportDialog.tsx (2)
50-50: LGTM!The custom error class is well-named and appropriately scoped for internal use within this module.
381-385: LGTM!The error handling logic correctly suppresses dialogs for
SilentErrorwhile preserving user feedback for genuine errors. State cleanup happens consistently regardless of error type.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/desktop/src-tauri/src/lib.rs (1)
48-48: Avoid relying on FFI constant if a safe wrapper existsUsing ffmpeg::ffi::AV_TIME_BASE couples to C FFI. Prefer a safe abstraction (e.g., rescaling via stream time_base or a crate-provided constant accessor) if available.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
apps/desktop/src-tauri/Cargo.toml(0 hunks)apps/desktop/src-tauri/src/lib.rs(4 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop/src-tauri/Cargo.toml
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/desktop/src-tauri/src/lib.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/lib.rs
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/lib.rs (2)
crates/enc-ffmpeg/src/mux/mp4.rs (1)
init(31-67)crates/media/src/lib.rs (1)
init(12-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (1)
apps/desktop/src-tauri/src/lib.rs (1)
67-67: LGTMBufWriter import is appropriate and used in PNG encoding.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/desktop/src-tauri/src/lib.rs (3)
889-892: sum::<Result<_, _>>() likely fails to compileThe previously flagged compile issue remains: use
try_fold(0.0, |acc, p| get_duration_for_path(p).map(|d| acc + d))?to properly accumulate Results and short-circuit on error.Based on past review comment.
680-682: is_valid_mp4 still accepts any FFmpeg-openable formatThis implementation returns true for any container FFmpeg can open, but callers assume MP4. The previously suggested fix remains applicable:
- Verify
.mp4extension (case-insensitive)- Check FFmpeg init with
is_err()pattern- Confirm at least one video stream exists
Based on past review comment suggesting the same fix.
866-871: Missing guard for unknown duration
input.duration()can be unknown (≤ 0). The previously suggested fix remains necessary: check if duration > 0 before dividing by AV_TIME_BASE, otherwise handle gracefully (return 0.0 or error with context).Based on past review comment.
🧹 Nitpick comments (1)
apps/desktop/src-tauri/src/lib.rs (1)
1818-1822: Consider failing fast on FFmpeg init failureWhile the current approach allows graceful continuation, FFmpeg operations will fail later anyway. Consider either:
- Making init failure more visible (e.g., show error dialog)
- Failing fast if FFmpeg is critical to core functionality
However,
is_valid_mp4(line 681) doesn't check if init succeeded before calling FFmpeg functions, which could cause silent failures.As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src-tauri/src/lib.rs(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/desktop/src-tauri/src/lib.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/lib.rs
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/lib.rs (2)
crates/enc-ffmpeg/src/mux/mp4.rs (1)
init(31-67)crates/media/src/lib.rs (1)
init(12-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (2)
apps/desktop/src-tauri/src/lib.rs (2)
48-48: LGTM!The AV_TIME_BASE import is necessary for FFmpeg duration calculations and is used correctly at line 869.
67-67: LGTM!Correct import for the BufWriter usage at line 1239 in the PNG encoding path.
oscartbeaumont
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src-tauri/src/lib.rs(8 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/desktop/src-tauri/src/lib.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/lib.rs
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/lib.rs (2)
crates/rendering/src/project_recordings.rs (4)
new(20-48)new(64-88)duration(178-180)duration(196-207)crates/media/src/lib.rs (1)
init(12-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
The issue seems to be because the
mp4library is reading bogus durations from the mp4. I have switched it forffmpegwhich seems to be correct.Also improved the UX. Previously it would show the error toast and the upgrade flow. Now it just shows the upgrade flow.
Summary by CodeRabbit
New Features
Bug Fixes
Chores