From 139be681854585bf24adb879111336f8ad5b4999 Mon Sep 17 00:00:00 2001 From: Robert Wu Date: Tue, 10 May 2022 23:01:24 +0000 Subject: [PATCH] USB MIDI: Don't call openLocked on device unplug When a USB MIDI device with multiple ports is unplugged, onDeviceStatusChanged gets called multiple times. The first time ends up opening a port since there are still other open ports. The fix here is to ignore onDeviceStatusChanged() if close() was called before. Also changed a continue to a break since that condition almost never gets hit. Bug: 232142589 Bug: 231496864 Test: Midi Keyboard that opens both input and output for both peripheral and host mode Change-Id: I973d55371cd16803ede192081531ca5f59847a51 --- .../server/usb/UsbDirectMidiDevice.java | 28 ++++++++-------- .../com/android/server/usb/UsbMidiDevice.java | 32 +++++++++---------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/services/usb/java/com/android/server/usb/UsbDirectMidiDevice.java b/services/usb/java/com/android/server/usb/UsbDirectMidiDevice.java index bbde882aabdc8..e127e312f7be0 100644 --- a/services/usb/java/com/android/server/usb/UsbDirectMidiDevice.java +++ b/services/usb/java/com/android/server/usb/UsbDirectMidiDevice.java @@ -89,38 +89,36 @@ public final class UsbDirectMidiDevice implements Closeable { private final Object mLock = new Object(); private boolean mIsOpen; + private boolean mServerAvailable; private final UsbMidiPacketConverter mUsbMidiPacketConverter = new UsbMidiPacketConverter(); private final MidiDeviceServer.Callback mCallback = new MidiDeviceServer.Callback() { - @Override public void onDeviceStatusChanged(MidiDeviceServer server, MidiDeviceStatus status) { MidiDeviceInfo deviceInfo = status.getDeviceInfo(); int numInputPorts = deviceInfo.getInputPortCount(); int numOutputPorts = deviceInfo.getOutputPortCount(); - boolean hasOpenPorts = false; + int numOpenPorts = 0; for (int i = 0; i < numInputPorts; i++) { if (status.isInputPortOpen(i)) { - hasOpenPorts = true; - break; + numOpenPorts++; } } - if (!hasOpenPorts) { - for (int i = 0; i < numOutputPorts; i++) { - if (status.getOutputPortOpenCount(i) > 0) { - hasOpenPorts = true; - break; - } + for (int i = 0; i < numOutputPorts; i++) { + if (status.getOutputPortOpenCount(i) > 0) { + numOpenPorts += status.getOutputPortOpenCount(i); } } synchronized (mLock) { - if (hasOpenPorts && !mIsOpen) { + Log.d(TAG, "numOpenPorts: " + numOpenPorts + " isOpen: " + mIsOpen + + " mServerAvailable: " + mServerAvailable); + if ((numOpenPorts > 0) && !mIsOpen && mServerAvailable) { openLocked(); - } else if (!hasOpenPorts && mIsOpen) { + } else if ((numOpenPorts == 0) && mIsOpen) { closeLocked(); } } @@ -343,7 +341,7 @@ public final class UsbDirectMidiDevice implements Closeable { final UsbRequest response = connectionFinal.requestWait(); if (response != request) { Log.w(TAG, "Unexpected response"); - continue; + break; } int bytesRead = byteBuffer.position(); @@ -457,7 +455,7 @@ public final class UsbDirectMidiDevice implements Closeable { mContext = context; MidiManager midiManager = context.getSystemService(MidiManager.class); if (midiManager == null) { - Log.e(TAG, "No MidiManager in UsbDirectMidiDevice.create()"); + Log.e(TAG, "No MidiManager in UsbDirectMidiDevice.register()"); return false; } @@ -494,6 +492,7 @@ public final class UsbDirectMidiDevice implements Closeable { mUsbDevice.getSerialNumber()); properties.putParcelable(MidiDeviceInfo.PROPERTY_USB_DEVICE, mUsbDevice); + mServerAvailable = true; mServer = midiManager.createDeviceServer(mMidiInputPortReceivers, mNumInputs, null, null, properties, MidiDeviceInfo.TYPE_USB, mDefaultMidiProtocol, mCallback); if (mServer == null) { @@ -509,6 +508,7 @@ public final class UsbDirectMidiDevice implements Closeable { if (mIsOpen) { closeLocked(); } + mServerAvailable = false; } if (mServer != null) { diff --git a/services/usb/java/com/android/server/usb/UsbMidiDevice.java b/services/usb/java/com/android/server/usb/UsbMidiDevice.java index 275f217551412..67955e155f191 100644 --- a/services/usb/java/com/android/server/usb/UsbMidiDevice.java +++ b/services/usb/java/com/android/server/usb/UsbMidiDevice.java @@ -71,40 +71,38 @@ public final class UsbMidiDevice implements Closeable { private final Object mLock = new Object(); private boolean mIsOpen; + private boolean mServerAvailable; // pipe file descriptor for signalling input thread to exit // only accessed from JNI code private int mPipeFD = -1; private final MidiDeviceServer.Callback mCallback = new MidiDeviceServer.Callback() { - @Override public void onDeviceStatusChanged(MidiDeviceServer server, MidiDeviceStatus status) { MidiDeviceInfo deviceInfo = status.getDeviceInfo(); - int inputPorts = deviceInfo.getInputPortCount(); - int outputPorts = deviceInfo.getOutputPortCount(); - boolean hasOpenPorts = false; + int numInputPorts = deviceInfo.getInputPortCount(); + int numOutputPorts = deviceInfo.getOutputPortCount(); + int numOpenPorts = 0; - for (int i = 0; i < inputPorts; i++) { + for (int i = 0; i < numInputPorts; i++) { if (status.isInputPortOpen(i)) { - hasOpenPorts = true; - break; + numOpenPorts++; } } - if (!hasOpenPorts) { - for (int i = 0; i < outputPorts; i++) { - if (status.getOutputPortOpenCount(i) > 0) { - hasOpenPorts = true; - break; - } + for (int i = 0; i < numOutputPorts; i++) { + if (status.getOutputPortOpenCount(i) > 0) { + numOpenPorts += status.getOutputPortOpenCount(i); } } synchronized (mLock) { - if (hasOpenPorts && !mIsOpen) { + Log.d(TAG, "numOpenPorts: " + numOpenPorts + " isOpen: " + mIsOpen + + " mServerAvailable: " + mServerAvailable); + if ((numOpenPorts > 0) && !mIsOpen && mServerAvailable) { openLocked(); - } else if (!hasOpenPorts && mIsOpen) { + } else if ((numOpenPorts == 0) && mIsOpen) { closeLocked(); } } @@ -298,10 +296,11 @@ public final class UsbMidiDevice implements Closeable { private boolean register(Context context, Bundle properties) { MidiManager midiManager = (MidiManager)context.getSystemService(Context.MIDI_SERVICE); if (midiManager == null) { - Log.e(TAG, "No MidiManager in UsbMidiDevice.create()"); + Log.e(TAG, "No MidiManager in UsbMidiDevice.register()"); return false; } + mServerAvailable = true; mServer = midiManager.createDeviceServer(mMidiInputPortReceivers, mNumInputs, null, null, properties, MidiDeviceInfo.TYPE_USB, MidiDeviceInfo.PROTOCOL_UNKNOWN, mCallback); @@ -318,6 +317,7 @@ public final class UsbMidiDevice implements Closeable { if (mIsOpen) { closeLocked(); } + mServerAvailable = false; } if (mServer != null) {