fix(core): Comprehensive fixes and performance optimizations#18
Conversation
16230a8 to
6d157bd
Compare
8eca424 to
aeb6f84
Compare
a7fefd2 to
b867225
Compare
There was a problem hiding this comment.
Pull request overview
This PR consolidates pixelflux into a single self-contained Rust (PyO3) extension module that implements both X11 and Wayland capture plus encoding/conversion, while adding substantial performance, safety, and operational improvements across the capture/encode pipeline (NVENC/VA-API/software), including multi-GPU container handling and runtime API/version negotiation.
Changes:
- Packaging/build refactor: drop the legacy Python/C++ packaging layers and build a single
pixelfluxRust extension viasetuptools-rust+ updated cibuildwheel configuration. - New/rewritten capture + pipeline components: add an X11 XShm capture path in Rust, a shared pipeline for full-frame decisions (paint-over/recovery IDR), and a bounded recording sink for H.264 fan-out.
- Encoder/runtime hardening & perf: multi-GPU NVENC ioctl filtering, NVENC API negotiation, VA-API cleanup + host-input path, software encoder optimizations (thread-local JPEG compressor, x264 open/close serialization, improved damage hashing).
Reviewed changes
Copilot reviewed 27 out of 36 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.py | Switches distribution to a single Rust extension module (pixelflux) and removes the legacy C++ build path. |
| pyproject.toml | Updates cibuildwheel build matrix and provisioning steps for the new single-extension build. |
| README.md | Updates installation/docs to match the new single Rust extension API and zero-copy frame model. |
| MANIFEST.in | Updates sdist includes for the new Rust crate layout. |
| pixelflux/Cargo.toml | Introduces the new unified Rust crate (cdylib) and dependency set (ffmpeg-next 8.1 pinned, smithay, x11rb, nvcodec-sys). |
| pixelflux/.gitignore | Adds Rust build outputs to ignore list. |
| pixelflux/init.py | Removes the legacy ctypes-based Python wrapper layer. |
| pixelflux/src/x11/mod.rs | Adds a Rust X11 XShm capture implementation with a bounded capture→encode handoff. |
| pixelflux/src/pipeline.rs | Adds shared full-frame decision logic (paint-over + recovery/on-demand IDR policy). |
| pixelflux/src/recording_sink.rs | Adds an out-of-band Unix socket H.264 recording sink with per-client bounded queues. |
| pixelflux/src/wayland/mod.rs | Defines Wayland module structure for the unified crate. |
| pixelflux/src/wayland/frontend.rs | Cursor hashing/stride hardening, cache eviction changes, and callback GIL attachment updates. |
| pixelflux/src/wayland/cursor.rs | Hardens xcursor parsing against empty-image cursor files. |
| pixelflux/src/encoders/mod.rs | Adds unified encoder module layout. |
| pixelflux/src/encoders/software.rs | Performance + correctness work: thread-local JPEG compressor, x264 open/close serialization, new damage hashing, header omission support, rate control improvements. |
| pixelflux/src/encoders/nvenc.rs | NVENC API negotiation, multi-GPU container filtering install, header omission, better error handling and resource teardown. |
| pixelflux/src/encoders/vaapi.rs | Improves unwind safety/cleanup, adds host-input VAAPI path, supports header omission, and rate-control fixes. |
| pixelflux/src/encoders/overlay.rs | Adds overlay/watermark state utilities for Smithay rendering. |
| pixelflux/src/nvgpufilter.rs | Adds GOT-patched ioctl filter to drop unreachable GPUs inside containers on affected drivers. |
| pixelflux/nvcodec-sys/* | Introduces an in-tree NVENC/CUDA binding crate with optional regen flow. |
| pixelflux_wayland/* | Removes the old separate Wayland extension crate (now unified). |
| example/screen_to_browser.py | Updates example usage to the new callback/frame object API and zero-copy send model. |
Comments suppressed due to low confidence (2)
pixelflux/src/encoders/software.rs:84
- Using
bitrate_kbps.abs()can overflow fori32::MIN(wrap in release / panic in debug), which could misconfigure x264 rate control. Prefersaturating_abs()for a safe, bounded value.
pixelflux/src/encoders/software.rs:155 - Using
bitrate_kbps.abs()can overflow fori32::MIN(wrap in release / panic in debug). Usesaturating_abs()so runtime rate updates can't trigger an overflow edge case.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
48fde0f to
d8973bf
Compare
6ff706b to
d276206
Compare
968a4f4 to
8dbc8ab
Compare
|
Two findings from second-eye testing this branch (together with selkies-project/selkies#254) on an Ubuntu noble (lsio-based) image, Intel laptop: 1. The module can't import on noble even for X11-only use: 2. VA-API encode on X11 always falls back to software: the filter graph is parsed before the VAAPI device is attached. On an Intel iGPU where the device derive and In 🤖 Generated with Claude Code |
|
Fixes incorporated in selkies-project/selkies@de77558, b32a178, linuxserver/pcmflux@0cbadcb. |
| let mut width: i32 = if initial_width > 0 { initial_width } else { 1024 }; | ||
| let mut height: i32 = if initial_height > 0 { initial_height } else { 768 }; | ||
|
|
||
| if let Ok(w_str) = std::env::var("SELKIES_MANUAL_WIDTH") { |
There was a problem hiding this comment.
@thelamer I moved all envvars into Selkies configurations and exposed them through internal ABI configurations pushed from Selkies (harder to manage lists of environments otherwise). Not desired.
There was a problem hiding this comment.
This will be the only one without re-writing the standard module init to accept vars on just loading the module or making a new function to just init the display without starting the stream. This seems like the easiest way and is battle tested.
There was a problem hiding this comment.
Then, I must refactor it on Sunday. Just let me know of similar things and I'll refactor it later on.
Complementary to selkies-project/selkies#254. Backward-incompatible components.