smasherprog / screen_capture_lite

cross platform screen/window capturing library
MIT License
638 stars 156 forks source link

Library fails to build on Raspberry Pi (32-bit architectures with XWindows) #101

Closed ijonglin closed 3 years ago

ijonglin commented 3 years ago

Trying to build on RasberryPi on the Debian , pi@raspberrypi:~/Git/screen_capture_lite $ uname -a Linux raspberrypi 5.4.72-v7+ #1356 SMP Thu Oct 22 13:56:54 BST 2020 armv7l GNU/Linux pi@raspberrypi:~/Git/screen_capture_lite $ pi@raspberrypi:~/Git/screen_capture_lite $ lsb_release -a No LSB modules are available. Distributor ID: Raspbian Description: Raspbian GNU/Linux 10 (buster) Release: 10 Codename: buster pi@raspberrypi:~/Git/screen_capture_lite $

I'm getting the following error on a clean cmake and make with the following problem:

[ 58%] Building CXX object CMakeFiles/screen_capture_lite.dir/src/linux/GetWindows.cpp.o /home/pi/Git/screen_capture_lite/src/linux/GetWindows.cpp: In function ‘void SL::Screen_Capture::AddWindow(Display*, XID&, std::vector<SL::Screen_Capture::Window>&)’: /home/pi/Git/screen_capture_lite/src/linux/GetWindows.cpp:103:51: error: invalid cast from type ‘XID’ {aka ‘long unsigned int’} to type ‘size_t’ {aka ‘unsigned int’} w.Handle = reinterpret_cast<size_t>(window); ^ make[2]: *** [CMakeFiles/screen_capture_lite.dir/build.make:141: CMakeFiles/screen_capture_lite.dir/src/linux/GetWindows.cpp.o] Error 1 make[1]: *** [CMakeFiles/Makefile2:105: CMakeFiles/screen_capture_lite.dir/all] Error 2 make: *** [Makefile:141: all] Error 2

Is this architecture and/or release not supported?

smasherprog commented 3 years ago

i haven't tested it on raspberry pi. feel free to improve the code and issue a PR :)

smasherprog commented 3 years ago

you do need to install these libs or their equivalents on linux linux: sudo apt-get install libxtst-dev libxinerama-dev libx11-dev libxfixes-dev

ijonglin commented 3 years ago

With your blessing, I'll take it on then. Feel free assign it to me then.

ijonglin commented 3 years ago

Looks like architecture cast issue on the size of the X Window handle type -- I believe I've installed all the dependencies. When I got some free cycles, I'll take a look. Also, there may be some big-endian vs little-endian issues, but I'll poke around a little more.

ijonglin commented 3 years ago

Question to @smasherprog:

It looks like the fundamental problem is that 1) I'm building it on a 32-bit architecture (not necessarily an ARM thing) , and 2) I'm grabbing the screen from XWindows subsystem. It seems XID inherent window handle is long unsigned int (defined by XWindows), but it's being cast to size_t (which for 32 bit systems) is just a regular unsigned int.

Looking for a good solution in my head, there are two ways go with this:

  1. Re-type to the w.Handle to fixed type (long unsigned bit), which would change a lot of code, methinks, but keep all the code paths the same.
  2. Insert a size_t to XID conversion function -- in only the case where XID type is smaller than size would do work -- all other times, it would be pass through. -- which would involve some interesting compiler tricks.

Any suggestions for your codebase, @smasherprog, before I put cycles into it?

smasherprog commented 3 years ago

that reinterpret cast can probably be changed to a static_cast? size_t and long unsigned int are the same

ijonglin commented 3 years ago

I've confirmed that size_t and long unsigned int are both 4 bytes on the Raspberry pi. I'll test the change to fix my build, but I'm no CMake expert, and I don't want to break it for other architectures. Are size_t and long unsigned int the same across all platforms? If so, I would be happy to put in this straightforward fix by forcing a cast on XID inside of the reinterpret_cast.

ijonglin commented 3 years ago

Confirmed that adding the cast here:

w.Handle = reinterpret_cast<size_t>( (size_t) window);

Allows the build to proceed. What's the basic regression tests I should run to make sure that I haven't broken anything.

ijonglin commented 3 years ago

make test command works on the raspberry pi, but since there's one test, not too sure if that's enough. I'll put together the fix.

smasherprog commented 3 years ago

Thata good enough. The bug was really just an 9versight. It's fine to issue the pr. 👍

smasherprog commented 3 years ago

Should be static cast instead of reinterpret cast though

ijonglin commented 3 years ago

Okay, I will change again. Opened the PR: https://github.com/smasherprog/screen_capture_lite/pull/102 , but don't know how to link the PR to the issue. I'll figure that ouot.

ijonglin commented 3 years ago

PR has been updated. Let me know if you need anything else.

The test doesn't run on my local machine, but I see you have some CI/CD stuff running right now, so hopefully, it'll be alright.

smasherprog commented 3 years ago

Fixed in the pr closing now