From ef441d965504dbf31c5db690e5b34fcdcecd92ff Mon Sep 17 00:00:00 2001 From: Mike Lockwood Date: Thu, 14 Jul 2011 21:00:02 -0400 Subject: MTP: Use a single packet for the data phase instead of sending 12 byte header in a separate packet. PTP on the Mac is much happier with this approach. Change-Id: I7d1ca498f6346afd88876d24332187b466fc469c Signed-off-by: Mike Lockwood --- media/mtp/MtpDataPacket.cpp | 46 +++++---------------- media/mtp/MtpDataPacket.h | 3 +- media/mtp/MtpServer.cpp | 97 ++++++++++++++++++++++++++------------------- 3 files changed, 66 insertions(+), 80 deletions(-) (limited to 'media/mtp') diff --git a/media/mtp/MtpDataPacket.cpp b/media/mtp/MtpDataPacket.cpp index 817eac0..20225ba 100644 --- a/media/mtp/MtpDataPacket.cpp +++ b/media/mtp/MtpDataPacket.cpp @@ -345,56 +345,28 @@ void MtpDataPacket::putString(const uint16_t* string) { #ifdef MTP_DEVICE int MtpDataPacket::read(int fd) { - // first read the header - int ret = ::read(fd, mBuffer, MTP_CONTAINER_HEADER_SIZE); - if (ret != MTP_CONTAINER_HEADER_SIZE) - return -1; - // then the following data - int total = MtpPacket::getUInt32(MTP_CONTAINER_LENGTH_OFFSET); - allocate(total); - int remaining = total - MTP_CONTAINER_HEADER_SIZE; - ret = ::read(fd, &mBuffer[0] + MTP_CONTAINER_HEADER_SIZE, remaining); - if (ret != remaining) + int ret = ::read(fd, mBuffer, mBufferSize); + if (ret < MTP_CONTAINER_HEADER_SIZE) return -1; - - mPacketSize = total; + mPacketSize = ret; mOffset = MTP_CONTAINER_HEADER_SIZE; - return total; -} - -int MtpDataPacket::readDataHeader(int fd) { - int ret = ::read(fd, mBuffer, MTP_CONTAINER_HEADER_SIZE); - if (ret > 0) - mPacketSize = ret; - else - mPacketSize = 0; return ret; } int MtpDataPacket::write(int fd) { MtpPacket::putUInt32(MTP_CONTAINER_LENGTH_OFFSET, mPacketSize); MtpPacket::putUInt16(MTP_CONTAINER_TYPE_OFFSET, MTP_CONTAINER_TYPE_DATA); - // send header separately from data - int ret = ::write(fd, mBuffer, MTP_CONTAINER_HEADER_SIZE); - if (ret == MTP_CONTAINER_HEADER_SIZE) - ret = ::write(fd, mBuffer + MTP_CONTAINER_HEADER_SIZE, - mPacketSize - MTP_CONTAINER_HEADER_SIZE); - return (ret < 0 ? ret : 0); -} - -int MtpDataPacket::writeDataHeader(int fd, uint32_t length) { - MtpPacket::putUInt32(MTP_CONTAINER_LENGTH_OFFSET, length); - MtpPacket::putUInt16(MTP_CONTAINER_TYPE_OFFSET, MTP_CONTAINER_TYPE_DATA); - int ret = ::write(fd, mBuffer, MTP_CONTAINER_HEADER_SIZE); + int ret = ::write(fd, mBuffer, mPacketSize); return (ret < 0 ? ret : 0); } int MtpDataPacket::writeData(int fd, void* data, uint32_t length) { - MtpPacket::putUInt32(MTP_CONTAINER_LENGTH_OFFSET, length + MTP_CONTAINER_HEADER_SIZE); + allocate(length); + memcpy(mBuffer + MTP_CONTAINER_HEADER_SIZE, data, length); + length += MTP_CONTAINER_HEADER_SIZE; + MtpPacket::putUInt32(MTP_CONTAINER_LENGTH_OFFSET, length); MtpPacket::putUInt16(MTP_CONTAINER_TYPE_OFFSET, MTP_CONTAINER_TYPE_DATA); - int ret = ::write(fd, mBuffer, MTP_CONTAINER_HEADER_SIZE); - if (ret == MTP_CONTAINER_HEADER_SIZE) - ret = ::write(fd, data, length); + int ret = ::write(fd, mBuffer, length); return (ret < 0 ? ret : 0); } diff --git a/media/mtp/MtpDataPacket.h b/media/mtp/MtpDataPacket.h index 8a08948..2b81063 100644 --- a/media/mtp/MtpDataPacket.h +++ b/media/mtp/MtpDataPacket.h @@ -41,6 +41,7 @@ public: void setOperationCode(MtpOperationCode code); void setTransactionID(MtpTransactionID id); + inline const uint8_t* getData() const { return mBuffer + MTP_CONTAINER_HEADER_SIZE; } inline uint8_t getUInt8() { return (uint8_t)mBuffer[mOffset++]; } inline int8_t getInt8() { return (int8_t)mBuffer[mOffset++]; } uint16_t getUInt16(); @@ -95,11 +96,9 @@ public: #ifdef MTP_DEVICE // fill our buffer with data from the given file descriptor int read(int fd); - int readDataHeader(int fd); // write our data to the given file descriptor int write(int fd); - int writeDataHeader(int fd, uint32_t length); int writeData(int fd, void* data, uint32_t length); #endif diff --git a/media/mtp/MtpServer.cpp b/media/mtp/MtpServer.cpp index 4047e2e..a9b539b 100644 --- a/media/mtp/MtpServer.cpp +++ b/media/mtp/MtpServer.cpp @@ -731,14 +731,12 @@ MtpResponseCode MtpServer::doGetObject() { } mfr.offset = 0; mfr.length = fileLength; - - // send data header - mData.setOperationCode(mRequest.getOperationCode()); - mData.setTransactionID(mRequest.getTransactionID()); - mData.writeDataHeader(mFD, fileLength + MTP_CONTAINER_HEADER_SIZE); + mfr.command = mRequest.getOperationCode(); + mfr.transaction_id = mRequest.getTransactionID(); // then transfer the file - int ret = ioctl(mFD, MTP_SEND_FILE, (unsigned long)&mfr); + int ret = ioctl(mFD, MTP_SEND_FILE_WITH_HEADER, (unsigned long)&mfr); + LOGV("MTP_SEND_FILE_WITH_HEADER returned %d\n", ret); close(mfr.fd); if (ret < 0) { if (errno == ECANCELED) @@ -798,15 +796,13 @@ MtpResponseCode MtpServer::doGetPartialObject(MtpOperationCode operation) { } mfr.offset = offset; mfr.length = length; + mfr.command = mRequest.getOperationCode(); + mfr.transaction_id = mRequest.getTransactionID(); mResponse.setParameter(1, length); - // send data header - mData.setOperationCode(mRequest.getOperationCode()); - mData.setTransactionID(mRequest.getTransactionID()); - mData.writeDataHeader(mFD, length + MTP_CONTAINER_HEADER_SIZE); - - // then transfer the file - int ret = ioctl(mFD, MTP_SEND_FILE, (unsigned long)&mfr); + // transfer the file + int ret = ioctl(mFD, MTP_SEND_FILE_WITH_HEADER, (unsigned long)&mfr); + LOGV("MTP_SEND_FILE_WITH_HEADER returned %d\n", ret); close(mfr.fd); if (ret < 0) { if (errno == ECANCELED) @@ -918,7 +914,7 @@ MtpResponseCode MtpServer::doSendObject() { return MTP_RESPONSE_GENERAL_ERROR; MtpResponseCode result = MTP_RESPONSE_OK; mode_t mask; - int ret; + int ret, initialData; if (mSendObjectHandle == kInvalidObjectHandle) { LOGE("Expected SendObjectInfo before SendObject"); @@ -926,12 +922,13 @@ MtpResponseCode MtpServer::doSendObject() { goto done; } - // read the header - ret = mData.readDataHeader(mFD); - // FIXME - check for errors here. - - // reset so we don't attempt to send this back - mData.reset(); + // read the header, and possibly some data + ret = mData.read(mFD); + if (ret < MTP_CONTAINER_HEADER_SIZE) { + result = MTP_RESPONSE_GENERAL_ERROR; + goto done; + } + initialData = ret - MTP_CONTAINER_HEADER_SIZE; mtp_file_range mfr; mfr.fd = open(mSendObjectFilePath, O_RDWR | O_CREAT | O_TRUNC); @@ -945,15 +942,19 @@ MtpResponseCode MtpServer::doSendObject() { fchmod(mfr.fd, mFilePermission); umask(mask); - mfr.offset = 0; - mfr.length = mSendObjectFileSize; + if (initialData > 0) + ret = write(mfr.fd, mData.getData(), initialData); - LOGV("receiving %s\n", (const char *)mSendObjectFilePath); - // transfer the file - ret = ioctl(mFD, MTP_RECEIVE_FILE, (unsigned long)&mfr); - close(mfr.fd); + if (mSendObjectFileSize - initialData > 0) { + mfr.offset = initialData; + mfr.length = mSendObjectFileSize - initialData; - LOGV("MTP_RECEIVE_FILE returned %d", ret); + LOGV("receiving %s\n", (const char *)mSendObjectFilePath); + // transfer the file + ret = ioctl(mFD, MTP_RECEIVE_FILE, (unsigned long)&mfr); + LOGV("MTP_RECEIVE_FILE returned %d\n", ret); + } + close(mfr.fd); if (ret < 0) { unlink(mSendObjectFilePath); @@ -964,6 +965,9 @@ MtpResponseCode MtpServer::doSendObject() { } done: + // reset so we don't attempt to send the data back + mData.reset(); + mDatabase->endSendObject(mSendObjectFilePath, mSendObjectHandle, mSendObjectFormat, result == MTP_RESPONSE_OK); mSendObjectHandle = kInvalidObjectHandle; @@ -1096,23 +1100,31 @@ MtpResponseCode MtpServer::doSendPartialObject() { return MTP_RESPONSE_GENERAL_ERROR; } - // read the header - int ret = mData.readDataHeader(mFD); - // FIXME - check for errors here. + const char* filePath = (const char *)edit->mPath; + LOGV("receiving partial %s %lld %lld\n", filePath, offset, length); - // reset so we don't attempt to send this back - mData.reset(); + // read the header, and possibly some data + int ret = mData.read(mFD); + if (ret < MTP_CONTAINER_HEADER_SIZE) + return MTP_RESPONSE_GENERAL_ERROR; + int initialData = ret - MTP_CONTAINER_HEADER_SIZE; - const char* filePath = (const char *)edit->mPath; - LOGV("receiving partial %s %lld %ld\n", filePath, offset, length); - mtp_file_range mfr; - mfr.fd = edit->mFD; - mfr.offset = offset; - mfr.length = length; + if (initialData > 0) { + ret = write(edit->mFD, mData.getData(), initialData); + offset += initialData; + length -= initialData; + } - // transfer the file - ret = ioctl(mFD, MTP_RECEIVE_FILE, (unsigned long)&mfr); - LOGV("MTP_RECEIVE_FILE returned %d", ret); + if (length > 0) { + mtp_file_range mfr; + mfr.fd = edit->mFD; + mfr.offset = offset; + mfr.length = length; + + // transfer the file + ret = ioctl(mFD, MTP_RECEIVE_FILE, (unsigned long)&mfr); + LOGV("MTP_RECEIVE_FILE returned %d", ret); + } if (ret < 0) { mResponse.setParameter(1, 0); if (errno == ECANCELED) @@ -1120,6 +1132,9 @@ MtpResponseCode MtpServer::doSendPartialObject() { else return MTP_RESPONSE_GENERAL_ERROR; } + + // reset so we don't attempt to send this back + mData.reset(); mResponse.setParameter(1, length); uint64_t end = offset + length; if (end > edit->mSize) { -- cgit v1.1