Merge lp:~cyphermox/mtp/rename into lp:mtp

[Mathieu Trudel-Lapierre]

Miscellaneous standard file operation fixes:

- fix renaming files
- correctly list file size after an edit
- return proper file size in ObjectInfo
- implement MTP_OPERATION_MOVE
- allow sending out events for ObjectInfo and ObjectProperty changes

Miscellaneous standard file operation fixes:

- fix renaming files
- correctly list file size after an edit
- return proper file size in ObjectInfo
- implement MTP_OPERATION_MOVE
- allow sending out events for ObjectInfo and ObjectProperty changes

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:39
http://jenkins.qa.ubuntu.com/job/mtp-ci/12/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mtp-trusty-amd64-ci/2
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mtp-trusty-armhf-ci/2
        deb: http://jenkins.qa.ubuntu.com/job/mtp-trusty-armhf-ci/2/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mtp-trusty-i386-ci/2

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mtp-ci/12/rebuild

review: Approve (continuous-integration)

[lp:~cyphermox/mtp/rename]

40. By Mathieu Trudel-Lapierre on 2013-12-03

Guess object format for media players support

41. By Mathieu Trudel-Lapierre on 2013-12-03

Add playback formats supported, fix folder send.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:41
http://jenkins.qa.ubuntu.com/job/mtp-ci/13/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mtp-trusty-amd64-ci/3
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mtp-trusty-armhf-ci/3
        deb: http://jenkins.qa.ubuntu.com/job/mtp-trusty-armhf-ci/3/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mtp-trusty-i386-ci/3

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mtp-ci/13/rebuild

review: Approve (continuous-integration)

[Ricardo Salveti (rsalveti)]

Looks good, tested with mako, copying over videos and mp3s, doing rename and removes, everything working as expected.

review: Approve

=== modified file 'debian/changelog'
--- debian/changelog	2013-10-23 05:07:47 +0000
+++ debian/changelog	2013-12-03 21:19:01 +0000
@@ -1,3 +1,9 @@
+mtp (0.0.2+14.04.20131127-0ubuntu1) UNRELEASED; urgency=low
+
+  * New release.
+
+ -- Mathieu Trudel-Lapierre <[e-mail]>  Wed, 27 Nov 2013 13:35:01 -0500
+
 mtp (0.0.1+13.10.20130926.3-0ubuntu2) trusty; urgency=low
 
   * No change rebuild for Boost 1.54 transition.

=== modified file 'include/MtpDatabase.h'
--- include/MtpDatabase.h	2013-06-13 10:22:21 +0000
+++ include/MtpDatabase.h	2013-12-03 21:19:01 +0000
@@ -93,6 +93,9 @@
 
     virtual MtpResponseCode         deleteFile(MtpObjectHandle handle) = 0;
 
+    virtual MtpResponseCode         moveFile(MtpObjectHandle handle,
+                                            MtpObjectHandle new_parent) = 0;
+
     virtual MtpObjectHandleList*    getObjectReferences(MtpObjectHandle handle) = 0;
 
     virtual MtpResponseCode         setObjectReferences(MtpObjectHandle handle,

=== modified file 'include/MtpServer.h'
--- include/MtpServer.h	2013-06-13 10:22:21 +0000
+++ include/MtpServer.h	2013-12-03 21:19:01 +0000
@@ -104,11 +104,17 @@
 
     void                sendObjectAdded(MtpObjectHandle handle);
     void                sendObjectRemoved(MtpObjectHandle handle);
+    void                sendObjectInfoChanged(MtpObjectHandle handle);
+    void                sendObjectPropChanged(MtpObjectHandle handle,
+                                              MtpObjectProperty prop);
 
 private:
     void                sendStoreAdded(MtpStorageID id);
     void                sendStoreRemoved(MtpStorageID id);
-    void                sendEvent(MtpEventCode code, uint32_t param1);
+    void                sendEvent(MtpEventCode code,
+                                  uint32_t param1,
+                                  uint32_t param2,
+                                  uint32_t param3);
 
     void                addEditObject(MtpObjectHandle handle, MtpString& path,
                                 uint64_t size, MtpObjectFormat format, int fd);
@@ -141,6 +147,7 @@
     MtpResponseCode     doSendObjectInfo();
     MtpResponseCode     doSendObject();
     MtpResponseCode     doDeleteObject();
+    MtpResponseCode     doMoveObject();
     MtpResponseCode     doGetObjectPropDesc();
     MtpResponseCode     doGetDevicePropDesc();
     MtpResponseCode     doSendPartialObject();

=== modified file 'server/UbuntuMtpDatabase.h'
--- server/UbuntuMtpDatabase.h	2013-09-13 17:11:57 +0000
+++ server/UbuntuMtpDatabase.h	2013-12-03 21:19:01 +0000
@@ -21,6 +21,7 @@
 #include <mtp.h>
 #include <MtpDatabase.h>
 #include <MtpDataPacket.h>
+#include <MtpStringBuffer.h>
 #include <MtpObjectInfo.h>
 #include <MtpProperty.h>
 
@@ -31,7 +32,10 @@
 #include <vector>
 #include <string>
 #include <tuple>
+#include <exception>
 
+#include <boost/algorithm/string.hpp>
+#include <boost/assign/list_of.hpp>
 #include <boost/foreach.hpp>
 #include <boost/filesystem.hpp>
 #include <boost/range/adaptors.hpp>
@@ -46,7 +50,6 @@
     struct DbEntry
     {
         MtpStorageID storage_id;
-        std::string object_name;
         MtpObjectFormat object_format;
         MtpObjectHandle parent;
         size_t object_size;
@@ -56,7 +59,31 @@
 
     uint32_t counter;
     std::map<MtpObjectHandle, DbEntry> db;
-    
+    std::map<std::string, MtpObjectFormat> formats = boost::assign::map_list_of
+        (".ogg", MTP_FORMAT_OGG)
+        (".mp3", MTP_FORMAT_MP3)
+        (".wav", MTP_FORMAT_WAV)
+        (".wma", MTP_FORMAT_WMA)
+        (".aac", MTP_FORMAT_AAC)
+        (".flac", MTP_FORMAT_FLAC);
+
+    MtpObjectFormat guess_object_format(std::string extension)
+    {
+        std::map<std::string, MtpObjectFormat>::iterator it;
+
+        std::cout << __PRETTY_FUNCTION__ << "  extension: " << extension << std::endl;
+        it = formats.find(extension);
+        if (it == formats.end()) {
+            boost::to_upper(extension);
+            it = formats.find(extension);
+            if (it == formats.end()) {
+                return MTP_FORMAT_UNDEFINED;
+            }
+	}
+
+	return it->second;
+    }
+
     void parse_directory(path p, MtpObjectHandle parent)
     {
 	DbEntry entry;
@@ -74,14 +101,15 @@
 
             entry.storage_id = MTP_STORAGE_FIXED_RAM;
             entry.parent = parent;
-            entry.object_name = it->filename().string();
-            entry.display_name = it->filename().string();
+            entry.display_name = std::string(it->filename().string());
             entry.path = it->string();
 
             if (is_regular_file (*it)) {
-                entry.object_format = MTP_FORMAT_DEFINED;
+                entry.object_format = guess_object_format(it->extension().string());
                 entry.object_size = file_size(*it);
 
+                std::cout << " format: " << std::hex << entry.object_format << std::endl;
+
                 db.insert( std::pair<MtpObjectHandle, DbEntry>(handle, entry) );
             } else if (is_directory (*it)) {
                 entry.object_format = MTP_FORMAT_ASSOCIATION;
@@ -107,8 +135,7 @@
 
                     entry.storage_id = MTP_STORAGE_FIXED_RAM;
                     entry.parent = MTP_PARENT_ROOT;
-                    entry.object_name = p.filename().string();
-                    entry.display_name = p.filename().string();
+                    entry.display_name = std::string(p.filename().string());
                     entry.path = p.string();
                     entry.object_format = MTP_FORMAT_ASSOCIATION;
                     entry.object_size = 0;
@@ -163,7 +190,6 @@
 
         entry.storage_id = storage;
         entry.parent = parent;
-        entry.object_name = std::string(basename(path.c_str()));
         entry.display_name = std::string(basename(path.c_str()));
         entry.path = path;
         entry.object_format = format;
@@ -188,8 +214,15 @@
         std::cout << __PRETTY_FUNCTION__ << ": " << path << std::endl;
 
 	if (!succeeded) {
-		db.erase(handle);
-	}
+            db.erase(handle);
+        } else {
+            boost::filesystem::path p (path);
+
+            if (format != MTP_FORMAT_ASSOCIATION) {
+                /* Resync file size, just in case this is actually an Edit. */
+                db.at(handle).object_size = file_size(p);
+            }
+        }
     }
 
     virtual MtpObjectHandleList* getObjectList(
@@ -238,7 +271,28 @@
     virtual MtpObjectFormatList* getSupportedPlaybackFormats()
     {
         std::cout << __PRETTY_FUNCTION__ << std::endl;
-        static const MtpObjectFormatList list = {MTP_FORMAT_PNG};
+        static const MtpObjectFormatList list = {
+            /* Generic files */
+            MTP_FORMAT_UNDEFINED,
+
+            /* Supported audio formats */
+            MTP_FORMAT_OGG,
+            MTP_FORMAT_MP3,
+            MTP_FORMAT_WAV,
+            MTP_FORMAT_WMA,
+            MTP_FORMAT_AAC,
+            MTP_FORMAT_FLAC,
+
+            /* Supported video formats */
+            // none listed yet, video apparently broken.
+
+            /* Audio album, and album art */
+            MTP_FORMAT_ABSTRACT_AUDIO_ALBUM,
+
+            /* Playlists for audio and video */
+            MTP_FORMAT_ABSTRACT_AV_PLAYLIST,
+        };
+
         return new MtpObjectFormatList{list};
     }
     
@@ -266,6 +320,7 @@
             MTP_PROPERTY_WIDTH,
             MTP_PROPERTY_HEIGHT,
             MTP_PROPERTY_IMAGE_BIT_DEPTH,
+            MTP_PROPERTY_OBJECT_FILE_NAME,
             MTP_PROPERTY_DISPLAY_NAME            
         };
          
@@ -292,6 +347,7 @@
             case MTP_PROPERTY_OBJECT_FORMAT: packet.putUInt32(db.at(handle).object_format); break;
             case MTP_PROPERTY_OBJECT_SIZE: packet.putUInt32(db.at(handle).object_size); break;
             case MTP_PROPERTY_DISPLAY_NAME: packet.putString(db.at(handle).display_name.c_str()); break;
+            case MTP_PROPERTY_OBJECT_FILE_NAME: packet.putString(db.at(handle).display_name.c_str()); break;
             default: return MTP_RESPONSE_GENERAL_ERROR; break;                
         }
         
@@ -303,8 +359,47 @@
         MtpObjectProperty property,
         MtpDataPacket& packet)
     {
+        DbEntry entry;
+        MtpStringBuffer buffer;
+        std::string oldname;
+        std::string newname;
+        path oldpath;
+        path newpath;
+
         std::cout << __PRETTY_FUNCTION__ << std::endl;
-        return MTP_RESPONSE_OPERATION_NOT_SUPPORTED;
+
+        switch(property)
+        {
+            case MTP_PROPERTY_OBJECT_FILE_NAME:
+                try {
+                    entry = db.at(handle);
+
+                    packet.getString(buffer);
+                    newname = strdup(buffer);
+
+                    oldpath /= entry.path;
+                    newpath /= oldpath.branch_path() / "/" / newname;
+
+                    boost::filesystem::rename(oldpath, newpath);
+
+                    db.at(handle).display_name = newname;
+                    db.at(handle).path = newpath.string();
+                } catch (filesystem_error& fe) {
+                    std::cout << "ERROR: " << fe.what() << std::endl;
+                    return MTP_RESPONSE_DEVICE_BUSY;
+                } catch (std::exception& e) {
+                    std::cout << "ERROR: " << e.what() << std::endl;
+                    return MTP_RESPONSE_GENERAL_ERROR;
+                } catch (...) {
+                    std::cout << "ERROR: autre exception" << std::endl;
+                    return MTP_RESPONSE_GENERAL_ERROR;
+		}
+
+                break;
+            default: return MTP_RESPONSE_OPERATION_NOT_SUPPORTED; break;
+        }
+        
+        return MTP_RESPONSE_OK;
     }
 
     virtual MtpResponseCode getDevicePropertyValue(
@@ -347,23 +442,20 @@
         MtpObjectInfo& info)
     {
         std::cout << __PRETTY_FUNCTION__ << std::endl;
+
         info.mHandle = handle;
         info.mStorageID = db.at(handle).storage_id;
         info.mFormat = db.at(handle).object_format;
         info.mProtectionStatus = 0x0;
-        info.mCompressedSize = 0;
-        info.mThumbFormat = db.at(handle).object_format;
-        info.mThumbCompressedSize = 20*20*4;
-        info.mThumbPixWidth = 20;
-        info.mThumbPixHeight  =20;
-        info.mImagePixWidth = 20;
-        info.mImagePixHeight = 20;
-        info.mImagePixDepth = 4;
+        info.mCompressedSize = db.at(handle).object_size;
+        info.mImagePixWidth = 0;
+        info.mImagePixHeight = 0;
+        info.mImagePixDepth = 0;
         info.mParent = db.at(handle).parent;
         info.mAssociationType = 0;
         info.mAssociationDesc = 0;
         info.mSequenceNumber = 0;
-        info.mName = ::strdup(db.at(handle).object_name.c_str());
+        info.mName = ::strdup(db.at(handle).display_name.c_str());
         info.mDateCreated = 0;
         info.mDateModified = 0;
         info.mKeywords = ::strdup("ubuntu,touch");
@@ -422,16 +514,17 @@
             return MTP_RESPONSE_GENERAL_ERROR;
     }
 
+    virtual MtpResponseCode moveFile(MtpObjectHandle handle, MtpObjectHandle new_parent)
+    {
+        std::cout << __PRETTY_FUNCTION__ << std::endl;
+
+        // change parent
+        db.at(handle).parent = new_parent;
+
+        return MTP_RESPONSE_OK;
+    }
+
     /*
-    virtual MtpResponseCode moveFile(MtpObjectHandle handle, MtpObjectHandle new_parent)
-    {
-        std::cout << __PRETTY_FUNCTION__ << std::endl;
-
-        // change parent
-
-        return MTP_RESPONSE_OK
-    }
-
     virtual MtpResponseCode copyFile(MtpObjectHandle handle, MtpObjectHandle new_parent)
     {
         std::cout << __PRETTY_FUNCTION__ << std::endl;
@@ -471,7 +564,8 @@
             case MTP_PROPERTY_WIDTH: result = new MtpProperty(property, MTP_TYPE_UINT32); break;
             case MTP_PROPERTY_HEIGHT: result = new MtpProperty(property, MTP_TYPE_UINT32); break;
             case MTP_PROPERTY_IMAGE_BIT_DEPTH: result = new MtpProperty(property, MTP_TYPE_UINT32); break;
-            case MTP_PROPERTY_DISPLAY_NAME: result = new MtpProperty(property, MTP_TYPE_STR); break;
+            case MTP_PROPERTY_DISPLAY_NAME: result = new MtpProperty(property, MTP_TYPE_STR, true); break;
+            case MTP_PROPERTY_OBJECT_FILE_NAME: result = new MtpProperty(property, MTP_TYPE_STR, true); break;
             default: break;                
         }
         

=== modified file 'src/MtpServer.cpp'
--- src/MtpServer.cpp	2013-09-13 16:37:39 +0000
+++ src/MtpServer.cpp	2013-12-03 21:19:01 +0000
@@ -68,7 +68,7 @@
     MTP_OPERATION_SET_DEVICE_PROP_VALUE,
     MTP_OPERATION_RESET_DEVICE_PROP_VALUE,
 //    MTP_OPERATION_TERMINATE_OPEN_CAPTURE,
-//    MTP_OPERATION_MOVE_OBJECT,
+    MTP_OPERATION_MOVE_OBJECT,
 //    MTP_OPERATION_COPY_OBJECT,
     MTP_OPERATION_GET_PARTIAL_OBJECT,
 //    MTP_OPERATION_INITIATE_OPEN_CAPTURE,
@@ -96,6 +96,8 @@
     MTP_EVENT_OBJECT_REMOVED,
     MTP_EVENT_STORE_ADDED,
     MTP_EVENT_STORE_REMOVED,
+    MTP_EVENT_OBJECT_INFO_CHANGED,
+    MTP_EVENT_OBJECT_PROP_CHANGED,
 };
 
 MtpServer::MtpServer(int fd, MtpDatabase* database, bool ptp,
@@ -246,29 +248,45 @@
 
 void MtpServer::sendObjectAdded(MtpObjectHandle handle) {
     ALOGV("sendObjectAdded %d\n", handle);
-    sendEvent(MTP_EVENT_OBJECT_ADDED, handle);
+    sendEvent(MTP_EVENT_OBJECT_ADDED, handle, 0, 0);
 }
 
 void MtpServer::sendObjectRemoved(MtpObjectHandle handle) {
     ALOGV("sendObjectRemoved %d\n", handle);
-    sendEvent(MTP_EVENT_OBJECT_REMOVED, handle);
+    sendEvent(MTP_EVENT_OBJECT_REMOVED, handle, 0, 0);
+}
+
+void MtpServer::sendObjectInfoChanged(MtpObjectHandle handle) {
+    ALOGV("sendObjectInfoChanged %d\n", handle);
+    sendEvent(MTP_EVENT_OBJECT_INFO_CHANGED, handle, 0, 0);
+}
+
+void MtpServer::sendObjectPropChanged(MtpObjectHandle handle,
+                                      MtpObjectProperty prop) {
+    ALOGV("sendObjectPropChanged %d %d\n", handle, prop);
+    sendEvent(MTP_EVENT_OBJECT_PROP_CHANGED, handle, prop, 0);
 }
 
 void MtpServer::sendStoreAdded(MtpStorageID id) {
     ALOGV("sendStoreAdded %08X\n", id);
-    sendEvent(MTP_EVENT_STORE_ADDED, id);
+    sendEvent(MTP_EVENT_STORE_ADDED, id, 0, 0);
 }
 
 void MtpServer::sendStoreRemoved(MtpStorageID id) {
     ALOGV("sendStoreRemoved %08X\n", id);
-    sendEvent(MTP_EVENT_STORE_REMOVED, id);
+    sendEvent(MTP_EVENT_STORE_REMOVED, id, 0, 0);
 }
 
-void MtpServer::sendEvent(MtpEventCode code, uint32_t param1) {
+void MtpServer::sendEvent(MtpEventCode code,
+                          uint32_t param1,
+                          uint32_t param2,
+                          uint32_t param3) {
     if (mSessionOpen) {
         mEvent.setEventCode(code);
         mEvent.setTransactionID(mRequest.getTransactionID());
         mEvent.setParameter(1, param1);
+        mEvent.setParameter(2, param2);
+        mEvent.setParameter(3, param3);
         int ret = mEvent.write(mFD);
         ALOGV("mEvent.write returned %d\n", ret);
     }
@@ -392,6 +410,9 @@
         case MTP_OPERATION_DELETE_OBJECT:
             response = doDeleteObject();
             break;
+        case MTP_OPERATION_MOVE_OBJECT:
+            response = doMoveObject();
+            break;
         case MTP_OPERATION_GET_OBJECT_PROP_DESC:
             response = doGetObjectPropDesc();
             break;
@@ -625,6 +646,8 @@
 }
 
 MtpResponseCode MtpServer::doSetObjectPropValue() {
+    MtpResponseCode response;
+
     if (!hasStorage())
         return MTP_RESPONSE_INVALID_OBJECT_HANDLE;
     MtpObjectHandle handle = mRequest.getParameter(1);
@@ -632,7 +655,11 @@
     ALOGV("SetObjectPropValue %d %s\n", handle,
             MtpDebug::getObjectPropCodeName(property));
 
-    return mDatabase->setObjectPropertyValue(handle, property, mData);
+    response = mDatabase->setObjectPropertyValue(handle, property, mData);
+
+    //sendObjectPropChanged(handle, property);
+
+    return response;
 }
 
 MtpResponseCode MtpServer::doGetDevicePropValue() {
@@ -1065,6 +1092,30 @@
     return result;
 }
 
+MtpResponseCode MtpServer::doMoveObject() {
+    if (!hasStorage())
+        return MTP_RESPONSE_INVALID_OBJECT_HANDLE;
+    MtpObjectHandle handle = mRequest.getParameter(1);
+    MtpObjectFormat format = mRequest.getParameter(2);
+    MtpObjectHandle newparent = mRequest.getParameter(3);
+
+    MtpString filePath;
+    MtpString newPath;
+    int64_t fileLength;
+    int result = mDatabase->getObjectFilePath(handle, filePath, fileLength, format);
+    result = mDatabase->getObjectFilePath(handle, newPath, fileLength, format);
+    if (result == MTP_RESPONSE_OK) {
+        ALOGV("moving %s to %s", filePath.c_str(), newPath.c_str());
+        result = mDatabase->moveFile(handle, newparent);
+        // Don't move the actual files unless the database deletion is allowed
+        if (result == MTP_RESPONSE_OK) {
+            rename(filePath.c_str(), newPath.c_str());
+        }
+    }
+
+    return result;
+}
+
 MtpResponseCode MtpServer::doGetObjectPropDesc() {
     MtpObjectProperty propCode = mRequest.getParameter(1);
     MtpObjectFormat format = mRequest.getParameter(2);

=== modified file 'tests/MockMtpDatabase.h'
--- tests/MockMtpDatabase.h	2013-09-13 14:21:51 +0000
+++ tests/MockMtpDatabase.h	2013-12-03 21:19:01 +0000
@@ -303,6 +303,15 @@
             return MTP_RESPONSE_GENERAL_ERROR;
     }
 
+    virtual MtpResponseCode moveFile(MtpObjectHandle handle, MtpObjectHandle new_parent)
+    {
+        std::cout << __PRETTY_FUNCTION__ << std::endl;
+
+        db.at(handle).parent = new_parent;
+
+        return MTP_RESPONSE_OK;
+    }
+
     virtual MtpObjectHandleList* getObjectReferences(MtpObjectHandle handle)
     {
         std::cout << __PRETTY_FUNCTION__ << std::endl;