build(android): support android platform (config and logging) #3741
Conversation
LGTM Co-authored-by: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com>
LGTM Co-authored-by: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com>
|
Sorry for the delayed response. Unfortunately, I don't have much time either. My main tech stack is Flutter (UI Framework) and Android Native. Typically, the compilation of an Android app differs significantly from C/C++ and requires the Java environment and Gradle. So, from my current perspective, it seems quite difficult to make the Sunshine workflow directly produce an APK. Additionally, I'm concerned that my lack of proficiency in C/C++ might disrupt the original workflow, so I plan to submit PRs in multiple stages. However, I can start with some compilation adaptations to ensure that future Sunshine APKs for Android can be imported and compiled smoothly. As seen in the patches, I can now directly use the Sunshine Source along with a Patch Script to make Android compilation work properly. My initial idea is to submit these patches to Sunshine so that the Android-compiled Sunshine and the Sunshine source code can remain consistent. Additionally, I’d like to share some recent comparisons with scrcpy. I’ve studied a lot of scrcpy’s code. In fact, scrcpy is already very efficient. It’s possible that the client’s use of SDL2 results in rendering that’s not as extremely efficient (just a guess). However, in terms of the server side, scrcpy and the ported Sunshine for Android should be roughly the same. That said, porting to Android still has significant value because it allows users to use Moonlight to stream their Android devices. I will continue to push this process forward. |
|
|
It looks like this PR has been idle for 90 days. If it's still something you're working on or would like to pursue, please leave a comment or update your branch. Otherwise, we'll be closing this PR in 10 days to reduce our backlog. Thanks! |
|
I've been too busy recently and have no idea how to move this PR forward. I used to open it occasionally, but there has been no progress on the parts awaiting review. So I haven't paid much attention to it since then. |
|
@mengyanshou there's a few comments above that haven't been addressed yet. I'd be happy to discuss in discord if there is some confusion to clear up? |
|
This comment was marked as resolved.
This comment was marked as resolved.
|
Sorry, my hand was shaking and I clicked the wrong one. But I think most of them should be the reviews waiting for a reply on my side. If these changes are accepted, I will modify these reviews as resolved to proceed with the subsequent merge. Then regarding one of the Todos, I currently don't know how to implement it, but I think it's necessary to keep the todo. |
|
Don't worry about the TODO warning, those are okay. |
|
Next, what can I do to help move forward? I have very little time recently and should not be able to continue submitting changes to the code until next month. Then, thank you for your cooperation @ReenigneArcher |
There was a problem hiding this comment.
Seems pretty good to me. In addition to my one question below, I have one more. If possible @FrogTheFrog, could you check the updated logging section?
What would it take to compile this in our CI (understanding no apk would be provided yet)? Could you provide instructions either here or in https://github.com/LizardByte/Sunshine/blob/master/docs/building.md
| @@ -1038,9 +1038,12 @@ namespace config { | |||
| } | |||
|
|
|||
| void apply_config(std::unordered_map<std::string, std::string> &&vars) { | |||
| #ifndef __ANDROID__ | |||
| // TODO: Android can possibly support this | |||
There was a problem hiding this comment.
Apologies, I wrote a comment on this for the last review but it didn't appear for some reason.
I think we discussed this on Discord some time back, but can you explain here why it's not supported right now? In my mind this should work, but I might be missing something.
In my mind, the default "Desktop" application could be renamed to "Screen" or something along those lines. Then the user could add the commands for other applications that would launch an app. Maybe I am missing something?
For reference, these are the current three apps.json files:
There was a problem hiding this comment.
Since I haven't decided on the form of Sunshine Android yet - whether it will be an APK, a binary launched via adb, or both options
The difference is that if it's an APK, the app will need to request screen recording permissions from users, with the readable directory being /data/data/com.xxx/files
If it's a binary launched through adb, no permissions would be needed, with the readable directory being /data/local/tmp
These two approaches have different readable directories, so I'd like to implement this when I have more time available
LGTM |
Regarding CI, as mentioned above, if it's a binary, we would need to compile the entire Sunshine using NDK, which is more complex and requires additional code. If it's an APK, we would need to introduce gradle for compilation. However, we're still in the very early stages - I've only merged some Android-compatible code into Sunshine, but currently we cannot compile either an APK or binary with these changes. Therefore, CI work will need to wait until the entire Android compatibility code is completed. The initial idea is to make it compatible with Android while not affecting any of Sunshine's original code |
Bundle ReportBundle size has no change ✅ |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3741 +/- ##
=========================================
Coverage 11.31% 11.31%
=========================================
Files 92 92
Lines 17571 17571
Branches 8239 8239
=========================================
Hits 1989 1989
+ Misses 14901 13063 -1838
- Partials 681 2519 +1838
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Description
Adapt config and logging for Android:
Type of Change
.github/...)Checklist