perf(preview): stop decoding pictures the hidden media elements never show - #976
Merged
Merged
Conversation
… show The native compositor draws every pixel of the preview, yet Chromium kept decoding the recording beside it. The screen <video> plays only as the playback clock and the <audio> on the same mp4 only for its sound, but both decoded every frame of the picture: measured on a 1080p60 take, ~6.7 % of an RTX 4070 Ti's decode engine apiece, on the engine the compositor's own decoders need. The camera <video> decoded and seeked the whole webcam recording for pixels CSS hid. The editor window enables the AudioVideoTracks Blink feature, and both elements deselect their video track once their metadata is in: zero frames decoded, with the clock, playbackRate and seeks unchanged. A recording without sound keeps its picture, since with neither track selected the element has nothing left to keep time with and races to its end. The camera element is now read for its size only and never played, which retires the playback clock VirtualPreview published for it.
Contributor
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (10)
💤 Files with no reviewable changes (2)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe preview enables Chromium media track support and drops video tracks after metadata loads when audio and video tracks are available. The shared playback clock is removed. WebcamOverlay now reads camera dimensions without playing its video. ChangesPreview media handling
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Other Sequence Diagram(s)sequenceDiagram
participant PreviewCanvas
participant VirtualPreview
participant ScreenMedia
participant WebcamOverlay
participant CameraVideo
PreviewCanvas->>VirtualPreview: Render clips and current time
VirtualPreview->>ScreenMedia: Load screen media
ScreenMedia-->>VirtualPreview: Report metadata loaded
VirtualPreview->>ScreenMedia: Deselect video tracks when audio and video tracks exist
PreviewCanvas->>WebcamOverlay: Render clips and current time
WebcamOverlay->>CameraVideo: Render muted video with metadata preload
CameraVideo-->>WebcamOverlay: Report native dimensions
Merge Risk: ⚪ Minimal · up to No demonstrated preview regression remains to block merging after normal checks. The Electron playback check is still useful for confirming the track behavior in-app. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change is limited to preview playback and media metadata. No expanded data access or privilege was demonstrated, but real-media timing and recovery behavior remain partly unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 6 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
4 of 16 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The native compositor draws every pixel of the preview, yet Chromium kept decoding the recording beside it, on the decode engine the compositor's own decoders need.
<video>plays only as the playback clock, and the<audio>on the same mp4 only for its sound, but both decoded every frame of the picture. The editor window now enables theAudioVideoTracksBlink feature, and both elements deselect their video track once their metadata is in (dropVideoTrack).WebcamOverlayis now read for the camera's size only and never played. That retires the playback clockVirtualPreviewpublished for it (playback-clock.ts).Measured in a hidden Electron 41.2.1 window, 1080p60 take, decode engine of Chromium's GPU process (RTX 4070 Ti):
<video><video>, video track deselectedreadyState4<audio>on the same mp4<audio>, video track deselectedRelated issue
None: comes out of a preview-fluidity investigation. Independent of the shared-texture transport PR.
Type of change
Release impact
Desktop impact
Screenshots / video
No visual change: the elements involved are hidden.
Testing
npx vitest --run: full suite green (298 files). NewVirtualPreview.tracks.test.ts(4) and aWebcamOverlaytest that the camera is never played.tsc --noEmit(app and tests), Biome,npm run docs:check.<video>(muted) and the sound<audio>(not muted) were both playing at 2.80 s withreadyState4 and their video track deselected, the imported voiceover track was playing, and the camera<video>stayed paused. Seeks and the 1.8× speed region played through as before.enableBlinkFeatures(security checklist):AudioVideoTracksonly exposes the media track lists, and the warning does not show in packaged builds.🤖 Generated with Claude Code