Don't send short URB packet when sending MtpDataPacket.
When sending a MtpDataPacket to a MTP device, the kernel driver splits
it into multiple URB packets so that the URB packet size does not exceed
the buffer size at the MTP device.
Previously MtpDataPacket sends its header first, then sends the
payload. It means the first URB packet only contains the header of
MtpDataPacket and the URB packet size is smaller than the maximum URB
packet size (short packet). Some MTP devices regard the short packet as
the end of the sequencail URB packets, thus the devices do not accept
the following URB packets that contain the payload.
The MTP spec says if the responder (MTP device) sends the data in a way
where the first pacekt contains only the header, the initiator (MTP
host) must send data in the same way. Otherwise the initiator must not
send a short packet in the sequencial URB packets.
The CL fixes the MTP host implementation so that it remembers how the
MTP device sends data, and uses the same way when sending data
from the host.
Bug: 31165557
Test: Manually invokes MtpDevice#sendObject
Change-Id: Ic76eb4241ed74957414aef2990be08cd77a9f5a9
(cherry picked from commit d4b4296b401162a7a42f757c96e3652b82255b13)
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
