Repository navigation
fix: verify 10-bit probe succeeded before advertising - #5690
Conversation
bitness
changed the title
bitness
changed the title
|
Hmm, SonarQube is displeased with the new variables I added not being |
|
Sunshine has a lot of existing sonar issues. I'll review if these should be fixed or not. Thanks. |
There was a problem hiding this comment.
The separate 10-bit capability checks address the reported 8-bit/10-bit advertisement mismatch. An isolated MSYS2 UCRT64/gtest harness using the current head's advertisement function and probe-result assignments passed four tests, including 18,432 mode/capability combinations. Please add the required regression coverage to the PR before merging.
Correction: the new arrays follow the established mutable probe-capability cache pattern. Under the maintainer's policy, the cpp:S5421 warnings do not require a refactor for this PR; that inline finding has been withdrawn and resolved. The request for changes now concerns only the missing regression tests.
Verification limit: this was isolated logic validation, not a full Sunshine build or hardware encoder test. Native CI, lint, and CodeQL await maintainer workflow approval; the GH-Pages Read the Docs failure is a timeout waiting for that unstarted workflow, not a demonstrated code failure. The current head is mergeable against master.
9a5bd66 to
53be22b
Compare
ReenigneArcher
left a comment
ReenigneArcher
left a comment
There was a problem hiding this comment.
The core advertisement fix is still needed on current master, and the new tests cover the reported 8-bit/10-bit 4:4:4 mismatch and probe-result replacement. Two items remain: fix the two new enum-related Sonar findings in the test body, and add the missing negative 10-bit 4:2:0 advertisement cases for HEVC and AV1. Both inline comments include copyable corrections.
The four cpp:S5421 findings are the previously discussed mutable probe-result arrays. They still follow the existing accepted pattern; I am not reopening the withdrawn refactor request.
Validation: an isolated MSYS2 UCRT64/gtest harness using this head's real project headers and extracted production function bodies passed all nine added PR tests. A second copy incorporating the suggested enum changes and two additional rejection cases passed all 11 tests. This was not a full Sunshine build, hardware encoder validation, or a new Sonar analysis.
Workflow status: native CI, common lint, CodeQL, and GH-Pages workflows await maintainer approval; the linked GH-Pages Read the Docs build is cancelled, not evidence of a change-related code failure. Head 53be22be099456312ebd5d38434766bd5f9dfb28 is mergeable against current master with no confirmed conflicts.
Review completed by Codex | Model: gpt-6.1-sol | Thinking: xhigh
53be22b to
648aee8
Compare
This makes it easier for tests to access the mode flags produced by probing.
Having this code in separate functions makes it possible for tests to call them without duplicating their functionality.
Adds two arrays that store the probe results for h264, hevc, and av1's 10-bit 4:2:0 and 4:4:4 encoding support. The naming scheme for the arrays follows the codebase's use of "dynamic_range" to indicate 10-bit, even though these results can be used for SDR and HDR encoding.
With this change, 10-bit 444 encoder advertisement will correctly reflect the results of the encoder probes. Before it, the code used the probe result for 8-bit 444 encoding to decide whether or not to advertise 10-bit 444 encoding, but not all hardware can do both. (Mostly it seems like older Intel Media Engines and older VA-API versions make up that category.) In those cases, the code would still offer the 10-bit 444 advertisement, but if the client selected it, it wouldn't have worked.
This helps ensure that Sunshine won't advertise incorrect codec capabilities to clients.
648aee8 to
f4ffa2b
Compare
Bundle ReportBundle size has no change ✅ |
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5690 +/- ##
==========================================
+ Coverage 49.20% 49.31% +0.11%
==========================================
Files 114 114
Lines 24789 24820 +31
Branches 10709 10718 +9
==========================================
+ Hits 12197 12240 +43
- Misses 10165 11839 +1674
+ Partials 2427 741 -1686
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 33 files with indirect coverage changes Continue to review full report in Codecov by Harness.
|
Screenshot ComparisonPR #5690 screenshots vs Matrix:
|
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
 |
sunshine_tray_menu.png |
 |
 |
sunshine_tray_pairing_request.png |
 |
 |
sunshine_tray_paused.png |
 |
 |
sunshine_tray_stopped.png |
 |
 |
sunshine_tray_streaming.png |
 |
 |
sunshine_tray_tooltip.png |
 |
 |
Matrix: AppImage-x86_64
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
 |
sunshine_tray_menu.png |
 |
 |
sunshine_tray_pairing_request.png |
 |
 |
sunshine_tray_paused.png |
 |
 |
sunshine_tray_stopped.png |
 |
 |
sunshine_tray_streaming.png |
 |
 |
sunshine_tray_tooltip.png |
 |
 |
Matrix: macOS-arm64
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
 |
sunshine_tray_menu.png |
 |
 |
sunshine_tray_pairing_request.png |
 |
 |
sunshine_tray_paused.png |
 |
 |
sunshine_tray_stopped.png |
 |
 |
sunshine_tray_streaming.png |
 |
 |
sunshine_tray_tooltip.png |
 |
 |
Matrix: macOS-x86_64
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
 |
sunshine_tray_menu.png |
 |
 |
sunshine_tray_pairing_request.png |
 |
 |
sunshine_tray_paused.png |
 |
 |
sunshine_tray_stopped.png |
 |
 |
sunshine_tray_streaming.png |
 |
 |
sunshine_tray_tooltip.png |
 |
 |
2 participants
Description
These changes make Sunshine separately track encoder probe results for 10-bit encoding with 4:2:0 and 4:4:4 chroma subsampling, and verifies that the probes succeeded before advertising support for those configurations. Without these patches, Sunshine conflates a successful 8-bit YUV 4:4:4 encoding probe with the ability to encode 10-bit YUV 4:4:4, which isn't always the case on some hardware or driver/API combinations.
Screenshot
Issues Fixed or Closed
Roadmap Issues
Type of Change
Checklist
AI Usage
See our AI usage policy.