Conversation
A small Java server, built with Gradle into a dex jar and pushed to the device, that mirrors a display into an ImageReader, encodes each frame as JPEG and writes it to an abstract unix socket. It also reads control messages back up the same socket and injects touch, scroll, key and text events. The jar is a build output and is not committed. Cherry picked from the livestream branch, at its latest state: the null virtualDisplay fix and the protocol version reset are folded in. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The jar the live stream pushes to the device is a build output and is not committed, so a clone has none. A build_server_jar job produces it and the pack job now depends on it, which is also what puts it in the tarball the test jobs install. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![u-pr[bot]](https://avatars.githubusercontent.com/in/1201017?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Harness Review
The change adds an on-device MJPEG server and wires its jar into package packing, but the mirroring call and several control-event paths behave incorrectly for supported devices or valid inputs.
Reviewed commit b90bedb
🤖 Helpful? 👍/👎
RunGradle runs a task in a Gradle project behind a cancelable progress bar, taking the JDK and SDK from Preferences > External Tools rather than from the environment, streaming Gradle's output to Editor.log as it arrives and killing the process tree - launcher script and daemon both - when a build is cancelled or runs past its timeout. Nothing calls it on this branch yet; the live stream's debug button that builds the server jar stayed behind on the livestream branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
/review |
There was a problem hiding this comment.
💡 Harness Review
The new server builds a package artifact and adds display capture and control handling; the frame conversion path can discard valid images on devices whose image rows have padding.
Reviewed commit 34ce873
🤖 Helpful? 👍/👎
|
/review |
|
/review |
There was a problem hiding this comment.
💡 Harness Review
The change adds an on-device JPEG streaming server and wires its generated jar into package packing. I examined the build and artifact paths, control protocol, and capture lifecycle and found no new actionable issues beyond those previously reported.
Reviewed commit c36685d
🤖 Helpful? 👍/👎
| LocalSocket socket = null; | ||
| ScreenStreamer streamer = null; | ||
| try { | ||
| socket = accept(serverSocket, options.getConnectTimeoutMs()); |
There was a problem hiding this comment.
Claude suggests to reject unwanted connections and to add this after accept.
// adbd opens `adb forward` connections, as shell (2000) or root (0). Every app has its own uid >= 10000.
int uid = socket.getPeerCredentials().getUid();
if (uid != 0 && uid != 2000) {
throw new IOException("Rejected a connection from uid " + uid + ", only adb may connect");
}
There was a problem hiding this comment.
Added
private static void rejectUnlessAdb(LocalSocket socket) throws IOException {
int uid = socket.getPeerCredentials().getUid();
if (uid != ROOT_UID && uid != SHELL_UID) {
throw new IOException("Rejected a connection from uid " + uid + ", only adb may connect");
}
}
| characterMap = KeyCharacterMap.load(KeyCharacterMap.VIRTUAL_KEYBOARD); | ||
| } | ||
|
|
||
| KeyEvent[] events = characterMap.getEvents(text.toCharArray()); |
There was a problem hiding this comment.
readme says that "non-US layouts work", but in reality if text contains non-ASCII characters, getEvents returns null, so the whole string would be lost
available options are:
- mention this behavior in readme
- call getEvents for each char separately - non-ASCII characters still be lost, but at least ASCII chars will get through
- use https://github.com/Genymobile/scrcpy/blob/master/server/src/main/java/com/genymobile/scrcpy/control/KeyComposition.java logic
There was a problem hiding this comment.
Claude supposedly fixed this..
|
|
||
| private void startSession(DisplayInfo info) throws IOException { | ||
| synchronized (sessionLock) { | ||
| displaySize = info.getSize(); |
There was a problem hiding this comment.
minor possibility of race condition, device rotated while close method is called, making handler null
makes sense to check for stopped and return early.
2 participants
This is a cherry pick from - #224, since that PR is too big.
This PR carries only the parts that no Editor UI depends on, so that #224 can be rebased onto it and reviewed as the Editor change alone.
What is here:
External/UnityLogcatServerapp_processas theshelluser - no manifest, no activity - which is what lets it call the hidden display mirroring APIs and holdINJECT_EVENTS. It mirrors a display into anImageReader(DisplayManagerGlobal.createVirtualDisplay, falling back toSurfaceControl.createDisplay), encodes each frame as JPEG and writes it to an abstract unix socket; the same socket carries touch, scroll, key and text messages back. JPEG rather than H.264 because the Editor has no reachable video decoder, and a JPEG frame goes straight intoTexture2D.LoadImage..yamato/build-server-jar.yml,Tools/CI/Settings/UnityMobileLogcatSettings.csbuild_server_jarjob produces it and the package pack job now depends on it, which is what puts it in the tarball the test jobs install.AndroidLogcatUtilities.RunGradleTools/CInet10.0, withglobal.jsonpinned to SDK 10 to match.Nothing in this PR is reachable by a user: with the Editor side still on #224, the jar ships in the package and is never started.