1039939 - [MTP] enable the functionality on windows 7

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Alphan Chen [:alchen]]

In current status, we can see a portable device called flame.
There is a volume called "firefox os" when entering this portable device.
However, we cannot see anything when entering "firefox os" volume.

The following is the MTP command which we received with no error response.
But the command just stop after "MTP_OPERATION_GET_OBJECT_INFO".

MTP_OPERATION_OPEN_SESSION -> MTP_OPERATION_GET_DEVICE_INFO -> 
MTP_OPERATION_GET_OBJECT_PROPS_SUPPORTED ->
MTP_OPERATION_GET_STORAGE_IDS -> MTP_OPERATION_GET_STORAGE_INFO -> MTP_OPERATION_GET_OBJECT_HANDLES -> MTP_OPERATION_GET_OBJECT_PROP_DESC -> MTP_OPERATION_GET_OBJECT_PROP_LIST ->
MTP_OPERATION_GET_OBJECT_INFO

Comment 1

[Alphan Chen [:alchen]]

I try to do modification on the response for MTP_OPERATION_GET_OBJECT_INFO and let it similar as what I see on Nexus 4 case. However, I still cannot see anything (after enter "firefox" volume) with this change.
So I will move forward on previous response.

Next target : "MTP_OPERATION_GET_OBJECT_PROP_LIST"

Comment 2

[Alphan Chen [:alchen]]

Update the new finding,
we can see folders with the following changes when entering "FirefoxOS" volume.


 MtpObjectFormatList*
 MozMtpDatabase::getSupportedPlaybackFormats()
 {
-  static const uint16_t init_data[] = {MTP_FORMAT_PNG};
+  static const uint16_t init_data[] = {MTP_FORMAT_UNDEFINED, MTP_FORMAT_ASSOCIATION, MTP_FORMAT_PNG};

However, the folder looks weird.
It show "local disk" without filename.

I also try to add the implementation of "getObjectReferences" but in vain.

Comment 3

[Alphan Chen [:alchen]]

Attachment: (0729) Enable MTP functionality on WIN7

With this patch, W7 have the same ability as Ubuntu.

Summarize this patch as below:

1. Add "MTP_FORMAT_UNDEFINED" and "MTP_FORMAT_ASSOCIATION" into the list of "SupportedPlaybackFormats". Without them, I even cannot see sdcard folder when entering "FirefoxOS" volume.

2. From MTP specification, "MTP_PROPERTY_NAME" and "MTP_PROPERTY_PERSISTENT_UID" is required. Add the support of these two properties.

3. Fix bug in "getObjectPropertyValue": different properties should have different type.

4. Fix bug in "getObjectPropertyList": the first data we need to reply should be the mount of properties we reply in the packet. (numObjectProperties * numEntries)

5. Correct the some object information in "getObjectInfo".

Comment 4

[Dave Hylands [:dhylands]]

Comment on attachment 8463834 [details] [diff] [review]
(0729) Enable MTP functionality on WIN7

Review of attachment 8463834 [details] [diff] [review]:
-----------------------------------------------------------------

Excellent.

The only thing I think I'd do (as commented) is to combine the storage ID and handle together in order to create the UID.

::: dom/system/gonk/MozMtpDatabase.cpp
@@ +365,2 @@
>      case MTP_PROPERTY_DISPLAY_NAME:   aPacket.putString(entry->mDisplayName.get()); break;
> +    case MTP_PROPERTY_PERSISTENT_UID: aPacket.putUInt128(entry->mHandle); break;

I think that this is probably insufficient. We should combine the storageID (in the upper 64 bits) and handle (in the lower 64 bits) together to cover the case where there are multiple storage areas.

Right now the handle is unique globally, but I think that as soon as we add support of multiple storage areas we'll wind up tracking files per storage area.

Comment 5

[Alphan Chen [:alchen]]

Attachment: Enable MTP functionality on WIN7. r=dhylands

Based on the comment 4, there is only one modification compared to attachment 8463834 [details] [diff] [review]. 
The value of "MTP_PROPERTY_PERSISTENT_UID" is storageID in the upper 64 bits and handle in the lower 64 bits. 

Try server result. It looks fine.
https://tbpl.mozilla.org/?tree=Try&rev=dd4792495e45

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bd47316a63d2

Any particular reason the Try run was across all platforms? Based on the code being touched and the bug title, that seems quite excessive. Please try to be conscientious about that as "all" runs consume a LOT of resources, often unnecessarily.
https://wiki.mozilla.org/Sheriffing/How:To:Recommended_Try_Practices

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bd47316a63d2