Merge "PTP: Improve performance and reliability of file importing"
diff --git a/core/java/android/provider/Mtp.java b/core/java/android/provider/Mtp.java
index de161e7..78110ef 100644
--- a/core/java/android/provider/Mtp.java
+++ b/core/java/android/provider/Mtp.java
@@ -113,6 +113,25 @@
}
/**
+ * 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 98da1f6..dcb1983 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 @@
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 @@
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 9b5ab95..daa3f4d 100644
--- a/media/java/android/media/MtpCursor.java
+++ b/media/java/android/media/MtpCursor.java
@@ -40,6 +40,7 @@
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 d23185b..6235fc0 100644
--- a/media/jni/android_media_MtpClient.cpp
+++ b/media/jni/android_media_MtpClient.cpp
@@ -39,19 +39,6 @@
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 @@
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 @@
{"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 @@
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 20dd94d..e1d1a92 100644
--- a/media/mtp/MtpDataPacket.cpp
+++ b/media/mtp/MtpDataPacket.cpp
@@ -413,20 +413,32 @@
}
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, 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 fab6a07..3ae6226 100644
--- a/media/mtp/MtpDataPacket.h
+++ b/media/mtp/MtpDataPacket.h
@@ -102,6 +102,8 @@
#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 @@
#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 fca0142..1b23a8e 100644
--- a/media/mtp/MtpDevice.cpp
+++ b/media/mtp/MtpDevice.cpp
@@ -348,97 +348,90 @@
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)
- {
- }
-
- virtual ~ReadObjectThread() {
- if (mFD >= 0)
- close(mFD);
- free(mInitialData);
- }
-
- // 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;
- }
-
- 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;
- }
-
- 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;
- } else {
- break;
- }
- }
-
- MtpResponseCode ret = mDevice->readResponse();
- mDevice->mMutex.unlock();
+// 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;
}
-};
- // returns the file descriptor for a pipe to read the object's data
-int MtpDevice::readObject(MtpObjectHandle handle, int objectSize) {
- mMutex.lock();
+ Mutex::Autolock autoLock(mMutex);
+ bool result = false;
- ReadObjectThread* thread = new ReadObjectThread(this, handle, objectSize);
- int fd = thread->init();
- if (fd < 0) {
- delete thread;
- mMutex.unlock();
- } else {
- thread->run("ReadObjectThread");
+ 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;
+ }
+ free(initialData);
+ }
+
+ // 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 {
+ readBuffer = NULL;
+ }
+
+ 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;
+ }
+
+ // wait for read to complete
+ if (readBuffer) {
+ int read = mData.readDataWait(mEndpointIn);
+ if (read < 0)
+ goto fail;
+
+ 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 57f492f..720502c 100644
--- a/media/mtp/MtpDevice.h
+++ b/media/mtp/MtpDevice.h
@@ -86,12 +86,9 @@
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 eae0b01..db4a336 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 9f2f98e..2373ee6 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.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.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 @@
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 @@
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 @@
}
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;
+ // copy file to /mnt/sdcard/imported/<filename>
+ File dest = Environment.getExternalStorageDirectory();
+ dest = new File(dest, "imported");
+ dest.mkdirs();
+ dest = new File(dest, mFileName);
- 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");
+ Uri requestUri = Mtp.Object.getContentUriForImport(mDeviceID, mObjectID,
+ dest.getAbsolutePath());
+ Uri resultUri = getContentResolver().insert(requestUri, new ContentValues());
+ Log.d(TAG, "save returned " + resultUri);
-
- 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);
+ 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 3202cae..9732944 100644
--- a/media/tests/mtp/mtp.cpp
+++ b/media/tests/mtp/mtp.cpp
@@ -158,32 +158,12 @@
}
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;
}