properly handle planar data in avfoundation encoder

[Brendonovich]

Summary by CodeRabbit

• Bug Fixes
  • Improved accuracy of audio frame data processing in the media encoder to ensure correct data alignment and copying for multi-channel audio streams.

[coderabbitai]

Walkthrough

The planar audio data copying logic in queue_audio_frame is corrected to compute the per-plane data length as frame samples multiplied by bytes per sample, replacing the previous entire buffer length calculation. This ensures accurate copying of each audio plane.

Changes

Cohort / File(s)	Summary
Audio plane data copying fix crates/enc-avfoundation/src/mp4.rs	Updated queue_audio_frame to calculate per-plane channel data size using channel_data_len (frame.samples() × frame.format().bytes()) instead of data.len(), improving accuracy of audio frame buffering

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Specific areas for attention:
  • Verify that frame.samples() * frame.format().bytes() correctly computes the per-plane data size for all supported audio formats
  • Confirm that the offset increment by channel_data_len properly handles multi-plane audio buffers without off-by-one or bounds errors

Poem

🐰 The audio frames hop so true,
Per-plane lengths now computed new,
No more buffer length mistakes,
Each sample sings as codec makes! 🎵

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The pull request title "properly handle planar data in avfoundation encoder" is directly and fully related to the main change in the changeset. The core modification involves fixing how planar audio data is handled in the queue_audio_frame function by computing the correct channel_data_len instead of using the entire data length, which is precisely what the title describes. The title is concise, specific, and uses clear language that accurately conveys the primary purpose of the change without vague terminology or unnecessary details.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch handle-planar-audio-avfoundation

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6be1d52 and 25428b4.

📒 Files selected for processing (1)

• crates/enc-avfoundation/src/mp4.rs (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Format Rust code using rustfmt and 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/enc-avfoundation/src/mp4.rs

crates/*/src/**/*

📄 CodeRabbit inference engine (AGENTS.md)

Rust crates should place tests within the src/ and/or a sibling tests/ directory for each crate inside crates/*.

Files:

• crates/enc-avfoundation/src/mp4.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/enc-avfoundation/src/mp4.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/enc-avfoundation/src/mp4.rs

⏰ 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)

• GitHub Check: Clippy
• GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
• GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
• GitHub Check: Analyze (rust)

🔇 Additional comments (1)

crates/enc-avfoundation/src/mp4.rs (1)

326-334: LGTM! Correct fix for planar audio data copying.

The calculation of channel_data_len as frame.samples() * frame.format().bytes() correctly determines the per-plane data size for planar audio, where each plane contains one channel's samples. This fixes the previous issue where the entire plane buffer length was used, which could include padding or extra allocation beyond the actual audio data.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}