diff options
| author | 2010-11-19 11:20:19 -0500 | |
|---|---|---|
| committer | 2010-11-19 11:30:10 -0500 | |
| commit | 954c267725d64a37655d6f3a00de6a5aa00ddaf8 (patch) | |
| tree | 9e3681b141c3bc3f6df5c93712bb56067baa06e3 | |
| parent | a8bbc11afc0f93143c1fd200108a51c95507cc43 (diff) | |
PTP: Improve performance and reliability of file importing
Now the file copy is done completely within the media process
rather than pushing data to the client via ContProvider.openFile().
File system writes are now interleaved with USB reads, which allows us
to copy the data faster and prevents the camera from timing out during transfer.
File is automatically inserted in the media provider after a successful import
and a Uri is returned to the client.
BUG: 2994234
Change-Id: Ie75c63da76f623343d3d966c6a707aa1ae871972
Signed-off-by: Mike Lockwood <lockwood@android.com>
| -rw-r--r-- | core/java/android/provider/Mtp.java | 19 | ||||
| -rw-r--r-- | media/java/android/media/MtpClient.java | 10 | ||||
| -rw-r--r-- | media/java/android/media/MtpCursor.java | 1 | ||||
| -rw-r--r-- | media/jni/android_media_MtpClient.cpp | 63 | ||||
| -rw-r--r-- | media/mtp/MtpDataPacket.cpp | 30 | ||||
| -rw-r--r-- | media/mtp/MtpDataPacket.h | 3 | ||||
| -rw-r--r-- | media/mtp/MtpDevice.cpp | 151 | ||||
| -rw-r--r-- | media/mtp/MtpDevice.h | 5 | ||||
| -rw-r--r-- | media/tests/CameraBrowser/AndroidManifest.xml | 2 | ||||
| -rw-r--r-- | media/tests/CameraBrowser/src/com/android/camerabrowser/ObjectViewer.java | 88 | ||||
| -rw-r--r-- | media/tests/mtp/mtp.cpp | 24 |
11 files changed, 146 insertions, 250 deletions
diff --git a/core/java/android/provider/Mtp.java b/core/java/android/provider/Mtp.java index de161e79e313..78110efe94cc 100644 --- a/core/java/android/provider/Mtp.java +++ b/core/java/android/provider/Mtp.java @@ -113,6 +113,25 @@ public final class Mtp } /** + * Used for copying files from device to host. + * Constructs a Uri based on the ID of the device and object for the source file, + * and the path for the destination file. + * When passed to the ContentProvider.insert() method, the file will be transferred + * to the specified destination directory and insert() will return a content Uri + * for the new file in the MediaProvider. + * ContentProvider.insert() will throw IllegalArgumentException if the destination + * path is not in the external storage or internal media directory. + */ + public static Uri getContentUriForImport(int deviceID, long objectID, String destPath) { + if (destPath.length() == 0 || destPath.charAt(0) != '/') { + throw new IllegalArgumentException( + "destPath must be a full path in getContentUriForImport"); + } + return Uri.parse(CONTENT_AUTHORITY_DEVICE_SLASH + deviceID + + "/import/" + objectID + "?" + destPath); + } + + /** * The following columns correspond to the fields in the ObjectInfo dataset * as described in the MTP specification. */ diff --git a/media/java/android/media/MtpClient.java b/media/java/android/media/MtpClient.java index 98da1f687d92..dcb1983525a3 100644 --- a/media/java/android/media/MtpClient.java +++ b/media/java/android/media/MtpClient.java @@ -16,7 +16,6 @@ package android.media; -import android.os.ParcelFileDescriptor; import android.util.Log; /** @@ -69,9 +68,10 @@ public class MtpClient { return native_get_storage_id(deviceID, objectID); } - // create a file descriptor for reading the contents of an object over MTP - public ParcelFileDescriptor openFile(int deviceID, long objectID) { - return native_open_file(deviceID, objectID); + // Reads a file from device to host to the specified destination. + // Returns true if the transfer succeeds. + public boolean importFile(int deviceID, long objectID, String destPath) { + return native_import_file(deviceID, objectID, destPath); } public interface Listener { @@ -104,5 +104,5 @@ public class MtpClient { private native boolean native_delete_object(int deviceID, long objectID); private native long native_get_parent(int deviceID, long objectID); private native long native_get_storage_id(int deviceID, long objectID); - private native ParcelFileDescriptor native_open_file(int deviceID, long objectID); + private native boolean native_import_file(int deviceID, long objectID, String destPath); } diff --git a/media/java/android/media/MtpCursor.java b/media/java/android/media/MtpCursor.java index 9b5ab95cd513..daa3f4d5486a 100644 --- a/media/java/android/media/MtpCursor.java +++ b/media/java/android/media/MtpCursor.java @@ -40,6 +40,7 @@ public final class MtpCursor extends AbstractWindowedCursor { public static final int OBJECT_ID = 6; public static final int STORAGE_CHILDREN = 7; public static final int OBJECT_CHILDREN = 8; + public static final int OBJECT_IMPORT = 9; /** The names of the columns in the projection */ private String[] mColumns; diff --git a/media/jni/android_media_MtpClient.cpp b/media/jni/android_media_MtpClient.cpp index d23185b647b9..6235fc04e1d1 100644 --- a/media/jni/android_media_MtpClient.cpp +++ b/media/jni/android_media_MtpClient.cpp @@ -39,19 +39,6 @@ static jmethodID method_deviceAdded; static jmethodID method_deviceRemoved; static jfieldID field_context; -static struct file_descriptor_offsets_t -{ - jclass mClass; - jmethodID mConstructor; - jfieldID mDescriptor; -} gFileDescriptorOffsets; - -static struct parcel_file_descriptor_offsets_t -{ - jclass mClass; - jmethodID mConstructor; -} gParcelFileDescriptorOffsets; - #ifdef HAVE_ANDROID_OS static void checkAndClearExceptionFromCallback(JNIEnv* env, const char* methodName) { @@ -201,34 +188,19 @@ android_media_MtpClient_get_storage_id(JNIEnv *env, jobject thiz, return -1; } -static jobject -android_media_MtpClient_open_file(JNIEnv *env, jobject thiz, - jint device_id, jlong object_id) +static jboolean +android_media_MtpClient_import_file(JNIEnv *env, jobject thiz, + jint device_id, jlong object_id, jstring dest_path) { #ifdef HAVE_ANDROID_OS MyClient *client = (MyClient *)env->GetIntField(thiz, field_context); MtpDevice* device = client->getDevice(device_id); - if (!device) - return NULL; - - MtpObjectInfo* info = device->getObjectInfo(object_id); - if (!info) - return NULL; - int object_size = info->mCompressedSize; - delete info; - int fd = device->readObject(object_id, object_size); - if (fd < 0) - return NULL; - - jobject fileDescriptor = env->NewObject(gFileDescriptorOffsets.mClass, - gFileDescriptorOffsets.mConstructor); - if (fileDescriptor != NULL) { - env->SetIntField(fileDescriptor, gFileDescriptorOffsets.mDescriptor, fd); - } else { - return NULL; + if (device) { + const char *destPathStr = env->GetStringUTFChars(dest_path, NULL); + bool result = device->readObject(object_id, destPathStr); + env->ReleaseStringUTFChars(dest_path, destPathStr); + return result; } - return env->NewObject(gParcelFileDescriptorOffsets.mClass, - gParcelFileDescriptorOffsets.mConstructor, fileDescriptor); #endif return NULL; } @@ -243,8 +215,8 @@ static JNINativeMethod gMethods[] = { {"native_delete_object", "(IJ)Z", (void *)android_media_MtpClient_delete_object}, {"native_get_parent", "(IJ)J", (void *)android_media_MtpClient_get_parent}, {"native_get_storage_id", "(IJ)J", (void *)android_media_MtpClient_get_storage_id}, - {"native_open_file", "(IJ)Landroid/os/ParcelFileDescriptor;", - (void *)android_media_MtpClient_open_file}, + {"native_import_file", "(IJLjava/lang/String;)Z", + (void *)android_media_MtpClient_import_file}, }; static const char* const kClassPathName = "android/media/MtpClient"; @@ -276,21 +248,6 @@ int register_android_media_MtpClient(JNIEnv *env) return -1; } - clazz = env->FindClass("java/io/FileDescriptor"); - LOG_FATAL_IF(clazz == NULL, "Unable to find class java.io.FileDescriptor"); - gFileDescriptorOffsets.mClass = (jclass) env->NewGlobalRef(clazz); - gFileDescriptorOffsets.mConstructor = env->GetMethodID(clazz, "<init>", "()V"); - gFileDescriptorOffsets.mDescriptor = env->GetFieldID(clazz, "descriptor", "I"); - LOG_FATAL_IF(gFileDescriptorOffsets.mDescriptor == NULL, - "Unable to find descriptor field in java.io.FileDescriptor"); - - clazz = env->FindClass("android/os/ParcelFileDescriptor"); - LOG_FATAL_IF(clazz == NULL, "Unable to find class android.os.ParcelFileDescriptor"); - gParcelFileDescriptorOffsets.mClass = (jclass) env->NewGlobalRef(clazz); - gParcelFileDescriptorOffsets.mConstructor = env->GetMethodID(clazz, "<init>", "(Ljava/io/FileDescriptor;)V"); - LOG_FATAL_IF(gParcelFileDescriptorOffsets.mConstructor == NULL, - "Unable to find constructor for android.os.ParcelFileDescriptor"); - return AndroidRuntime::registerNativeMethods(env, "android/media/MtpClient", gMethods, NELEM(gMethods)); } diff --git a/media/mtp/MtpDataPacket.cpp b/media/mtp/MtpDataPacket.cpp index 20dd94d0e922..e1d1a928a284 100644 --- a/media/mtp/MtpDataPacket.cpp +++ b/media/mtp/MtpDataPacket.cpp @@ -413,20 +413,32 @@ int MtpDataPacket::read(struct usb_endpoint *ep) { } int MtpDataPacket::readData(struct usb_endpoint *ep, void* buffer, int length) { - int packetSize = usb_endpoint_max_packet(ep); int read = 0; while (read < length) { - int ret = transfer(ep, (char *)buffer + read, packetSize); + int ret = transfer(ep, (char *)buffer + read, length - read); if (ret < 0) { -printf("MtpDataPacket::readData returning %d\n", ret); return ret; } read += ret; } -printf("MtpDataPacket::readData returning %d\n", read); return read; } +// Queue a read request. Call readDataWait to wait for result +int MtpDataPacket::readDataAsync(struct usb_endpoint *ep, void* buffer, int length) { + if (usb_endpoint_queue(ep, buffer, length)) { + LOGE("usb_endpoint_queue failed, errno: %d", errno); + return -1; + } + return 0; +} + +// Wait for result of readDataAsync +int MtpDataPacket::readDataWait(struct usb_endpoint *ep) { + int ep_num; + return usb_endpoint_wait(usb_endpoint_get_device(ep), &ep_num); +} + int MtpDataPacket::readDataHeader(struct usb_endpoint *ep) { int length = transfer(ep, mBuffer, usb_endpoint_max_packet(ep)); if (length >= 0) @@ -454,15 +466,7 @@ int MtpDataPacket::write(struct usb_endpoint *ep) { } int MtpDataPacket::write(struct usb_endpoint *ep, void* buffer, uint32_t length) { - int ret = 0; - int packetSize = usb_endpoint_max_packet(ep); - while (length > 0) { - int write = (length > packetSize ? packetSize : length); - int ret = transfer(ep, buffer, write); - if (ret < 0) - break; - length -= ret; - } + int ret = transfer(ep, buffer, length); return (ret < 0 ? ret : 0); } diff --git a/media/mtp/MtpDataPacket.h b/media/mtp/MtpDataPacket.h index fab6a072aa82..3ae622630283 100644 --- a/media/mtp/MtpDataPacket.h +++ b/media/mtp/MtpDataPacket.h @@ -102,6 +102,8 @@ public: #ifdef MTP_HOST int read(struct usb_endpoint *ep); int readData(struct usb_endpoint *ep, void* buffer, int length); + int readDataAsync(struct usb_endpoint *ep, void* buffer, int length); + int readDataWait(struct usb_endpoint *ep); int readDataHeader(struct usb_endpoint *ep); int writeDataHeader(struct usb_endpoint *ep, uint32_t length); @@ -110,6 +112,7 @@ public: #endif inline bool hasData() const { return mPacketSize > MTP_CONTAINER_HEADER_SIZE; } + inline uint32_t getContainerLength() const { return MtpPacket::getUInt32(MTP_CONTAINER_LENGTH_OFFSET); } void* getData(int& outLength) const; }; diff --git a/media/mtp/MtpDevice.cpp b/media/mtp/MtpDevice.cpp index fca014286e48..1b23a8ee65dd 100644 --- a/media/mtp/MtpDevice.cpp +++ b/media/mtp/MtpDevice.cpp @@ -348,97 +348,90 @@ MtpProperty* MtpDevice::getDevicePropDesc(MtpDeviceProperty code) { return NULL; } -class ReadObjectThread : public Thread { -private: - MtpDevice* mDevice; - MtpObjectHandle mHandle; - int mObjectSize; - void* mInitialData; - int mInitialDataLength; - int mFD; - -public: - ReadObjectThread(MtpDevice* device, MtpObjectHandle handle, int objectSize) - : mDevice(device), - mHandle(handle), - mObjectSize(objectSize), - mInitialData(NULL), - mInitialDataLength(0) - { +// reads the object's data and writes it to the specified file path +bool MtpDevice::readObject(MtpObjectHandle handle, const char* destPath) { + LOGD("readObject: %s", destPath); + int fd = ::open(destPath, O_RDWR | O_CREAT | O_TRUNC); + if (fd < 0) { + LOGE("open failed for %s", destPath); + return false; } - virtual ~ReadObjectThread() { - if (mFD >= 0) - close(mFD); - free(mInitialData); - } + Mutex::Autolock autoLock(mMutex); + bool result = false; - // returns file descriptor - int init() { - mDevice->mRequest.reset(); - mDevice->mRequest.setParameter(1, mHandle); - if (mDevice->sendRequest(MTP_OPERATION_GET_OBJECT) - && mDevice->mData.readDataHeader(mDevice->mEndpointIn)) { - - // mData will contain header and possibly the beginning of the object data - mInitialData = mDevice->mData.getData(mInitialDataLength); - - // create a pipe for the client to read from - int pipefd[2]; - if (pipe(pipefd) < 0) { - LOGE("pipe failed (%s)", strerror(errno)); - return -1; + mRequest.reset(); + mRequest.setParameter(1, handle); + if (sendRequest(MTP_OPERATION_GET_OBJECT) + && mData.readDataHeader(mEndpointIn)) { + uint32_t length = mData.getContainerLength(); + if (length < MTP_CONTAINER_HEADER_SIZE) + goto fail; + length -= MTP_CONTAINER_HEADER_SIZE; + uint32_t remaining = length; + + int initialDataLength = 0; + void* initialData = mData.getData(initialDataLength); + if (initialData) { + if (initialDataLength > 0) { + if (write(fd, initialData, initialDataLength) != initialDataLength) + goto fail; + remaining -= initialDataLength; } - - mFD = pipefd[1]; - return pipefd[0]; - } else { - return -1; - } - } - - virtual bool threadLoop() { - int remaining = mObjectSize; - if (mInitialData) { - write(mFD, mInitialData, mInitialDataLength); - remaining -= mInitialDataLength; - free(mInitialData); - mInitialData = NULL; + free(initialData); } - char buffer[16384]; - while (remaining > 0) { - int readSize = (remaining > sizeof(buffer) ? sizeof(buffer) : remaining); - int count = mDevice->mData.readData(mDevice->mEndpointIn, buffer, readSize); - int written; - if (count >= 0) { - int written = write(mFD, buffer, count); - // FIXME check error - remaining -= count; + // USB reads greater than 16K don't work + char buffer1[16384], buffer2[16384]; + char* readBuffer = buffer1; + char* writeBuffer = NULL; + int writeLength = 0; + + while (remaining > 0 || writeBuffer) { + if (remaining > 0) { + // queue up a read request + int readSize = (remaining > sizeof(buffer1) ? sizeof(buffer1) : remaining); + if (mData.readDataAsync(mEndpointIn, readBuffer, readSize)) { + LOGE("readDataAsync failed"); + goto fail; + } } else { - break; + readBuffer = NULL; } - } - MtpResponseCode ret = mDevice->readResponse(); - mDevice->mMutex.unlock(); - return false; - } -}; + if (writeBuffer) { + // write previous buffer + if (write(fd, writeBuffer, writeLength) != writeLength) { + LOGE("write failed"); + // wait for pending read before failing + if (readBuffer) + mData.readDataWait(mEndpointIn); + goto fail; + } + writeBuffer = NULL; + } - // returns the file descriptor for a pipe to read the object's data -int MtpDevice::readObject(MtpObjectHandle handle, int objectSize) { - mMutex.lock(); + // wait for read to complete + if (readBuffer) { + int read = mData.readDataWait(mEndpointIn); + if (read < 0) + goto fail; - ReadObjectThread* thread = new ReadObjectThread(this, handle, objectSize); - int fd = thread->init(); - if (fd < 0) { - delete thread; - mMutex.unlock(); - } else { - thread->run("ReadObjectThread"); + writeBuffer = readBuffer; + writeLength = read; + remaining -= read; + readBuffer = (readBuffer == buffer1 ? buffer2 : buffer1); + } + } + + MtpResponseCode response = readResponse(); + if (response == MTP_RESPONSE_OK) + result = true; } - return fd; + +fail: + ::close(fd); + return result; } bool MtpDevice::sendRequest(MtpOperationCode operation) { diff --git a/media/mtp/MtpDevice.h b/media/mtp/MtpDevice.h index 57f492f06a5f..720502c80b5b 100644 --- a/media/mtp/MtpDevice.h +++ b/media/mtp/MtpDevice.h @@ -86,12 +86,9 @@ public: MtpProperty* getDevicePropDesc(MtpDeviceProperty code); - // returns the file descriptor for a pipe to read the object's data - int readObject(MtpObjectHandle handle, int objectSize); + bool readObject(MtpObjectHandle handle, const char* destPath); private: - friend class ReadObjectThread; - bool sendRequest(MtpOperationCode operation); bool sendData(); bool readData(); diff --git a/media/tests/CameraBrowser/AndroidManifest.xml b/media/tests/CameraBrowser/AndroidManifest.xml index eae0b015b916..db4a336972c1 100644 --- a/media/tests/CameraBrowser/AndroidManifest.xml +++ b/media/tests/CameraBrowser/AndroidManifest.xml @@ -1,8 +1,6 @@ <manifest xmlns:android="http://schemas.android.com/apk/res/android" package="com.android.camerabrowser"> - <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" /> - <application android:label="@string/app_label"> <activity android:name="CameraBrowser" android:label="Camera Browser"> <intent-filter> diff --git a/media/tests/CameraBrowser/src/com/android/camerabrowser/ObjectViewer.java b/media/tests/CameraBrowser/src/com/android/camerabrowser/ObjectViewer.java index 9f2f98e388cf..2373ee6cc7e9 100644 --- a/media/tests/CameraBrowser/src/com/android/camerabrowser/ObjectViewer.java +++ b/media/tests/CameraBrowser/src/com/android/camerabrowser/ObjectViewer.java @@ -16,6 +16,7 @@ package com.android.camerabrowser; import android.app.Activity; +import android.content.ContentValues; import android.content.Intent; import android.database.Cursor; import android.graphics.Bitmap; @@ -23,9 +24,6 @@ import android.graphics.BitmapFactory; import android.net.Uri; import android.os.Bundle; import android.os.Environment; -import android.os.FileUtils; -import android.os.ParcelFileDescriptor; -import android.os.Process; import android.provider.Mtp; import android.util.Log; import android.view.Menu; @@ -37,9 +35,6 @@ import android.widget.TextView; import android.widget.Toast; import java.io.File; -import java.io.FileDescriptor; -import java.io.FileInputStream; -import java.io.FileOutputStream; import java.util.Calendar; import java.util.Date; @@ -53,6 +48,7 @@ public class ObjectViewer extends Activity { private int mDeviceID; private long mStorageID; private long mObjectID; + private String mFileName; private static final String[] OBJECT_COLUMNS = new String[] { Mtp.Object._ID, @@ -93,7 +89,8 @@ public class ObjectViewer extends Activity { OBJECT_COLUMNS, null, null, null); c.moveToFirst(); TextView view = (TextView)findViewById(R.id.name); - view.setText(c.getString(1)); + mFileName = c.getString(1); + view.setText(mFileName); view = (TextView)findViewById(R.id.size); view.setText(Long.toString(c.getLong(2))); view = (TextView)findViewById(R.id.thumb_width); @@ -167,71 +164,18 @@ public class ObjectViewer extends Activity { } private void save() { - boolean success = false; - Uri uri = Mtp.Object.getContentUri(mDeviceID, mObjectID); - File destFile = null; - ParcelFileDescriptor pfd = null; - FileInputStream fis = null; - FileOutputStream fos = null; - - try { - pfd = getContentResolver().openFileDescriptor(uri, "r"); - Log.d(TAG, "save got pfd " + pfd); - if (pfd != null) { - fis = new FileInputStream(pfd.getFileDescriptor()); - Log.d(TAG, "save got fis " + fis); - File destDir = Environment.getExternalStoragePublicDirectory( - Environment.DIRECTORY_DCIM); - destDir.mkdirs(); - destFile = new File(destDir, "CameraBrowser-" + getTimestamp() + ".jpeg"); - - - Log.d(TAG, "save got destFile " + destFile); - - if (destFile.exists()) { - destFile.delete(); - } - fos = new FileOutputStream(destFile); - - byte[] buffer = new byte[65536]; - int bytesRead; - while ((bytesRead = fis.read(buffer)) >= 0) { - fos.write(buffer, 0, bytesRead); - } - - // temporary workaround until we straighten out permissions in /data/media - FileUtils.setPermissions(destDir.getPath(), 0775, Process.myUid(), Process.SDCARD_RW_GID); - FileUtils.setPermissions(destFile.getPath(), 0664, Process.myUid(), Process.SDCARD_RW_GID); - - success = true; - } - } catch (Exception e) { - Log.e(TAG, "Exception in ObjectView.save", e); - } finally { - if (fis != null) { - try { - fis.close(); - } catch (Exception e) { - } - } - if (fos != null) { - try { - fos.close(); - } catch (Exception e) { - } - } - if (pfd != null) { - try { - pfd.close(); - } catch (Exception e) { - } - } - } - - if (success) { - Intent intent = new Intent(Intent.ACTION_MEDIA_SCANNER_SCAN_FILE); - intent.setData(Uri.fromFile(destFile)); - sendBroadcast(intent); + // copy file to /mnt/sdcard/imported/<filename> + File dest = Environment.getExternalStorageDirectory(); + dest = new File(dest, "imported"); + dest.mkdirs(); + dest = new File(dest, mFileName); + + Uri requestUri = Mtp.Object.getContentUriForImport(mDeviceID, mObjectID, + dest.getAbsolutePath()); + Uri resultUri = getContentResolver().insert(requestUri, new ContentValues()); + Log.d(TAG, "save returned " + resultUri); + + if (resultUri != null) { Toast.makeText(this, R.string.object_saved_message, Toast.LENGTH_SHORT).show(); } else { Toast.makeText(this, R.string.save_failed_message, Toast.LENGTH_SHORT).show(); diff --git a/media/tests/mtp/mtp.cpp b/media/tests/mtp/mtp.cpp index 3202cae65e17..9732944b7ed7 100644 --- a/media/tests/mtp/mtp.cpp +++ b/media/tests/mtp/mtp.cpp @@ -158,32 +158,12 @@ static int get_file(int argc, char* argv[]) { } dest = (argc > 1 ? argv[1] : info->mName); - destFD = open(dest, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); - if (destFD < 0) { - fprintf(stderr, "could not create %s\n", dest); - goto fail; - } - srcFD = srcFile->getDevice()->readObject(info->mHandle, info->mCompressedSize); - if (srcFD < 0) - goto fail; - - char buffer[65536]; - while (1) { - int count = read(srcFD, buffer, sizeof(buffer)); - if (count <= 0) - break; - write(destFD, buffer, count); - } - // FIXME - error checking and reporting - ret = 0; + if (srcFile->getDevice()->readObject(info->mHandle, dest)) + ret = 0; fail: delete srcFile; delete info; - if (srcFD >= 0) - close(srcFD); - if (destFD >= 0) - close(destFD); return ret; } |