chore(release): cherry-pick RC-window bugfixes for 1.7.0 rc.3#124
Merged
Conversation
… span Auto zoom regions (created by the magic-wand cursor-dwell suggestion pass) were built with focusMode left undefined unless the separate, global "Auto Focus All" toggle was on. That meant the pan/zoom used the static dwell-centroid focus captured at suggestion time and never tracked the cursor afterward, even though the cursor-follow interpolation logic (zoomRegionUtils/cursorFollowUtils) already existed and just wasn't wired up by default for these regions. Auto-suggested regions now always get focusMode "auto" so they pan to track the cursor telemetry across their whole span, independent of the "Auto Focus All" toggle (which still only controls the default for manually-drawn zoom regions). Fixes #72 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The video-writer thread held the shared frame-state mutex across IMFSinkWriter::WriteSample, which the main thread's stop-wait also locks to check control.stopRequested. With the software H.264 encoder fallback (preferSoftwareEncoder: true), WriteSample is slow enough that the writer thread could keep re-acquiring the lock every frame faster than the main thread could win it after a stop request, starving the stop-wait indefinitely — explaining why no [stop-timing] line ever printed. Split MFEncoder::writeFrame/writeBgraFrame into a capture step (GPU readback + IMFSample construction, still under the shared mutex since it touches latestFrameTexture) and a submit step (WriteSample, only under the encoder's own low-contention writerMutex_), and call submit outside the shared mutex in main.cpp's writeVideoFrames. Fixes #115. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fixes #60: recording a single window (as opposed to a full display) on Windows produced a solid black video for the entire clip. Root cause: GraphicsCaptureItem::Size() for a window capture item reports the window's real client-area pixel dimensions verbatim, which are frequently odd (arbitrary resize, DPI rounding) — unlike monitor/display capture items, which are always even. H.264 encoding (and the RGB32->NV12 conversion feeding it) requires even width/height. The frame pool and captureWidth()/ captureHeight() (which configures the encoder's input media type) both used the raw, possibly-odd item size, so an odd-dimensioned window capture fed the Media Foundation H.264 encoder a size it can't encode correctly, producing black output. Fix: round window capture dimensions up to the nearest even value once, in WgcSession::createCaptureItem(HWND), and use that same rounded size (not the raw item size) for both the Direct3D11CaptureFramePool buffer and captureWidth()/captureHeight(), so every consumer of the session agrees on one even-dimensioned size. Monitor/display capture is untouched (always even in practice). Verified: built wgc-capture.exe via CMake/Ninja (VS 18 Insiders x64), opened a real Notepad window resized to an odd 789x595 client area, and drove the helper directly against it (sourceType:"window", real HWND). - Original (pre-fix) binary: 5.4KB output, ffmpeg blackdetect flags the entire clip as black (black_duration ~= full clip length). - Fixed binary: 36.5KB output (consistent with real, compressible frame content vs. a solid-color clip), ffmpeg blackdetect reports zero black segments across the same clip. - Sanity-checked normal display/monitor capture is unaffected (unchanged code path).
…ks playback The timeline playhead read its position from `currentTime` React state, set from VideoEditor (a ~3300-line component) on every rAF tick of video playback and on every `seeking` event while dragging. Each tick forced a full re-render of VideoEditor + VideoPlayback + TimelineEditor before the playhead's DOM moved, so the visual position lagged behind actual video playback and felt sluggish while scrubbing during playback. - PlaybackCursor now reads the video element's currentTime directly via a `getPlaybackTimeMs` getter, in its own requestAnimationFrame loop and its own local `useState`. Only this small component re-renders per frame, independent of how long the ancestor tree's render takes. - Coalesce the `onTimeUpdate` React state commit (still used by everything else that reads `currentTime`) to at most once per animation frame via a new `createRafCoalescer` helper, so a burst of `seeking` events from a fast timeline drag doesn't force one state commit per event. Fixes #111
Per CodeRabbit review on #120: PlaybackCursor wrote to getPlaybackTimeMsRef.current directly in the render body instead of in an effect. Writing to a ref during render is a React anti-pattern — it can leave the ref holding a stale value from a discarded render pass under concurrent rendering. Moved the assignment into a useEffect keyed on getPlaybackTimeMs, matching the recommended pattern for "latest callback" refs. Verified: tsc --noEmit clean, biome clean, rafCoalescer.test.ts and zoomRegionUtils.test.ts pass (7/7).
Follow-up to #119 (Fixes #115). CodeRabbit flagged, on the now-superseded #121, that the video-writer thread's per-frame cv.wait() has no timeout: if a notification were ever missed, the thread would block indefinitely, hanging videoWriterThread.join() and the whole stop sequence — exactly the failure mode this release is eliminating. #119 already fixed the actual root cause (the blocking WriteSample call no longer runs under this mutex, so the main thread's stop-wait is never starved), but the wait itself was still unbounded. Switch it to wait_for(100ms) so the loop always re-checks stopRequested/encodeFailed even in that circumstance, instead of relying solely on a notification reaching an already-waiting thread. Verified: rebuilt wgc-capture.exe on top of current main (includes #119), re-ran the #115 repro (screen-only, all extras disabled) and the full-featured regression (system audio + cursor) - both stop in under 110ms with a full [stop-timing] log and a valid MP4.
Native macOS recordings write system audio and the microphone as two separate AAC tracks in the screen recording, and both are flagged `default`. The exporter decoded audio through web-demuxer's bare "audio" selector, which resolves to the single stream FFmpeg's `av_find_best_stream` picks — the first (system-audio) track. When nothing was playing, that track is silent, so the exported video had no audible audio even though the mic was recorded fine. The browser recorder already blends system + mic into one track; the native path never did. Decode every audio stream and mix them into one timeline before encoding, mirroring the browser recorder: - `mixPlanarSources` (pure, unit-tested) sums each decoded source, downmixed to the target channels and aligned at its source-time offset, clamped to [-1, 1]. - Per-stream decode via `readAVPacket(streamIndex)` targets each track by container index instead of the best-stream heuristic. - The trim-only and offline (speed) export paths both mix multi-track sources; multi-track speed projects are routed to the offline path because the real-time <audio> capture can only play one track. - The source-copy fast path is disabled for multi-track sources, since copying verbatim carries both tracks over and players fall back to the silent first one. Single-track recordings keep the original fast path unchanged. Adds a real-browser regression test (fixture: silent track + 440 Hz tone) that asserts the exported audio carries the tone through both mixing paths. Closes #108 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Preserve the signed startFrame so AAC preroll (negative timestamp) no longer collides with the timestamp-zero frame; mixPlanarSources already discards pre-zero frames. - Keep the sole decodable stream instead of returning null, so a partial decode failure can't fall back to the demuxer's best stream and export silence. - Log per-track sample rates when a cross-rate mix is skipped. - Document the hard-clip trade-off in mixPlanarSources. - Tests: pin the source-copy audioStreamCount>1 blocker, cover the signed startFrame case, and tighten the browser RMS thresholds (0.01 -> 0.05). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
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
2 participants
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
Cherry-picks the 8 bugfix commits merged to
mainduring the 1.7.0 RC-testing window, per the release-branch-freeze contract in.harness/docs/git-workflow.md§ Release branches — these are cherry-picks of already-merged main fixes, not new work.Fixes bundled:
Verification
Cherry-picked cleanly onto
release/v1.7.0(no conflicts). On the resulting tree:tsc --noEmit— cleanbiome check— cleanwgc-capture.exerebuilds cleanly (CMake/Ninja, VS 18 Insiders)Once merged, this branch tip is ready for an
rc.3cut (Actions → "Cut a release candidate", or the manual tag fallback) for a final round of testing before promotion to stable.