-
Notifications
You must be signed in to change notification settings - Fork 27
feat: add device.screencapture.setConfiguration RPC #290
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| package devices | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "io" | ||
| "sync" | ||
| ) | ||
|
|
||
| // avcControls maps a device ID to the stdin writer of its running AvcServer | ||
| // process, so a separate JSON-RPC call (device.screencapture.setConfiguration) | ||
| // can push live encoder commands into an in-flight capture without restarting it. | ||
| // | ||
| // ponytail: one writer per device — a device streams at most one AVC capture at | ||
| // a time. Revisit if concurrent captures per device ever become a thing. | ||
| var ( | ||
| avcControlsMu sync.Mutex | ||
| avcControls = map[string]io.Writer{} | ||
| ) | ||
|
|
||
| func registerAvcControl(deviceID string, w io.Writer) { | ||
| avcControlsMu.Lock() | ||
| defer avcControlsMu.Unlock() | ||
| avcControls[deviceID] = w | ||
| } | ||
|
|
||
| func unregisterAvcControl(deviceID string) { | ||
| avcControlsMu.Lock() | ||
| defer avcControlsMu.Unlock() | ||
| delete(avcControls, deviceID) | ||
| } | ||
|
|
||
| // SetAvcBitrate pushes a live bitrate command to the running AvcServer for the | ||
| // given device. Returns an error if no AVC capture is currently active. | ||
| func SetAvcBitrate(deviceID string, bitrate int) error { | ||
| avcControlsMu.Lock() | ||
| w := avcControls[deviceID] | ||
| avcControlsMu.Unlock() | ||
|
|
||
| if w == nil { | ||
| return fmt.Errorf("no active AVC capture for device %s", deviceID) | ||
| } | ||
|
|
||
| // AvcServer reads newline-delimited commands from stdin. | ||
| if _, err := fmt.Fprintf(w, "bitrate %d\n", bitrate); err != nil { | ||
| return fmt.Errorf("failed to send bitrate command: %w", err) | ||
| } | ||
| return nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -999,7 +999,7 @@ | |
| // start DeviceKit | ||
| // Note: passing nil registry since this is internal call from StartScreenCapture | ||
| // ScreenCapture callers should have already registered the device via StartAgent | ||
| deviceKitInfo, err = d.StartDeviceKit(nil) | ||
| deviceKitInfo, err = d.StartDeviceKit264(nil) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift Wire iOS AVC streams into the bitrate-control path, or reject them explicitly. This branch starts DeviceKit H.264 streaming, but no iOS code here registers a writer/control endpoint for 🤖 Prompt for AI Agents |
||
| if err != nil { | ||
| return fmt.Errorf("failed to start DeviceKit: %w", err) | ||
| } | ||
|
|
@@ -1417,10 +1417,10 @@ | |
| return true | ||
| } | ||
|
|
||
| // StartDeviceKit starts the devicekit-ios XCUITest which provides: | ||
| // StartDeviceKit264 starts the devicekit-ios XCUITest which provides: | ||
| // - An HTTP server for tap/dumpUI commands (port 12004) | ||
| // - A broadcast extension for H.264 screen streaming (port 12005) | ||
| func (d *IOSDevice) StartDeviceKit(hook *ShutdownHook) (*DeviceKitInfo, error) { | ||
| func (d *IOSDevice) StartDeviceKit264(hook *ShutdownHook) (*DeviceKitInfo, error) { | ||
| // register cleanup hook for this device | ||
| if hook != nil { | ||
| hookName := fmt.Sprintf("ios-devicekit-%s", d.Udid) | ||
|
|
@@ -1444,8 +1444,12 @@ | |
|
|
||
| var devicekitMainAppBundleId string | ||
| for _, app := range apps { | ||
| // look for the main app, not the test runner | ||
| if strings.HasPrefix(app.PackageName, "com.") && strings.Contains(app.PackageName, "devicekit-ios") && !strings.Contains(app.PackageName, "UITests") { | ||
| // look for the main app, not the test runner. the bundle id can carry a | ||
| // signing/team prefix when re-signed (e.g. com.megidish1.devicekit-ios), so | ||
| // match on suffix rather than substring. the runner ends with | ||
| // "...devicekit-iosUITests.xctrunner" and is excluded by this suffix. | ||
| fmt.Println("-> %s", app.PackageName) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- devices/ios.go context ---'
nl -ba devices/ios.go | sed -n '1420,1475p'
echo
echo '--- search for utils.Verbose and fmt.Println usage in devices/ios.go ---'
rg -n 'utils\.Verbose|fmt\.Print(ln|f)\(' devices/ios.go
echo
echo '--- search for utils.Verbose in repo ---'
rg -n 'utils\.Verbose' .
echo
echo '--- search for startup/app bundle logging in devices/ios.go ---'
rg -n 'PackageName|bundle|startup|app bundle|debug' devices/ios.goRepository: mobile-next/mobilecli Length of output: 230 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- devices/ios.go context ---'
sed -n '1420,1475p' devices/ios.go | cat -n
echo
echo '--- search for utils.Verbose and fmt.Println usage in devices/ios.go ---'
rg -n 'utils\.Verbose|fmt\.Print(ln|f)\(' devices/ios.go || true
echo
echo '--- search for utils.Verbose in repo ---'
rg -n 'utils\.Verbose' . || true
echo
echo '--- search for startup/app bundle logging in devices/ios.go ---'
rg -n 'PackageName|bundle|startup|app bundle|debug' devices/ios.go || trueRepository: mobile-next/mobilecli Length of output: 27752 Remove the stray stdout debug print. 🤖 Prompt for AI Agents |
||
| if strings.HasSuffix(app.PackageName, "devicekit-h264") { | ||
| utils.Verbose("DeviceKit main app found, bundle ID: %s", app.PackageName) | ||
| devicekitMainAppBundleId = app.PackageName | ||
| break | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| package server | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "strings" | ||
| "testing" | ||
| ) | ||
|
|
||
| // rejects bad input before any device lookup, so these run without a device. | ||
| func TestSetConfigurationRejectsNonPositiveBitrate(t *testing.T) { | ||
| _, err := handleScreenCaptureSetConfiguration(json.RawMessage(`{"deviceId":"x","bitrate":0}`)) | ||
| if err == nil || !strings.Contains(err.Error(), "bitrate must be positive") { | ||
| t.Fatalf("expected positive-bitrate error, got: %v", err) | ||
| } | ||
| } | ||
|
|
||
| func TestSetConfigurationRejectsInvalidJSON(t *testing.T) { | ||
| _, err := handleScreenCaptureSetConfiguration(json.RawMessage(`not json`)) | ||
| if err == nil || !strings.Contains(err.Error(), "invalid parameters") { | ||
| t.Fatalf("expected invalid-parameters error, got: %v", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard the registry against overlapping capture lifetimes.
registerAvcControlsilently overwrites an existing writer, andunregisterAvcControldeletes bydeviceIDonly. If a second AVC capture starts before the first deferred cleanup runs, the first cleanup can remove the new writer and makeSetAvcBitratefail with “no active AVC capture”.Suggested direction
Callers should then handle duplicate registration by closing the new pipe and stopping the newly started process.
📝 Committable suggestion
🤖 Prompt for AI Agents