Keep video rotation metadata when transcoding (#5008)

Author: jmartinesp

libraries/mediaupload/impl/src/main/kotlin/io/element/android/libraries/mediaupload/impl/VideoCompressor.kt

@@ -90,6 +90,7 @@ class VideoCompressor @Inject constructor(
                 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 rotation = it.extractMetadata(MediaMetadataRetriever.METADATA_KEY_VIDEO_ROTATION)?.toIntOrNull() ?: 0
 
                 val (actualWidth, actualHeight) = if (width == -1 || height == -1) {
                     // Try getting the first frame instead
@@ -103,7 +104,8 @@ class VideoCompressor @Inject constructor(
                     width = actualWidth,
                     height = actualHeight,
                     bitrate = bitrate,
-                    frameRate = framerate
+                    frameRate = framerate,
+                    rotation = rotation,
                 )
             }
         }.onFailure {
@@ -113,10 +115,11 @@ class VideoCompressor @Inject constructor(
 }
 
 internal data class VideoFileMetadata(
-    val width: Int?,
-    val height: Int?,
-    val bitrate: Long?,
-    val frameRate: Int?,
+    val width: Int,
+    val height: Int,
+    val bitrate: Long,
+    val frameRate: Int,
+    val rotation: Int,
 )
 
 sealed interface VideoTranscodingEvent {
@@ -136,10 +139,11 @@ internal object VideoStrategyFactory {
         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
+        val width = metadata?.width?.takeIf { it >= 0 } ?: Int.MAX_VALUE
+        val height = metadata?.height?.takeIf { it >= 0 } ?: Int.MAX_VALUE
+        val bitrate = metadata?.bitrate?.takeIf { it >= 0 }
+        val frameRate = metadata?.frameRate?.takeIf { it >= 0 }
+        val rotation = metadata?.rotation?.takeIf { it >= 0 }
 
         // We only create a resizer if needed
         val resizer = when {
@@ -148,8 +152,9 @@ internal object VideoStrategyFactory {
             else -> null
         }
 
-        return if (resizer == null && expectedExtension == MP4_EXTENSION) {
+        return if (resizer == null && rotation == 0 && 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
+            // Rotation is not kept by the PassThroughTrackStrategy, so we need to ensure the video is not rotated
             PassThroughTrackStrategy()
         } else {
             DefaultVideoStrategy.Builder()

libraries/mediaupload/impl/src/test/kotlin/io/element/android/libraries/mediaupload/impl/VideoStrategyFactoryTest.kt

@@ -39,7 +39,7 @@ class VideoStrategyFactoryTest {
     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 metadata = VideoFileMetadata(width = 1920, height = 1080, bitrate = 1_000_000, frameRate = 50, rotation = 0)
         val shouldBeCompressed = true
 
         // When
@@ -57,7 +57,7 @@ class VideoStrategyFactoryTest {
     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 metadata = VideoFileMetadata(width = 1280, height = 720, bitrate = 1_000_000, frameRate = 50, rotation = 0)
         val shouldBeCompressed = true
 
         // When
@@ -75,7 +75,7 @@ class VideoStrategyFactoryTest {
     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 metadata = VideoFileMetadata(width = 2560, height = 1440, bitrate = 1_000_000, frameRate = 50, rotation = 0)
         val shouldBeCompressed = false
 
         // When
@@ -93,7 +93,7 @@ class VideoStrategyFactoryTest {
     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 metadata = VideoFileMetadata(width = 1920, height = 1080, bitrate = 1_000_000, frameRate = 50, rotation = 0)
         val shouldBeCompressed = false
 
         // When
@@ -111,7 +111,7 @@ class VideoStrategyFactoryTest {
     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 metadata = VideoFileMetadata(width = 320, height = 240, bitrate = 1_000_000, frameRate = 50, rotation = 0)
         val shouldBeCompressed = false
 
         // When
@@ -129,7 +129,7 @@ class VideoStrategyFactoryTest {
     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 metadata = VideoFileMetadata(width = 320, height = 240, bitrate = 1_000_000, frameRate = 50, rotation = 0)
         val shouldBeCompressed = true
 
         // When
@@ -143,6 +143,24 @@ class VideoStrategyFactoryTest {
         assertIsTranscoded(videoStrategy)
     }
 
+    @Test
+    fun `if the video should not be compressed but has a rotation not zero it will be transcoded`() {
+        // Given
+        val expectedExtension = "mp4"
+        val metadata = VideoFileMetadata(width = 320, height = 240, bitrate = 1_000_000, frameRate = 50, rotation = 90)
+        val shouldBeCompressed = false
+
+        // When
+        val videoStrategy = VideoStrategyFactory.create(
+            expectedExtension = expectedExtension,
+            metadata = metadata,
+            shouldBeCompressed = shouldBeCompressed
+        )
+
+        // Then
+        assertIsTranscoded(videoStrategy)
+    }
+
     private inline fun assertIsTranscoded(videoStrategy: TrackStrategy) {
         assert(videoStrategy is DefaultVideoStrategy)
     }