From 1cfc0c749a5c9b2f27863220c2a8089513ac1679 Mon Sep 17 00:00:00 2001 From: Zim Date: Tue, 2 Nov 2021 17:41:38 +0000 Subject: [PATCH 1/2] Workaround to fixup MTP transcode file size Some Windows MTP clients ignore the file size sent from the server while sending MTP objects, and instead rely on the cached MTP metadata containing the size. This causes file corruption when transcoded files are transferred from phone to PC because the file is truncated to the original file size from the cached metadata which is smaller than the transcoded file size. To workaround this, MediaProvider added an ugly workaround to the struct stat st_nlink field to signal when a file requires transcoding. In such cases, the MTP server can double the metadata size, the same heuristic used in the MediaProvider to accommodate the transcoded file size. Test: atest MtpTests Bug: 184117074 Bug: 190422448 Change-Id: I0581978fb346e3dc2d7a5204ffd7c601b61ec749 --- media/java/android/mtp/MtpStorageManager.java | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/media/java/android/mtp/MtpStorageManager.java b/media/java/android/mtp/MtpStorageManager.java index 0bede0dccbed2..fd3741ebec234 100644 --- a/media/java/android/mtp/MtpStorageManager.java +++ b/media/java/android/mtp/MtpStorageManager.java @@ -18,7 +18,11 @@ package android.mtp; import android.media.MediaFile; import android.os.FileObserver; +import android.os.SystemProperties; import android.os.storage.StorageVolume; +import android.system.ErrnoException; +import android.system.Os; +import android.system.StructStat; import android.util.Log; import com.android.internal.util.Preconditions; @@ -199,7 +203,38 @@ public class MtpStorageManager { } public long getSize() { - return mIsDir ? 0 : getPath().toFile().length(); + return mIsDir ? 0 : maybeApplyTranscodeLengthWorkaround(getPath().toFile().length()); + } + + private long maybeApplyTranscodeLengthWorkaround(long length) { + // Windows truncates transferred files to the size advertised in the object property. + if (isTranscodeMtpEnabled() && isFileTranscodeSupported()) { + // If the file supports transcoding, we double the returned size to accommodate + // the increase in size from transcoding to AVC. This is the same heuristic + // applied in the FUSE daemon (MediaProvider). + return length * 2; + } + return length; + } + + private boolean isTranscodeMtpEnabled() { + return SystemProperties.getBoolean("sys.fuse.transcode_mtp", false); + } + + private boolean isFileTranscodeSupported() { + // Check if the file supports transcoding by reading the |st_nlinks| struct stat + // field. This will be > 1 if the file supports transcoding. The FUSE daemon + // sets the field accordingly to enable the MTP stack workaround some Windows OS + // MTP client bug where they ignore the size returned as part of getting the MTP + // object, see MtpServer#doGetObject. + final Path path = getPath(); + try { + StructStat stat = Os.stat(path.toString()); + return stat.st_nlink > 1; + } catch (ErrnoException e) { + Log.w(TAG, "Failed to stat path: " + getPath() + ". Ignoring transcoding."); + return false; + } } public Path getPath() { From f0fbd6439e3d1d0d20b0f932b76414f4147a3a0e Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Fri, 12 Nov 2021 16:13:55 +0000 Subject: [PATCH 2/2] Limit transcoding size workaround to windows Test: Connect android to windows through usbc. Enable mtp, then enable the transcoding option. Copy H265 video from the camera folder to windows' desktop. Transcoded H264 video plays correctly in WMPcorrectly. Bug: 184117074 Bug: 190422448 Change-Id: I17264f0ee3e742315569d4c50817b1da7f3f2eb4 --- media/java/android/mtp/MtpDatabase.java | 10 +++++++--- media/java/android/mtp/MtpStorage.java | 15 ++++++++++++++- media/java/android/mtp/MtpStorageManager.java | 11 +++++++---- .../src/android/mtp/MtpStorageManagerTest.java | 7 ++++--- 4 files changed, 32 insertions(+), 11 deletions(-) diff --git a/media/java/android/mtp/MtpDatabase.java b/media/java/android/mtp/MtpDatabase.java index d8f48c2cf0c68..20d711cf4c54d 100755 --- a/media/java/android/mtp/MtpDatabase.java +++ b/media/java/android/mtp/MtpDatabase.java @@ -103,6 +103,7 @@ public class MtpDatabase implements AutoCloseable { private int mDeviceType; private String mHostType; private boolean mSkipThumbForHost = false; + private volatile boolean mHostIsWindows = false; private MtpServer mServer; private MtpStorageManager mManager; @@ -358,7 +359,7 @@ public class MtpDatabase implements AutoCloseable { } public void addStorage(StorageVolume storage) { - MtpStorage mtpStorage = mManager.addMtpStorage(storage); + MtpStorage mtpStorage = mManager.addMtpStorage(storage, () -> mHostIsWindows); mStorageMap.put(storage.getPath(), mtpStorage); if (mServer != null) { mServer.addStorage(mtpStorage); @@ -413,6 +414,7 @@ public class MtpDatabase implements AutoCloseable { } mHostType = ""; mSkipThumbForHost = false; + mHostIsWindows = false; } @VisibleForNative @@ -736,10 +738,12 @@ public class MtpDatabase implements AutoCloseable { : MtpConstants.RESPONSE_GENERAL_ERROR); case MtpConstants.DEVICE_PROPERTY_SESSION_INITIATOR_VERSION_INFO: mHostType = stringValue; + Log.d(TAG, "setDeviceProperty." + Integer.toHexString(property) + + "=" + stringValue); if (stringValue.startsWith("Android/")) { - Log.d(TAG, "setDeviceProperty." + Integer.toHexString(property) - + "=" + stringValue); mSkipThumbForHost = true; + } else if (stringValue.startsWith("Windows/")) { + mHostIsWindows = true; } return MtpConstants.RESPONSE_OK; } diff --git a/media/java/android/mtp/MtpStorage.java b/media/java/android/mtp/MtpStorage.java index 88c32a3ea72b8..a3754e90a875c 100644 --- a/media/java/android/mtp/MtpStorage.java +++ b/media/java/android/mtp/MtpStorage.java @@ -19,6 +19,8 @@ package android.mtp; import android.compat.annotation.UnsupportedAppUsage; import android.os.storage.StorageVolume; +import java.util.function.Supplier; + /** * This class represents a storage unit on an MTP device. * Used only for MTP support in USB responder mode. @@ -33,14 +35,16 @@ public class MtpStorage { private final boolean mRemovable; private final long mMaxFileSize; private final String mVolumeName; + private final Supplier mIsHostWindows; - public MtpStorage(StorageVolume volume, int storageId) { + public MtpStorage(StorageVolume volume, int storageId, Supplier isHostWindows) { mStorageId = storageId; mPath = volume.getPath(); mDescription = volume.getDescription(null); mRemovable = volume.isRemovable(); mMaxFileSize = volume.getMaxFileSize(); mVolumeName = volume.getMediaStoreVolumeName(); + mIsHostWindows = isHostWindows; } /** @@ -93,4 +97,13 @@ public class MtpStorage { public String getVolumeName() { return mVolumeName; } + + /** + * Returns true if the mtp host of this storage is Windows. + * + * @return is host Windows + */ + public boolean isHostWindows() { + return mIsHostWindows.get(); + } } diff --git a/media/java/android/mtp/MtpStorageManager.java b/media/java/android/mtp/MtpStorageManager.java index fd3741ebec234..e9426cf2ce316 100644 --- a/media/java/android/mtp/MtpStorageManager.java +++ b/media/java/android/mtp/MtpStorageManager.java @@ -39,6 +39,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.function.Supplier; /** * MtpStorageManager provides functionality for listing, tracking, and notifying MtpServer of @@ -208,7 +209,7 @@ public class MtpStorageManager { private long maybeApplyTranscodeLengthWorkaround(long length) { // Windows truncates transferred files to the size advertised in the object property. - if (isTranscodeMtpEnabled() && isFileTranscodeSupported()) { + if (mStorage.isHostWindows() && isTranscodeMtpEnabled() && isFileTranscodeSupported()) { // If the file supports transcoding, we double the returned size to accommodate // the increase in size from transcoding to AVC. This is the same heuristic // applied in the FUSE daemon (MediaProvider). @@ -455,10 +456,12 @@ public class MtpStorageManager { * @param volume Storage to add. * @return the associated MtpStorage */ - public synchronized MtpStorage addMtpStorage(StorageVolume volume) { + public synchronized MtpStorage addMtpStorage(StorageVolume volume, + Supplier isHostWindows) { int storageId = ((getNextStorageId() & 0x0000FFFF) << 16) + 1; - MtpStorage storage = new MtpStorage(volume, storageId); - MtpObject root = new MtpObject(storage.getPath(), storageId, storage, null, true); + MtpStorage storage = new MtpStorage(volume, storageId, isHostWindows); + MtpObject root = new MtpObject(storage.getPath(), storageId, storage, /* parent= */ null, + /* isDir= */ true); mRoots.put(storageId, root); return storage; } diff --git a/media/tests/MtpTests/src/android/mtp/MtpStorageManagerTest.java b/media/tests/MtpTests/src/android/mtp/MtpStorageManagerTest.java index fdf65823b1f33..eb357f67caea2 100644 --- a/media/tests/MtpTests/src/android/mtp/MtpStorageManagerTest.java +++ b/media/tests/MtpTests/src/android/mtp/MtpStorageManagerTest.java @@ -159,10 +159,11 @@ public class MtpStorageManagerTest { Log.d(TAG, "sendObjectInfoChanged: " + id); objectsInfoChanged.add(id); } - }, null); + }, /* subdirectories= */ null); - mainMtpStorage = manager.addMtpStorage(mainStorage); - secondaryMtpStorage = manager.addMtpStorage(secondaryStorage); + mainMtpStorage = manager.addMtpStorage(mainStorage, /* isHostWindows= */ () -> false); + secondaryMtpStorage = manager.addMtpStorage(secondaryStorage, + /* isHostWindows= */ () -> false); } @After