Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions devices/android.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"encoding/json"
"encoding/xml"
"fmt"
"io"
"os"
"os/exec"
"os/signal"
Expand Down Expand Up @@ -1157,10 +1158,29 @@ func (d *AndroidDevice) StartScreenCapture(config ScreenCaptureConfig) error {
return fmt.Errorf("failed to create stdout pipe: %v", err)
}

// For AVC, keep stdin open as a live control channel: AvcServer reads
// newline-delimited "bitrate <bps>" commands so the host can adapt the
// encoder to the viewer's measured downlink without restarting the stream.
var stdin io.WriteCloser
if config.Format == "avc" {
stdin, err = cmd.StdinPipe()
if err != nil {
return fmt.Errorf("failed to create stdin pipe: %v", err)
}
}

if err := cmd.Start(); err != nil {
return fmt.Errorf("failed to start %s: %v", serverClass, err)
}

if config.Format == "avc" {
registerAvcControl(d.ID(), stdin)
defer func() {
unregisterAvcControl(d.ID())
_ = stdin.Close()
}()
}

// Read bytes from the command output and send to callback
buffer := make([]byte, 65536)
for {
Expand Down
48 changes: 48 additions & 0 deletions devices/avc_control.go
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
}
Comment on lines +15 to +48

Copy link
Copy Markdown

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.

registerAvcControl silently overwrites an existing writer, and unregisterAvcControl deletes by deviceID only. If a second AVC capture starts before the first deferred cleanup runs, the first cleanup can remove the new writer and make SetAvcBitrate fail with “no active AVC capture”.

Suggested direction
+type avcControl struct {
+	w io.Writer
+}
+
 var (
 	avcControlsMu sync.Mutex
-	avcControls   = map[string]io.Writer{}
+	avcControls   = map[string]*avcControl{}
 )
 
-func registerAvcControl(deviceID string, w io.Writer) {
+func registerAvcControl(deviceID string, w io.Writer) (*avcControl, error) {
 	avcControlsMu.Lock()
 	defer avcControlsMu.Unlock()
-	avcControls[deviceID] = w
+	if _, exists := avcControls[deviceID]; exists {
+		return nil, fmt.Errorf("AVC capture already active for device %s", deviceID)
+	}
+	control := &avcControl{w: w}
+	avcControls[deviceID] = control
+	return control, nil
 }
 
-func unregisterAvcControl(deviceID string) {
+func unregisterAvcControl(deviceID string, control *avcControl) {
 	avcControlsMu.Lock()
 	defer avcControlsMu.Unlock()
-	delete(avcControls, deviceID)
+	if avcControls[deviceID] == control {
+		delete(avcControls, deviceID)
+	}
 }
 
 func SetAvcBitrate(deviceID string, bitrate int) error {
 	avcControlsMu.Lock()
-	w := avcControls[deviceID]
+	control := avcControls[deviceID]
 	avcControlsMu.Unlock()
 
-	if w == nil {
+	if control == nil {
 		return fmt.Errorf("no active AVC capture for device %s", deviceID)
 	}
 
-	if _, err := fmt.Fprintf(w, "bitrate %d\n", bitrate); err != nil {
+	if _, err := fmt.Fprintf(control.w, "bitrate %d\n", bitrate); err != nil {
 		return fmt.Errorf("failed to send bitrate command: %w", err)
 	}

Callers should then handle duplicate registration by closing the new pipe and stopping the newly started process.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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
}
type avcControl struct {
w io.Writer
}
var (
avcControlsMu sync.Mutex
avcControls = map[string]*avcControl{}
)
func registerAvcControl(deviceID string, w io.Writer) (*avcControl, error) {
avcControlsMu.Lock()
defer avcControlsMu.Unlock()
if _, exists := avcControls[deviceID]; exists {
return nil, fmt.Errorf("AVC capture already active for device %s", deviceID)
}
control := &avcControl{w: w}
avcControls[deviceID] = control
return control, nil
}
func unregisterAvcControl(deviceID string, control *avcControl) {
avcControlsMu.Lock()
defer avcControlsMu.Unlock()
if avcControls[deviceID] == control {
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()
control := avcControls[deviceID]
avcControlsMu.Unlock()
if control == nil {
return fmt.Errorf("no active AVC capture for device %s", deviceID)
}
// AvcServer reads newline-delimited commands from stdin.
if _, err := fmt.Fprintf(control.w, "bitrate %d\n", bitrate); err != nil {
return fmt.Errorf("failed to send bitrate command: %w", err)
}
return nil
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@devices/avc_control.go` around lines 15 - 48, The AVC control registry
currently allows overlapping captures to overwrite each other, so stale cleanup
can remove the active writer. Update registerAvcControl and unregisterAvcControl
to track ownership per capture instance (not just by deviceID) and reject or
detect duplicate registration in SetAvcBitrate’s lifecycle path. If
registerAvcControl finds an existing writer for the same deviceID, have the
caller treat that as a conflict and stop the new capture/close its pipe instead
of replacing the entry. Ensure unregisterAvcControl only removes the writer it
originally registered so a previous deferred cleanup cannot clear a newer
capture.

14 changes: 9 additions & 5 deletions devices/ios.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 SetAvcBitrate. The new RPC handler calls devices.SetAvcBitrate(targetDevice.ID(), req.Bitrate), so active iOS AVC streams will still return “no active AVC capture”.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@devices/ios.go` at line 1002, The iOS DeviceKit H.264 start path in
StartDeviceKit264 currently creates a stream but never registers it with the AVC
bitrate-control path, so SetAvcBitrate has no active target to update. Update
the ios.go flow around deviceKitInfo and StartDeviceKit264 to either attach the
started stream to the same writer/control endpoint used by devices.SetAvcBitrate
or explicitly fail/skip AVC bitrate control for iOS streams. Make sure the
active iOS AVC capture is discoverable by the new RPC handler path.

if err != nil {
return fmt.Errorf("failed to start DeviceKit: %w", err)
}
Expand Down Expand Up @@ -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)
Expand All @@ -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)

Check failure on line 1451 in devices/ios.go

View workflow job for this annotation

GitHub Actions / test

fmt.Println call has possible Printf formatting directive %s

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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.go

Repository: 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 || true

Repository: mobile-next/mobilecli

Length of output: 27752


Remove the stray stdout debug print.
fmt.Println("-> %s", app.PackageName) prints the format string literally and writes a line for every app scanned during startup. Use utils.Verbose if this output is needed.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@devices/ios.go` at line 1451, Remove the stray stdout debug print in the app
scan path: the fmt.Println call in the iOS device logic is emitting a line for
every app and printing the format string literally. Update the code near the
app-processing flow in devices/ios.go to stop writing directly to stdout, and if
this diagnostic output is still needed, route it through utils.Verbose instead.

if strings.HasSuffix(app.PackageName, "devicekit-h264") {
utils.Verbose("DeviceKit main app found, bundle ID: %s", app.PackageName)
devicekitMainAppBundleId = app.PackageName
break
Expand Down
97 changes: 49 additions & 48 deletions server/dispatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,54 +12,55 @@ type HandlerFunc func(params json.RawMessage) (any, error)
// This is used by both the HTTP server and embedded clients
func GetMethodRegistry() map[string]HandlerFunc {
return map[string]HandlerFunc{
"devices.list": handleDevicesList,
"device.screenshot": handleScreenshot,
"device.screencapture": handleScreenCaptureSession,
"device.io.tap": handleIoTap,
"device.io.longpress": handleIoLongPress,
"device.io.text": handleIoText,
"device.io.keys": handleIoKeys,
"device.io.button": handleIoButton,
"device.io.swipe": handleIoSwipe,
"device.io.gesture": handleIoGesture,
"device.url": handleURL,
"device.info": handleDeviceInfo,
"device.io.orientation.get": handleIoOrientationGet,
"device.io.orientation.set": handleIoOrientationSet,
"device.boot": handleDeviceBoot,
"device.shutdown": handleDeviceShutdown,
"device.reboot": handleDeviceReboot,
"device.settings.apply": handleSettingsApply,
"device.dump.ui": handleDumpUI,
"device.apps.launch": handleAppsLaunch,
"device.apps.terminate": handleAppsTerminate,
"device.apps.list": handleAppsList,
"device.apps.foreground": handleAppsForeground,
"device.apps.install": handleAppsInstall,
"device.apps.uninstall": handleAppsUninstall,
"device.screenrecord": handleScreenRecord,
"device.screenrecord.stop": handleScreenRecordStop,
"device.crashes.list": handleCrashesList,
"device.crashes.get": handleCrashesGet,
"device.webview.list": handleWebViewList,
"device.webview.content": handleWebViewContent,
"device.webview.goto": handleWebViewGoto,
"device.webview.reload": handleWebViewReload,
"device.webview.goBack": handleWebViewGoBack,
"device.webview.goForward": handleWebViewGoForward,
"device.webview.url": handleWebViewURL,
"device.webview.title": handleWebViewTitle,
"device.webview.query": handleWebViewQuery,
"device.webview.evaluate": handleWebViewEvaluate,
"device.webview.waitForLoadState": handleWebViewWaitForLoadState,
"server.info": handleServerInfo,
"server.shutdown": handleServerShutdown,
"device.apps.path": handleAppsPath,
"device.fs.ls": handleFsLs,
"device.fs.pull": handleFsPull,
"device.fs.push": handleFsPush,
"device.fs.mkdir": handleFsMkdir,
"device.fs.rm": handleFsRm,
"devices.list": handleDevicesList,
"device.screenshot": handleScreenshot,
"device.screencapture": handleScreenCaptureSession,
"device.screencapture.setConfiguration": handleScreenCaptureSetConfiguration,
"device.io.tap": handleIoTap,
"device.io.longpress": handleIoLongPress,
"device.io.text": handleIoText,
"device.io.keys": handleIoKeys,
"device.io.button": handleIoButton,
"device.io.swipe": handleIoSwipe,
"device.io.gesture": handleIoGesture,
"device.url": handleURL,
"device.info": handleDeviceInfo,
"device.io.orientation.get": handleIoOrientationGet,
"device.io.orientation.set": handleIoOrientationSet,
"device.boot": handleDeviceBoot,
"device.shutdown": handleDeviceShutdown,
"device.reboot": handleDeviceReboot,
"device.settings.apply": handleSettingsApply,
"device.dump.ui": handleDumpUI,
"device.apps.launch": handleAppsLaunch,
"device.apps.terminate": handleAppsTerminate,
"device.apps.list": handleAppsList,
"device.apps.foreground": handleAppsForeground,
"device.apps.install": handleAppsInstall,
"device.apps.uninstall": handleAppsUninstall,
"device.screenrecord": handleScreenRecord,
"device.screenrecord.stop": handleScreenRecordStop,
"device.crashes.list": handleCrashesList,
"device.crashes.get": handleCrashesGet,
"device.webview.list": handleWebViewList,
"device.webview.content": handleWebViewContent,
"device.webview.goto": handleWebViewGoto,
"device.webview.reload": handleWebViewReload,
"device.webview.goBack": handleWebViewGoBack,
"device.webview.goForward": handleWebViewGoForward,
"device.webview.url": handleWebViewURL,
"device.webview.title": handleWebViewTitle,
"device.webview.query": handleWebViewQuery,
"device.webview.evaluate": handleWebViewEvaluate,
"device.webview.waitForLoadState": handleWebViewWaitForLoadState,
"server.info": handleServerInfo,
"server.shutdown": handleServerShutdown,
"device.apps.path": handleAppsPath,
"device.fs.ls": handleFsLs,
"device.fs.pull": handleFsPull,
"device.fs.push": handleFsPush,
"device.fs.mkdir": handleFsMkdir,
"device.fs.rm": handleFsRm,
}
}

Expand Down
22 changes: 22 additions & 0 deletions server/screencapture_setconfig_test.go
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)
}
}
31 changes: 31 additions & 0 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1330,6 +1330,37 @@ func handleScreenCaptureSession(params json.RawMessage) (any, error) {
return result, nil
}

// screenCaptureSetConfigRequest is the parameter shape for the
// device.screencapture.setConfiguration RPC.
type screenCaptureSetConfigRequest struct {
DeviceID string `json:"deviceId"`
Bitrate int `json:"bitrate"`
}

// handleScreenCaptureSetConfiguration applies live encoder settings to an
// in-flight AVC capture (currently only bitrate) without restarting the stream.
func handleScreenCaptureSetConfiguration(params json.RawMessage) (any, error) {
var req screenCaptureSetConfigRequest
if err := json.Unmarshal(params, &req); err != nil {
return nil, fmt.Errorf("invalid parameters: %w", err)
}

if req.Bitrate <= 0 {
return nil, fmt.Errorf("bitrate must be positive")
}

targetDevice, err := commands.FindDeviceOrAutoSelect(req.DeviceID)
if err != nil {
return nil, fmt.Errorf("error finding device: %w", err)
}

if err := devices.SetAvcBitrate(targetDevice.ID(), req.Bitrate); err != nil {
return nil, err
}

return map[string]any{"deviceId": targetDevice.ID(), "bitrate": req.Bitrate}, nil
}

// handleStream handles the /stream endpoint for screen capture streaming
func handleStream(w http.ResponseWriter, r *http.Request) {
// extract session ID from query parameter
Expand Down
Loading