feat: Implement core video export functionality using WebCodecs#460
feat: Implement core video export functionality using WebCodecs#460alamshafil wants to merge 1 commit into
Conversation
- Add main API exports for video exporting including VideoExporter and related utilities. - Introduce RenderPipeline for processing frames with various processors. - Create base classes for effects and transitions, including Blur, Brightness, Fade, Slide, and Wipe effects. - Define types for export settings, progress, and timeline data. - Implement utility functions for audio processing and time formatting. - Develop a TimelineDataTransformer for converting OpenCut timeline data to export format. - Add a debug logger for improved logging and error tracking.
|
@alamshafil is attempting to deploy a commit to the OpenCut OSS Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughA comprehensive video export system was implemented for the OpenCut application. This includes a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EditorHeader
participant VideoExportDialog
participant VideoExporter
participant VideoExportEngine
participant StreamTarget
User->>EditorHeader: Clicks Export
EditorHeader->>VideoExportDialog: Opens dialog
User->>VideoExportDialog: Configures settings, starts export
VideoExportDialog->>VideoExporter: exportTimeline(...)
VideoExporter->>VideoExportEngine: export()
VideoExportEngine->>StreamTarget: create()
VideoExportEngine->>VideoExportEngine: Render frames, encode video/audio
VideoExportEngine->>StreamTarget: saveBuffer()
VideoExportEngine-->>VideoExporter: Emit progress/completion
VideoExporter-->>VideoExportDialog: Notify progress/completion
VideoExportDialog-->>User: Show progress, allow download
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 27
♻️ Duplicate comments (1)
packages/video-export/src/pipeline/transitions/base-transition.ts (1)
55-56: Add cleanup for temporary canvases to prevent memory leaks.All transition classes create temporary canvases without proper cleanup, similar to the effects.
Apply the same pattern suggested for effects - wrap canvas operations in try/finally blocks and reset canvas dimensions to help garbage collection.
Also applies to: 109-111, 139-141
🧹 Nitpick comments (24)
packages/video-export/src/utils/time-utils.ts (2)
11-11: Consider using stricter input validation.The condition
!timeInSecondswill treat0as falsy, but0is a valid time value that should format to "0s". This works correctly due to the fallback logic, but the condition could be more explicit.- if (!timeInSeconds || timeInSeconds < 0) { + if (timeInSeconds <= 0) {
42-42: Same input validation consideration applies here.Similar to
formatTime, the condition treats0as falsy when it's a valid input.- if (!timeInSeconds || timeInSeconds < 0) { + if (timeInSeconds <= 0) {packages/video-export/package.json (1)
6-7: Consider build output for main/module fields.The main and module fields point to TypeScript source files. While this works for development in a monorepo, consider pointing to compiled JavaScript output for production builds.
For production builds, these should typically point to:
"main": "./dist/index.js", "module": "./dist/index.js"packages/video-export/src/core/renderers/media-renderer.ts (2)
111-117: Fix typo in comment.- // Simple direct rendering` + // Simple direct rendering
265-273: Improve video cleanup in dispose method.Setting
src = ''and callingload()may not properly release video resources.dispose(): void { for (const media of this.mediaCache.values()) { if (media instanceof HTMLVideoElement) { - media.src = ''; - media.load(); + media.pause(); + media.removeAttribute('src'); + media.load(); } } this.mediaCache.clear(); }packages/video-export/src/core/renderers/subtitle-renderer.ts (2)
135-143: Implement actual font loading for web fonts.The current implementation assumes fonts are already loaded, which may cause text to render with fallback fonts.
Would you like me to implement proper web font loading using the Font Loading API? I can open a new issue to track this enhancement.
103-106: Consider more accurate text height measurement.Using
fontSizeas text height is an approximation that may not work well for all fonts.Consider using font metrics for more accurate height calculation:
const metrics = this.context.measureText(element.content); const textWidth = metrics.width; - const textHeight = element.fontSize; + // Use actual font metrics if available + const textHeight = (metrics.actualBoundingBoxAscent + metrics.actualBoundingBoxDescent) || element.fontSize;packages/video-export/src/core/frame-renderer.ts (1)
152-163: Complete the effect processor creation logic.The method checks for processor existence but doesn't create one when missing.
The current implementation only logs but doesn't create effect processors. Would you like me to implement the processor creation logic based on effectType and parameters? I can open a new issue to track this.
packages/video-export/src/core/stream-target.ts (1)
55-63: Improve download reliability and cleanup timingThe current implementation has potential issues:
- The 1-second timeout might be too short for large files
- The anchor element should be clicked after being added to DOM for better browser compatibility
const a = document.createElement('a'); a.href = url; a.download = this.fileName; +a.style.display = 'none'; document.body.appendChild(a); a.click(); document.body.removeChild(a); // Clean up -setTimeout(() => URL.revokeObjectURL(url), 1000); +// Wait longer for large files and ensure download starts +setTimeout(() => URL.revokeObjectURL(url), 60000); // 60 secondspackages/video-export/src/core/config-generator.ts (1)
19-29: Consider supporting additional video codecs for better performanceCurrently only H.264 is attempted. Modern browsers support VP9 and AV1 which can provide better compression.
const configOptions: VideoEncoderConfig[] = [ { codec: 'avc1.64002a', // H.264 High Profile Level 4.2 - Best codec for compatibility across all browsers, devices, phones, and web platforms width, height, bitrate: settings.videoBitrate, framerate: settings.fps, hardwareAcceleration: settings.hardwareAcceleration ? 'prefer-hardware' : 'prefer-software', }, + { + codec: 'vp09.00.41.08', // VP9 Profile 0 Level 4.1 - Better compression, widely supported + width, + height, + bitrate: Math.floor(settings.videoBitrate * 0.7), // VP9 is more efficient + framerate: settings.fps, + hardwareAcceleration: settings.hardwareAcceleration ? 'prefer-hardware' : 'prefer-software', + }, + { + codec: 'av01.0.08M.10', // AV1 Main Profile - Best compression, growing support + width, + height, + bitrate: Math.floor(settings.videoBitrate * 0.5), // AV1 is very efficient + framerate: settings.fps, + hardwareAcceleration: settings.hardwareAcceleration ? 'prefer-hardware' : 'prefer-software', + }, ];packages/video-export/src/core/renderers/overlay-renderer.ts (2)
135-191: Consider making pattern parameters configurableThe pattern rendering methods use hardcoded values for sizes and spacing. Consider making these configurable through element properties or configuration objects for better flexibility.
Example refactor for the dot pattern method:
- private renderDotPattern(element: OverlayExportElement): void { - const dotSize = 5; - const spacing = 15; + private renderDotPattern(element: OverlayExportElement): void { + // Allow customization through element properties or use defaults + const dotSize = element.patternConfig?.dotSize ?? 5; + const spacing = element.patternConfig?.spacing ?? 15;This approach could be applied to all pattern methods to make them more flexible and reusable.
203-204: Consider flexible CORS handling for imagesSetting
crossOrigin = 'anonymous'might cause issues with images from servers that don't support CORS. Consider making this configurable or implementing a fallback mechanism.- image.crossOrigin = 'anonymous'; + // Only set crossOrigin if the URL is cross-origin + try { + const url = new URL(source); + const currentOrigin = window.location.origin; + if (url.origin !== currentOrigin) { + image.crossOrigin = 'anonymous'; + } + } catch { + // Invalid URL or relative path, no CORS needed + }packages/video-export/src/pipeline/pipeline.ts (1)
78-80: Improve type safety in getProcessorsByTypeThe current implementation uses unsafe type assertion. Consider a more type-safe approach.
- getProcessorsByType(type: string): PipelineProcessor[] { - return this.processors.filter(p => (p as any).type === type); - } + getProcessorsByType<T extends 'effect' | 'transition'>( + type: T + ): T extends 'effect' ? EffectProcessor[] : TransitionProcessor[] { + return this.processors.filter(p => + 'type' in p && p.type === type + ) as any; + }This provides better type safety and IntelliSense support while avoiding the
as anyassertion.apps/web/src/components/export/VideoExportDialog.tsx (2)
106-127: Simplify time formatting logicThe
formatTimefunction has complex nested conditions that could be simplified for better readability.function formatTime(seconds: number): string { - if (seconds < 60) { - return `${Math.round(seconds)}s`; - } else if (seconds < 3600) { - const minutes = Math.floor(seconds / 60); - const remainingSeconds = Math.round(seconds % 60); - return remainingSeconds > 0 ? `${minutes}m ${remainingSeconds}s` : `${minutes}m`; - } else { - const hours = Math.floor(seconds / 3600); - const minutes = Math.floor((seconds % 3600) / 60); - const remainingSeconds = Math.round(seconds % 60); - - if (minutes === 0 && remainingSeconds === 0) { - return `${hours}h`; - } else if (remainingSeconds === 0) { - return `${hours}h ${minutes}m`; - } else { - return `${hours}h ${minutes}m ${remainingSeconds}s`; - } - } + const hours = Math.floor(seconds / 3600); + const minutes = Math.floor((seconds % 3600) / 60); + const secs = Math.round(seconds % 60); + + const parts = []; + if (hours > 0) parts.push(`${hours}h`); + if (minutes > 0) parts.push(`${minutes}m`); + if (secs > 0 || parts.length === 0) parts.push(`${secs}s`); + + return parts.join(' '); }
247-258: Consolidate error handling logicThe error handling logic is duplicated between the
onErrorcallback and the catch block. Consider extracting this into a shared function.+ const handleExportError = (error: Error | unknown) => { + console.error('Export error:', error); + setCurrentExporter(null); + const errorMessage = error instanceof Error ? error.message : "Unknown error"; + setExportState({ + isExporting: false, + progress: null, + error: errorMessage, + completed: false + }); + toast.error(`Export failed: ${errorMessage}`); + }; try { // ... export logic ... } catch (error) { - console.error('Export catch block error:', error); - setCurrentExporter(null); - const errorMessage = error instanceof Error ? error.message : "Unknown error"; - setExportState({ - isExporting: false, - progress: null, - error: errorMessage, - completed: false - }); - toast.error(`Export failed: ${errorMessage}`); + handleExportError(error); }Then use
handleExportErrorin theonErrorcallback as well.packages/video-export/src/core/audio-processor.ts (1)
20-24: Improve type safety for audio track filteringThe current implementation uses type assertions (
as any) which reduces type safety. Consider defining proper types for media elements.const audioTracks = timelineData.tracks.filter(track => track.type === 'audio' || track.elements.some(el => - el.type === 'media' && ((el as any).mediaType === 'audio' || (el as any).mediaType === 'video') + el.type === 'media' && 'mediaType' in el && (el.mediaType === 'audio' || el.mediaType === 'video') ) );packages/video-export/src/api/video-exporter.ts (2)
153-153: Document the resolution multiplier calculation for 720p.The value
0.67for 720p resolution is a magic number that should be documented or defined as a constant.- '720p_high': { - resolution: 0.67, // 720p + '720p_high': { + resolution: 0.67, // 720p = 1280x720, which is 0.67 of 1920x1080Or better yet, define resolution constants:
const RESOLUTION_MULTIPLIERS = { '720p': 720 / 1080, // 0.667 '1080p': 1, '4k': 2 } as const;Also applies to: 159-159
210-221: Extract magic numbers to named constants for clarity.The complexity calculation uses hardcoded thresholds that should be defined as constants for better maintainability.
+ // Complexity thresholds + const COMPLEXITY_THRESHOLDS = { + high: { elements: 20, tracks: 10, resolution: 2 }, + medium: { elements: 10, tracks: 5, resolution: 1 } + }; + + // Base render time per frame in seconds + const BASE_RENDER_TIMES = { + low: 0.1, + medium: 0.2, + high: 0.5 + }; + let complexity: 'low' | 'medium' | 'high' = 'low'; - let baseRenderTime = 0.1; // Base seconds per frame + let baseRenderTime = BASE_RENDER_TIMES.low; // Estimate complexity - if (elementCount > 20 || trackCount > 10 || resolution >= 2) { + if (elementCount > COMPLEXITY_THRESHOLDS.high.elements || + trackCount > COMPLEXITY_THRESHOLDS.high.tracks || + resolution >= COMPLEXITY_THRESHOLDS.high.resolution) { complexity = 'high'; - baseRenderTime = 0.5; // Higher for complex projects + baseRenderTime = BASE_RENDER_TIMES.high;packages/video-export/src/utils/timeline-transformer.ts (1)
9-61: Consider importing shared types instead of redefining them.These OpenCut interfaces are redefined here but should ideally be imported from a shared types module to ensure consistency across the codebase.
Consider creating a shared types package or module that both the main app and this export package can import from. This would prevent type drift and ensure consistency.
packages/video-export/README.md (3)
5-5: Consider removing or rephrasing the LLM authorship note.While transparency is good, mentioning LLM authorship might reduce confidence in the documentation. Consider rephrasing or moving this to a contributors section.
-Note: Document written by LLM to help agents understand OpenCut's video export system. +Note: This document provides a comprehensive overview of OpenCut's video export system architecture and usage.
609-618: Improve error type checking in the error handling example.The error handling example could be more robust by checking error types more specifically.
} catch (error) { - if (error.message.includes('WebCodecs')) { + if (error instanceof Error && error.message.includes('WebCodecs')) { // Browser doesn't support WebCodecs - } else if (error.message.includes('validation')) { + } else if (error instanceof Error && error.message.includes('validation')) { // Timeline data validation failed } else { // Other export error + console.error('Unexpected export error:', error); } }
259-259: Track TODO items for UI integration.The README mentions TODOs for adding effects and transitions to the Editor UI. These should be tracked as issues.
Would you like me to create GitHub issues to track these UI integration tasks for effects and transitions?
Also applies to: 284-284
packages/video-export/src/types/index.ts (2)
167-194: Consider default-optional fields & safer trimming semantics
trimStartandtrimEndare required numbers. A caller that hasn’t trimmed must still supply explicit0, leading to noisy object literals and potential NaN bugs if a field is forgotten.Recommend:
- trimStart: number; - trimEnd: number; + trimStart?: number; // default 0 + trimEnd?: number; // default 0and let the engine coerce
undefinedto0. This mirrors common NLE data models and simplifies authoring.
351-356: Event payloads could be richer & type-safe
'complete': stringonly returns a path/URL. Consumers usually need stats (duration, size) and theExportSettingsused. Consider:interface ExportCompletePayload { fileUrl: string; settings: ExportSettings; totalElapsed: number; sizeBytes?: number; }and update the map to
'complete': ExportCompletePayload. This prevents the inevitable follow-up “can we also return …?” change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (26)
apps/web/package.json(1 hunks)apps/web/src/components/editor-header.tsx(2 hunks)apps/web/src/components/export/VideoExportDialog.tsx(1 hunks)packages/video-export/README.md(1 hunks)packages/video-export/package.json(1 hunks)packages/video-export/src/api/video-exporter.ts(1 hunks)packages/video-export/src/core/audio-processor.ts(1 hunks)packages/video-export/src/core/config-generator.ts(1 hunks)packages/video-export/src/core/frame-renderer.ts(1 hunks)packages/video-export/src/core/renderers/base-renderer.ts(1 hunks)packages/video-export/src/core/renderers/index.ts(1 hunks)packages/video-export/src/core/renderers/media-renderer.ts(1 hunks)packages/video-export/src/core/renderers/overlay-renderer.ts(1 hunks)packages/video-export/src/core/renderers/subtitle-renderer.ts(1 hunks)packages/video-export/src/core/renderers/text-renderer.ts(1 hunks)packages/video-export/src/core/stream-target.ts(1 hunks)packages/video-export/src/core/video-export-engine.ts(1 hunks)packages/video-export/src/index.ts(1 hunks)packages/video-export/src/pipeline/effects/base-effect.ts(1 hunks)packages/video-export/src/pipeline/pipeline.ts(1 hunks)packages/video-export/src/pipeline/transitions/base-transition.ts(1 hunks)packages/video-export/src/types/index.ts(1 hunks)packages/video-export/src/utils/audio-utils.ts(1 hunks)packages/video-export/src/utils/debug-logger.ts(1 hunks)packages/video-export/src/utils/time-utils.ts(1 hunks)packages/video-export/src/utils/timeline-transformer.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: alamshafil
PR: OpenCut-app/OpenCut#325
File: apps/web/src/components/ui/font-picker.tsx:27-27
Timestamp: 2025-07-18T15:22:59.093Z
Learning: In the OpenCut video export system, the FontPicker component uses getGoogleFonts() instead of FONT_OPTIONS to limit font selection to Google Fonts only. This is because the video export functionality requires actual font files to be loaded into FFmpeg's virtual filesystem through the FontManager, rather than CSS-based fonts. System fonts like Arial and Helvetica are not available as downloadable files in the browser environment, making Google Fonts the appropriate choice for video export compatibility.
packages/video-export/src/api/video-exporter.ts (1)
Learnt from: alamshafil
PR: #325
File: apps/web/src/components/ui/font-picker.tsx:27-27
Timestamp: 2025-07-18T15:22:59.093Z
Learning: In the OpenCut video export system, the FontPicker component uses getGoogleFonts() instead of FONT_OPTIONS to limit font selection to Google Fonts only. This is because the video export functionality requires actual font files to be loaded into FFmpeg's virtual filesystem through the FontManager, rather than CSS-based fonts. System fonts like Arial and Helvetica are not available as downloadable files in the browser environment, making Google Fonts the appropriate choice for video export compatibility.
packages/video-export/README.md (1)
Learnt from: alamshafil
PR: #325
File: apps/web/src/components/ui/font-picker.tsx:27-27
Timestamp: 2025-07-18T15:22:59.093Z
Learning: In the OpenCut video export system, the FontPicker component uses getGoogleFonts() instead of FONT_OPTIONS to limit font selection to Google Fonts only. This is because the video export functionality requires actual font files to be loaded into FFmpeg's virtual filesystem through the FontManager, rather than CSS-based fonts. System fonts like Arial and Helvetica are not available as downloadable files in the browser environment, making Google Fonts the appropriate choice for video export compatibility.
🧬 Code Graph Analysis (6)
packages/video-export/src/core/renderers/subtitle-renderer.ts (2)
packages/video-export/src/core/renderers/index.ts (2)
SubtitleElementRenderer(6-6)BaseElementRenderer(2-2)packages/video-export/src/types/index.ts (1)
SubtitleExportElement(301-330)
packages/video-export/src/core/frame-renderer.ts (7)
packages/video-export/src/types/index.ts (4)
ExportSettings(55-88)TimelineExportData(124-137)RenderFrame(334-344)ExportElement(332-332)packages/video-export/src/pipeline/pipeline.ts (1)
RenderPipeline(21-91)packages/video-export/src/core/renderers/media-renderer.ts (1)
MediaElementRenderer(8-274)packages/video-export/src/core/renderers/text-renderer.ts (1)
TextElementRenderer(8-171)packages/video-export/src/core/renderers/overlay-renderer.ts (1)
OverlayElementRenderer(8-230)packages/video-export/src/core/renderers/subtitle-renderer.ts (1)
SubtitleElementRenderer(8-151)packages/video-export/src/utils/debug-logger.ts (2)
error(26-40)debugLogger(80-80)
packages/video-export/src/core/config-generator.ts (1)
packages/video-export/src/types/index.ts (2)
ExportSettings(55-88)EncoderConfig(346-349)
packages/video-export/src/pipeline/pipeline.ts (2)
packages/video-export/src/index.ts (7)
PipelineProcessor(33-33)RenderFrame(24-24)ExportSettings(17-17)EffectProcessor(33-33)TransitionProcessor(33-33)RenderPipeline(32-32)TimelineExportData(19-19)packages/video-export/src/types/index.ts (3)
RenderFrame(334-344)ExportSettings(55-88)TimelineExportData(124-137)
packages/video-export/src/pipeline/effects/base-effect.ts (3)
packages/video-export/src/index.ts (7)
BaseEffect(34-34)EffectProcessor(33-33)RenderFrame(24-24)ExportSettings(17-17)BlurEffect(34-34)BrightnessEffect(34-34)ColorFilterEffect(34-34)packages/video-export/src/pipeline/pipeline.ts (1)
EffectProcessor(10-13)packages/video-export/src/types/index.ts (2)
RenderFrame(334-344)ExportSettings(55-88)
packages/video-export/src/utils/timeline-transformer.ts (2)
packages/video-export/src/index.ts (6)
TimelineDataTransformer(12-12)TimelineExportData(19-19)ExportTrack(20-20)ExportElement(21-21)MediaExportElement(22-22)TextExportElement(23-23)packages/video-export/src/types/index.ts (5)
TimelineExportData(124-137)ExportTrack(140-164)ExportElement(332-332)MediaExportElement(197-217)TextExportElement(220-261)
🔇 Additional comments (16)
packages/video-export/src/utils/time-utils.ts (2)
65-68: Functions look well-implemented.Both
formatProgressandformatRenderSpeedhandle edge cases appropriately and provide clean, readable output formatting.Also applies to: 76-79
51-51: Seconds padding is correct and requires no change.Tests for various edge cases confirm that
secondsis always computed astimeInSeconds % 60(i.e., in [0, 60)), soseconds.toFixed(3).padStart(6, '0')consistently yields the expected six-character output:
- 65.123 → “1:05.123”
- 3599.999 → “59:59.999”
- 0.001 → “0.001s”
No updates needed.
apps/web/package.json (1)
26-26: Dependency addition looks correct.The workspace dependency for the new video export package is properly added and will enable the web app to use the export functionality.
packages/video-export/src/core/renderers/index.ts (1)
1-6: Clean barrel export implementation.The index file properly exports all renderer classes, providing a clean API surface for the rendering system.
apps/web/src/components/editor-header.tsx (2)
11-11: Import statement is correct.The VideoExportDialog component is properly imported from the export directory.
102-111: Clean integration of the video export dialog.The existing export button is now properly wrapped with the VideoExportDialog component, maintaining the same UI while adding the new export functionality. The component composition pattern is well-executed.
packages/video-export/package.json (1)
17-18: TypeScript type dependencies look appropriate.The WebCodecs and File System Access API type definitions are correctly placed in devDependencies, and the peer dependency on @opencut/db makes sense for the export functionality.
Also applies to: 21-21
packages/video-export/src/core/renderers/text-renderer.ts (1)
57-67: Well-structured text style setupThe method correctly composes the font string and aligns text properties with the center-origin coordinate system.
packages/video-export/src/core/video-export-engine.ts (2)
210-264: Verify frame counting logic for accurate progress reportingThe loop processes
totalFrames + 1frames (0 to totalFrames inclusive), but progress calculation usesframe / totalFrameswhich could exceed 100% on the last frame.Either adjust the loop condition or the progress calculation:
-for (let frame = 0; frame <= totalFrames; frame++) { +for (let frame = 0; frame < totalFrames; frame++) {Or update progress calculation to account for the extra frame:
-percentage: (frame / totalFrames) * 100, +percentage: (frame / (totalFrames + 1)) * 100,
329-340: Good fault tolerance with fallback canvasThe fallback canvas implementation ensures export continues even if individual frames fail to render, providing a solid background instead of corrupting the video.
packages/video-export/src/core/config-generator.ts (1)
106-124: Well-designed settings validationThe validation method properly constrains all parameters to reasonable ranges and handles edge cases well. Good future-proofing with support for up to 8K resolution.
packages/video-export/src/core/renderers/base-renderer.ts (1)
1-76: Well-structured base renderer implementationThe abstract base class provides a solid foundation for element renderers with proper context management, timing calculations, and property application. The save/restore pattern ensures clean state management between renders.
packages/video-export/src/core/audio-processor.ts (1)
126-128: Good audio clipping prevention!The clamping logic properly prevents audio clipping by keeping values within the valid range of -1 to 1. This is a best practice for audio processing.
packages/video-export/src/index.ts (1)
1-35: Well-organized package exports!The index file is well-structured with clear categorization of exports (API, Core engine, Utilities, Types, Pipeline system). This makes the package API clear and easy to use.
packages/video-export/src/utils/timeline-transformer.ts (1)
181-183: Duration Calculation Verified
ThetrimEndfield represents the amount trimmed from the end (not an absolute timestamp). Across the export and rendering code—including theaudio-processor(wheretrimEndis converted to samples) and the base/frame renderers—the math consistently subtractstrimStartandtrimEndfromduration. Therefore,const actualDuration = element.duration - element.trimStart - element.trimEnd;is correct and does not need changing.
packages/video-export/src/pipeline/transitions/base-transition.ts (1)
44-47: Fix incorrect cross-fade alpha calculation.The cross-fade alpha calculation doesn't make sense. It goes from 1→0 in the first half, then 0→0.5 in the second half, which would make the content disappear and partially reappear.
For a proper cross-fade between two sources, you need:
case 'cross': - // For cross-fade, this would need two frames to blend - alpha = this.progress < 0.5 ? (1 - this.progress * 2) : (this.progress - 0.5) * 2; + // Cross-fade requires blending two frames, not just alpha + // This should either: + // 1. Throw an error indicating cross-fade needs special handling + // 2. Default to fade-out behavior with a comment + throw new Error('Cross-fade requires two frames to blend and should be handled differently');Likely an incorrect or invalid review comment.
| <Input | ||
| id="video-bitrate" | ||
| type="number" | ||
| min="1" | ||
| max="100" | ||
| step="0.5" | ||
| value={customSettings.videoBitrate || 10} | ||
| onChange={(e) => | ||
| setCustomSettings(prev => ({ | ||
| ...prev, | ||
| videoBitrate: parseFloat(e.target.value) | ||
| })) | ||
| } | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation for bitrate values
The numeric inputs rely only on HTML5 constraints which can be bypassed. Consider adding validation to ensure values stay within acceptable ranges.
onChange={(e) =>
setCustomSettings(prev => ({
...prev,
- videoBitrate: parseFloat(e.target.value)
+ videoBitrate: Math.max(1, Math.min(100, parseFloat(e.target.value) || 10))
}))
}Apply similar validation to the audio bitrate input as well.
📝 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.
| <Input | |
| id="video-bitrate" | |
| type="number" | |
| min="1" | |
| max="100" | |
| step="0.5" | |
| value={customSettings.videoBitrate || 10} | |
| onChange={(e) => | |
| setCustomSettings(prev => ({ | |
| ...prev, | |
| videoBitrate: parseFloat(e.target.value) | |
| })) | |
| } | |
| /> | |
| </div> | |
| <Input | |
| id="video-bitrate" | |
| type="number" | |
| min="1" | |
| max="100" | |
| step="0.5" | |
| value={customSettings.videoBitrate || 10} | |
| onChange={(e) => | |
| setCustomSettings(prev => ({ | |
| ...prev, | |
| videoBitrate: Math.max( | |
| 1, | |
| Math.min( | |
| 100, | |
| parseFloat(e.target.value) || 10 | |
| ) | |
| ) | |
| })) | |
| } | |
| /> |
🤖 Prompt for AI Agents
In apps/web/src/components/export/VideoExportDialog.tsx between lines 517 and
531, the video bitrate input lacks proper validation beyond HTML5 attributes,
which can be bypassed. Add explicit validation in the onChange handler to clamp
the videoBitrate value within the allowed range (e.g., 1 to 100) before updating
the state. Similarly, locate the audio bitrate input and apply the same
validation logic to ensure its value also stays within acceptable bounds.
| } | ||
| }, | ||
| "dependencies": { | ||
| "mediabunny": "^1.3.1" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify mediabunny version and security.
Ensure that mediabunny ^1.3.1 is the latest stable version and check for any known security vulnerabilities.
🌐 Web query:
What is the latest version of the mediabunny npm package and are there any known security vulnerabilities in version 1.3.1?
💡 Result:
The latest version of the mediabunny npm package is 1.1.1, published two days ago. (socket.dev) Version 1.3.1 does not exist; the most recent release is 1.1.1.
Regarding security vulnerabilities, there are no known issues reported for version 1.1.1. The package has been analyzed by Socket, which monitors for malicious and high-risk dependencies, and no alerts have been issued. (socket.dev)
For more information or to report any concerns, you can visit the mediabunny GitHub repository.
Citations:
- 1: https://socket.dev/npm/package/mediabunny?utm_source=openai
- 2: https://socket.dev/npm/package/mediabunny?utm_source=openai
Invalid mediabunny version—please update to an existing release
The dependency in packages/video-export/package.json is set to
"mediabunny": "^1.3.1"but version 1.3.1 does not exist on npm. The latest published version is 1.1.1, and there are no known security vulnerabilities in that release.
• Update line 14 of packages/video-export/package.json to:
- "mediabunny": "^1.3.1"
+ "mediabunny": "^1.1.1"🤖 Prompt for AI Agents
In packages/video-export/package.json at line 14, the mediabunny dependency
version is set to 1.3.1, which does not exist on npm. Update the version to the
latest published release, 1.1.1, by changing the version string to "^1.1.1" to
ensure the dependency is valid and installable.
| timelineTracks: any[], | ||
| mediaItems: any[], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace any[] with proper types for better type safety.
Using any[] bypasses TypeScript's type checking and could lead to runtime errors. Define proper interfaces for the timeline data structures.
- timelineTracks: any[],
- mediaItems: any[],
+ timelineTracks: OpenCutTimelineTrack[],
+ mediaItems: OpenCutMediaItem[],Consider importing these types from a shared types module or defining them in this package's types file.
📝 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.
| timelineTracks: any[], | |
| mediaItems: any[], | |
| timelineTracks: OpenCutTimelineTrack[], | |
| mediaItems: OpenCutMediaItem[], |
🤖 Prompt for AI Agents
In packages/video-export/src/api/video-exporter.ts at lines 46 to 47, replace
the use of `any[]` for `timelineTracks` and `mediaItems` with specific,
well-defined TypeScript interfaces to improve type safety. Define or import
appropriate interfaces that describe the structure of timeline tracks and media
items, either from a shared types module or by creating new type definitions
within this package. Update the variable declarations to use these interfaces
instead of `any[]`.
| console.log('Starting export with:', { | ||
| timelineTracks: timelineTracks.length, | ||
| mediaItems: mediaItems.length, | ||
| projectSettings | ||
| }); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use debugLogger instead of console.log for consistency.
The file imports debugLogger but uses console.log directly. This inconsistency could make debugging harder in production.
- console.log('Starting export with:', {
+ debugLogger.log('🎬 Starting export with:', {
timelineTracks: timelineTracks.length,
mediaItems: mediaItems.length,
projectSettings
});
- console.log('Transformed timeline data:', timelineData);
+ debugLogger.log('📊 Transformed timeline data:', timelineData);Also applies to: 66-66
🤖 Prompt for AI Agents
In packages/video-export/src/api/video-exporter.ts at lines 53 to 58 and line
66, replace the console.log calls with debugLogger calls to maintain logging
consistency. Use debugLogger to output the same messages instead of console.log,
ensuring all debug information is handled uniformly across the file.
| const hasAudioElements = timelineData.tracks.some(track => | ||
| track.elements.some(el => | ||
| el.type === 'media' && ((el as any).mediaType === 'audio' || (el as any).mediaType === 'video') | ||
| ) | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid type casting by properly typing the element.
The use of (el as any) indicates missing type information in the element definition.
- track.elements.some(el =>
- el.type === 'media' && ((el as any).mediaType === 'audio' || (el as any).mediaType === 'video')
+ track.elements.some(el =>
+ el.type === 'media' && (el as MediaExportElement).mediaType === 'audio' || (el as MediaExportElement).mediaType === 'video')This requires importing MediaExportElement from the types module.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/video-export/src/api/video-exporter.ts around lines 256 to 260, the
code uses (el as any) to access mediaType, indicating missing type information.
To fix this, import MediaExportElement from the types module and properly type
the elements in the tracks so that el is recognized as MediaExportElement. This
will eliminate the need for type casting and improve type safety.
| export interface RenderFrame { | ||
| /** Frame number */ | ||
| frameNumber: number; | ||
|
|
||
| /** Timestamp in seconds */ | ||
| timestamp: number; | ||
|
|
||
| /** Canvas context for this frame */ | ||
| canvas: HTMLCanvasElement; | ||
| context: CanvasRenderingContext2D; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use OffscreenCanvas or generic context to support worker rendering
HTMLCanvasElement & CanvasRenderingContext2D tie RenderFrame to the main thread DOM. The encoder pipeline likely runs in a worker where only OffscreenCanvas exists. Suggest loosening the contract:
-export interface RenderFrame {
- /** Frame number */
- frameNumber: number;
- /** Timestamp in seconds */
- timestamp: number;
- /** Canvas context for this frame */
- canvas: HTMLCanvasElement | OffscreenCanvas;
- context: CanvasRenderingContext2D | OffscreenCanvasRenderingContext2D;
-}
+export interface RenderFrame<C = CanvasRenderingContext2D | OffscreenCanvasRenderingContext2D> {
+ frameNumber: number;
+ timestamp: number;
+ canvas: OffscreenCanvas | HTMLCanvasElement;
+ context: C;
+}This keeps the type safe while unlocking worker-based rendering and WebCodecs’ VideoTrackGenerator.
📝 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.
| export interface RenderFrame { | |
| /** Frame number */ | |
| frameNumber: number; | |
| /** Timestamp in seconds */ | |
| timestamp: number; | |
| /** Canvas context for this frame */ | |
| canvas: HTMLCanvasElement; | |
| context: CanvasRenderingContext2D; | |
| } | |
| export interface RenderFrame<C = CanvasRenderingContext2D | OffscreenCanvasRenderingContext2D> { | |
| frameNumber: number; | |
| timestamp: number; | |
| canvas: OffscreenCanvas | HTMLCanvasElement; | |
| context: C; | |
| } |
🤖 Prompt for AI Agents
In packages/video-export/src/types/index.ts around lines 334 to 344, the
RenderFrame interface currently uses HTMLCanvasElement and
CanvasRenderingContext2D, which restricts it to main thread DOM usage. To
support worker rendering and WebCodecs VideoTrackGenerator, update the types to
use OffscreenCanvas or a more generic canvas and context type that works in both
main thread and worker environments, ensuring type safety while enabling broader
compatibility.
| let sample = audioBuffer.getChannelData(channel)[i] * 32767; // Convert float [-1,1] to 16-bit PCM | ||
|
|
||
| // Clamp values to the Int16 range | ||
| if (sample > 32767) sample = 32767; | ||
| if (sample < -32767) sample = -32767; | ||
|
|
||
| interleaved[i * numberOfChannels + channel] = sample; | ||
| } |
There was a problem hiding this comment.
Fix Int16 clamping range.
The minimum value for Int16 should be -32768, not -32767.
let sample = audioBuffer.getChannelData(channel)[i] * 32767; // Convert float [-1,1] to 16-bit PCM
// Clamp values to the Int16 range
if (sample > 32767) sample = 32767;
- if (sample < -32767) sample = -32767;
+ if (sample < -32768) sample = -32768;📝 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.
| let sample = audioBuffer.getChannelData(channel)[i] * 32767; // Convert float [-1,1] to 16-bit PCM | |
| // Clamp values to the Int16 range | |
| if (sample > 32767) sample = 32767; | |
| if (sample < -32767) sample = -32767; | |
| interleaved[i * numberOfChannels + channel] = sample; | |
| } | |
| let sample = audioBuffer.getChannelData(channel)[i] * 32767; // Convert float [-1,1] to 16-bit PCM | |
| // Clamp values to the Int16 range | |
| if (sample > 32767) sample = 32767; | |
| if (sample < -32768) sample = -32768; | |
| interleaved[i * numberOfChannels + channel] = sample; | |
| } |
🤖 Prompt for AI Agents
In packages/video-export/src/utils/audio-utils.ts around lines 33 to 40, the
clamping for the Int16 minimum value is incorrect; it should be -32768 instead
of -32767. Update the condition that clamps the sample value to use -32768 as
the lower bound to correctly represent the Int16 range.
| log(message: string, ...args: any[]) { | ||
| const timestamp = new Date().toLocaleTimeString(); | ||
| const logEntry = `[${timestamp}] ${message}${args.length > 0 ? ' ' + args.map(arg => | ||
| typeof arg === 'object' ? JSON.stringify(arg) : String(arg) | ||
| ).join(' ') : ''}`; | ||
|
|
||
| this.logs.push(logEntry); | ||
|
|
||
| // Keep only the last maxLogs entries | ||
| if (this.logs.length > this.maxLogs) { | ||
| this.logs = this.logs.slice(-this.maxLogs); | ||
| } | ||
|
|
||
| // Also log to console | ||
| console.log(message, ...args); | ||
|
|
||
| // Notify listeners | ||
| this.notifyListeners(); | ||
| } | ||
|
|
||
| error(message: string, ...args: any[]) { | ||
| const timestamp = new Date().toLocaleTimeString(); | ||
| const logEntry = `[${timestamp}] ERROR: ${message}${args.length > 0 ? ' ' + args.map(arg => | ||
| typeof arg === 'object' ? JSON.stringify(arg) : String(arg) | ||
| ).join(' ') : ''}`; | ||
|
|
||
| this.logs.push(logEntry); | ||
|
|
||
| if (this.logs.length > this.maxLogs) { | ||
| this.logs = this.logs.slice(-this.maxLogs); | ||
| } | ||
|
|
||
| console.error(message, ...args); | ||
| this.notifyListeners(); | ||
| } | ||
|
|
||
| warn(message: string, ...args: any[]) { | ||
| const timestamp = new Date().toLocaleTimeString(); | ||
| const logEntry = `[${timestamp}] WARN: ${message}${args.length > 0 ? ' ' + args.map(arg => | ||
| typeof arg === 'object' ? JSON.stringify(arg) : String(arg) | ||
| ).join(' ') : ''}`; | ||
|
|
||
| this.logs.push(logEntry); | ||
|
|
||
| if (this.logs.length > this.maxLogs) { | ||
| this.logs = this.logs.slice(-this.maxLogs); | ||
| } | ||
|
|
||
| console.warn(message, ...args); | ||
| this.notifyListeners(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract duplicated logging logic into a private method.
The log formatting and array trimming logic is duplicated across log, error, and warn methods. This violates the DRY principle.
Apply this refactor to reduce duplication:
+ private formatLogEntry(level: string, message: string, args: any[]): string {
+ const timestamp = new Date().toLocaleTimeString();
+ const prefix = level ? `${level}: ` : '';
+ const formattedArgs = args.length > 0 ? ' ' + args.map(arg =>
+ typeof arg === 'object' ? JSON.stringify(arg) : String(arg)
+ ).join(' ') : '';
+ return `[${timestamp}] ${prefix}${message}${formattedArgs}`;
+ }
+
+ private addLogEntry(entry: string): void {
+ this.logs.push(entry);
+ if (this.logs.length > this.maxLogs) {
+ this.logs = this.logs.slice(-this.maxLogs);
+ }
+ this.notifyListeners();
+ }
log(message: string, ...args: any[]) {
- const timestamp = new Date().toLocaleTimeString();
- const logEntry = `[${timestamp}] ${message}${args.length > 0 ? ' ' + args.map(arg =>
- typeof arg === 'object' ? JSON.stringify(arg) : String(arg)
- ).join(' ') : ''}`;
-
- this.logs.push(logEntry);
-
- // Keep only the last maxLogs entries
- if (this.logs.length > this.maxLogs) {
- this.logs = this.logs.slice(-this.maxLogs);
- }
-
+ const logEntry = this.formatLogEntry('', message, args);
+ this.addLogEntry(logEntry);
// Also log to console
console.log(message, ...args);
-
- // Notify listeners
- this.notifyListeners();
}
error(message: string, ...args: any[]) {
- const timestamp = new Date().toLocaleTimeString();
- const logEntry = `[${timestamp}] ERROR: ${message}${args.length > 0 ? ' ' + args.map(arg =>
- typeof arg === 'object' ? JSON.stringify(arg) : String(arg)
- ).join(' ') : ''}`;
-
- this.logs.push(logEntry);
-
- if (this.logs.length > this.maxLogs) {
- this.logs = this.logs.slice(-this.maxLogs);
- }
-
+ const logEntry = this.formatLogEntry('ERROR', message, args);
+ this.addLogEntry(logEntry);
console.error(message, ...args);
- this.notifyListeners();
}
warn(message: string, ...args: any[]) {
- const timestamp = new Date().toLocaleTimeString();
- const logEntry = `[${timestamp}] WARN: ${message}${args.length > 0 ? ' ' + args.map(arg =>
- typeof arg === 'object' ? JSON.stringify(arg) : String(arg)
- ).join(' ') : ''}`;
-
- this.logs.push(logEntry);
-
- if (this.logs.length > this.maxLogs) {
- this.logs = this.logs.slice(-this.maxLogs);
- }
-
+ const logEntry = this.formatLogEntry('WARN', message, args);
+ this.addLogEntry(logEntry);
console.warn(message, ...args);
- this.notifyListeners();
}📝 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.
| log(message: string, ...args: any[]) { | |
| const timestamp = new Date().toLocaleTimeString(); | |
| const logEntry = `[${timestamp}] ${message}${args.length > 0 ? ' ' + args.map(arg => | |
| typeof arg === 'object' ? JSON.stringify(arg) : String(arg) | |
| ).join(' ') : ''}`; | |
| this.logs.push(logEntry); | |
| // Keep only the last maxLogs entries | |
| if (this.logs.length > this.maxLogs) { | |
| this.logs = this.logs.slice(-this.maxLogs); | |
| } | |
| // Also log to console | |
| console.log(message, ...args); | |
| // Notify listeners | |
| this.notifyListeners(); | |
| } | |
| error(message: string, ...args: any[]) { | |
| const timestamp = new Date().toLocaleTimeString(); | |
| const logEntry = `[${timestamp}] ERROR: ${message}${args.length > 0 ? ' ' + args.map(arg => | |
| typeof arg === 'object' ? JSON.stringify(arg) : String(arg) | |
| ).join(' ') : ''}`; | |
| this.logs.push(logEntry); | |
| if (this.logs.length > this.maxLogs) { | |
| this.logs = this.logs.slice(-this.maxLogs); | |
| } | |
| console.error(message, ...args); | |
| this.notifyListeners(); | |
| } | |
| warn(message: string, ...args: any[]) { | |
| const timestamp = new Date().toLocaleTimeString(); | |
| const logEntry = `[${timestamp}] WARN: ${message}${args.length > 0 ? ' ' + args.map(arg => | |
| typeof arg === 'object' ? JSON.stringify(arg) : String(arg) | |
| ).join(' ') : ''}`; | |
| this.logs.push(logEntry); | |
| if (this.logs.length > this.maxLogs) { | |
| this.logs = this.logs.slice(-this.maxLogs); | |
| } | |
| console.warn(message, ...args); | |
| this.notifyListeners(); | |
| } | |
| private formatLogEntry(level: string, message: string, args: any[]): string { | |
| const timestamp = new Date().toLocaleTimeString(); | |
| const prefix = level ? `${level}: ` : ''; | |
| const formattedArgs = args.length > 0 | |
| ? ' ' + args.map(arg => | |
| typeof arg === 'object' ? JSON.stringify(arg) : String(arg) | |
| ).join(' ') | |
| : ''; | |
| return `[${timestamp}] ${prefix}${message}${formattedArgs}`; | |
| } | |
| private addLogEntry(entry: string): void { | |
| this.logs.push(entry); | |
| if (this.logs.length > this.maxLogs) { | |
| this.logs = this.logs.slice(-this.maxLogs); | |
| } | |
| this.notifyListeners(); | |
| } | |
| log(message: string, ...args: any[]) { | |
| const logEntry = this.formatLogEntry('', message, args); | |
| this.addLogEntry(logEntry); | |
| // Also log to console | |
| console.log(message, ...args); | |
| } | |
| error(message: string, ...args: any[]) { | |
| const logEntry = this.formatLogEntry('ERROR', message, args); | |
| this.addLogEntry(logEntry); | |
| console.error(message, ...args); | |
| } | |
| warn(message: string, ...args: any[]) { | |
| const logEntry = this.formatLogEntry('WARN', message, args); | |
| this.addLogEntry(logEntry); | |
| console.warn(message, ...args); | |
| } |
🤖 Prompt for AI Agents
In packages/video-export/src/utils/debug-logger.ts between lines 6 and 56, the
log, error, and warn methods duplicate code for formatting log entries and
trimming the logs array. Refactor by extracting this common logic into a private
helper method that takes the log level and message with arguments, formats the
log entry, trims the logs array if needed, and pushes the entry. Then update the
log, error, and warn methods to call this helper and handle console output and
listener notification accordingly.
| const logEntry = `[${timestamp}] ${message}${args.length > 0 ? ' ' + args.map(arg => | ||
| typeof arg === 'object' ? JSON.stringify(arg) : String(arg) | ||
| ).join(' ') : ''}`; |
There was a problem hiding this comment.
Add error handling for JSON.stringify to prevent logging failures.
JSON.stringify can throw errors on circular references or unsupported types like BigInt, which would cause the entire logging operation to fail.
Wrap JSON.stringify in a try-catch block:
- typeof arg === 'object' ? JSON.stringify(arg) : String(arg)
+ typeof arg === 'object' ? (() => {
+ try {
+ return JSON.stringify(arg);
+ } catch (e) {
+ return '[Circular or unserializable object]';
+ }
+ })() : String(arg)Also applies to: 28-30, 44-46
🤖 Prompt for AI Agents
In packages/video-export/src/utils/debug-logger.ts around lines 8 to 10, 28 to
30, and 44 to 46, the use of JSON.stringify can throw errors on circular
references or unsupported types, causing logging to fail. Wrap each
JSON.stringify call in a try-catch block to safely handle errors by returning a
fallback string like "[Unable to stringify]" when an exception occurs, ensuring
the logging operation continues without interruption.
| for (const element of track.elements) { | ||
| if (!element.id || element.startTime < 0 || element.duration <= 0) { | ||
| errors.push(`Invalid element: ${element.id || 'unknown'} in track ${track.name}`); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for trim values and calculated duration.
The validation should check if trim values are valid and if the calculated duration after trimming is positive.
for (const element of track.elements) {
if (!element.id || element.startTime < 0 || element.duration <= 0) {
errors.push(`Invalid element: ${element.id || 'unknown'} in track ${track.name}`);
}
+
+ // Validate trim values
+ if (element.trimStart < 0 || element.trimEnd < 0) {
+ errors.push(`Invalid trim values for element: ${element.id}`);
+ }
+
+ if (element.trimStart + element.trimEnd >= element.duration) {
+ errors.push(`Trim values exceed duration for element: ${element.id}`);
+ }📝 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 element of track.elements) { | |
| if (!element.id || element.startTime < 0 || element.duration <= 0) { | |
| errors.push(`Invalid element: ${element.id || 'unknown'} in track ${track.name}`); | |
| } | |
| for (const element of track.elements) { | |
| if (!element.id || element.startTime < 0 || element.duration <= 0) { | |
| errors.push(`Invalid element: ${element.id || 'unknown'} in track ${track.name}`); | |
| } | |
| // Validate trim values | |
| if (element.trimStart < 0 || element.trimEnd < 0) { | |
| errors.push(`Invalid trim values for element: ${element.id}`); | |
| } | |
| if (element.trimStart + element.trimEnd >= element.duration) { | |
| errors.push(`Trim values exceed duration for element: ${element.id}`); | |
| } |
🤖 Prompt for AI Agents
In packages/video-export/src/utils/timeline-transformer.ts around lines 219 to
222, the current validation only checks element id, startTime, and duration but
does not validate trim values or the duration after trimming. Add checks to
ensure trim start and end values are valid (e.g., non-negative and within
element duration) and verify that the duration after applying trims is positive.
If these conditions fail, push an appropriate error message to the errors array.
1 participant
Description
This pull request introduces a video export feature, using WebCodecs and other modern browser APIs.
Integration of Video Export Feature:
VideoExportDialogcomponent into the editor headerImplementation of Video Export Core:
@opencut/video-exportpackage with initial setup inpackage.json, including dependencies likemediabunnyand type definitions for WebCodecs.VideoExporterclass inpackages/video-export/src/api/video-exporter.ts, providing methods for exporting timelines, aborting exports, and estimating export times. It supports configurations like resolution, bitrate, framerate, and hardware acceleration.Read the
README.mdinpackages/video-exportfor more info.Type of change
Please delete options that are not relevant.
Screenshots (if applicable)
Checklist:
Additional context
The performance of this PR can be improved.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation