fix(ddprobe): verifying frames should test up to 10 frames #3414
Conversation
Fixed a bug that caused the verify frame code to exit the loop early if it failed to aquire a frame (due to timeouts or other reasons). It should now properly try up to 10 times.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3414 +/- ##
==========================================
- Coverage 11.13% 11.12% -0.01%
==========================================
Files 99 99
Lines 17253 17253
Branches 8045 8045
==========================================
- Hits 1921 1920 -1
Misses 12639 12639
- Partials 2693 2694 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
ReenigneArcher
changed the title
|
Does this mean in the worst case we're potentially waiting 5 seconds (500 ms * 10 tries) per invocation of ddprobe? That seems bad. It already takes quite a while (like 30 seconds or so) to reprobe decoders when a display isn't connected due to ddprobe running over and over. |
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
Files not reviewed (1)
- tools/ddprobe.cpp: Language not supported
|
I was looking into this According to MS (https://learn.microsoft.com/en-us/windows/win32/api/dxgi1_6/ne-dxgi1_6-dxgi_gpu_preference) there are only 3 possible values for GPU preference. If we discard the first one, it's 2 - We could then (I know it's not nice, but it's a solution) copy We could further uglify this by also running |
Just to follow up on this, as we discussed in #3441 you mentioned that this PR might fix or attempt to fix the issue we went through. I tried this PR's Here is a log of this PR's Sunshine log |
The registry values don't actually correspond to the values in If you do something like run a 3 GPU setup (ex: iGPU + 2 dGPUs), the valid values will be:
|
Whatever the case is, the current solution from this branch does not seem to work for @RebelliousX. I have made him a special What if we allow this GPU preference to be also user-configurable? Newest VDD by MikeTheTech also allows to specify which GPU should be used, so maybe a preference with value With that said, I would propose to add a new parameter with the following values (at least until
|
Sounds reasonable to me. |
|
@FrogTheFrog The problem happening in #3521 (comment) is that the display topology that ddprobe.exe executed under initially is not the topology that will be used while streaming. When running initially, ddprobe.exe determines that the primary display is attached to the iGPU, so it configured Sunshine to use that. Later on when Sunshine updates the display topology during streaming initialization to a (virtual) display attached to a secondary GPU, it's bound to the wrong GPU and so DXGI capture fails. Unfortunately the binding of a process to a GPU is permanent (unless we could do something clever/nasty like unloading DXGI.dll). I haven't looked into your display reconfiguration code, but if you know ahead of time that the display you will be configuring is virtual, I think we could try to opt for the OS default GPU (0) rather than probing and probably getting it wrong. |
For me, when I set it to 0, it worked on both GPUs the iGPU and dGPU and I didn't have to even fill in the Adapter Name. If I put the UUID for the monitor (in my case, the virtual one attached to dGPU), the right GPU is chosen. If I leave the device ID for the monitor empty, the iGPU is chosen for the main physical display. -1 is defaulting to 1 and that forces the iGPU to be selected. 2 works for the dGPU but not the iGPU on main display. I mentioned this in #3521 but understandably @ReenigneArcher wanted to be safe and set it as current behavior lest it cause problems. |
|
Interesting, I suppose it's possible that 24H2 changed behavior here because DXGI capture definitely didn't work across GPUs before without determining the correct GPU ahead of time. That's the reason we had to write all this probing code in the first place. |
|
Its not trivial to detect whether the display is virtual or not. There is an enum that could be set accordingly by the driver itself somehow to indicate that it's a virtual display, but so far I have not seen any of the popular VDDs doing that. Instead thay say that it's a HDMI display. In the new VDD release, you can select which GPU shall be used by it. I don't know what it does exactly, but maybe it effects the enumeration somehow. |
Description
Fixed a bug that caused the verify frame code to exit the loop early if it failed to acquire a frame (due to timeouts or other reasons).
It should now properly try up to 10 times.
Issues Fixed or Closed
N/A
Type of Change
.github/...)Checklist