Conversation
There was a problem hiding this comment.
Copilot reviewed 8 out of 12 changed files in this pull request and generated no comments.
Files not reviewed (4)
- CMakeLists.txt: Language not supported
- fbs/CMakeLists.txt: Language not supported
- unicapture/CMakeLists.txt: Language not supported
- unicapture/unicapture.c: Language not supported
sundermann
left a comment
sundermann
left a comment
There was a problem hiding this comment.
Looks good, though I'm wondering if ARGB is needed for VNC. Blending in the alpha channel saves 25% bandwidth at the cost of some computation but it could be worth it.
fbs/CMakeLists.txt
Outdated
| URL_HASH SHA512=46ba5ca75facc7d3360dba797d24ae7bfe539a854a48831e1c7b96528cf9594d8bea22b267678fd7c6d742b6636d9e52930987119b4c6b2e38d4abe89b990cae | ||
| DOWNLOAD_EXTRACT_TIMESTAMP TRUE | ||
| CMAKE_ARGS | ||
| -DCMAKE_C_COMPILER=/usr/bin/cc |
There was a problem hiding this comment.
I'm not against changing this, but I'm just wondering why it's needed as in my testing the CMAKE_TOOLCHAIN_FILE from the main cmake was not passed automatically when using ExternalProject_Add.
There was a problem hiding this comment.
On my machine, environment-setup was sourced, so CC environment variable was set. Maybe I could try changing that on my computer instead
There was a problem hiding this comment.
environment-setup is not needed with cmake projects. It's enough to pass the toolchain file only.
There was a problem hiding this comment.
I just tried a few combination:
- Set
-DCMAKE_TOOLCHAIN_FILEin configuration step:ExternalProjectwon't have toolchain specified - Set CMAKE_TOOLCHAIN_FILE in environment variable (just like sourcing
environment-setup):ExternalProjectwill have that toolchain too
This change seems to be out of scope for this time, so I'll revert the last commit :)
|
@sundermann Yes, for VNC library, it only expects 1/2/4 bytes per pixel. We need to give it XRGB. |
mariotaku
force-pushed
the
feature/separated-unicapture
branch
from
27cde35 to
177e8cf
Compare
|
@mariotaku Hi, is it normal that we no longer have the .so libraries included in the zip artifacts build? |
|
@Syspoke Sorry, I have introduced this build issue. I'll fix it in another commit. |
3 participants
Make unicapture (somewhat) separated. This library does great job capturing screen content, and can be used by webos-vncserver: Informatic/webos-vncserver#6 .