fix(android): scale avc capture to fit the encoder and report agent failures as errors

Author: mobile-kevin

agents/android/java/AvcServer.java

@@ -84,6 +84,40 @@ private static int clampBitrate(int bps) {
 		return Math.max(MIN_BITRATE, Math.min(MAX_BITRATE, bps));
 	}
 
+	// H.264 encoders only accept dimensions that are a multiple of their
+	// alignment (usually 2); a raw (display * scale) can be odd. Round down.
+	private static int alignDown(float size, int alignment) {
+		int align = Math.max(alignment, 2);
+		return ((int) size / align) * align;
+	}
+
+	/**
+	 * Returns the largest {width, height} the encoder supports that is no bigger
+	 * than the one asked for and keeps its aspect ratio, or null if there is none.
+	 * Encoders often report their limits for landscape only (e.g. 4096x2176), so a
+	 * tall portrait screen can exceed the maximum height while its area is well
+	 * within what the encoder handles; it is then scaled down to fit.
+	 */
+	static int[] fitToEncoder(MediaCodecInfo.VideoCapabilities caps, float width, float height) {
+		int maxWidth = caps.getSupportedWidths().getUpper();
+		int maxHeight = caps.getSupportedHeights().getUpper();
+		float factor = Math.min(1.0f, Math.min(maxWidth / width, maxHeight / height));
+		// past the width/height limits there can still be a block count or aspect
+		// ratio limit; shrink a little at a time until the encoder takes it
+		for (int attempt = 0; attempt < 100; attempt++) {
+			int w = alignDown(width * factor, caps.getWidthAlignment());
+			int h = alignDown(height * factor, caps.getHeightAlignment());
+			if (w <= 0 || h <= 0) {
+				return null;
+			}
+			if (caps.isSizeSupported(w, h)) {
+				return new int[] {w, h};
+			}
+			factor *= 0.95f;
+		}
+		return null;
+	}
+
 	private void startControlServer(MediaCodec codec, DisplayUtils.DisplayMirror mirror) {
 		new JsonRpcSocketServer(CONTROL_SOCKET, (method, params) -> {
 			switch (method) {
@@ -127,23 +161,27 @@ private void stream() throws Exception {
 		MediaCodecInfo.VideoCapabilities caps = codec.getCodecInfo()
 				.getCapabilitiesForType(MediaFormat.MIMETYPE_VIDEO_AVC).getVideoCapabilities();
 
-		// H.264 encoders only accept dimensions that are a multiple of their
-		// alignment (usually 2); a raw (display * scale) can be odd. Round down.
-		int widthAlign = Math.max(caps.getWidthAlignment(), 2);
-		int heightAlign = Math.max(caps.getHeightAlignment(), 2);
-		int width = ((int) (display.width * scale) / widthAlign) * widthAlign;
-		int height = ((int) (display.height * scale) / heightAlign) * heightAlign;
-
-		if (width <= 0 || height <= 0) {
+		int requestedWidth = alignDown(display.width * scale, caps.getWidthAlignment());
+		int requestedHeight = alignDown(display.height * scale, caps.getHeightAlignment());
+		if (requestedWidth <= 0 || requestedHeight <= 0) {
 			codec.release();
-			throw new IllegalArgumentException("Invalid dimensions: " + width + "x" + height);
+			throw new IllegalArgumentException("Invalid dimensions: " + requestedWidth + "x" + requestedHeight);
 		}
-		if (!caps.isSizeSupported(width, height)) {
+
+		int[] size = fitToEncoder(caps, display.width * scale, display.height * scale);
+		if (size == null) {
 			String max = caps.getSupportedWidths().getUpper() + "x" + caps.getSupportedHeights().getUpper();
 			codec.release();
-			throw new IllegalArgumentException("Video dimensions " + width + "x" + height
-					+ " exceed codec capabilities. Maximum supported: " + max
-					+ ". Try using --scale parameter to reduce resolution (e.g., --scale 0.5)");
+			throw new IllegalArgumentException("Video dimensions " + requestedWidth + "x" + requestedHeight
+					+ " are not supported by the encoder, and no smaller size with the same aspect ratio is."
+					+ " Maximum supported: " + max);
+		}
+		int width = size[0];
+		int height = size[1];
+		if (width != requestedWidth || height != requestedHeight) {
+			// not on stderr: adb exec-out merges it into the video stream
+			Log.i(TAG, "Encoder does not support " + requestedWidth + "x" + requestedHeight
+					+ ", encoding at " + width + "x" + height);
 		}
 
 		MediaFormat format = MediaFormat.createVideoFormat(MediaFormat.MIMETYPE_VIDEO_AVC, width, height);

devices/android.go

@@ -9,6 +9,7 @@ import (
 	"encoding/xml"
 	"errors"
 	"fmt"
+	"io"
 	"os"
 	"os/exec"
 	"os/signal"
@@ -1524,24 +1525,54 @@ func (d *AndroidDevice) StartScreenCapture(config ScreenCaptureConfig) error {
 		return fmt.Errorf("failed to start %s: %v", serverClass, err)
 	}
 
-	// Read bytes from the command output and send to callback
+	err = forwardAgentStream(stdout, config.OnData)
+	_ = cmd.Process.Kill()
+	_ = cmd.Wait()
+	if err != nil {
+		return fmt.Errorf("%s failed: %w", serverClass, err)
+	}
+	return nil
+}
+
+// agentStreamErrorPrefix starts the message MjpegServer and AvcServer print when
+// they fail to start. `adb exec-out` merges the agent's stderr into its stdout,
+// and does not pass on its exit code, so the message arrives in place of the
+// stream and is the only sign that the agent failed.
+const agentStreamErrorPrefix = "Error: "
+
+// agentStreamErrorLimit bounds how much of a failed agent's output is read.
+const agentStreamErrorLimit = 64 * 1024
+
+// forwardAgentStream sends every chunk read from the agent's output to onData,
+// until the output ends or onData returns false. If the output starts with the
+// agent's error message instead of a stream, it returns that message as an error.
+func forwardAgentStream(r io.Reader, onData func([]byte) bool) error {
+	head := make([]byte, len(agentStreamErrorPrefix))
+	n, err := io.ReadFull(r, head)
+	head = head[:n]
+	if bytes.HasPrefix(head, []byte(agentStreamErrorPrefix)) {
+		rest, _ := io.ReadAll(io.LimitReader(r, agentStreamErrorLimit))
+		message := strings.TrimSpace(strings.TrimPrefix(string(head)+string(rest), agentStreamErrorPrefix))
+		return errors.New(message)
+	}
+	if n > 0 && !onData(head) {
+		return nil
+	}
+	if err != nil {
+		// the output ended within its first few bytes
+		return nil
+	}
+
 	buffer := make([]byte, 65536)
 	for {
-		n, err := stdout.Read(buffer)
-		if err != nil {
-			break
+		n, err := r.Read(buffer)
+		if n > 0 && !onData(buffer[:n]) {
+			return nil
 		}
-
-		if n > 0 {
-			// Send bytes to callback, break if it returns false
-			if !config.OnData(buffer[:n]) {
-				break
-			}
+		if err != nil {
+			return nil
 		}
 	}
-
-	_ = cmd.Process.Kill()
-	return nil
 }
 
 // ScreenRecord records the device screen to a local MP4 file using adb screenrecord.

devices/android_screencapture_test.go

@@ -0,0 +1,102 @@
+package devices
+
+import (
+	"bytes"
+	"io"
+	"strings"
+	"testing"
+	"testing/iotest"
+)
+
+// agentOutput is what the on-device agent wrote, delivered by r.
+type agentOutput struct {
+	r io.Reader
+}
+
+func agentWrote(output string) agentOutput {
+	return agentOutput{r: strings.NewReader(output)}
+}
+
+// agentWroteOneByteAtATime delivers the output in 1-byte reads, the worst case
+// for spotting a prefix that spans several reads.
+func agentWroteOneByteAtATime(output string) agentOutput {
+	return agentOutput{r: iotest.OneByteReader(strings.NewReader(output))}
+}
+
+// forwardEverything forwards the agent's output and returns what reached the
+// caller, and the error.
+func forwardEverything(output agentOutput) (string, error) {
+	var received bytes.Buffer
+	err := forwardAgentStream(output.r, func(chunk []byte) bool {
+		received.Write(chunk)
+		return true
+	})
+	return received.String(), err
+}
+
+const avcStreamStart = "\x00\x00\x00\x01\x67\x42\xc0\x29"
+
+func TestForwardAgentStream_PassesTheStreamThrough(t *testing.T) {
+	stream := avcStreamStart + strings.Repeat("\x01\x02\x03", 50000)
+
+	for name, output := range map[string]agentOutput{
+		"one read":         agentWrote(stream),
+		"one byte at time": agentWroteOneByteAtATime(stream),
+	} {
+		t.Run(name, func(t *testing.T) {
+			received, err := forwardEverything(output)
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if received != stream {
+				t.Errorf("received %d bytes, want the %d bytes of the stream", len(received), len(stream))
+			}
+		})
+	}
+}
+
+func TestForwardAgentStream_ReturnsTheAgentErrorInsteadOfForwardingIt(t *testing.T) {
+	agentError := "Error: Invalid dimensions: 0x0\n"
+
+	for name, output := range map[string]agentOutput{
+		"one read":         agentWrote(agentError),
+		"one byte at time": agentWroteOneByteAtATime(agentError),
+	} {
+		t.Run(name, func(t *testing.T) {
+			received, err := forwardEverything(output)
+			if err == nil {
+				t.Fatal("expected the agent's error, got none")
+			}
+			if err.Error() != "Invalid dimensions: 0x0" {
+				t.Errorf("error = %q, want %q", err.Error(), "Invalid dimensions: 0x0")
+			}
+			if received != "" {
+				t.Errorf("forwarded %q as stream data", received)
+			}
+		})
+	}
+}
+
+func TestForwardAgentStream_ForwardsAnOutputShorterThanTheErrorPrefix(t *testing.T) {
+	received, err := forwardEverything(agentWrote("\x00\x00"))
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if received != "\x00\x00" {
+		t.Errorf("received %q, want %q", received, "\x00\x00")
+	}
+}
+
+func TestForwardAgentStream_StopsWhenTheCallerStops(t *testing.T) {
+	chunks := 0
+	err := forwardAgentStream(agentWroteOneByteAtATime(avcStreamStart+"more stream").r, func([]byte) bool {
+		chunks++
+		return chunks < 2
+	})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if chunks != 2 {
+		t.Errorf("onData called %d times after asking to stop at 2", chunks)
+	}
+}