improved handling of motion photo with incorrect video offset

CHANGELOG.md

@@ -22,6 +22,7 @@ All notable changes to this project will be documented in this file.
 - editing TIFF metadata increasing file size
 - region decoding for some RAW files
 - incorrect video size or orientation as reported by Media Store
+- corrupting image when removing video from motion photo with incorrect metadata
 
 ## <a id="v1.12.2"></a>[v1.12.2] - 2025-01-13
 

android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/EmbeddedDataHandler.kt

@@ -186,7 +186,7 @@ class EmbeddedDataHandler(private val context: Context) : MethodCallHandler {
             return
         }
 
-        MultiPage.getMotionPhotoOffset(context, uri, mimeType, sizeBytes)?.let { videoSizeBytes ->
+        MultiPage.getMotionPhotoVideoSize(context, uri, mimeType, sizeBytes)?.let { videoSizeBytes ->
             val imageSizeBytes = sizeBytes - videoSizeBytes
             StorageUtils.openInputStream(context, uri)?.let { input ->
                 copyEmbeddedBytes(result, mimeType, displayName, input, imageSizeBytes)
@@ -207,7 +207,7 @@ class EmbeddedDataHandler(private val context: Context) : MethodCallHandler {
             return
         }
 
-        MultiPage.getMotionPhotoOffset(context, uri, mimeType, sizeBytes)?.let { videoSizeBytes ->
+        MultiPage.getMotionPhotoVideoSize(context, uri, mimeType, sizeBytes)?.let { videoSizeBytes ->
             val videoStartOffset = sizeBytes - videoSizeBytes
             StorageUtils.openInputStream(context, uri)?.let { input ->
                 input.skip(videoStartOffset)

android/app/src/main/kotlin/deckers/thibault/aves/metadata/MediaMetadataRetrieverHelper.kt

@@ -111,20 +111,25 @@ object MediaMetadataRetrieverHelper {
                 // format
                 MediaMetadataRetriever.METADATA_KEY_IMAGE_ROTATION,
                 MediaMetadataRetriever.METADATA_KEY_VIDEO_ROTATION -> "$value°"
+
                 MediaMetadataRetriever.METADATA_KEY_IMAGE_HEIGHT, MediaMetadataRetriever.METADATA_KEY_IMAGE_WIDTH,
                 MediaMetadataRetriever.METADATA_KEY_VIDEO_HEIGHT, MediaMetadataRetriever.METADATA_KEY_VIDEO_WIDTH -> "$value pixels"
+
                 MediaMetadataRetriever.METADATA_KEY_BITRATE -> {
                     val bitrate = value.toLongOrNull() ?: 0
                     if (bitrate > 0) formatBitrate(bitrate) else null
                 }
+
                 MediaMetadataRetriever.METADATA_KEY_CAPTURE_FRAMERATE -> {
                     val framerate = value.toDoubleOrNull() ?: 0.0
                     if (framerate > 0.0) "$framerate" else null
                 }
+
                 MediaMetadataRetriever.METADATA_KEY_DURATION -> {
                     val dateMillis = value.toLongOrNull() ?: 0
                     if (dateMillis > 0) durationFormat.format(Date(dateMillis)) else null
                 }
+
                 MediaMetadataRetriever.METADATA_KEY_COLOR_RANGE -> {
                     when (value.toIntOrNull()) {
                         MediaFormat.COLOR_RANGE_FULL -> "Full"
@@ -132,6 +137,7 @@ object MediaMetadataRetrieverHelper {
                         else -> value
                     }
                 }
+
                 MediaMetadataRetriever.METADATA_KEY_COLOR_STANDARD -> {
                     when (value.toIntOrNull()) {
                         MediaFormat.COLOR_STANDARD_BT709 -> "BT.709"
@@ -141,6 +147,7 @@ object MediaMetadataRetrieverHelper {
                         else -> value
                     }
                 }
+
                 MediaMetadataRetriever.METADATA_KEY_COLOR_TRANSFER -> {
                     when (value.toIntOrNull()) {
                         MediaFormat.COLOR_TRANSFER_LINEAR -> "Linear"
@@ -154,6 +161,7 @@ object MediaMetadataRetrieverHelper {
                 MediaMetadataRetriever.METADATA_KEY_COMPILATION,
                 MediaMetadataRetriever.METADATA_KEY_DISC_NUMBER,
                 MediaMetadataRetriever.METADATA_KEY_YEAR -> if (value != "0") value else null
+
                 MediaMetadataRetriever.METADATA_KEY_CD_TRACK_NUMBER -> if (value != "0/0") value else null
                 MediaMetadataRetriever.METADATA_KEY_DATE -> {
                     val dateMillis = Metadata.parseVideoMetadataDate(value)
@@ -168,4 +176,12 @@ object MediaMetadataRetrieverHelper {
             }?.let { save(it) }
         }
     }
+
+    fun MediaFormat.getSafeInt(key: String, save: (value: Int) -> Unit) {
+        if (this.containsKey(key)) save(this.getInteger(key))
+    }
+
+    fun MediaFormat.getSafeLong(key: String, save: (value: Long) -> Unit) {
+        if (this.containsKey(key)) save(this.getLong(key))
+    }
 }

android/app/src/main/kotlin/deckers/thibault/aves/metadata/MultiPage.kt

@@ -15,6 +15,8 @@ import com.drew.metadata.exif.ExifDirectoryBase
 import com.drew.metadata.exif.ExifIFD0Directory
 import com.drew.metadata.xmp.XmpDirectory
 import deckers.thibault.aves.metadata.ExifInterfaceHelper.getSafeInt
+import deckers.thibault.aves.metadata.MediaMetadataRetrieverHelper.getSafeInt
+import deckers.thibault.aves.metadata.MediaMetadataRetrieverHelper.getSafeLong
 import deckers.thibault.aves.metadata.metadataextractor.Helper
 import deckers.thibault.aves.metadata.metadataextractor.Helper.getSafeInt
 import deckers.thibault.aves.metadata.metadataextractor.mpf.MpEntry
@@ -47,14 +49,6 @@ object MultiPage {
     private const val KEY_ROTATION_DEGREES = "rotationDegrees"
 
     fun getHeicTracks(context: Context, uri: Uri): ArrayList<FieldMap> {
-        fun MediaFormat.getSafeInt(key: String, save: (value: Int) -> Unit) {
-            if (this.containsKey(key)) save(this.getInteger(key))
-        }
-
-        fun MediaFormat.getSafeLong(key: String, save: (value: Long) -> Unit) {
-            if (this.containsKey(key)) save(this.getLong(key))
-        }
-
         val tracks = ArrayList<FieldMap>()
         val extractor = MediaExtractor()
         extractor.setDataSource(context, uri, null)
@@ -250,23 +244,9 @@ object MultiPage {
     }
 
     fun getMotionPhotoPages(context: Context, uri: Uri, mimeType: String, sizeBytes: Long): ArrayList<FieldMap> {
-        fun MediaFormat.getSafeInt(key: String, save: (value: Int) -> Unit) {
-            if (this.containsKey(key)) save(this.getInteger(key))
-        }
-
-        fun MediaFormat.getSafeLong(key: String, save: (value: Long) -> Unit) {
-            if (this.containsKey(key)) save(this.getLong(key))
-        }
-
         val pages = ArrayList<FieldMap>()
-        val extractor = MediaExtractor()
+        getMotionPhotoVideoSize(context, uri, mimeType, sizeBytes)?.let { videoSizeBytes ->
-        var pfd: ParcelFileDescriptor? = null
+            getTrailerVideoInfo(context, uri, fileSizeBytes = sizeBytes, videoSizeBytes = videoSizeBytes)?.let { videoInfo ->
-        try {
-            getMotionPhotoOffset(context, uri, mimeType, sizeBytes)?.let { videoSizeBytes ->
-                val videoStartOffset = sizeBytes - videoSizeBytes
-                pfd = context.contentResolver.openFileDescriptor(uri, "r")
-                pfd?.fileDescriptor?.let { fd ->
-                    extractor.setDataSource(fd, videoStartOffset, videoSizeBytes)
                 // set the original image as the first and default track
                 var pageIndex = 0
                 pages.add(
@@ -277,43 +257,28 @@ object MultiPage {
                     )
                 )
                 // add video tracks from the appended video
-                    if (extractor.trackCount > 0) {
+                videoInfo.getString(MediaFormat.KEY_MIME)?.let { mime ->
-                        // only consider the first track to represent the appended video
-                        val trackIndex = 0
-                        try {
-                            val format = extractor.getTrackFormat(trackIndex)
-                            format.getString(MediaFormat.KEY_MIME)?.let { mime ->
                     if (MimeTypes.isVideo(mime)) {
                         val page: FieldMap = hashMapOf(
                             KEY_PAGE to pageIndex++,
                             KEY_MIME_TYPE to MimeTypes.MP4,
                             KEY_IS_DEFAULT to false,
                         )
-                                    format.getSafeInt(MediaFormat.KEY_WIDTH) { page[KEY_WIDTH] = it }
+                        videoInfo.getSafeInt(MediaFormat.KEY_WIDTH) { page[KEY_WIDTH] = it }
-                                    format.getSafeInt(MediaFormat.KEY_HEIGHT) { page[KEY_HEIGHT] = it }
+                        videoInfo.getSafeInt(MediaFormat.KEY_HEIGHT) { page[KEY_HEIGHT] = it }
                         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
-                                        format.getSafeInt(MediaFormat.KEY_ROTATION) { page[KEY_ROTATION_DEGREES] = it }
+                            videoInfo.getSafeInt(MediaFormat.KEY_ROTATION) { page[KEY_ROTATION_DEGREES] = it }
                         }
-                                    format.getSafeLong(MediaFormat.KEY_DURATION) { page[KEY_DURATION] = it / 1000 }
+                        videoInfo.getSafeLong(MediaFormat.KEY_DURATION) { page[KEY_DURATION] = it / 1000 }
                         pages.add(page)
                     }
                 }
-                        } catch (e: Exception) {
-                            Log.w(LOG_TAG, "failed to get motion photo track information for uri=$uri, track num=$trackIndex", e)
             }
         }
-                }
-            }
-        } catch (e: Exception) {
-            Log.w(LOG_TAG, "failed to open motion photo for uri=$uri", e)
-        } finally {
-            extractor.release()
-            pfd?.close()
-        }
         return pages
     }
 
-    fun getMotionPhotoOffset(context: Context, uri: Uri, mimeType: String, sizeBytes: Long): Long? {
+    fun getMotionPhotoVideoSize(context: Context, uri: Uri, mimeType: String, sizeBytes: Long): Long? {
         if (MimeTypes.isHeic(mimeType)) {
             // XMP in HEIC motion photos (as taken with a Samsung Camera v12.0.01.50) indicates an `Item:Length` of 68 bytes for the video.
             // This item does not contain the video itself, but only some kind of metadata (no doc, no spec),
@@ -360,6 +325,34 @@ object MultiPage {
         return offsetFromEnd
     }
 
+    fun getTrailerVideoInfo(context: Context, uri: Uri, fileSizeBytes: Long, videoSizeBytes: Long): MediaFormat? {
+        var format: MediaFormat? = null
+        val extractor = MediaExtractor()
+        var pfd: ParcelFileDescriptor? = null
+        try {
+            val videoStartOffset = fileSizeBytes - videoSizeBytes
+            pfd = context.contentResolver.openFileDescriptor(uri, "r")
+            pfd?.fileDescriptor?.let { fd ->
+                extractor.setDataSource(fd, videoStartOffset, videoSizeBytes)
+                if (extractor.trackCount > 0) {
+                    // only consider the first track to represent the appended video
+                    val trackIndex = 0
+                    try {
+                        format = extractor.getTrackFormat(trackIndex)
+                    } catch (e: Exception) {
+                        Log.w(LOG_TAG, "failed to get motion photo track information for uri=$uri, track num=$trackIndex", e)
+                    }
+                }
+            }
+        } catch (e: Exception) {
+            Log.w(LOG_TAG, "failed to open motion photo for uri=$uri", e)
+        } finally {
+            extractor.release()
+            pfd?.close()
+        }
+        return format
+    }
+
     fun getTiffPages(context: Context, uri: Uri): ArrayList<FieldMap> {
         fun toMap(pageIndex: Int, options: TiffBitmapFactory.Options): FieldMap {
             return hashMapOf(

android/app/src/main/kotlin/deckers/thibault/aves/metadata/xmp/GoogleXMP.kt

@@ -183,7 +183,7 @@ object GoogleXMP {
         return offsetFromEnd
     }
 
-    fun updateTrailingVideoOffset(xmp: String, oldOffset: Int, newOffset: Int): String {
+    fun updateTrailingVideoOffset(xmp: String, oldOffset: Number, newOffset: Number): String {
         return xmp.replace(
             // GCamera motion photo
             "${GCAMERA_VIDEO_OFFSET_PROP_NAME}=\"$oldOffset\"",
@@ -195,7 +195,6 @@ object GoogleXMP {
         )
     }
 
-
     fun getDeviceContainer(meta: XMPMeta): GoogleDeviceContainer? {
         return if (meta.doesPropPathExist(listOf(GDEVICE_CONTAINER_PROP_NAME, GDEVICE_CONTAINER_DIRECTORY_PROP_NAME))) {
             GoogleDeviceContainer().apply { findItems(meta) }

android/app/src/main/kotlin/deckers/thibault/aves/model/provider/ImageProvider.kt

@@ -646,19 +646,21 @@ abstract class ImageProvider {
         }
 
         val originalFileSize = File(path).length()
-        val videoSize = MultiPage.getMotionPhotoOffset(context, uri, mimeType, originalFileSize)?.let { it.toInt() + trailerDiff }
+        var trailerVideoBytes: ByteArray? = null
-        var videoBytes: ByteArray? = null
         val editableFile = StorageUtils.createTempFile(context).apply {
+            val videoSize = MultiPage.getMotionPhotoVideoSize(context, uri, mimeType, originalFileSize)?.let { it + trailerDiff }
+            val isTrailerVideoValid = videoSize != null && MultiPage.getTrailerVideoInfo(context, uri, originalFileSize, videoSize) != null
             try {
-                if (videoSize != null) {
+                if (videoSize != null && isTrailerVideoValid) {
                     // handle motion photo and embedded video separately
                     val imageSize = (originalFileSize - videoSize).toInt()
-                    videoBytes = ByteArray(videoSize)
+                    val videoByteSize = videoSize.toInt()
+                    trailerVideoBytes = ByteArray(videoByteSize)
 
                     StorageUtils.openInputStream(context, uri)?.let { input ->
                         val imageBytes = ByteArray(imageSize)
                         input.read(imageBytes, 0, imageSize)
-                        input.read(videoBytes, 0, videoSize)
+                        input.read(trailerVideoBytes, 0, videoByteSize)
 
                         // copy only the image to a temporary file for editing
                         // video will be appended after metadata modification
@@ -693,15 +695,15 @@ abstract class ImageProvider {
                 ImageDecoder.decodeBitmap(ImageDecoder.createSource(editableFile))
             }
 
-            if (videoBytes != null) {
+            if (trailerVideoBytes != null) {
                 // append trailer video, if any
-                editableFile.appendBytes(videoBytes!!)
+                editableFile.appendBytes(trailerVideoBytes!!)
             }
 
             // copy the edited temporary file back to the original
             editableFile.transferTo(outputStream(context, mimeType, uri, path))
 
-            if (autoCorrectTrailerOffset && !checkTrailerOffset(context, path, uri, mimeType, videoSize, editableFile, callback)) {
+            if (autoCorrectTrailerOffset && !checkTrailerOffset(context, path, uri, mimeType, trailerVideoBytes?.size, editableFile, callback)) {
                 return false
             }
             editableFile.delete()
@@ -729,19 +731,21 @@ abstract class ImageProvider {
         }
 
         val originalFileSize = File(path).length()
-        val videoSize = MultiPage.getMotionPhotoOffset(context, uri, mimeType, originalFileSize)?.let { it.toInt() + trailerDiff }
+        var trailerVideoBytes: ByteArray? = null
-        var videoBytes: ByteArray? = null
         val editableFile = StorageUtils.createTempFile(context).apply {
+            val videoSize = MultiPage.getMotionPhotoVideoSize(context, uri, mimeType, originalFileSize)?.let { it + trailerDiff }
+            val isTrailerVideoValid = videoSize != null && MultiPage.getTrailerVideoInfo(context, uri, originalFileSize, videoSize) != null
             try {
-                if (videoSize != null) {
+                if (videoSize != null && isTrailerVideoValid) {
                     // handle motion photo and embedded video separately
                     val imageSize = (originalFileSize - videoSize).toInt()
-                    videoBytes = ByteArray(videoSize)
+                    val videoByteSize = videoSize.toInt()
+                    trailerVideoBytes = ByteArray(videoByteSize)
 
                     StorageUtils.openInputStream(context, uri)?.let { input ->
                         val imageBytes = ByteArray(imageSize)
                         input.read(imageBytes, 0, imageSize)
-                        input.read(videoBytes, 0, videoSize)
+                        input.read(trailerVideoBytes, 0, videoByteSize)
 
                         // copy only the image to a temporary file for editing
                         // video will be appended after metadata modification
@@ -777,15 +781,15 @@ abstract class ImageProvider {
                 }
             }
 
-            if (videoBytes != null) {
+            if (trailerVideoBytes != null) {
                 // append trailer video, if any
-                editableFile.appendBytes(videoBytes!!)
+                editableFile.appendBytes(trailerVideoBytes!!)
             }
 
             // copy the edited temporary file back to the original
             editableFile.transferTo(outputStream(context, mimeType, uri, path))
 
-            if (autoCorrectTrailerOffset && !checkTrailerOffset(context, path, uri, mimeType, videoSize, editableFile, callback)) {
+            if (autoCorrectTrailerOffset && !checkTrailerOffset(context, path, uri, mimeType, trailerVideoBytes?.size, editableFile, callback)) {
                 return false
             }
             editableFile.delete()
@@ -895,7 +899,7 @@ abstract class ImageProvider {
         }
 
         val originalFileSize = File(path).length()
-        val videoSize = MultiPage.getMotionPhotoOffset(context, uri, mimeType, originalFileSize)?.let { it.toInt() + trailerDiff }
+        val videoSize = MultiPage.getMotionPhotoVideoSize(context, uri, mimeType, originalFileSize)?.let { it.toInt() + trailerDiff }
         val editableFile = StorageUtils.createTempFile(context).apply {
             try {
                 editXmpWithPixy(
@@ -978,7 +982,7 @@ abstract class ImageProvider {
         path: String,
         uri: Uri,
         mimeType: String,
-        trailerOffset: Int?,
+        trailerOffset: Number?,
         editedFile: File,
         callback: ImageOpCallback,
     ): Boolean {
@@ -993,7 +997,7 @@ abstract class ImageProvider {
             LOG_TAG, "Edited file length=$expectedLength does not match final document file length=$actualLength. " +
                     "We need to edit XMP to adjust trailer video offset by $diff bytes."
         )
-        val newTrailerOffset = trailerOffset + diff
+        val newTrailerOffset = trailerOffset.toLong() + diff
         return editXmp(context, path, uri, mimeType, callback, trailerDiff = diff, editCoreXmp = { xmp ->
             GoogleXMP.updateTrailingVideoOffset(xmp, trailerOffset, newTrailerOffset)
         })
@@ -1258,12 +1262,18 @@ abstract class ImageProvider {
         callback: ImageOpCallback,
     ) {
         val originalFileSize = File(path).length()
-        val videoSize = MultiPage.getMotionPhotoOffset(context, uri, mimeType, originalFileSize)?.toInt()
+        val videoSize = MultiPage.getMotionPhotoVideoSize(context, uri, mimeType, originalFileSize)
         if (videoSize == null) {
             callback.onFailure(Exception("failed to get trailer video size"))
             return
         }
 
+        val isTrailerVideoValid = MultiPage.getTrailerVideoInfo(context, uri, fileSizeBytes = originalFileSize, videoSizeBytes = videoSize) != null
+        if (!isTrailerVideoValid) {
+            callback.onFailure(Exception("failed to open trailer video with size=$videoSize"))
+            return
+        }
+
         val editableFile = StorageUtils.createTempFile(context).apply {
             try {
                 val inputStream = StorageUtils.openInputStream(context, uri)
@@ -1303,7 +1313,8 @@ abstract class ImageProvider {
         }
 
         val originalFileSize = File(path).length()
-        val videoSize = MultiPage.getMotionPhotoOffset(context, uri, mimeType, originalFileSize)?.toInt()
+        val videoSize = MultiPage.getMotionPhotoVideoSize(context, uri, mimeType, originalFileSize)
+        val isTrailerVideoValid = videoSize != null && MultiPage.getTrailerVideoInfo(context, uri, originalFileSize, videoSize) != null
         val editableFile = StorageUtils.createTempFile(context).apply {
             try {
                 outputStream().use { output ->
@@ -1323,7 +1334,7 @@ abstract class ImageProvider {
             // copy the edited temporary file back to the original
             editableFile.transferTo(outputStream(context, mimeType, uri, path))
 
-            if (!types.contains(TYPE_XMP) && !checkTrailerOffset(context, path, uri, mimeType, videoSize, editableFile, callback)) {
+            if (!types.contains(TYPE_XMP) && isTrailerVideoValid && !checkTrailerOffset(context, path, uri, mimeType, videoSize, editableFile, callback)) {
                 return
             }
             editableFile.delete()