Merge "Don't send short URB packet when sending MtpDataPacket." into nyc-mr2-dev
diff --git a/media/mtp/MtpDataPacket.cpp b/media/mtp/MtpDataPacket.cpp
index 0381edf..0e9bc34 100644
--- a/media/mtp/MtpDataPacket.cpp
+++ b/media/mtp/MtpDataPacket.cpp
@@ -16,17 +16,38 @@
#define LOG_TAG "MtpDataPacket"
+#include "MtpDataPacket.h"
+
+#include <algorithm>
+#include <fcntl.h>
#include <stdio.h>
#include <sys/types.h>
-#include <fcntl.h>
-
#include <usbhost/usbhost.h>
-
-#include "MtpDataPacket.h"
#include "MtpStringBuffer.h"
namespace android {
+namespace {
+// Reads the exact |count| bytes from |fd| to |buf|.
+// Returns |count| if it succeed to read the bytes. Otherwise returns -1. If it reaches EOF, the
+// function regards it as an error.
+ssize_t readExactBytes(int fd, void* buf, size_t count) {
+ if (count > SSIZE_MAX) {
+ return -1;
+ }
+ size_t read_count = 0;
+ while (read_count < count) {
+ int result = read(fd, static_cast<int8_t*>(buf) + read_count, count - read_count);
+ // Assume that EOF is error.
+ if (result <= 0) {
+ return -1;
+ }
+ read_count += result;
+ }
+ return read_count == count ? count : -1;
+}
+} // namespace
+
MtpDataPacket::MtpDataPacket()
: MtpPacket(MTP_BUFFER_SIZE), // MAX_USBFS_BUFFER_SIZE
mOffset(MTP_CONTAINER_HEADER_SIZE)
@@ -511,29 +532,104 @@
return length;
}
-int MtpDataPacket::writeDataHeader(struct usb_request *request, uint32_t length) {
- MtpPacket::putUInt32(MTP_CONTAINER_LENGTH_OFFSET, length);
- MtpPacket::putUInt16(MTP_CONTAINER_TYPE_OFFSET, MTP_CONTAINER_TYPE_DATA);
- request->buffer = mBuffer;
- request->buffer_length = MTP_CONTAINER_HEADER_SIZE;
- int ret = transfer(request);
- return (ret < 0 ? ret : 0);
-}
+int MtpDataPacket::write(struct usb_request *request, UrbPacketDivisionMode divisionMode) {
+ if (mPacketSize < MTP_CONTAINER_HEADER_SIZE || mPacketSize > MTP_BUFFER_SIZE) {
+ ALOGE("Illegal packet size.");
+ return -1;
+ }
-int MtpDataPacket::write(struct usb_request *request) {
MtpPacket::putUInt32(MTP_CONTAINER_LENGTH_OFFSET, mPacketSize);
MtpPacket::putUInt16(MTP_CONTAINER_TYPE_OFFSET, MTP_CONTAINER_TYPE_DATA);
- request->buffer = mBuffer;
- request->buffer_length = mPacketSize;
- int ret = transfer(request);
- return (ret < 0 ? ret : 0);
+
+ size_t processedBytes = 0;
+ while (processedBytes < mPacketSize) {
+ const size_t write_size =
+ processedBytes == 0 && divisionMode == FIRST_PACKET_ONLY_HEADER ?
+ MTP_CONTAINER_HEADER_SIZE : mPacketSize - processedBytes;
+ request->buffer = mBuffer + processedBytes;
+ request->buffer_length = write_size;
+ const int result = transfer(request);
+ if (result < 0) {
+ ALOGE("Failed to write bytes to the device.");
+ return -1;
+ }
+ processedBytes += result;
+ }
+
+ return processedBytes == mPacketSize ? processedBytes : -1;
}
-int MtpDataPacket::write(struct usb_request *request, void* buffer, uint32_t length) {
- request->buffer = buffer;
- request->buffer_length = length;
- int ret = transfer(request);
- return (ret < 0 ? ret : 0);
+int MtpDataPacket::write(struct usb_request *request,
+ UrbPacketDivisionMode divisionMode,
+ int fd,
+ size_t payloadSize) {
+ // Obtain the greatest multiple of minimum packet size that is not greater than
+ // MTP_BUFFER_SIZE.
+ if (request->max_packet_size <= 0) {
+ ALOGE("Cannot determine bulk transfer size due to illegal max packet size %d.",
+ request->max_packet_size);
+ return -1;
+ }
+ const size_t maxBulkTransferSize =
+ MTP_BUFFER_SIZE - (MTP_BUFFER_SIZE % request->max_packet_size);
+ const size_t containerLength = payloadSize + MTP_CONTAINER_HEADER_SIZE;
+ size_t processedBytes = 0;
+ bool readError = false;
+
+ // Bind the packet with given request.
+ request->buffer = mBuffer;
+ allocate(maxBulkTransferSize);
+
+ while (processedBytes < containerLength) {
+ size_t bulkTransferSize = 0;
+
+ // prepare header.
+ const bool headerSent = processedBytes != 0;
+ if (!headerSent) {
+ MtpPacket::putUInt32(MTP_CONTAINER_LENGTH_OFFSET, containerLength);
+ MtpPacket::putUInt16(MTP_CONTAINER_TYPE_OFFSET, MTP_CONTAINER_TYPE_DATA);
+ bulkTransferSize += MTP_CONTAINER_HEADER_SIZE;
+ }
+
+ // Prepare payload.
+ if (headerSent || divisionMode == FIRST_PACKET_HAS_PAYLOAD) {
+ const size_t processedPayloadBytes =
+ headerSent ? processedBytes - MTP_CONTAINER_HEADER_SIZE : 0;
+ const size_t maxRead = payloadSize - processedPayloadBytes;
+ const size_t maxWrite = maxBulkTransferSize - bulkTransferSize;
+ const size_t bulkTransferPayloadSize = std::min(maxRead, maxWrite);
+ // prepare payload.
+ if (!readError) {
+ const ssize_t result = readExactBytes(
+ fd,
+ mBuffer + bulkTransferSize,
+ bulkTransferPayloadSize);
+ if (result < 0) {
+ ALOGE("Found an error while reading data from FD. Send 0 data instead.");
+ readError = true;
+ }
+ }
+ if (readError) {
+ memset(mBuffer + bulkTransferSize, 0, bulkTransferPayloadSize);
+ }
+ bulkTransferSize += bulkTransferPayloadSize;
+ }
+
+ // Bulk transfer.
+ mPacketSize = bulkTransferSize;
+ request->buffer_length = bulkTransferSize;
+ const int result = transfer(request);
+ if (result != static_cast<ssize_t>(bulkTransferSize)) {
+ // Cannot recover writing error.
+ ALOGE("Found an error while write data to MtpDevice.");
+ return -1;
+ }
+
+ // Update variables.
+ processedBytes += bulkTransferSize;
+ }
+
+ return readError ? -1 : processedBytes;
}
#endif // MTP_HOST
diff --git a/media/mtp/MtpDataPacket.h b/media/mtp/MtpDataPacket.h
index 6240f28..82e0ee4 100644
--- a/media/mtp/MtpDataPacket.h
+++ b/media/mtp/MtpDataPacket.h
@@ -93,7 +93,6 @@
inline void putEmptyString() { putUInt8(0); }
inline void putEmptyArray() { putUInt32(0); }
-
#ifdef MTP_DEVICE
// fill our buffer with data from the given file descriptor
int read(int fd);
@@ -110,9 +109,15 @@
int readDataWait(struct usb_device *device);
int readDataHeader(struct usb_request *ep);
- int writeDataHeader(struct usb_request *ep, uint32_t length);
- int write(struct usb_request *ep);
- int write(struct usb_request *ep, void* buffer, uint32_t length);
+ // Write a whole data packet with payload to the end point given by a request. |divisionMode|
+ // specifies whether to divide header and payload. See |UrbPacketDivisionMode| for meanings of
+ // each value. Return the number of bytes (including header size) sent to the device on success.
+ // Otherwise -1.
+ int write(struct usb_request *request, UrbPacketDivisionMode divisionMode);
+ // Similar to previous write method but it reads the payload from |fd|. If |size| is larger than
+ // MTP_BUFFER_SIZE, the data will be sent by multiple bulk transfer requests.
+ int write(struct usb_request *request, UrbPacketDivisionMode divisionMode,
+ int fd, size_t size);
#endif
inline bool hasData() const { return mPacketSize > MTP_CONTAINER_HEADER_SIZE; }
diff --git a/media/mtp/MtpDevice.cpp b/media/mtp/MtpDevice.cpp
index 7301193..e3d2ec6 100644
--- a/media/mtp/MtpDevice.cpp
+++ b/media/mtp/MtpDevice.cpp
@@ -222,7 +222,8 @@
mProcessingEvent(false),
mCurrentEventHandle(0),
mLastSendObjectInfoTransactionID(0),
- mLastSendObjectInfoObjectHandle(0)
+ mLastSendObjectInfoObjectHandle(0),
+ mPacketDivisionMode(FIRST_PACKET_HAS_PAYLOAD)
{
mRequestIn1 = usb_request_new(device, ep_in);
mRequestIn2 = usb_request_new(device, ep_in);
@@ -512,30 +513,15 @@
return false;
}
- int remaining = size;
mRequest.reset();
- bool error = false;
if (sendRequest(MTP_OPERATION_SEND_OBJECT)) {
- // send data header
- writeDataHeader(MTP_OPERATION_SEND_OBJECT, remaining);
-
- // USB writes greater than 16K don't work
- char buffer[MTP_BUFFER_SIZE];
- while (remaining > 0) {
- int count = read(srcFD, buffer, sizeof(buffer));
- if (count > 0) {
- if (mData.write(mRequestOut, buffer, count) < 0) {
- error = true;
- }
- // FIXME check error
- remaining -= count;
- } else {
- break;
- }
- }
+ mData.setOperationCode(mRequest.getOperationCode());
+ mData.setTransactionID(mRequest.getTransactionID());
+ const int writeResult = mData.write(mRequestOut, mPacketDivisionMode, srcFD, size);
+ const MtpResponseCode ret = readResponse();
+ return ret == MTP_RESPONSE_OK && writeResult > 0;
}
- MtpResponseCode ret = readResponse();
- return (remaining == 0 && ret == MTP_RESPONSE_OK && !error);
+ return false;
}
bool MtpDevice::deleteObject(MtpObjectHandle handle) {
@@ -707,8 +693,8 @@
return false;
}
- // If object size 0 byte, the remote device can reply response packet
- // without sending any data packets.
+ // If object size 0 byte, the remote device may reply a response packet without sending any data
+ // packets.
if (mData.getContainerType() == MTP_CONTAINER_TYPE_RESPONSE) {
mResponse.copyFrom(mData);
return mResponse.getResponseCode() == MTP_RESPONSE_OK;
@@ -731,6 +717,14 @@
{
int initialDataLength = 0;
void* const initialData = mData.getData(&initialDataLength);
+ if (fullLength > MTP_CONTAINER_HEADER_SIZE && initialDataLength == 0) {
+ // According to the MTP spec, the responder (MTP device) can choose two ways of sending
+ // data. a) The first packet contains the head and as much of the payload as possible
+ // b) The first packet contains only the header. The initiator (MTP host) needs
+ // to remember which way the responder used, and send upcoming data in the same way.
+ ALOGD("Found short packet that contains only a header.");
+ mPacketDivisionMode = FIRST_PACKET_ONLY_HEADER;
+ }
if (initialData) {
if (initialDataLength > 0) {
if (!callback(initialData, offset, initialDataLength, clientData)) {
@@ -854,7 +848,7 @@
ALOGV("sendData\n");
mData.setOperationCode(mRequest.getOperationCode());
mData.setTransactionID(mRequest.getTransactionID());
- int ret = mData.write(mRequestOut);
+ int ret = mData.write(mRequestOut, mPacketDivisionMode);
mData.dump();
return (ret >= 0);
}
@@ -881,12 +875,6 @@
}
}
-bool MtpDevice::writeDataHeader(MtpOperationCode operation, int dataLength) {
- mData.setOperationCode(operation);
- mData.setTransactionID(mRequest.getTransactionID());
- return (!mData.writeDataHeader(mRequestOut, dataLength));
-}
-
MtpResponseCode MtpDevice::readResponse() {
ALOGV("readResponse\n");
if (mReceivedResponse) {
diff --git a/media/mtp/MtpDevice.h b/media/mtp/MtpDevice.h
index b65e098..c84c842 100644
--- a/media/mtp/MtpDevice.h
+++ b/media/mtp/MtpDevice.h
@@ -71,6 +71,9 @@
Mutex mEventMutex;
Mutex mEventMutexForInterrupt;
+ // Remember the device's packet division mode.
+ UrbPacketDivisionMode mPacketDivisionMode;
+
public:
typedef bool (*ReadObjectCallback)
(void* data, uint32_t offset, uint32_t length, void* clientData);
diff --git a/media/mtp/MtpTypes.h b/media/mtp/MtpTypes.h
index 720c854..c749c66 100644
--- a/media/mtp/MtpTypes.h
+++ b/media/mtp/MtpTypes.h
@@ -73,6 +73,13 @@
typedef String8 MtpString;
+enum UrbPacketDivisionMode {
+ // First packet only contains a header.
+ FIRST_PACKET_ONLY_HEADER,
+ // First packet contains payload much as possible.
+ FIRST_PACKET_HAS_PAYLOAD
+};
+
}; // namespace android
#endif // _MTP_TYPES_H