streamer(vulkan): prioritize capable dGPUs and fix AppImage GLib clash - #992
Merged
Merged
Conversation
- Filter and score Vulkan physical devices based on video decode queue isolation and PRIME offload variables instead of indexing device 0. - Fall through to alternative physical devices when queue isolation fails. - Exclude host GLib and GObject binaries from the AppImage packaging pipeline to resolve host VA-API driver dlopen symbol clashes. - Ensure proper sync object drain before Vulkan teardown under Wayland.
Score computation now takes an explicit VulkanDevicePreferences value, with a thin from_env wrapper preserving the OPENNOW_VK_* / PRIME behavior. The unit tests exercise the pure function directly, so they no longer mutate process-wide env vars and cannot flake when cargo runs them in parallel. Also addresses review nits: sort_by_key, drop the redundant unsafe around the safe options builder, and fall back to default device selection if the index string ever fails CString conversion instead of unwrapping.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
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.
Merges #989 (by @quadsteel) with CI fixes applied on top. The original commit is preserved verbatim with authorship intact as
68dc0b16; the follow-up commit addresses the review findings.Original change (quadsteel): intelligent Vulkan device selection and scoring that prioritizes capable discrete GPUs on hybrid systems, clean
vkDeviceWaitIdleteardown for Wayland surfaces, and AppImage GLib/GObject host exclusion to fix driver symbol collisions. Full rationale and hardware verification are in #989.Fixes applied on top:
VulkanDevicePreferencesvalue with a thinfrom_envwrapper, so the unit tests exercise a pure function instead of mutating process-wide env state. The original tests flaked ~60% of runs when cargo scheduled the two scoring tests on parallel threads.cargo fmtthe touched package (CICheck Rust formattingfailed on linux-x64 and macos-arm64).clippy::unnecessary_sort_bylint in both ranking call sites.unsafeblock around the safe FFmpeg options builder and replace aCString::unwrapwith a graceful fallback to default device selection.Validation:
cargo test --workspace(streamer): all suites green, scoring tests stable across repeated runs.cargo clippy -p opennow-streamer-platform-linux --all-targets: clean.cargo fmt --all --check(streamer + core): clean.test_ci_workflow.py+test_bundled_deb.py: 23 passed.Closes #989 (superseded; will close that PR once this merges).