When transcoding a video fails, send it as a file (#4257)

- If the video can't be transcoded it will be uploaded as a file instead. - If the video already has the right format and dimensions, don't transcode it. - Update the dimensions to 720p max when enabling media compression and 1080p otherwise, matching Element X iOS.
2025-05-13 13:04:51 +02:00
parent 6513534ff1
commit 3d64afa937
3 changed files with 277 additions and 28 deletions
--- a/libraries/mediaupload/impl/src/main/kotlin/io/element/android/libraries/mediaupload/impl/AndroidMediaPreProcessor.kt
+++ b/libraries/mediaupload/impl/src/main/kotlin/io/element/android/libraries/mediaupload/impl/AndroidMediaPreProcessor.kt
@@ -188,20 +188,29 @@ class AndroidMediaPreProcessor @Inject constructor(
    }

    private suspend fun processVideo(uri: Uri, mimeType: String?, shouldBeCompressed: Boolean): MediaUploadInfo {
-        val resultFile = videoCompressor.compress(uri, shouldBeCompressed)
-            .onEach {
-                // TODO handle progress
-            }
-            .filterIsInstance<VideoTranscodingEvent.Completed>()
-            .first()
-            .file
-        val thumbnailInfo = thumbnailFactory.createVideoThumbnail(resultFile)
-        val videoInfo = extractVideoMetadata(resultFile, mimeType, thumbnailInfo)
-        return MediaUploadInfo.Video(
-            file = resultFile,
-            videoInfo = videoInfo,
-            thumbnailFile = thumbnailInfo?.file
-        )
+        val resultFile = runCatching {
+            videoCompressor.compress(uri, shouldBeCompressed)
+                .onEach {
+                    // TODO handle progress
+                }
+                .filterIsInstance<VideoTranscodingEvent.Completed>()
+                .first()
+                .file
+        }
+            .getOrNull()
+
+        if (resultFile != null) {
+            val thumbnailInfo = thumbnailFactory.createVideoThumbnail(resultFile)
+            val videoInfo = extractVideoMetadata(resultFile, mimeType, thumbnailInfo)
+            return MediaUploadInfo.Video(
+                file = resultFile,
+                videoInfo = videoInfo,
+                thumbnailFile = thumbnailInfo?.file
+            )
+        } else {
+            // If the video could not be compressed, just use the original one, but send it as a file
+            return processFile(uri, MimeTypes.OctetStream)
+        }
    }

    private suspend fun processAudio(uri: Uri, mimeType: String?): MediaUploadInfo {
--- a/libraries/mediaupload/impl/src/main/kotlin/io/element/android/libraries/mediaupload/impl/VideoCompressor.kt
+++ b/libraries/mediaupload/impl/src/main/kotlin/io/element/android/libraries/mediaupload/impl/VideoCompressor.kt
@@ -8,39 +8,49 @@
 package io.element.android.libraries.mediaupload.impl

 import android.content.Context
+import android.media.MediaMetadataRetriever
 import android.net.Uri
+import android.webkit.MimeTypeMap
 import com.otaliastudios.transcoder.Transcoder
 import com.otaliastudios.transcoder.TranscoderListener
+import com.otaliastudios.transcoder.internal.media.MediaFormatConstants
 import com.otaliastudios.transcoder.resize.AtMostResizer
 import com.otaliastudios.transcoder.strategy.DefaultVideoStrategy
+import com.otaliastudios.transcoder.strategy.PassThroughTrackStrategy
+import com.otaliastudios.transcoder.strategy.TrackStrategy
+import com.otaliastudios.transcoder.validator.WriteAlwaysValidator
 import io.element.android.libraries.androidutils.file.createTmpFile
+import io.element.android.libraries.androidutils.file.getMimeType
 import io.element.android.libraries.androidutils.file.safeDelete
 import io.element.android.libraries.di.ApplicationContext
 import kotlinx.coroutines.channels.awaitClose
 import kotlinx.coroutines.flow.callbackFlow
+import timber.log.Timber
 import java.io.File
 import javax.inject.Inject

+private const val MP4_EXTENSION = "mp4"
+
 class VideoCompressor @Inject constructor(
    @ApplicationContext private val context: Context,
 ) {
    fun compress(uri: Uri, shouldBeCompressed: Boolean) = callbackFlow {
-        val tmpFile = context.createTmpFile(extension = "mp4")
+        val metadata = getVideoMetadata(uri)
+
+        val expectedExtension = MimeTypeMap.getSingleton().getExtensionFromMimeType(context.getMimeType(uri))
+
+        val videoStrategy = VideoStrategyFactory.create(
+            expectedExtension = expectedExtension,
+            metadata = metadata,
+            shouldBeCompressed = shouldBeCompressed
+        )
+
+        val tmpFile = context.createTmpFile(extension = MP4_EXTENSION)
        val future = Transcoder.into(tmpFile.path)
-            .setVideoTrackStrategy(
-                DefaultVideoStrategy.Builder()
-                    .addResizer(
-                        AtMostResizer(
-                            if (shouldBeCompressed) {
-                                720
-                            } else {
-                                1080
-                            }
-                        )
-                    )
-                    .build()
-            )
+            .setVideoTrackStrategy(videoStrategy)
            .addDataSource(context, uri)
+            // Force the output to be written, even if no transcoding was actually needed
+            .setValidator(WriteAlwaysValidator())
            .setListener(object : TranscoderListener {
                override fun onTranscodeProgress(progress: Double) {
                    trySend(VideoTranscodingEvent.Progress(progress.toFloat()))
@@ -69,9 +79,86 @@ class VideoCompressor @Inject constructor(
            }
        }
    }
+
+    private fun getVideoMetadata(uri: Uri): VideoFileMetadata? {
+        return runCatching {
+            MediaMetadataRetriever().use {
+                it.setDataSource(context, uri)
+
+                val width = it.extractMetadata(MediaMetadataRetriever.METADATA_KEY_VIDEO_WIDTH)?.toIntOrNull() ?: -1
+                val height = it.extractMetadata(MediaMetadataRetriever.METADATA_KEY_VIDEO_HEIGHT)?.toIntOrNull() ?: -1
+                val bitrate = it.extractMetadata(MediaMetadataRetriever.METADATA_KEY_BITRATE)?.toLongOrNull() ?: -1
+                val framerate = it.extractMetadata(MediaMetadataRetriever.METADATA_KEY_CAPTURE_FRAMERATE)?.toIntOrNull() ?: -1
+
+                val (actualWidth, actualHeight) = if (width == -1 || height == -1) {
+                    // Try getting the first frame instead
+                    val bitmap = it.getFrameAtTime(0) ?: return null
+                    bitmap.width to bitmap.height
+                } else {
+                    width to height
+                }
+
+                VideoFileMetadata(
+                    width = actualWidth,
+                    height = actualHeight,
+                    bitrate = bitrate,
+                    frameRate = framerate
+                )
+            }
+        }.onFailure {
+            Timber.e(it, "Failed to get video dimensions")
+        }.getOrNull()
+    }
 }

+internal data class VideoFileMetadata(
+    val width: Int?,
+    val height: Int?,
+    val bitrate: Long?,
+    val frameRate: Int?,
+)
+
 sealed interface VideoTranscodingEvent {
    data class Progress(val value: Float) : VideoTranscodingEvent
    data class Completed(val file: File) : VideoTranscodingEvent
 }
+
+internal object VideoStrategyFactory {
+    // 720p
+    private const val MAX_COMPRESSED_PIXEL_SIZE = 1280
+
+    // 1080p
+    private const val MAX_PIXEL_SIZE = 1920
+
+    fun create(
+        expectedExtension: String?,
+        metadata: VideoFileMetadata?,
+        shouldBeCompressed: Boolean,
+    ): TrackStrategy {
+        val width = metadata?.width ?: Int.MAX_VALUE
+        val height = metadata?.height ?: Int.MAX_VALUE
+        val bitrate = metadata?.bitrate
+        val frameRate = metadata?.frameRate
+
+        // We only create a resizer if needed
+        val resizer = when {
+            shouldBeCompressed && (width > MAX_COMPRESSED_PIXEL_SIZE || height > MAX_COMPRESSED_PIXEL_SIZE) -> AtMostResizer(MAX_COMPRESSED_PIXEL_SIZE)
+            width > MAX_PIXEL_SIZE || height > MAX_PIXEL_SIZE -> AtMostResizer(MAX_PIXEL_SIZE)
+            else -> null
+        }
+
+        return if (resizer == null && expectedExtension == MP4_EXTENSION) {
+            // If there's no transcoding or resizing needed for the video file, just create a new file with the same contents but no metadata
+            PassThroughTrackStrategy()
+        } else {
+            DefaultVideoStrategy.Builder()
+                .apply {
+                    resizer?.let { addResizer(it) }
+                    bitrate?.let { bitRate(it) }
+                    frameRate?.let { frameRate(it) }
+                }
+                .mimeType(MediaFormatConstants.MIMETYPE_VIDEO_AVC)
+                .build()
+        }
+    }
+}
--- a/libraries/mediaupload/impl/src/test/kotlin/io/element/android/libraries/mediaupload/impl/VideoStrategyFactoryTest.kt
+++ b/libraries/mediaupload/impl/src/test/kotlin/io/element/android/libraries/mediaupload/impl/VideoStrategyFactoryTest.kt
@@ -0,0 +1,153 @@
+/*
+ * Copyright 2025 New Vector Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
+ * Please see LICENSE files in the repository root for full details.
+ */
+
+package io.element.android.libraries.mediaupload.impl
+
+import com.otaliastudios.transcoder.strategy.DefaultVideoStrategy
+import com.otaliastudios.transcoder.strategy.PassThroughTrackStrategy
+import com.otaliastudios.transcoder.strategy.TrackStrategy
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+
+@Suppress("NOTHING_TO_INLINE")
+@RunWith(RobolectricTestRunner::class)
+class VideoStrategyFactoryTest {
+    @Test
+    fun `if we don't have metadata the video will be transcoded just in case`() {
+        // Given
+        val expectedExtension = "mp4"
+        val metadata = null
+        val shouldBeCompressed = true
+
+        // When
+        val videoStrategy = VideoStrategyFactory.create(
+            expectedExtension = expectedExtension,
+            metadata = metadata,
+            shouldBeCompressed = shouldBeCompressed
+        )
+
+        // Then
+        assertIsTranscoded(videoStrategy)
+    }
+
+    @Test
+    fun `if the video should be compressed and is larger than 720p it will be transcoded`() {
+        // Given
+        val expectedExtension = "mp4"
+        val metadata = VideoFileMetadata(width = 1920, height = 1080, bitrate = 1_000_000, frameRate = 50)
+        val shouldBeCompressed = true
+
+        // When
+        val videoStrategy = VideoStrategyFactory.create(
+            expectedExtension = expectedExtension,
+            metadata = metadata,
+            shouldBeCompressed = shouldBeCompressed
+        )
+
+        // Then
+        assertIsTranscoded(videoStrategy)
+    }
+
+    @Test
+    fun `if the video should be compressed, has the right format and is smaller or equal to 720p it will not be transcoded`() {
+        // Given
+        val expectedExtension = "mp4"
+        val metadata = VideoFileMetadata(width = 1280, height = 720, bitrate = 1_000_000, frameRate = 50)
+        val shouldBeCompressed = true
+
+        // When
+        val videoStrategy = VideoStrategyFactory.create(
+            expectedExtension = expectedExtension,
+            metadata = metadata,
+            shouldBeCompressed = shouldBeCompressed
+        )
+
+        // Then
+        assertIsNotTranscoded(videoStrategy)
+    }
+
+    @Test
+    fun `if the video should not be compressed and is larger than 1080p it will be transcoded`() {
+        // Given
+        val expectedExtension = "mp4"
+        val metadata = VideoFileMetadata(width = 2560, height = 1440, bitrate = 1_000_000, frameRate = 50)
+        val shouldBeCompressed = false
+
+        // When
+        val videoStrategy = VideoStrategyFactory.create(
+            expectedExtension = expectedExtension,
+            metadata = metadata,
+            shouldBeCompressed = shouldBeCompressed
+        )
+
+        // Then
+        assertIsTranscoded(videoStrategy)
+    }
+
+    @Test
+    fun `if the video should not be compressed, has the right format and is smaller or equal than 1080p it will not be transcoded`() {
+        // Given
+        val expectedExtension = "mp4"
+        val metadata = VideoFileMetadata(width = 1920, height = 1080, bitrate = 1_000_000, frameRate = 50)
+        val shouldBeCompressed = false
+
+        // When
+        val videoStrategy = VideoStrategyFactory.create(
+            expectedExtension = expectedExtension,
+            metadata = metadata,
+            shouldBeCompressed = shouldBeCompressed
+        )
+
+        // Then
+        assertIsNotTranscoded(videoStrategy)
+    }
+
+    @Test
+    fun `if the video should not be compressed but has a wrong format it will be transcoded`() {
+        // Given
+        val expectedExtension = "mkv"
+        val metadata = VideoFileMetadata(width = 320, height = 240, bitrate = 1_000_000, frameRate = 50)
+        val shouldBeCompressed = false
+
+        // When
+        val videoStrategy = VideoStrategyFactory.create(
+            expectedExtension = expectedExtension,
+            metadata = metadata,
+            shouldBeCompressed = shouldBeCompressed
+        )
+
+        // Then
+        assertIsTranscoded(videoStrategy)
+    }
+
+    @Test
+    fun `if the video should be compressed and has a wrong format it will be transcoded`() {
+        // Given
+        val expectedExtension = "mkv"
+        val metadata = VideoFileMetadata(width = 320, height = 240, bitrate = 1_000_000, frameRate = 50)
+        val shouldBeCompressed = true
+
+        // When
+        val videoStrategy = VideoStrategyFactory.create(
+            expectedExtension = expectedExtension,
+            metadata = metadata,
+            shouldBeCompressed = shouldBeCompressed
+        )
+
+        // Then
+        assertIsTranscoded(videoStrategy)
+    }
+
+    private inline fun assertIsTranscoded(videoStrategy: TrackStrategy) {
+        assert(videoStrategy is DefaultVideoStrategy)
+    }
+
+    private inline fun assertIsNotTranscoded(videoStrategy: TrackStrategy) {
+        assert(videoStrategy is PassThroughTrackStrategy)
+    }
+}