feat: Implement Windows ARM build #1354
Conversation
WalkthroughAdded Windows ARM64 target across configs and CI; expanded per-target Cargo dependencies; extended publish workflow with platform/arch matrix and RUN_BUILD gating; made FFmpeg setup architecture-aware; validated/clamped recording durations and updated TimelineSegment field; adjusted Windows encoder locking/flush and FPS/duration fallbacks; bumped desktop version. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as Trigger
participant Publish as .github/workflows/publish.yml
participant Matrix as build matrix
participant Runner as GitHub runner
User->>Publish: start workflow (inputs: windowsTarget, buildMac)
Publish->>Matrix: expand to entries with platform + arch
Publish->>Publish: compute RUN_BUILD based on matrix + inputs
alt RUN_BUILD == true
Matrix->>Runner: run gated build/sign/upload steps
else RUN_BUILD == false
Publish-->>Runner: skip build steps
end
sequenceDiagram
autonumber
participant Setup as scripts/setup.js
participant GH as GitHub Releases
participant Cache as local cache
Setup->>GH: request "latest" release assets
GH-->>Setup: return asset list
Setup->>Setup: pick asset by arch (aarch64 or x86_64), fallback to x86_64
alt asset found
Setup->>Cache: download ZIP -> extract -> rename -> cache
else asset missing
Setup-->>Setup: fallback handling (use x86_64)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Points to review closely:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.rs📄 CodeRabbit inference engine (AGENTS.md)
Files:
crates/*/src/**/*📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (2)📚 Learning: 2025-10-17T05:58:22.586ZApplied to files:
📚 Learning: 2025-10-28T08:39:42.230ZApplied to files:
🧬 Code graph analysis (1)crates/rendering/src/project_recordings.rs (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.config/hakari.toml(1 hunks).github/workflows/ci.yml(4 hunks).github/workflows/publish.yml(3 hunks)crates/workspace-hack/Cargo.toml(3 hunks)scripts/setup.js(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
scripts/setup.js
🧠 Learnings (1)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to **/*.rs : Format Rust code using `rustfmt` and ensure all Rust code passes workspace-level clippy lints.
Applied to files:
.github/workflows/ci.yml
🧬 Code graph analysis (1)
scripts/setup.js (1)
apps/desktop/scripts/prodBeforeBundle.js (2)
targetDir(14-14)exec(9-9)
🪛 YAMLlint (1.37.1)
.github/workflows/publish.yml
[error] 147-147: syntax error: expected , but found '}'
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (rust)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
crates/rendering/src/project_recordings.rs (1)
102-128: Consider applying similar duration validation to Audio.For consistency with the robust Video duration validation (lines 42-75), Audio::new could benefit from similar fallback logic and validation checks to handle invalid container durations.
Example approach:
let container_duration = input.duration(); let mut duration = if container_duration > 0 && container_duration != i64::MIN { container_duration as f64 / 1_000_000.0 } else { 0.0 }; // Add stream duration fallback if needed if duration <= 0.0 { let stream = input.streams().best(ffmpeg::media::Type::Audio) .ok_or_else(|| "No audio stream found".to_string())?; let stream_duration = stream.duration(); if stream_duration > 0 && stream_duration != i64::MIN { let time_base = stream.time_base(); if time_base.denominator() != 0 { duration = (stream_duration as f64 * time_base.numerator() as f64) / time_base.denominator() as f64; } } } // Validate and warn if !duration.is_finite() || duration <= 0.0 { tracing::warn!(?path, "Failed to determine audio duration; defaulting to zero"); duration = 0.0; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.github/workflows/publish.yml(9 hunks)apps/desktop/src-tauri/Cargo.toml(1 hunks)apps/desktop/src-tauri/src/recording.rs(1 hunks)crates/recording/src/output_pipeline/win.rs(2 hunks)crates/rendering/src/project_recordings.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
crates/recording/src/output_pipeline/win.rscrates/rendering/src/project_recordings.rsapps/desktop/src-tauri/src/recording.rs
crates/*/src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Rust crates should place tests within the
src/and/or a siblingtests/directory for each crate insidecrates/*.
Files:
crates/recording/src/output_pipeline/win.rscrates/rendering/src/project_recordings.rs
🧠 Learnings (3)
📚 Learning: 2025-10-28T08:39:42.230Z
Learnt from: Brendonovich
Repo: CapSoftware/Cap PR: 1305
File: crates/recording/src/output_pipeline/macos.rs:80-90
Timestamp: 2025-10-28T08:39:42.230Z
Learning: In `crates/recording/src/output_pipeline/macos.rs`, the `AVFoundationMp4Muxer` intentionally holds the `Mutex<MP4Encoder>` lock during retry attempts in `send_video_frame()` and `send_audio_frame()`. This blocking behavior is correct because frame processing must happen sequentially to prevent audio and video frames from being interleaved incorrectly in the encoder.
Applied to files:
crates/recording/src/output_pipeline/win.rscrates/rendering/src/project_recordings.rs
📚 Learning: 2025-10-17T05:58:22.586Z
Learnt from: Brendonovich
Repo: CapSoftware/Cap PR: 1219
File: crates/enc-avfoundation/src/mp4.rs:350-373
Timestamp: 2025-10-17T05:58:22.586Z
Learning: In crates/enc-avfoundation/src/mp4.rs, the `finish()` method intentionally skips video extension when `is_paused` is true. This is correct behavior because if recording is paused, the video should not be extended beyond the pause point—the pause is user-initiated, unlike the case where ScreenCaptureKit stops providing frames during static content.
Applied to files:
crates/recording/src/output_pipeline/win.rscrates/rendering/src/project_recordings.rs
📚 Learning: 2025-09-23T12:34:13.729Z
Learnt from: Brendonovich
Repo: CapSoftware/Cap PR: 1053
File: .github/workflows/publish.yml:222-230
Timestamp: 2025-09-23T12:34:13.729Z
Learning: In the Cap desktop app, there's a beforeBundle Tauri script (prodBeforeBundle.js) that creates Cap.dSYM in the target/ directory using dsymutil, so the Sentry debug symbols upload path "target/Cap.dSYM" is correct and doesn't need to be changed to the matrix target subdirectory.
Applied to files:
.github/workflows/publish.yml
🧬 Code graph analysis (2)
crates/rendering/src/project_recordings.rs (2)
apps/desktop/src-tauri/src/export.rs (1)
fps(17-22)apps/desktop/src/utils/tauri.ts (1)
Video(484-484)
apps/desktop/src-tauri/src/recording.rs (3)
crates/rendering/src/project_recordings.rs (2)
duration(217-219)duration(235-246)crates/project/src/configuration.rs (2)
duration(477-479)duration(542-544)apps/desktop/src/utils/tauri.ts (1)
TimelineSegment(478-478)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (rust)
🔇 Additional comments (13)
apps/desktop/src-tauri/Cargo.toml (1)
3-3: Version bump aligns with release workflow.The version bump from 0.3.82 to 0.3.83 is appropriate and consistent with the coordinated publish workflow changes mentioned in the AI summary. Per the summary, this version information is used by the publish workflow for tag creation and messaging.
.github/workflows/publish.yml (6)
11-24: Workflow inputs correctly define Windows and macOS targeting.The new
windowsTarget(all/x64/arm64) andbuildMac(boolean) inputs provide good control over selective builds. The defaults (all, true) ensure backward compatibility with existing workflows.
163-180: Matrix expansion properly supports Windows ARM64.The addition of platform and arch fields for all four combinations (macOS x64/arm64, Windows x64/arm64) is clean and well-organized. Each target correctly specifies its
aarch64-pc-windows-msvcoraarch64-apple-darwintriple where applicable.
181-184: RUN_BUILD logic correctly gates execution across platforms.The expression properly evaluates selective builds: macOS jobs run only when
buildMac=true, Windows jobs when the target matches the current arch or whenwindowsTarget=all. The single-line format resolves the previous YAML syntax error.
188-233: Step-level gating is consistently applied.All critical steps (checkout, signing, Rust/JS setup, env file, build) are properly gated by
env.RUN_BUILD == 'true'. The .env file augmentation now includesRUST_TARGET_TRIPLE, which enables architecture-aware build behavior in downstream tooling.
310-327: Sentry uploads correctly handle architecture-specific debug symbols.The split logic is sound: macOS uploads
target/Cap.dSYM(per learnings, created by beforeBundle script in target/ root), while Windows uploads the architecture-specifictarget/${{ matrix.settings.target }}/release/cap_desktop.pdb. Both steps are properly gated.Please verify that the
blacksmith-32vcpu-windows-2025runner has native ARM64 support for theaarch64-pc-windows-msvctarget, or whether cross-compilation is expected. The runner name suggests x86-based infrastructure, which may require additional setup.
129-153: Discord notification refactoring maintains correct payload handling.The restructured fetch calls in both draft and done jobs (using cleaner multiline formatting) preserve the original logic and error handling. Payload construction is consistent (
type,interactionId,version,releaseUrl,cnReleaseId) and theresp.status !== 200check appropriately surfaces errors.Also applies to: 345-368
apps/desktop/src-tauri/src/recording.rs (1)
1312-1329: Duration validation is correctly implemented with safe downstream handling.The duration validation logic is solid defensive programming. Clamping non-finite or non-positive durations to 0.0 prevents downstream issues while maintaining segment array consistency.
Verification confirms zero-duration segments are handled safely throughout the codebase:
- Time lookup accumulates durations correctly (zero-duration segments contribute nothing)
- Duration calculations sum correctly (zero values are valid)
- Rendering/playback iterates by segment index (unaffected by zero duration)
- No division-by-zero or loop termination risks
The field name change to
recording_segmentcorrectly aligns with the public API update.crates/rendering/src/project_recordings.rs (4)
36-40: Good defensive check for FPS calculation.The guard against division by zero prevents potential panics and defaults to a safe value.
42-47: Container duration extraction looks good.The sentinel value check (
i64::MIN) and unit conversion from microseconds to seconds are correct.
58-63: Frame-based fallback implemented correctly.The calculation and positive value checks for both frames and fps are appropriate.
65-75: Excellent validation and diagnostic logging.The finite check and comprehensive warning message with all attempted values will help diagnose issues in production.
crates/recording/src/output_pipeline/win.rs (1)
256-266: Appreciate the explicit flushLocking the muxer output and flushing the software encoder before returning guarantees buffered frames hit the file. Nice reliability win.
This reverts commit fe8a8b8.
|
looks like a merge went wrong or something, a bunch of changes have been undone 
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
crates/rendering/src/project_recordings.rs (3)
60-61: Reintroduce guard for zerotime_basedenominator.This fallback divides by
stream.time_base().denominator()without checking for zero, so malformed streams will panic. This was fixed earlier and appears to have regressed—please restore the guard before doing the division.- if stream_duration > 0 && stream_duration != i64::MIN { - let time_base = stream.time_base(); - duration = (stream_duration as f64 * time_base.numerator() as f64) - / time_base.denominator() as f64; + if stream_duration > 0 && stream_duration != i64::MIN { + let time_base = stream.time_base(); + let denom = time_base.denominator(); + if denom != 0 { + duration = (stream_duration as f64 * time_base.numerator() as f64) + / (denom as f64); + } }
83-84: Guard the secondtime_basedivision as well.Same regression here:
tb.denominator()can be zero, so this division can panic. Add the zero check before dividing.- if last_ts >= 0 { - let tb = stream.time_base(); - duration = (last_ts as f64 * tb.numerator() as f64) - / tb.denominator() as f64; + if last_ts >= 0 { + let tb = stream.time_base(); + let denom = tb.denominator(); + if denom != 0 { + duration = (last_ts as f64 * tb.numerator() as f64) + / (denom as f64); + } }
107-112: Restore finite check before casting FPS.Without
fps.is_finite(), an infinite value passes the> 0.0check andround()saturates tou32::MAX. This was previously fixed; please reapply the guard.Ok(Video { width: video_decoder.width(), height: video_decoder.height(), duration, - fps: if fps > 0.0 { fps.round() as u32 } else { 0 }, + fps: if fps.is_finite() && fps > 0.0 { + fps.round() as u32 + } else { + 0 + }, start_time, })
🧹 Nitpick comments (1)
crates/recording/src/output_pipeline/win.rs (1)
182-196: Good explicit header write with proper error handling.The mutex locking, error propagation, and header write logic are correct. The explicit handling of
PoisonErrorand propagation through the ready channel ensures failures are properly communicated.As a minor optimization, consider dropping
output_guardexplicitly before sending the ready signal:if let Err(e) = output_guard.write_header() { error!("Failed to write header: {:#}", e); let _ = ready_tx.send(Err(anyhow!("write_header: {e}"))); return Err(anyhow!("write_header: {e}")); } + + drop(output_guard); if ready_tx.send(Ok(())).is_err() {This minimizes the lock holding duration, though the impact is minimal since this occurs during setup before frame processing begins.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/recording/src/output_pipeline/win.rs(4 hunks)crates/rendering/src/project_recordings.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
crates/recording/src/output_pipeline/win.rscrates/rendering/src/project_recordings.rs
crates/*/src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Rust crates should place tests within the
src/and/or a siblingtests/directory for each crate insidecrates/*.
Files:
crates/recording/src/output_pipeline/win.rscrates/rendering/src/project_recordings.rs
🧠 Learnings (2)
📚 Learning: 2025-10-28T08:39:42.230Z
Learnt from: Brendonovich
Repo: CapSoftware/Cap PR: 1305
File: crates/recording/src/output_pipeline/macos.rs:80-90
Timestamp: 2025-10-28T08:39:42.230Z
Learning: In `crates/recording/src/output_pipeline/macos.rs`, the `AVFoundationMp4Muxer` intentionally holds the `Mutex<MP4Encoder>` lock during retry attempts in `send_video_frame()` and `send_audio_frame()`. This blocking behavior is correct because frame processing must happen sequentially to prevent audio and video frames from being interleaved incorrectly in the encoder.
Applied to files:
crates/recording/src/output_pipeline/win.rscrates/rendering/src/project_recordings.rs
📚 Learning: 2025-10-17T05:58:22.586Z
Learnt from: Brendonovich
Repo: CapSoftware/Cap PR: 1219
File: crates/enc-avfoundation/src/mp4.rs:350-373
Timestamp: 2025-10-17T05:58:22.586Z
Learning: In crates/enc-avfoundation/src/mp4.rs, the `finish()` method intentionally skips video extension when `is_paused` is true. This is correct behavior because if recording is paused, the video should not be extended beyond the pause point—the pause is user-initiated, unlike the case where ScreenCaptureKit stops providing frames during static content.
Applied to files:
crates/recording/src/output_pipeline/win.rscrates/rendering/src/project_recordings.rs
🧬 Code graph analysis (1)
crates/rendering/src/project_recordings.rs (1)
apps/desktop/src/utils/tauri.ts (1)
Video(484-484)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build (aarch64-pc-windows-msvc, windows-latest-l, windows, arm64)
- GitHub Check: Clippy (aarch64-apple-darwin, macos-latest)
- GitHub Check: Clippy (aarch64-pc-windows-msvc, windows-latest)
- GitHub Check: Clippy (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (3)
crates/recording/src/output_pipeline/win.rs (3)
245-250: LGTM - Clear message-driven pattern.The change to explicitly receive and destructure messages makes the None case more explicit and easier to follow. The scoped import of
AsFFmpegis appropriate.
272-281: Excellent error handling for the flush operation.The explicit mutex locking with
PoisonErrorhandling and the proper error context on the flush call ensure robust cleanup. This is consistent with the error handling patterns added elsewhere in this change.
233-233: Verify inconsistent mutex error handling.The new changes add careful
PoisonErrorhandling when locking the output mutex (lines 182-189, 272-279), but Line 233 in the native encoder path still uses.unwrap()which will panic if the mutex is poisoned.For consistency and robustness, consider handling this error gracefully as well.
Based on learnings
Summary by CodeRabbit
New Features
Bug Fixes
Chores