Moehammered / switch-remote-play

Let the switch remotely play PC games (similar to steam link or remote play)
GNU General Public License v3.0
301 stars 14 forks source link

[BUG] Cannot stream secondary monitors located below or to the left of the primary monitor #52

Open IBNobody opened 2 years ago

IBNobody commented 2 years ago

This is a follow up from the GBA thread.

Bug:

SRP cannot stream secondary monitors located below or to the left of the primary monitor if the secondary monitor is resized by SRD.

Cause:

After running ChangeDisplaySettings or ChangeDisplaySettingsEx to resize the monitor, the origin data stored in display.x and display.y remains constant. This causes an issue because resizing a display with a negative X or Y origin value such as (-1920,0), (0,-1080), or (-1920,-1080) causes the display's origin to change. (The monitor's origin "scoots closer" to (0,0).)

Because the monitor's origin is no longer at (display.x, display.y), FFMPEG will fail to start the stream because the viewport it was passed no longer exists (or "exists outside of the desktop area")

[gdigrab @ 0000023e2e7dcc80] Capture area (-1920,0),(-640,720) extends outside window area (-1280,0),(2560,1080)desktop: I/O error

Recommended solution:

Either DisplayDeviceService::QueryDevices or a subset of that function needs to be re-run, or display.x and display.y need to be updated with the new origin. (I recommend re-detecting the monitor rather than trying to solve this with math because there is more of a chance for a bug to occur with the math.)

Moehammered commented 2 years ago

@IBNobody you're a champion mate. Thanks so much for making this report and troubleshooting the bug. I like your idea for the solution too. I already have the ability to query the devices so I think I will simply run the display service again after resizing to avoid having to do the math like you've mentioned. This means there is a potential bug trying to stream a monitor that lies between 2 monitors that sit in the positive quadrant space.

I'll look into making a rushed change for you to test and see if it works but I won't have time to properly tackle the issue until after a few weeks.

I'll post a comment or edit this one once I have an edited version for you to try out. Maybe by tomorrow fingers crossed.

You've helped a lot finding a great bug like this one. Thank you very much. :)

IBNobody commented 2 years ago

Again, no rush on this. It was fun to debug, and showing me what I needed to turn on verbose debugging helped me deduce the issue.

Regarding the other minor bug with the display.x and display.y showing large numbers in the log ((x,y): 18446744073709549696, 0) rather than negative, the issue is that your logger overload has uint64_t as the datatype and you are (correctly) storing display.x and display.y as int64_t.

https://github.com/Moehammered/switch-remote-play/blob/6e36fa8dd78bee3cb564d695778048c045b0bd2c/windows-project/switch-remote-play-host/switch-remote-play-host/Log.cpp#L76

I don't recall off the top of my head if C++ lets you overload << for both int and uint datatypes. If it does, that would be the solution to fix the logging.