feat: port Linux v4l2loopback backend to Rust core - #5
Merged
Conversation
Introduce a Rust workspace with shared format conversion and a PyO3 Linux backend while keeping the Python API unchanged. Enable mock tests in Linux CI and document the migration for reviewers. Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR introduces a Rust workspace ( ChangesRust Core and Python Bindings for Virtual Camera
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Enable org-standard luxie-tronic reviews on pull requests, with a pyvirtualcam-specific policy covering Rust and Python paths. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
crates/pyvirtualcam-core/src/error.rs (1)
43-43: ⚡ Quick winPreserve
std::error::Errorsource chaining forError::IoLine 43:
impl std::error::Error for Error {}does not exposesourcefor theIovariant, so callers lose causal chain inspection.Suggested fix
-impl std::error::Error for Error {} +impl std::error::Error for Error { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Self::Io { source, .. } => Some(source), + _ => None, + } + } +}🤖 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 `@crates/pyvirtualcam-core/src/error.rs` at line 43, The impl of std::error::Error for the Error enum currently doesn't expose a source for Error::Io; update the impl of std::error::Error for Error to override source(&self) -> Option<&(dyn std::error::Error + 'static)> (or the modern source method signature) and return the inner io::Error as Some(&inner) when matching Error::Io(...), otherwise return None; locate the Error enum and the impl block for std::error::Error to add this match for the Io variant so callers can inspect the causal chain.crates/pyvirtualcam-core/src/linux.rs (1)
456-467: ⚡ Quick winLayout tests verify size but not field offsets — add offset checks.
v4l2_struct_layout_matches_linux_headersonly asserts total sizes, which is exactly why theV4l2Formatoffset mismatch (see Line 440) slips through. Addoffset_of!assertions for the critical fields (V4l2Format::fmt,V4l2PixFormat::pixelformat) to guard the ABI against future edits.🤖 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 `@crates/pyvirtualcam-core/src/linux.rs` around lines 456 - 467, The test v4l2_struct_layout_matches_linux_headers currently only checks total sizes; add offset assertions to ensure field layouts match the kernel headers: use offset_of! to assert the offset of V4l2Format::fmt and V4l2PixFormat::pixelformat (in the same test or a new one) against the expected byte offsets from the Linux headers so ABI breaks like the V4l2Format mismatch cannot slip through; update the test function v4l2_struct_layout_matches_linux_headers (or add v4l2_field_offsets_match_linux_headers) to include these offset_of! checks alongside the existing size assertions.
🤖 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 @.github/scripts/build-linux.sh:
- Around line 30-34: The script currently unconditionally sources
"$HOME/.cargo/env" which can fail if rustup wasn't installed and the file
doesn't exist; update the post-install logic by checking for the file's
existence before sourcing it (i.e., after the existing cargo presence
check/installation block, wrap the source "$HOME/.cargo/env" call in a
conditional that tests -f "$HOME/.cargo/env" and only sources when present) so
the build continues when the image already provides cargo or the env file is
absent.
In `@crates/pyvirtualcam-core/cpp/libyuv_wrapper.cpp`:
- Around line 17-25: The chroma plane stride/offsets in pyvc_rgb_to_i420 and
pyvc_bgr_to_i420 are computed with floor halves causing incorrect U/V pointers
for odd dimensions; change half_width to (width + 1) / 2 and half_height to
(height_abs + 1) / 2, use chroma_width (half_width) as the U/V stride and
compute the V-plane pointer offset as i420 + width * height_abs + chroma_width *
chroma_height so the calls to libyuv::RAWToI420 pass the corrected chroma
strides and pointers consistent with libyuv’s (width+1)/2 rounding.
In `@crates/pyvirtualcam-core/src/convert.rs`:
- Around line 43-44: The current narrowing of width/height with `as i32` in
convert functions can wrap large u32 values; before casting in convert_to_i420
(and in rgb_to_i420/bgr_to_i420 callers that forward u32), validate that width
and height are <= i32::MAX and return a clear error (or clamp) if not, then cast
to i32 and call the extern pyvc_*_to_i420 APIs; also harden
PixelFormat::frame_size by doing checked multiplication (using checked_mul on
usize) and return an error on overflow so buffer-length checks don't rely on
unchecked width*height arithmetic.
In `@crates/pyvirtualcam-core/src/formats.rs`:
- Around line 47-54: The I420/NV12 size math in frame_size undercounts when
width or height is odd; update the I420 | Nv12 arm in pub fn frame_size(self,
width: u32, height: u32) -> usize to compute chroma width and height with
ceiling division (chroma_w = (width as usize + 1) / 2, chroma_h = (height as
usize + 1) / 2), compute chroma_pixels = chroma_w * chroma_h, and return pixels
+ 2 * chroma_pixels (instead of pixels * 3 / 2). Also apply the same ceil-chroma
logic to any other places that compute chroma plane sizes (e.g., the
corresponding conversion/stride calculations referenced near the other match
arms) so all I420/NV12 code paths use (w+1)/2 and (h+1)/2 for chroma dimensions.
In `@crates/pyvirtualcam-core/src/linux.rs`:
- Around line 253-258: The TOCTOU bug: `try_open` only checks active_devices()
then releases the mutex while opening/configuring, allowing races with
concurrent V4l2LoopbackCamera::new; fix by making the check+reserve atomic under
the same mutex—either (A) modify try_open to lock active_devices(), check
contains(device_name) and insert a reservation before returning the fd, and
ensure any failure path (including subsequent configure_device errors) removes
that reservation, or (B) change V4l2LoopbackCamera::new to hold the
active_devices() guard across contains → insert → open → configure so the device
cannot be opened concurrently; reference active_devices()/ACTIVE_DEVICES,
try_open, and V4l2LoopbackCamera::new when applying the change and ensure
cleanup removes the inserted device on any error.
In `@crates/pyvirtualcam-py/src/lib.rs`:
- Around line 21-27: The constructor currently accepts an fps parameter but
ignores it when creating the backend (fps is not passed into
V4l2LoopbackCamera::new), so update the call site in the function that invokes
V4l2LoopbackCamera::new to either (A) forward the fps value into
V4l2LoopbackCamera::new (and update that constructor signature/implementation
accordingly) or (B) validate/reject unsupported fps values before constructing
the backend; specifically modify the function that calls parse_devices(...) and
V4l2LoopbackCamera::new(width, height, fourcc, devices) to include fps (e.g.
V4l2LoopbackCamera::new(width, height, fps, fourcc, devices) or validate fps and
return a PyErr via to_py_err) so pyvirtualcam.Camera semantics remain consistent
with PixelFormat/Backend/register_backend.
In `@setup.py`:
- Around line 182-184: Update the unpinned setuptools-rust requirement to a
Python-3.8-compatible bound across all build manifests and scripts: change the
setup_requires entry in setup.py (currently "pybind11>=2.6.0",
"setuptools-rust") to pin setuptools-rust (e.g., "setuptools-rust<1.11.0" or
"setuptools-rust==1.10.2"); apply the exact same constraint to pyproject.toml's
build-system.requires and to any CI install commands that call pip install
setuptools-rust (e.g., in .github/scripts/build-windows.ps1, build-macos.sh,
build-linux.sh) so all places use the same pinned version range.
---
Nitpick comments:
In `@crates/pyvirtualcam-core/src/error.rs`:
- Line 43: The impl of std::error::Error for the Error enum currently doesn't
expose a source for Error::Io; update the impl of std::error::Error for Error to
override source(&self) -> Option<&(dyn std::error::Error + 'static)> (or the
modern source method signature) and return the inner io::Error as Some(&inner)
when matching Error::Io(...), otherwise return None; locate the Error enum and
the impl block for std::error::Error to add this match for the Io variant so
callers can inspect the causal chain.
In `@crates/pyvirtualcam-core/src/linux.rs`:
- Around line 456-467: The test v4l2_struct_layout_matches_linux_headers
currently only checks total sizes; add offset assertions to ensure field layouts
match the kernel headers: use offset_of! to assert the offset of V4l2Format::fmt
and V4l2PixFormat::pixelformat (in the same test or a new one) against the
expected byte offsets from the Linux headers so ABI breaks like the V4l2Format
mismatch cannot slip through; update the test function
v4l2_struct_layout_matches_linux_headers (or add
v4l2_field_offsets_match_linux_headers) to include these offset_of! checks
alongside the existing size assertions.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ff108083-f101-4925-8e7d-f829c7f44909
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
.github/scripts/build-linux.sh.github/scripts/build-macos.sh.github/scripts/build-windows.ps1.github/scripts/test-linux.sh.gitignoreAGENTS.mdCHANGELOG.mdCargo.tomlMANIFEST.inREADME.mdcrates/pyvirtualcam-core/Cargo.tomlcrates/pyvirtualcam-core/README.mdcrates/pyvirtualcam-core/build.rscrates/pyvirtualcam-core/cpp/libyuv_wrapper.cppcrates/pyvirtualcam-core/examples/simple.rscrates/pyvirtualcam-core/src/camera.rscrates/pyvirtualcam-core/src/convert.rscrates/pyvirtualcam-core/src/error.rscrates/pyvirtualcam-core/src/formats.rscrates/pyvirtualcam-core/src/fourcc.rscrates/pyvirtualcam-core/src/lib.rscrates/pyvirtualcam-core/src/linux.rscrates/pyvirtualcam-py/Cargo.tomlcrates/pyvirtualcam-py/src/lib.rsdocs/conf.pydocs/index.rstexamples/README.mdpyproject.tomlsetup.pytest/test_backend_contract.pytest/test_camera.py
Fix I420 buffer sizing for odd dimensions, guard V4L2 device reservation, validate conversion dimensions, pin setuptools-rust for Python 3.8, and apply other review-driven hardening. Co-authored-by: Cursor <cursoragent@cursor.com>
Author
|
Replying to the two CodeRabbit nitpick items from the summary (no inline threads):
|
Pin manylinux2014 images that still ship cp38 for 3.8 matrix jobs and enable PyO3 ABI3 forward compatibility when building cp313 wheels. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
37-37: ⚖️ Poor tradeoffPinning is inconsistent across the matrix.
Only the cp38 rows pin to the dated tag
:2026.05.01-2; the cp39–cp313 rows still use floatingmanylinux2014_x86_64/manylinux_2_28_aarch64tags. Floating tags pull whatever the registry serves at build time, which undermines reproducibility and can cause non-cp38 jobs to drift while cp38 stays fixed. Consider pinning all rows to a consistent dated tag (or digest) for reproducible builds.Also applies to: 74-74, 235-235, 272-272
🤖 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 @.github/workflows/ci.yml at line 37, The workflow uses a dated pinned image only for the cp38 matrix row (docker-image: quay.io/pypa/manylinux2014_x86_64:2026.05.01-2) while other matrix rows still reference floating tags (manylinux2014_x86_64 / manylinux_2_28_aarch64); update all matrix entries that currently use the unpinned images to the same dated tag (or preferably to the corresponding image digest) so every docker-image entry is consistently pinned (apply the same change for the other occurrences noted in the matrix).
🤖 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 @.github/scripts/build-macos.sh:
- Line 32: The macOS build installs setuptools-rust even though setup.py only
builds the RustExtension (RustExtension,
pyvirtualcam._native_linux_v4l2loopback) on Linux, which can force a Rust
toolchain on macOS; modify .github/scripts/build-macos.sh to remove
'setuptools-rust>=1.10.2,<1.11.0' from the pip install line and update setup.py
so that setuptools-rust is only declared/required (e.g. in setup_requires or
conditional imports) when platform.system() == 'Linux' and RustExtension will be
used, ensuring macOS builds do not attempt to pull or build the Rust toolchain.
In @.github/workflows/luxie-tronic.yml:
- Around line 5-33: The workflow uses pull_request_target with broad write
permissions and forwards all repo secrets via secrets: inherit to a mutable
reusable workflow ref (automation_ref/v1.6); update the generator config
(.github/lux-ci-config.json) to pin automation_ref to a full commit SHA
(immutable) instead of a tag, regenerate the workflow so the reusable-workflow
reference in luxie-tronic.yml points to that SHA, remove secrets: inherit and
instead pass only the minimal required secrets explicitly to the reusable
workflow, and tighten permissions (contents/pull-requests/issues) to least
privilege for the review job to eliminate secret leakage risk.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 37: The workflow uses a dated pinned image only for the cp38 matrix row
(docker-image: quay.io/pypa/manylinux2014_x86_64:2026.05.01-2) while other
matrix rows still reference floating tags (manylinux2014_x86_64 /
manylinux_2_28_aarch64); update all matrix entries that currently use the
unpinned images to the same dated tag (or preferably to the corresponding image
digest) so every docker-image entry is consistently pinned (apply the same
change for the other occurrences noted in the matrix).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 71f0da3f-56cd-4d7d-91ff-f60aade313eb
📒 Files selected for processing (16)
.codex/luxie-tronic-policy.json.github/lux-ci-config.json.github/scripts/build-linux.sh.github/scripts/build-macos.sh.github/scripts/build-windows.ps1.github/workflows/ci.yml.github/workflows/luxie-tronic.yml.gitignorecrates/pyvirtualcam-core/cpp/libyuv_wrapper.cppcrates/pyvirtualcam-core/src/convert.rscrates/pyvirtualcam-core/src/error.rscrates/pyvirtualcam-core/src/formats.rscrates/pyvirtualcam-core/src/linux.rscrates/pyvirtualcam-py/src/lib.rspyproject.tomlsetup.py
✅ Files skipped from review due to trivial changes (2)
- pyproject.toml
- .codex/luxie-tronic-policy.json
🚧 Files skipped from review as they are similar to previous changes (9)
- .github/scripts/build-windows.ps1
- crates/pyvirtualcam-core/src/convert.rs
- .gitignore
- crates/pyvirtualcam-core/src/error.rs
- crates/pyvirtualcam-py/src/lib.rs
- crates/pyvirtualcam-core/cpp/libyuv_wrapper.cpp
- crates/pyvirtualcam-core/src/formats.rs
- setup.py
- crates/pyvirtualcam-core/src/linux.rs
GitHub retired the macos-13 runner image in December 2025, which left Intel matrix jobs queued indefinitely and blocked downstream test/docs. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
43-70:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPin all manylinux Docker images to fixed tags in
ci.yml(avoid untaggedquay.io/pypa/...).
buildpinsquay.io/pypa/manylinux2014_x86_64:2026.05.01-2for Python 3.8, but uses untaggedquay.io/pypa/manylinux2014_x86_64for Python 3.9–3.13 (implicitly:latest, which can drift) at lines 43-70; same issue on ARM at lines 80-107 (manylinux_2_28_aarch64is untagged for 3.9–3.13). Thetestjob repeats this pattern at lines 242-269 and 279-306. Pin the remaining x86_64/aarch64 entries to the same fixed tag already used for Python 3.8 to keep CI reproducible.🤖 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 @.github/workflows/ci.yml around lines 43 - 70, The CI uses untagged quay.io/pypa manylinux images for Python 3.9–3.13 (fields docker-image for entries with python-version '3.9'..'3.13' and the ARM entries using manylinux_2_28_aarch64), which can drift; update those docker-image values to the fixed tag used for Python 3.8 (quay.io/pypa/manylinux2014_x86_64:2026.05.01-2 and the corresponding manylinux_2_28_aarch64:2026.05.01-2) in the matrix entries for x86_64 and aarch64 across the build and test job matrices so all python-version rows use the pinned tag.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
109-109: 💤 Low valueConsider clarifying the retirement date format in the comment.
The comment references "2025-12-04" but mixes ISO date format with explanatory text. While the information is correct and helpful, consider making it more precise.
💬 Optional clarity improvement
- # GitHub retired macos-13 on 2025-12-04; use macos-15-intel for x86_64. + # GitHub retired macos-13 (December 4, 2025); use macos-15-intel for x86_64.Or with a reference:
- # GitHub retired macos-13 on 2025-12-04; use macos-15-intel for x86_64. + # macos-13 was retired on 2025-12-04; macos-15-intel provides x86_64 support.🤖 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 @.github/workflows/ci.yml at line 109, Update the inline comment string "GitHub retired macos-13 on 2025-12-04; use macos-15-intel for x86_64." to clarify the retirement date format (e.g., "GitHub retired macos-13 on 2025-12-04 (YYYY-MM-DD) — Dec 4, 2025; use macos-15-intel for x86_64." or similar) so the date is unambiguous while preserving the guidance about using macos-15-intel.
🤖 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.
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 43-70: The CI uses untagged quay.io/pypa manylinux images for
Python 3.9–3.13 (fields docker-image for entries with python-version
'3.9'..'3.13' and the ARM entries using manylinux_2_28_aarch64), which can
drift; update those docker-image values to the fixed tag used for Python 3.8
(quay.io/pypa/manylinux2014_x86_64:2026.05.01-2 and the corresponding
manylinux_2_28_aarch64:2026.05.01-2) in the matrix entries for x86_64 and
aarch64 across the build and test job matrices so all python-version rows use
the pinned tag.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 109: Update the inline comment string "GitHub retired macos-13 on
2025-12-04; use macos-15-intel for x86_64." to clarify the retirement date
format (e.g., "GitHub retired macos-13 on 2025-12-04 (YYYY-MM-DD) — Dec 4, 2025;
use macos-15-intel for x86_64." or similar) so the date is unambiguous while
preserving the guidance about using macos-15-intel.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5a9f18c8-ac85-4ce3-9d69-313b480bcf2d
📒 Files selected for processing (1)
.github/workflows/ci.yml
Gate setuptools-rust to Linux-only builds, pin all manylinux matrix images to dated tags, and harden luxie-tronic with SHA-pinned workflow ref and explicit secret passing. Co-authored-by: Cursor <cursoragent@cursor.com>
Author
|
Addressed the outside-diff ci.yml manylinux pinning comment in 658d668: all x86_64 matrix rows now use |
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # .github/lux-ci-config.json # .github/workflows/luxie-tronic.yml
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
pyvirtualcam-core,pyvirtualcam-py) with shared pixel-format conversion (libyuv), a native Rust camera API, and PyO3 bindings for the Linuxv4l2loopbackbackend.pyvirtualcam.Camera, backends, pixel formats); macOS and Windows still use the existing C++/ObjC++ extensions.AGENTS.md, crate README, Rust example).Design notes
setuptools-rust; CI installs Rust on Linux only.linux.rs) after fixing a struct-size mismatch found on aarch64 (GB8).Upstream divergence (intentionally deferred)
This fork is based on upstream
letmaik/pyvirtualcambut does not include these recent upstream commits yet:manylinux_2_28(Update x86_64 Linux builds to manylinux_2_28 letmaik/pyvirtualcam#138)mypy/py.typed(Add mypy type checking and py.typed marker letmaik/pyvirtualcam#139)Reason: this fork maintains Python 3.8 support and a Linux/macOS-only CI matrix. We can cherry-pick
mypy/py.typedin a follow-up if desired.Out of scope for this PR
pyvirtualcam/native_linux_v4l2loopback/)Test plan
cargo fmt --check,cargo clippy --workspace --locked --all-targets -- -D warningscargo test --workspace --lockedpytest test/test_backend_contract.py test/test_util.pysphinx-build -b html docs dist-docspython setup.py bdist_wheel(x86_64 Linux)/dev/video0smoke test)Made with Cursor
Summary by CodeRabbit
Release Notes
New Features
Changed
Documentation