From 20821ecbe81ba52b260ae232096bc2bfb3e92ad0 Mon Sep 17 00:00:00 2001 From: Mike Lockwood Date: Tue, 24 Feb 2015 09:06:30 -0800 Subject: [PATCH 1/5] Eliminate MidiPort base class for MidiInputPort and MidiOutputPort Change-Id: I628c0468ac980eee909add53a4d6e55e9b358603 --- .../android/media/midi/MidiInputPort.java | 25 ++++++++-- .../android/media/midi/MidiOutputPort.java | 25 +++++++--- .../midi/{MidiPort.java => MidiPortImpl.java} | 50 ++----------------- .../com/android/server/usb/UsbMidiDevice.java | 5 +- 4 files changed, 44 insertions(+), 61 deletions(-) rename media/java/android/media/midi/{MidiPort.java => MidiPortImpl.java} (71%) diff --git a/media/java/android/media/midi/MidiInputPort.java b/media/java/android/media/midi/MidiInputPort.java index 730d36442336c..9ded0b1140156 100644 --- a/media/java/android/media/midi/MidiInputPort.java +++ b/media/java/android/media/midi/MidiInputPort.java @@ -20,6 +20,7 @@ import android.os.ParcelFileDescriptor; import libcore.io.IoUtils; +import java.io.Closeable; import java.io.FileOutputStream; import java.io.IOException; @@ -29,18 +30,32 @@ import java.io.IOException; * CANDIDATE FOR PUBLIC API * @hide */ -public class MidiInputPort extends MidiPort implements MidiReceiver { +public class MidiInputPort implements MidiReceiver, Closeable { + private final int mPortNumber; private final FileOutputStream mOutputStream; // buffer to use for sending messages out our output stream - private final byte[] mBuffer = new byte[MAX_PACKET_SIZE]; + private final byte[] mBuffer = new byte[MidiPortImpl.MAX_PACKET_SIZE]; /* package */ MidiInputPort(ParcelFileDescriptor pfd, int portNumber) { - super(portNumber); + mPortNumber = portNumber; mOutputStream = new ParcelFileDescriptor.AutoCloseOutputStream(pfd); } + /** + * Returns the port number of this port + * + * @return the port's port number + */ + public final int getPortNumber() { + return mPortNumber; + } + + //FIXME + public void onIOException() { + } + /** * Writes a MIDI message to the input port * @@ -56,9 +71,9 @@ public class MidiInputPort extends MidiPort implements MidiReceiver { synchronized (mBuffer) { try { while (count > 0) { - int length = packMessage(msg, offset, count, timestamp, mBuffer); + int length = MidiPortImpl.packMessage(msg, offset, count, timestamp, mBuffer); mOutputStream.write(mBuffer, 0, length); - int sent = getMessageSize(mBuffer, length); + int sent = MidiPortImpl.getMessageSize(mBuffer, length); assert(sent >= 0 && sent <= length); offset += sent; diff --git a/media/java/android/media/midi/MidiOutputPort.java b/media/java/android/media/midi/MidiOutputPort.java index c195603a204c6..3efd03fc580eb 100644 --- a/media/java/android/media/midi/MidiOutputPort.java +++ b/media/java/android/media/midi/MidiOutputPort.java @@ -21,6 +21,7 @@ import android.util.Log; import libcore.io.IoUtils; +import java.io.Closeable; import java.io.FileInputStream; import java.io.IOException; @@ -30,9 +31,10 @@ import java.io.IOException; * CANDIDATE FOR PUBLIC API * @hide */ -public class MidiOutputPort extends MidiPort implements MidiSender { +public class MidiOutputPort implements MidiSender, Closeable { private static final String TAG = "MidiOutputPort"; + private final int mPortNumber; private final FileInputStream mInputStream; private final MidiDispatcher mDispatcher = new MidiDispatcher(); @@ -41,7 +43,7 @@ public class MidiOutputPort extends MidiPort implements MidiSender { private final Thread mThread = new Thread() { @Override public void run() { - byte[] buffer = new byte[MAX_PACKET_SIZE]; + byte[] buffer = new byte[MidiPortImpl.MAX_PACKET_SIZE]; try { while (true) { @@ -52,9 +54,9 @@ public class MidiOutputPort extends MidiPort implements MidiSender { // FIXME - inform receivers here? } - int offset = getMessageOffset(buffer, count); - int size = getMessageSize(buffer, count); - long timestamp = getMessageTimeStamp(buffer, count); + int offset = MidiPortImpl.getMessageOffset(buffer, count); + int size = MidiPortImpl.getMessageSize(buffer, count); + long timestamp = MidiPortImpl.getMessageTimeStamp(buffer, count); // dispatch to all our receivers mDispatcher.post(buffer, offset, size, timestamp); @@ -68,12 +70,21 @@ public class MidiOutputPort extends MidiPort implements MidiSender { } }; - /* package */ MidiOutputPort(ParcelFileDescriptor pfd, int portNumber) { - super(portNumber); + /* package */ MidiOutputPort(ParcelFileDescriptor pfd, int portNumber) { + mPortNumber = portNumber; mInputStream = new ParcelFileDescriptor.AutoCloseInputStream(pfd); mThread.start(); } + /** + * Returns the port number of this port + * + * @return the port's port number + */ + public final int getPortNumber() { + return mPortNumber; + } + @Override public void connect(MidiReceiver receiver) { mDispatcher.getSender().connect(receiver); diff --git a/media/java/android/media/midi/MidiPort.java b/media/java/android/media/midi/MidiPortImpl.java similarity index 71% rename from media/java/android/media/midi/MidiPort.java rename to media/java/android/media/midi/MidiPortImpl.java index 3aa03f2de0d8b..fcb23c17bc73c 100644 --- a/media/java/android/media/midi/MidiPort.java +++ b/media/java/android/media/midi/MidiPortImpl.java @@ -19,28 +19,19 @@ package android.media.midi; import java.io.Closeable; /** - * This class represents a MIDI input or output port. - * Base class for {@link MidiInputPort} and {@link MidiOutputPort} - * - * CANDIDATE FOR PUBLIC API - * @hide + * This class contains utilities for socket communication between a + * MidiInputPort and MidiOutputPort */ -abstract public class MidiPort implements Closeable { +/* package */ class MidiPortImpl { private static final String TAG = "MidiPort"; - private final int mPortNumber; - /** * Maximum size of a packet that can pass through our ParcelFileDescriptor. - * For internal use only. Implementation details may change in the future. - * @hide */ public static final int MAX_PACKET_SIZE = 1024; /** * size of message timestamp in bytes - * For internal use only. Implementation details may change in the future. - * @hide */ private static final int TIMESTAMP_SIZE = 8; @@ -49,29 +40,6 @@ abstract public class MidiPort implements Closeable { */ public static final int MAX_PACKET_DATA_SIZE = MAX_PACKET_SIZE - TIMESTAMP_SIZE; - - /* package */ MidiPort(int portNumber) { - mPortNumber = portNumber; - } - - /** - * Returns the port number of this port - * - * @return the port's port number - */ - public final int getPortNumber() { - return mPortNumber; - } - - /** - * Called when an IOExeption occurs while sending or receiving data. - * Subclasses can override to be notified of such errors - * - * @hide - */ - public void onIOException() { - } - /** * Utility function for packing a MIDI message to be sent through our ParcelFileDescriptor * @@ -80,9 +48,6 @@ abstract public class MidiPort implements Closeable { * timestamp is message timestamp to pack * dest is buffer to pack into * returns size of packed message - * - * For internal use only. Implementation details may change in the future. - * @hide */ public static int packMessage(byte[] message, int offset, int size, long timestamp, byte[] dest) { @@ -104,9 +69,6 @@ abstract public class MidiPort implements Closeable { /** * Utility function for unpacking a MIDI message received from our ParcelFileDescriptor * returns the offset of the MIDI message in packed buffer - * - * For internal use only. Implementation details may change in the future. - * @hide */ public static int getMessageOffset(byte[] buffer, int bufferLength) { // message is at the beginning @@ -116,9 +78,6 @@ abstract public class MidiPort implements Closeable { /** * Utility function for unpacking a MIDI message received from our ParcelFileDescriptor * returns size of MIDI data in packed buffer - * - * For internal use only. Implementation details may change in the future. - * @hide */ public static int getMessageSize(byte[] buffer, int bufferLength) { // message length is total buffer length minus size of the timestamp @@ -128,9 +87,6 @@ abstract public class MidiPort implements Closeable { /** * Utility function for unpacking a MIDI message received from our ParcelFileDescriptor * unpacks timestamp from packed buffer - * - * For internal use only. Implementation details may change in the future. - * @hide */ public static long getMessageTimeStamp(byte[] buffer, int bufferLength) { // timestamp is at end of the packet diff --git a/services/usb/java/com/android/server/usb/UsbMidiDevice.java b/services/usb/java/com/android/server/usb/UsbMidiDevice.java index a6e9677a90c49..b5187d6dd147d 100644 --- a/services/usb/java/com/android/server/usb/UsbMidiDevice.java +++ b/services/usb/java/com/android/server/usb/UsbMidiDevice.java @@ -21,7 +21,6 @@ import android.media.midi.MidiDeviceInfo; import android.media.midi.MidiDeviceServer; import android.media.midi.MidiDispatcher; import android.media.midi.MidiManager; -import android.media.midi.MidiPort; import android.media.midi.MidiReceiver; import android.media.midi.MidiSender; import android.os.Bundle; @@ -46,6 +45,8 @@ public final class UsbMidiDevice implements Closeable { private final MidiReceiver[] mInputPortReceivers; + private static final int BUFFER_SIZE = 512; + // for polling multiple FileDescriptors for MIDI events private final StructPollfd[] mPollFDs; // streams for reading from ALSA driver @@ -130,7 +131,7 @@ public final class UsbMidiDevice implements Closeable { new Thread() { @Override public void run() { - byte[] buffer = new byte[MidiPort.MAX_PACKET_DATA_SIZE]; + byte[] buffer = new byte[BUFFER_SIZE]; try { boolean done = false; while (!done) { From 3b7664589be22ddad34b72e11ced937d48660ebb Mon Sep 17 00:00:00 2001 From: Mike Lockwood Date: Wed, 4 Mar 2015 14:50:39 -0800 Subject: [PATCH 2/5] Make MidiSender and MidiReceiver abstract classes, rename MidiReceiver.post() to receive() Change-Id: I1cef3bd48ca0acf2968c9de223f78445f3434404 --- .../android/media/midi/MidiDeviceServer.java | 8 ++-- .../android/media/midi/MidiDispatcher.java | 8 ++-- .../android/media/midi/MidiInputPort.java | 41 +++++++------------ .../android/media/midi/MidiOutputPort.java | 4 +- .../java/android/media/midi/MidiReceiver.java | 7 ++-- media/java/android/media/midi/MidiSender.java | 6 +-- .../com/android/server/usb/UsbMidiDevice.java | 4 +- 7 files changed, 34 insertions(+), 44 deletions(-) diff --git a/media/java/android/media/midi/MidiDeviceServer.java b/media/java/android/media/midi/MidiDeviceServer.java index 24ef5288f9d6b..87fb3c5eccf38 100644 --- a/media/java/android/media/midi/MidiDeviceServer.java +++ b/media/java/android/media/midi/MidiDeviceServer.java @@ -85,10 +85,10 @@ public final class MidiDeviceServer implements Closeable { outputPort.connect(new MidiReceiver() { @Override - public void post(byte[] msg, int offset, int count, long timestamp) + public void receive(byte[] msg, int offset, int count, long timestamp) throws IOException { try { - inputPortReceviver.post(msg, offset, count, timestamp); + inputPortReceviver.receive(msg, offset, count, timestamp); } catch (IOException e) { IoUtils.closeQuietly(mInputPortOutputPorts[portNumberF]); mInputPortOutputPorts[portNumberF] = null; @@ -125,10 +125,10 @@ public final class MidiDeviceServer implements Closeable { final MidiSender sender = mOutputPortDispatchers[portNumber].getSender(); sender.connect(new MidiReceiver() { @Override - public void post(byte[] msg, int offset, int count, long timestamp) + public void receive(byte[] msg, int offset, int count, long timestamp) throws IOException { try { - inputPort.post(msg, offset, count, timestamp); + inputPort.receive(msg, offset, count, timestamp); } catch (IOException e) { IoUtils.closeQuietly(inputPort); sender.disconnect(this); diff --git a/media/java/android/media/midi/MidiDispatcher.java b/media/java/android/media/midi/MidiDispatcher.java index 165061f1f09dc..b2d8b6fd01e69 100644 --- a/media/java/android/media/midi/MidiDispatcher.java +++ b/media/java/android/media/midi/MidiDispatcher.java @@ -24,12 +24,12 @@ import java.util.ArrayList; * This class subclasses {@link MidiReceiver} and dispatches any data it receives * to its receiver list. Any receivers that throw an exception upon receiving data will * be automatically removed from the receiver list, but no IOException will be returned - * from the dispatcher's {@link #post} in that case. + * from the dispatcher's {@link #receive} in that case. * * CANDIDATE FOR PUBLIC API * @hide */ -public class MidiDispatcher implements MidiReceiver { +public class MidiDispatcher extends MidiReceiver { private final ArrayList mReceivers = new ArrayList(); @@ -71,12 +71,12 @@ public class MidiDispatcher implements MidiReceiver { } @Override - public void post(byte[] msg, int offset, int count, long timestamp) throws IOException { + public void receive(byte[] msg, int offset, int count, long timestamp) throws IOException { synchronized (mReceivers) { for (int i = 0; i < mReceivers.size(); ) { MidiReceiver receiver = mReceivers.get(i); try { - receiver.post(msg, offset, count, timestamp); + receiver.receive(msg, offset, count, timestamp); i++; // increment only on success. on failure we remove the receiver // so i should not be incremented } catch (IOException e) { diff --git a/media/java/android/media/midi/MidiInputPort.java b/media/java/android/media/midi/MidiInputPort.java index 9ded0b1140156..fb70e8927b093 100644 --- a/media/java/android/media/midi/MidiInputPort.java +++ b/media/java/android/media/midi/MidiInputPort.java @@ -30,12 +30,12 @@ import java.io.IOException; * CANDIDATE FOR PUBLIC API * @hide */ -public class MidiInputPort implements MidiReceiver, Closeable { +public class MidiInputPort extends MidiReceiver implements Closeable { private final int mPortNumber; private final FileOutputStream mOutputStream; - // buffer to use for sending messages out our output stream + // buffer to use for sending data out our output stream private final byte[] mBuffer = new byte[MidiPortImpl.MAX_PACKET_SIZE]; /* package */ MidiInputPort(ParcelFileDescriptor pfd, int portNumber) { @@ -52,38 +52,27 @@ public class MidiInputPort implements MidiReceiver, Closeable { return mPortNumber; } - //FIXME - public void onIOException() { - } - /** - * Writes a MIDI message to the input port + * Writes MIDI data to the input port * - * @param msg byte array containing the message - * @param offset offset of first byte of the message in msg byte array - * @param count size of the message in bytes - * @param timestamp future time to post the message (based on + * @param msg byte array containing the data + * @param offset offset of first byte of the data in msg byte array + * @param count size of the data in bytes + * @param timestamp future time to post the data (based on * {@link java.lang.System#nanoTime} */ - public void post(byte[] msg, int offset, int count, long timestamp) throws IOException { + public void receive(byte[] msg, int offset, int count, long timestamp) throws IOException { assert(offset >= 0 && count >= 0 && offset + count <= msg.length); synchronized (mBuffer) { - try { - while (count > 0) { - int length = MidiPortImpl.packMessage(msg, offset, count, timestamp, mBuffer); - mOutputStream.write(mBuffer, 0, length); - int sent = MidiPortImpl.getMessageSize(mBuffer, length); - assert(sent >= 0 && sent <= length); + while (count > 0) { + int length = MidiPortImpl.packMessage(msg, offset, count, timestamp, mBuffer); + mOutputStream.write(mBuffer, 0, length); + int sent = MidiPortImpl.getMessageSize(mBuffer, length); + assert(sent >= 0 && sent <= length); - offset += sent; - count -= sent; - } - } catch (IOException e) { - IoUtils.closeQuietly(mOutputStream); - // report I/O failure - onIOException(); - throw e; + offset += sent; + count -= sent; } } } diff --git a/media/java/android/media/midi/MidiOutputPort.java b/media/java/android/media/midi/MidiOutputPort.java index 3efd03fc580eb..8b99e9079ba63 100644 --- a/media/java/android/media/midi/MidiOutputPort.java +++ b/media/java/android/media/midi/MidiOutputPort.java @@ -31,7 +31,7 @@ import java.io.IOException; * CANDIDATE FOR PUBLIC API * @hide */ -public class MidiOutputPort implements MidiSender, Closeable { +public class MidiOutputPort extends MidiSender implements Closeable { private static final String TAG = "MidiOutputPort"; private final int mPortNumber; @@ -59,7 +59,7 @@ public class MidiOutputPort implements MidiSender, Closeable { long timestamp = MidiPortImpl.getMessageTimeStamp(buffer, count); // dispatch to all our receivers - mDispatcher.post(buffer, offset, size, timestamp); + mDispatcher.receive(buffer, offset, size, timestamp); } } catch (IOException e) { // FIXME report I/O failure? diff --git a/media/java/android/media/midi/MidiReceiver.java b/media/java/android/media/midi/MidiReceiver.java index 64c0c072fa409..674c9743500ff 100644 --- a/media/java/android/media/midi/MidiReceiver.java +++ b/media/java/android/media/midi/MidiReceiver.java @@ -24,7 +24,7 @@ import java.io.IOException; * CANDIDATE FOR PUBLIC API * @hide */ -public interface MidiReceiver { +abstract public class MidiReceiver { /** * Called to pass MIDI data to the receiver. * @@ -32,7 +32,7 @@ public interface MidiReceiver { * The msg bytes should be copied by the receiver rather than retaining a reference * to this parameter. * Also, modifying the contents of the msg array parameter may result in other receivers - * in the same application receiving incorrect values in their post() method. + * in the same application receiving incorrect values in their receive() method. * * @param msg a byte array containing the MIDI data * @param offset the offset of the first byte of the data in the byte array @@ -40,5 +40,6 @@ public interface MidiReceiver { * @param timestamp the timestamp of the message (based on {@link java.lang.System#nanoTime} * @throws IOException */ - public void post(byte[] msg, int offset, int count, long timestamp) throws IOException; + abstract public void receive(byte[] msg, int offset, int count, long timestamp) + throws IOException; } diff --git a/media/java/android/media/midi/MidiSender.java b/media/java/android/media/midi/MidiSender.java index 4550476701675..9285973234e03 100644 --- a/media/java/android/media/midi/MidiSender.java +++ b/media/java/android/media/midi/MidiSender.java @@ -23,18 +23,18 @@ package android.media.midi; * CANDIDATE FOR PUBLIC API * @hide */ -public interface MidiSender { +abstract public class MidiSender { /** * Called to connect a {@link MidiReceiver} to the sender * * @param receiver the receiver to connect */ - public void connect(MidiReceiver receiver); + abstract public void connect(MidiReceiver receiver); /** * Called to disconnect a {@link MidiReceiver} from the sender * * @param receiver the receiver to disconnect */ - public void disconnect(MidiReceiver receiver); + abstract public void disconnect(MidiReceiver receiver); } diff --git a/services/usb/java/com/android/server/usb/UsbMidiDevice.java b/services/usb/java/com/android/server/usb/UsbMidiDevice.java index b5187d6dd147d..51d61bbb4b387 100644 --- a/services/usb/java/com/android/server/usb/UsbMidiDevice.java +++ b/services/usb/java/com/android/server/usb/UsbMidiDevice.java @@ -103,7 +103,7 @@ public final class UsbMidiDevice implements Closeable { final int portF = port; mInputPortReceivers[port] = new MidiReceiver() { @Override - public void post(byte[] data, int offset, int count, long timestamp) + public void receive(byte[] data, int offset, int count, long timestamp) throws IOException { // FIXME - timestamps are ignored, future posting not supported yet. mOutputStreams[portF].write(data, offset, count); @@ -144,7 +144,7 @@ public final class UsbMidiDevice implements Closeable { int count = mInputStreams[index].read(buffer); long timestamp = System.nanoTime(); - outputReceivers[index].post(buffer, 0, count, timestamp); + outputReceivers[index].receive(buffer, 0, count, timestamp); } else if ((pfd.revents & (OsConstants.POLLERR | OsConstants.POLLHUP)) != 0) { done = true; From 35110d1ed72ee7fe687125831b4dd91b87b515ee Mon Sep 17 00:00:00 2001 From: Mike Lockwood Date: Wed, 4 Mar 2015 16:05:17 -0800 Subject: [PATCH 3/5] Add MidiDevice.close() method so we can clean up our ServiceConnection Change-Id: I65cd4cfd940b02709daeffef6dab814305b8a6b0 --- media/java/android/media/midi/MidiDevice.java | 34 +++++++++++++++---- .../java/android/media/midi/MidiManager.java | 2 +- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/media/java/android/media/midi/MidiDevice.java b/media/java/android/media/midi/MidiDevice.java index 87af362d21da6..a273915039ae5 100644 --- a/media/java/android/media/midi/MidiDevice.java +++ b/media/java/android/media/midi/MidiDevice.java @@ -16,10 +16,15 @@ package android.media.midi; +import android.content.Context; +import android.content.ServiceConnection; import android.os.ParcelFileDescriptor; import android.os.RemoteException; import android.util.Log; +import java.io.Closeable; +import java.io.IOException; + /** * This class is used for sending and receiving data to and from an MIDI device * Instances of this class are created by {@link MidiManager#openDevice}. @@ -27,19 +32,27 @@ import android.util.Log; * CANDIDATE FOR PUBLIC API * @hide */ -public final class MidiDevice { +public final class MidiDevice implements Closeable { private static final String TAG = "MidiDevice"; private final MidiDeviceInfo mDeviceInfo; private final IMidiDeviceServer mServer; + private Context mContext; + private ServiceConnection mServiceConnection; - /** - * MidiDevice should only be instantiated by MidiManager - * @hide - */ - public MidiDevice(MidiDeviceInfo deviceInfo, IMidiDeviceServer server) { + /* package */ MidiDevice(MidiDeviceInfo deviceInfo, IMidiDeviceServer server) { mDeviceInfo = deviceInfo; mServer = server; + mContext = null; + mServiceConnection = null; + } + + /* package */ MidiDevice(MidiDeviceInfo deviceInfo, IMidiDeviceServer server, + Context context, ServiceConnection serviceConnection) { + mDeviceInfo = deviceInfo; + mServer = server; + mContext = context; + mServiceConnection = serviceConnection; } /** @@ -89,6 +102,15 @@ public final class MidiDevice { } } + @Override + public void close() throws IOException { + if (mContext != null && mServiceConnection != null) { + mContext.unbindService(mServiceConnection); + mContext = null; + mServiceConnection = null; + } + } + @Override public String toString() { return ("MidiDevice: " + mDeviceInfo.toString()); diff --git a/media/java/android/media/midi/MidiManager.java b/media/java/android/media/midi/MidiManager.java index ca7d3c25c293e..08ac25a145db3 100644 --- a/media/java/android/media/midi/MidiManager.java +++ b/media/java/android/media/midi/MidiManager.java @@ -223,7 +223,7 @@ public class MidiManager { public void onServiceConnected(ComponentName name, IBinder binder) { IMidiDeviceServer server = IMidiDeviceServer.Stub.asInterface(binder); - MidiDevice device = new MidiDevice(deviceInfoF, server); + MidiDevice device = new MidiDevice(deviceInfoF, server, mContext, this); sendOpenDeviceResponse(deviceInfoF, device, callbackF, handlerF); } From eebc98ff18c1ee92dff3fcd505158ea161d552be Mon Sep 17 00:00:00 2001 From: Mike Lockwood Date: Fri, 6 Mar 2015 08:17:33 -0800 Subject: [PATCH 4/5] MidiDeviceService: Add getDeviceInfo() accessor method so service implementations can access their own device info object. Change-Id: I93e0c449e72d76568d7b4c9f7f7db00a846b5a33 --- .../java/android/media/midi/MidiDeviceService.java | 14 ++++++++++++++ media/java/android/media/midi/MidiPortImpl.java | 2 -- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/media/java/android/media/midi/MidiDeviceService.java b/media/java/android/media/midi/MidiDeviceService.java index 1d91be2ae1922..64f69cdcd1320 100644 --- a/media/java/android/media/midi/MidiDeviceService.java +++ b/media/java/android/media/midi/MidiDeviceService.java @@ -55,6 +55,7 @@ abstract public class MidiDeviceService extends Service { private IMidiManager mMidiManager; private MidiDeviceServer mServer; + private MidiDeviceInfo mDeviceInfo; @Override public void onCreate() { @@ -64,6 +65,11 @@ abstract public class MidiDeviceService extends Service { try { MidiDeviceInfo deviceInfo = mMidiManager.getServiceDeviceInfo(getPackageName(), this.getClass().getName()); + if (deviceInfo == null) { + Log.e(TAG, "Could not find MidiDeviceInfo for MidiDeviceService " + this); + return; + } + mDeviceInfo = deviceInfo; MidiReceiver[] inputPortReceivers = getInputPortReceivers(); if (inputPortReceivers == null) { inputPortReceivers = new MidiReceiver[0]; @@ -100,6 +106,14 @@ abstract public class MidiDeviceService extends Service { } } + /** + * returns the {@link MidiDeviceInfo} instance for this service + * @return our MidiDeviceInfo + */ + public MidiDeviceInfo getDeviceInfo() { + return mDeviceInfo; + } + @Override public IBinder onBind(Intent intent) { if (SERVICE_INTERFACE.equals(intent.getAction()) && mServer != null) { diff --git a/media/java/android/media/midi/MidiPortImpl.java b/media/java/android/media/midi/MidiPortImpl.java index fcb23c17bc73c..5795045edc7d8 100644 --- a/media/java/android/media/midi/MidiPortImpl.java +++ b/media/java/android/media/midi/MidiPortImpl.java @@ -16,8 +16,6 @@ package android.media.midi; -import java.io.Closeable; - /** * This class contains utilities for socket communication between a * MidiInputPort and MidiOutputPort From 4a3d7ed45d98ad2fe900221755845b87f26b554a Mon Sep 17 00:00:00 2001 From: Mike Lockwood Date: Fri, 6 Mar 2015 09:18:09 -0800 Subject: [PATCH 5/5] MIDI Manager: Add explicit close mechanism for input and output ports Relying on errors from closing the file descriptor is not reliable and was resulting in file descriptor leaks in device servers. Change-Id: Ib5cc22dba493eae6608a12cc6d4178d8390da77b --- .../android/media/midi/IMidiDeviceServer.aidl | 5 +- media/java/android/media/midi/MidiDevice.java | 18 +-- .../android/media/midi/MidiDeviceServer.java | 118 ++++++++++++------ .../android/media/midi/MidiInputPort.java | 40 +++++- .../android/media/midi/MidiOutputPort.java | 38 +++++- 5 files changed, 172 insertions(+), 47 deletions(-) diff --git a/media/java/android/media/midi/IMidiDeviceServer.aidl b/media/java/android/media/midi/IMidiDeviceServer.aidl index 71914ad5892d7..3331aae649bf9 100644 --- a/media/java/android/media/midi/IMidiDeviceServer.aidl +++ b/media/java/android/media/midi/IMidiDeviceServer.aidl @@ -21,6 +21,7 @@ import android.os.ParcelFileDescriptor; /** @hide */ interface IMidiDeviceServer { - ParcelFileDescriptor openInputPort(int portNumber); - ParcelFileDescriptor openOutputPort(int portNumber); + ParcelFileDescriptor openInputPort(IBinder token, int portNumber); + ParcelFileDescriptor openOutputPort(IBinder token, int portNumber); + void closePort(IBinder token); } diff --git a/media/java/android/media/midi/MidiDevice.java b/media/java/android/media/midi/MidiDevice.java index a273915039ae5..1a39485b5e47b 100644 --- a/media/java/android/media/midi/MidiDevice.java +++ b/media/java/android/media/midi/MidiDevice.java @@ -18,6 +18,8 @@ package android.media.midi; import android.content.Context; import android.content.ServiceConnection; +import android.os.Binder; +import android.os.IBinder; import android.os.ParcelFileDescriptor; import android.os.RemoteException; import android.util.Log; @@ -36,13 +38,13 @@ public final class MidiDevice implements Closeable { private static final String TAG = "MidiDevice"; private final MidiDeviceInfo mDeviceInfo; - private final IMidiDeviceServer mServer; + private final IMidiDeviceServer mDeviceServer; private Context mContext; private ServiceConnection mServiceConnection; /* package */ MidiDevice(MidiDeviceInfo deviceInfo, IMidiDeviceServer server) { mDeviceInfo = deviceInfo; - mServer = server; + mDeviceServer = server; mContext = null; mServiceConnection = null; } @@ -50,7 +52,7 @@ public final class MidiDevice implements Closeable { /* package */ MidiDevice(MidiDeviceInfo deviceInfo, IMidiDeviceServer server, Context context, ServiceConnection serviceConnection) { mDeviceInfo = deviceInfo; - mServer = server; + mDeviceServer = server; mContext = context; mServiceConnection = serviceConnection; } @@ -72,11 +74,12 @@ public final class MidiDevice implements Closeable { */ public MidiInputPort openInputPort(int portNumber) { try { - ParcelFileDescriptor pfd = mServer.openInputPort(portNumber); + IBinder token = new Binder(); + ParcelFileDescriptor pfd = mDeviceServer.openInputPort(token, portNumber); if (pfd == null) { return null; } - return new MidiInputPort(pfd, portNumber); + return new MidiInputPort(mDeviceServer, token, pfd, portNumber); } catch (RemoteException e) { Log.e(TAG, "RemoteException in openInputPort"); return null; @@ -91,11 +94,12 @@ public final class MidiDevice implements Closeable { */ public MidiOutputPort openOutputPort(int portNumber) { try { - ParcelFileDescriptor pfd = mServer.openOutputPort(portNumber); + IBinder token = new Binder(); + ParcelFileDescriptor pfd = mDeviceServer.openOutputPort(token, portNumber); if (pfd == null) { return null; } - return new MidiOutputPort(pfd, portNumber); + return new MidiOutputPort(mDeviceServer, token, pfd, portNumber); } catch (RemoteException e) { Log.e(TAG, "RemoteException in openOutputPort"); return null; diff --git a/media/java/android/media/midi/MidiDeviceServer.java b/media/java/android/media/midi/MidiDeviceServer.java index 87fb3c5eccf38..4d59c6395faf5 100644 --- a/media/java/android/media/midi/MidiDeviceServer.java +++ b/media/java/android/media/midi/MidiDeviceServer.java @@ -28,6 +28,7 @@ import libcore.io.IoUtils; import java.io.Closeable; import java.io.IOException; +import java.util.HashMap; /** * Internal class used for providing an implementation for a MIDI device. @@ -53,11 +54,68 @@ public final class MidiDeviceServer implements Closeable { // MidiOutputPorts for clients connected to our input ports private final MidiOutputPort[] mInputPortOutputPorts; + abstract private class PortClient implements IBinder.DeathRecipient { + final IBinder mToken; + + PortClient(IBinder token) { + mToken = token; + + try { + token.linkToDeath(this, 0); + } catch (RemoteException e) { + close(); + } + } + + abstract void close(); + + @Override + public void binderDied() { + close(); + } + } + + private class InputPortClient extends PortClient { + private final MidiOutputPort mOutputPort; + + InputPortClient(IBinder token, MidiOutputPort outputPort) { + super(token); + mOutputPort = outputPort; + } + + @Override + void close() { + mToken.unlinkToDeath(this, 0); + synchronized (mInputPortOutputPorts) { + mInputPortOutputPorts[mOutputPort.getPortNumber()] = null; + } + IoUtils.closeQuietly(mOutputPort); + } + } + + private class OutputPortClient extends PortClient { + private final MidiInputPort mInputPort; + + OutputPortClient(IBinder token, MidiInputPort inputPort) { + super(token); + mInputPort = inputPort; + } + + @Override + void close() { + mToken.unlinkToDeath(this, 0); + mOutputPortDispatchers[mInputPort.getPortNumber()].getSender().disconnect(mInputPort); + IoUtils.closeQuietly(mInputPort); + } + } + + private final HashMap mPortClients = new HashMap(); + // Binder interface stub for receiving connection requests from clients private final IMidiDeviceServer mServer = new IMidiDeviceServer.Stub() { @Override - public ParcelFileDescriptor openInputPort(int portNumber) { + public ParcelFileDescriptor openInputPort(IBinder token, int portNumber) { if (mDeviceInfo.isPrivate()) { if (Binder.getCallingUid() != Process.myUid()) { throw new SecurityException("Can't access private device from different UID"); @@ -78,25 +136,13 @@ public final class MidiDeviceServer implements Closeable { try { ParcelFileDescriptor[] pair = ParcelFileDescriptor.createSocketPair( OsConstants.SOCK_SEQPACKET); - final MidiOutputPort outputPort = new MidiOutputPort(pair[0], portNumber); + MidiOutputPort outputPort = new MidiOutputPort(pair[0], portNumber); mInputPortOutputPorts[portNumber] = outputPort; - final int portNumberF = portNumber; - final MidiReceiver inputPortReceviver = mInputPortReceivers[portNumber]; - - outputPort.connect(new MidiReceiver() { - @Override - public void receive(byte[] msg, int offset, int count, long timestamp) - throws IOException { - try { - inputPortReceviver.receive(msg, offset, count, timestamp); - } catch (IOException e) { - IoUtils.closeQuietly(mInputPortOutputPorts[portNumberF]); - mInputPortOutputPorts[portNumberF] = null; - // FIXME also flush the receiver - } - } - }); - + outputPort.connect(mInputPortReceivers[portNumber]); + InputPortClient client = new InputPortClient(token, outputPort); + synchronized (mPortClients) { + mPortClients.put(token, client); + } return pair[1]; } catch (IOException e) { Log.e(TAG, "unable to create ParcelFileDescriptors in openInputPort"); @@ -106,7 +152,7 @@ public final class MidiDeviceServer implements Closeable { } @Override - public ParcelFileDescriptor openOutputPort(int portNumber) { + public ParcelFileDescriptor openOutputPort(IBinder token, int portNumber) { if (mDeviceInfo.isPrivate()) { if (Binder.getCallingUid() != Process.myUid()) { throw new SecurityException("Can't access private device from different UID"); @@ -121,28 +167,28 @@ public final class MidiDeviceServer implements Closeable { try { ParcelFileDescriptor[] pair = ParcelFileDescriptor.createSocketPair( OsConstants.SOCK_SEQPACKET); - final MidiInputPort inputPort = new MidiInputPort(pair[0], portNumber); - final MidiSender sender = mOutputPortDispatchers[portNumber].getSender(); - sender.connect(new MidiReceiver() { - @Override - public void receive(byte[] msg, int offset, int count, long timestamp) - throws IOException { - try { - inputPort.receive(msg, offset, count, timestamp); - } catch (IOException e) { - IoUtils.closeQuietly(inputPort); - sender.disconnect(this); - // FIXME also flush the receiver? - } - } - }); - + MidiInputPort inputPort = new MidiInputPort(pair[0], portNumber); + mOutputPortDispatchers[portNumber].getSender().connect(inputPort); + OutputPortClient client = new OutputPortClient(token, inputPort); + synchronized (mPortClients) { + mPortClients.put(token, client); + } return pair[1]; } catch (IOException e) { Log.e(TAG, "unable to create ParcelFileDescriptors in openOutputPort"); return null; } } + + @Override + public void closePort(IBinder token) { + synchronized (mPortClients) { + PortClient client = mPortClients.remove(token); + if (client != null) { + client.close(); + } + } + } }; /* package */ MidiDeviceServer(IMidiManager midiManager, MidiReceiver[] inputPortReceivers, diff --git a/media/java/android/media/midi/MidiInputPort.java b/media/java/android/media/midi/MidiInputPort.java index fb70e8927b093..5d944cb30da70 100644 --- a/media/java/android/media/midi/MidiInputPort.java +++ b/media/java/android/media/midi/MidiInputPort.java @@ -16,7 +16,12 @@ package android.media.midi; +import android.os.IBinder; import android.os.ParcelFileDescriptor; +import android.os.RemoteException; +import android.util.Log; + +import dalvik.system.CloseGuard; import libcore.io.IoUtils; @@ -31,16 +36,29 @@ import java.io.IOException; * @hide */ public class MidiInputPort extends MidiReceiver implements Closeable { + private static final String TAG = "MidiInputPort"; + private final IMidiDeviceServer mDeviceServer; + private final IBinder mToken; private final int mPortNumber; private final FileOutputStream mOutputStream; + private final CloseGuard mGuard = CloseGuard.get(); + // buffer to use for sending data out our output stream private final byte[] mBuffer = new byte[MidiPortImpl.MAX_PACKET_SIZE]; - /* package */ MidiInputPort(ParcelFileDescriptor pfd, int portNumber) { + /* package */ MidiInputPort(IMidiDeviceServer server, IBinder token, + ParcelFileDescriptor pfd, int portNumber) { + mDeviceServer = server; + mToken = token; mPortNumber = portNumber; mOutputStream = new ParcelFileDescriptor.AutoCloseOutputStream(pfd); + mGuard.open("close"); + } + + /* package */ MidiInputPort(ParcelFileDescriptor pfd, int portNumber) { + this(null, null, pfd, portNumber); } /** @@ -79,6 +97,26 @@ public class MidiInputPort extends MidiReceiver implements Closeable { @Override public void close() throws IOException { + mGuard.close(); mOutputStream.close(); + if (mDeviceServer != null) { + try { + mDeviceServer.closePort(mToken); + } catch (RemoteException e) { + Log.e(TAG, "RemoteException in MidiInputPort.close()"); + } + } + } + + @Override + protected void finalize() throws Throwable { + try { + if (mGuard != null) { + mGuard.warnIfOpen(); + } + close(); + } finally { + super.finalize(); + } } } diff --git a/media/java/android/media/midi/MidiOutputPort.java b/media/java/android/media/midi/MidiOutputPort.java index 8b99e9079ba63..d46b2028fbb1e 100644 --- a/media/java/android/media/midi/MidiOutputPort.java +++ b/media/java/android/media/midi/MidiOutputPort.java @@ -16,9 +16,13 @@ package android.media.midi; +import android.os.IBinder; import android.os.ParcelFileDescriptor; +import android.os.RemoteException; import android.util.Log; +import dalvik.system.CloseGuard; + import libcore.io.IoUtils; import java.io.Closeable; @@ -34,10 +38,14 @@ import java.io.IOException; public class MidiOutputPort extends MidiSender implements Closeable { private static final String TAG = "MidiOutputPort"; + private final IMidiDeviceServer mDeviceServer; + private final IBinder mToken; private final int mPortNumber; private final FileInputStream mInputStream; private final MidiDispatcher mDispatcher = new MidiDispatcher(); + private final CloseGuard mGuard = CloseGuard.get(); + // This thread reads MIDI events from a socket and distributes them to the list of // MidiReceivers attached to this device. private final Thread mThread = new Thread() { @@ -70,10 +78,18 @@ public class MidiOutputPort extends MidiSender implements Closeable { } }; - /* package */ MidiOutputPort(ParcelFileDescriptor pfd, int portNumber) { + /* package */ MidiOutputPort(IMidiDeviceServer server, IBinder token, + ParcelFileDescriptor pfd, int portNumber) { + mDeviceServer = server; + mToken = token; mPortNumber = portNumber; mInputStream = new ParcelFileDescriptor.AutoCloseInputStream(pfd); mThread.start(); + mGuard.open("close"); + } + + /* package */ MidiOutputPort(ParcelFileDescriptor pfd, int portNumber) { + this(null, null, pfd, portNumber); } /** @@ -97,6 +113,26 @@ public class MidiOutputPort extends MidiSender implements Closeable { @Override public void close() throws IOException { + mGuard.close(); mInputStream.close(); + if (mDeviceServer != null) { + try { + mDeviceServer.closePort(mToken); + } catch (RemoteException e) { + Log.e(TAG, "RemoteException in MidiOutputPort.close()"); + } + } + } + + @Override + protected void finalize() throws Throwable { + try { + if (mGuard != null) { + mGuard.warnIfOpen(); + } + close(); + } finally { + super.finalize(); + } } }