From 8c7028e8381ac030ff56f55c9be3b99da64820b6 Mon Sep 17 00:00:00 2001 From: Robert Wu Date: Mon, 3 Oct 2022 22:03:51 +0000 Subject: [PATCH] USB MIDI: Extend bulkTransfer timeout If a UsbDeviceConnection is used both for sending and receiving data, bulkTransfer may time out. The solution is to extend the timeout from 10ms to a second. Also, we need to split the packet into multiple packets if they are greater than the endpoint's max packet size. Linux would do this for us, but with the new retry code, we may be sending some parts of the packet multiple times otherwise. This is because if Linux would split the packet into 3 packets and only the third packet fails, the first and second packets would resend if we don't split the packets. Bug: 250616636 Bug: 251551233 Change-Id: Ic4dc2d4434731d34b26b0e073221e4ef5ebbd920 Test: CTS Verifier MIDI tests --- .../server/usb/UsbDirectMidiDevice.java | 62 ++++++++++++++++--- 1 file changed, 53 insertions(+), 9 deletions(-) diff --git a/services/usb/java/com/android/server/usb/UsbDirectMidiDevice.java b/services/usb/java/com/android/server/usb/UsbDirectMidiDevice.java index 2ae328b0d8e94..394d6e774aa13 100644 --- a/services/usb/java/com/android/server/usb/UsbDirectMidiDevice.java +++ b/services/usb/java/com/android/server/usb/UsbDirectMidiDevice.java @@ -19,6 +19,7 @@ package com.android.server.usb; import android.annotation.NonNull; import android.content.Context; import android.hardware.usb.UsbConfiguration; +import android.hardware.usb.UsbConstants; import android.hardware.usb.UsbDevice; import android.hardware.usb.UsbDeviceConnection; import android.hardware.usb.UsbEndpoint; @@ -76,10 +77,10 @@ public final class UsbDirectMidiDevice implements Closeable { // event schedulers for each input port of the physical device private MidiEventScheduler[] mEventSchedulers; - // Arbitrary number for timeout to not continue sending to - // an inactive device. This number tries to balances the number - // of cycles and not being permanently stuck. - private static final int BULK_TRANSFER_TIMEOUT_MILLISECONDS = 10; + // Timeout for sending a packet to a device. + // If bulkTransfer times out, retry sending the packet up to 20 times. + private static final int BULK_TRANSFER_TIMEOUT_MILLISECONDS = 50; + private static final int BULK_TRANSFER_NUMBER_OF_RETRIES = 20; // Arbitrary number for timeout when closing a thread private static final int THREAD_JOIN_TIMEOUT_MILLISECONDS = 200; @@ -386,10 +387,15 @@ public final class UsbDirectMidiDevice implements Closeable { break; } final UsbRequest response = connectionFinal.requestWait(); - if (response != request) { - Log.w(TAG, "Unexpected response"); + if (response == null) { + Log.w(TAG, "Response is null"); break; } + if (request != response) { + Log.w(TAG, "Skipping response"); + continue; + } + int bytesRead = byteBuffer.position(); if (bytesRead > 0) { @@ -513,9 +519,47 @@ public final class UsbDirectMidiDevice implements Closeable { convertedArray.length); } - connectionFinal.bulkTransfer(endpointFinal, convertedArray, - convertedArray.length, - BULK_TRANSFER_TIMEOUT_MILLISECONDS); + boolean isInterrupted = false; + // Split the packet into multiple if they are greater than the + // endpoint's max packet size. + for (int curPacketStart = 0; + curPacketStart < convertedArray.length && + isInterrupted == false; + curPacketStart += endpointFinal.getMaxPacketSize()) { + int transferResult = -1; + int retryCount = 0; + int curPacketSize = Math.min(endpointFinal.getMaxPacketSize(), + convertedArray.length - curPacketStart); + + // Keep trying to send the packet until the result is + // successful or until the retry limit is reached. + while (transferResult < 0 && retryCount <= + BULK_TRANSFER_NUMBER_OF_RETRIES) { + transferResult = connectionFinal.bulkTransfer( + endpointFinal, + convertedArray, + curPacketStart, + curPacketSize, + BULK_TRANSFER_TIMEOUT_MILLISECONDS); + retryCount++; + + if (Thread.currentThread().interrupted()) { + Log.w(TAG, "output thread interrupted after send"); + isInterrupted = true; + break; + } + if (transferResult < 0) { + Log.d(TAG, "retrying packet. retryCount = " + + retryCount + " result = " + transferResult); + if (retryCount > BULK_TRANSFER_NUMBER_OF_RETRIES) { + Log.w(TAG, "Skipping packet because timeout"); + } + } + } + } + if (isInterrupted == true) { + break; + } eventSchedulerFinal.addEventToPool(event); } } catch (NullPointerException e) {