Merge pull request #2058 from element-hq/feature/bma/optionalThumbnails

Optional thumbnails

.idea/dictionaries/shared.xml

@@ -2,6 +2,7 @@
     <dictionary name="shared">
         <words>
             <w>backstack</w>
+            <w>blurhash</w>
             <w>ftue</w>
             <w>homeserver</w>
             <w>konsist</w>

libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt

@@ -100,9 +100,9 @@ interface MatrixRoom : Closeable {
 
     suspend fun redactEvent(eventId: EventId, reason: String? = null): Result<Unit>
 
-    suspend fun sendImage(file: File, thumbnailFile: File, imageInfo: ImageInfo, progressCallback: ProgressCallback?): Result<MediaUploadHandler>
+    suspend fun sendImage(file: File, thumbnailFile: File?, imageInfo: ImageInfo, progressCallback: ProgressCallback?): Result<MediaUploadHandler>
 
-    suspend fun sendVideo(file: File, thumbnailFile: File, videoInfo: VideoInfo, progressCallback: ProgressCallback?): Result<MediaUploadHandler>
+    suspend fun sendVideo(file: File, thumbnailFile: File?, videoInfo: VideoInfo, progressCallback: ProgressCallback?): Result<MediaUploadHandler>
 
     suspend fun sendAudio(file: File, audioInfo: AudioInfo, progressCallback: ProgressCallback?): Result<MediaUploadHandler>
 

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt

@@ -360,15 +360,25 @@ class RustMatrixRoom(
         }
     }
 
-    override suspend fun sendImage(file: File, thumbnailFile: File, imageInfo: ImageInfo, progressCallback: ProgressCallback?): Result<MediaUploadHandler> {
+    override suspend fun sendImage(
-        return sendAttachment(listOf(file, thumbnailFile)) {
+        file: File,
-            innerTimeline.sendImage(file.path, thumbnailFile.path, imageInfo.map(), progressCallback?.toProgressWatcher())
+        thumbnailFile: File?,
+        imageInfo: ImageInfo,
+        progressCallback: ProgressCallback?,
+    ): Result<MediaUploadHandler> {
+        return sendAttachment(listOfNotNull(file, thumbnailFile)) {
+            innerTimeline.sendImage(file.path, thumbnailFile?.path, imageInfo.map(), progressCallback?.toProgressWatcher())
         }
     }
 
-    override suspend fun sendVideo(file: File, thumbnailFile: File, videoInfo: VideoInfo, progressCallback: ProgressCallback?): Result<MediaUploadHandler> {
+    override suspend fun sendVideo(
-        return sendAttachment(listOf(file, thumbnailFile)) {
+        file: File,
-            innerTimeline.sendVideo(file.path, thumbnailFile.path, videoInfo.map(), progressCallback?.toProgressWatcher())
+        thumbnailFile: File?,
+        videoInfo: VideoInfo,
+        progressCallback: ProgressCallback?,
+    ): Result<MediaUploadHandler> {
+        return sendAttachment(listOfNotNull(file, thumbnailFile)) {
+            innerTimeline.sendVideo(file.path, thumbnailFile?.path, videoInfo.map(), progressCallback?.toProgressWatcher())
         }
     }
 

libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt

@@ -299,14 +299,14 @@ class FakeMatrixRoom(
 
     override suspend fun sendImage(
         file: File,
-        thumbnailFile: File,
+        thumbnailFile: File?,
         imageInfo: ImageInfo,
         progressCallback: ProgressCallback?
     ): Result<MediaUploadHandler> = fakeSendMedia(progressCallback)
 
     override suspend fun sendVideo(
         file: File,
-        thumbnailFile: File,
+        thumbnailFile: File?,
         videoInfo: VideoInfo,
         progressCallback: ProgressCallback?
     ): Result<MediaUploadHandler> = fakeSendMedia(

libraries/mediaupload/api/src/main/kotlin/io/element/android/libraries/mediaupload/api/MediaSender.kt

@@ -101,7 +101,6 @@ class MediaSender @Inject constructor(
                     progressCallback = progressCallback
                 )
             }
-
             is MediaUploadInfo.Video -> {
                 sendVideo(
                     file = uploadInfo.file,

libraries/mediaupload/api/src/main/kotlin/io/element/android/libraries/mediaupload/api/MediaUploadInfo.kt

@@ -26,8 +26,8 @@ sealed interface MediaUploadInfo {
 
     val file: File
 
-    data class Image(override val file: File, val imageInfo: ImageInfo, val thumbnailFile: File) : MediaUploadInfo
+    data class Image(override val file: File, val imageInfo: ImageInfo, val thumbnailFile: File?) : MediaUploadInfo
-    data class Video(override val file: File, val videoInfo: VideoInfo, val thumbnailFile: File) : MediaUploadInfo
+    data class Video(override val file: File, val videoInfo: VideoInfo, val thumbnailFile: File?) : MediaUploadInfo
     data class Audio(override val file: File, val audioInfo: AudioInfo) : MediaUploadInfo
     data class VoiceMessage(override val file: File, val audioInfo: AudioInfo, val waveform: List<Float>) : MediaUploadInfo
     data class AnyFile(override val file: File, val fileInfo: FileInfo) : MediaUploadInfo

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

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-package io.element.android.libraries.mediaupload
+package io.element.android.libraries.mediaupload.impl
 
 import android.content.Context
 import android.graphics.BitmapFactory
@@ -137,7 +137,7 @@ class AndroidMediaPreProcessor @Inject constructor(
                 resizeMode = ResizeMode.Approximate(IMAGE_SCALE_REF_SIZE, IMAGE_SCALE_REF_SIZE),
                 orientation = orientation,
             ).getOrThrow()
-            val thumbnailResult: ThumbnailResult = thumbnailFactory.createImageThumbnail(compressionResult.file)
+            val thumbnailResult = thumbnailFactory.createImageThumbnail(compressionResult.file)
             val imageInfo = compressionResult.toImageInfo(
                 mimeType = mimeType,
                 thumbnailResult = thumbnailResult
@@ -146,13 +146,13 @@ class AndroidMediaPreProcessor @Inject constructor(
             return MediaUploadInfo.Image(
                 file = compressionResult.file,
                 imageInfo = imageInfo,
-                thumbnailFile = thumbnailResult.file
+                thumbnailFile = thumbnailResult?.file
             )
         }
 
         suspend fun processImageWithoutCompression(): MediaUploadInfo {
             val file = copyToTmpFile(uri)
-            val thumbnailResult: ThumbnailResult = thumbnailFactory.createImageThumbnail(file)
+            val thumbnailResult = thumbnailFactory.createImageThumbnail(file)
             val imageInfo = contentResolver.openInputStream(uri).use { input ->
                 val bitmap = BitmapFactory.decodeStream(input, null, null)!!
                 ImageInfo(
@@ -160,16 +160,16 @@ class AndroidMediaPreProcessor @Inject constructor(
                     height = bitmap.height.toLong(),
                     mimetype = mimeType,
                     size = file.length(),
-                    thumbnailInfo = thumbnailResult.info,
+                    thumbnailInfo = thumbnailResult?.info,
                     thumbnailSource = null,
-                    blurhash = thumbnailResult.blurhash,
+                    blurhash = thumbnailResult?.blurhash,
                 )
             }
             removeSensitiveImageMetadata(file)
             return MediaUploadInfo.Image(
                 file = file,
                 imageInfo = imageInfo,
-                thumbnailFile = thumbnailResult.file
+                thumbnailFile = thumbnailResult?.file
             )
         }
 
@@ -197,7 +197,7 @@ class AndroidMediaPreProcessor @Inject constructor(
         return MediaUploadInfo.Video(
             file = resultFile,
             videoInfo = videoInfo,
-            thumbnailFile = thumbnailInfo.file
+            thumbnailFile = thumbnailInfo?.file
         )
     }
 
@@ -235,7 +235,7 @@ class AndroidMediaPreProcessor @Inject constructor(
         }
     }
 
-    private fun extractVideoMetadata(file: File, mimeType: String?, thumbnailResult: ThumbnailResult): VideoInfo =
+    private fun extractVideoMetadata(file: File, mimeType: String?, thumbnailResult: ThumbnailResult?): VideoInfo =
         MediaMetadataRetriever().runAndRelease {
             setDataSource(context, Uri.fromFile(file))
             VideoInfo(
@@ -244,10 +244,10 @@ class AndroidMediaPreProcessor @Inject constructor(
                 height = extractMetadata(MediaMetadataRetriever.METADATA_KEY_VIDEO_HEIGHT)?.toLong() ?: 0L,
                 mimetype = mimeType,
                 size = file.length(),
-                thumbnailInfo = thumbnailResult.info,
+                thumbnailInfo = thumbnailResult?.info,
                 // Will be computed by the rust sdk
                 thumbnailSource = null,
-                blurhash = thumbnailResult.blurhash,
+                blurhash = thumbnailResult?.blurhash,
             )
         }
 
@@ -257,15 +257,15 @@ class AndroidMediaPreProcessor @Inject constructor(
     }
 }
 
-fun ImageCompressionResult.toImageInfo(mimeType: String, thumbnailResult: ThumbnailResult) = ImageInfo(
+private fun ImageCompressionResult.toImageInfo(mimeType: String, thumbnailResult: ThumbnailResult?) = ImageInfo(
     width = width.toLong(),
     height = height.toLong(),
     mimetype = mimeType,
     size = size,
-    thumbnailInfo = thumbnailResult.info,
+    thumbnailInfo = thumbnailResult?.info,
     // Will be computed by the rust sdk
     thumbnailSource = null,
-    blurhash = thumbnailResult.blurhash,
+    blurhash = thumbnailResult?.blurhash,
 )
 
 private fun MediaMetadataRetriever.extractDuration(): Duration {

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

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-package io.element.android.libraries.mediaupload
+package io.element.android.libraries.mediaupload.impl
 
 import android.content.Context
 import android.graphics.Bitmap

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

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-package io.element.android.libraries.mediaupload
+package io.element.android.libraries.mediaupload.impl
 
 import android.annotation.SuppressLint
 import android.content.Context
@@ -34,7 +34,9 @@ import io.element.android.libraries.di.ApplicationContext
 import io.element.android.libraries.matrix.api.media.ThumbnailInfo
 import io.element.android.services.toolbox.api.sdk.BuildVersionSdkIntProvider
 import kotlinx.coroutines.suspendCancellableCoroutine
+import timber.log.Timber
 import java.io.File
+import java.io.IOException
 import javax.inject.Inject
 import kotlin.coroutines.resume
 
@@ -61,26 +63,36 @@ class ThumbnailFactory @Inject constructor(
 ) {
 
     @SuppressLint("NewApi")
-    suspend fun createImageThumbnail(file: File): ThumbnailResult {
+    suspend fun createImageThumbnail(file: File): ThumbnailResult? {
         return createThumbnail { cancellationSignal ->
-            // This API works correctly with GIF
+            try {
-            if (sdkIntProvider.isAtLeast(Build.VERSION_CODES.Q)) {
+                // This API works correctly with GIF
-                ThumbnailUtils.createImageThumbnail(
+                if (sdkIntProvider.isAtLeast(Build.VERSION_CODES.Q)) {
-                    file,
+                    try {
-                    Size(THUMB_MAX_WIDTH, THUMB_MAX_HEIGHT),
+                        ThumbnailUtils.createImageThumbnail(
-                    cancellationSignal
+                            file,
-                )
+                            Size(THUMB_MAX_WIDTH, THUMB_MAX_HEIGHT),
-            } else {
+                            cancellationSignal
-                @Suppress("DEPRECATION")
+                        )
-                ThumbnailUtils.createImageThumbnail(
+                    } catch (ioException: IOException) {
-                    file.path,
+                        Timber.w(ioException, "Failed to create thumbnail for $file")
-                    MediaStore.Images.Thumbnails.MINI_KIND,
+                        null
-                )
+                    }
+                } else {
+                    @Suppress("DEPRECATION")
+                    ThumbnailUtils.createImageThumbnail(
+                        file.path,
+                        MediaStore.Images.Thumbnails.MINI_KIND,
+                    )
+                }
+            } catch (throwable: Throwable) {
+                Timber.w(throwable, "Failed to create thumbnail for $file")
+                null
             }
         }
     }
 
-    suspend fun createVideoThumbnail(file: File): ThumbnailResult {
+    suspend fun createVideoThumbnail(file: File): ThumbnailResult? {
         return createThumbnail {
             MediaMetadataRetriever().runAndRelease {
                 setDataSource(context, file.toUri())
@@ -89,37 +101,38 @@ class ThumbnailFactory @Inject constructor(
         }
     }
 
-    private suspend fun createThumbnail(bitmapFactory: (CancellationSignal) -> Bitmap?): ThumbnailResult = suspendCancellableCoroutine { continuation ->
+    private suspend fun createThumbnail(bitmapFactory: (CancellationSignal) -> Bitmap?): ThumbnailResult? = suspendCancellableCoroutine { continuation ->
         val cancellationSignal = CancellationSignal()
         continuation.invokeOnCancellation {
             cancellationSignal.cancel()
         }
         val bitmapThumbnail: Bitmap? = bitmapFactory(cancellationSignal)
+        if (bitmapThumbnail == null) {
+            continuation.resume(null)
+            return@suspendCancellableCoroutine
+        }
         val thumbnailFile = context.createTmpFile(extension = "jpeg")
         thumbnailFile.outputStream().use { outputStream ->
-            bitmapThumbnail?.compress(Bitmap.CompressFormat.JPEG, 80, outputStream)
+            bitmapThumbnail.compress(Bitmap.CompressFormat.JPEG, 80, outputStream)
-        }
-        val blurhash = bitmapThumbnail?.let {
-            BlurHash.encode(it, 3, 3)
         }
+        val blurhash = BlurHash.encode(bitmapThumbnail, 3, 3)
         val thumbnailResult = ThumbnailResult(
             file = thumbnailFile,
             info = ThumbnailInfo(
-                height = bitmapThumbnail?.height?.toLong(),
+                height = bitmapThumbnail.height.toLong(),
-                width = bitmapThumbnail?.width?.toLong(),
+                width = bitmapThumbnail.width.toLong(),
                 mimetype = MimeTypes.Jpeg,
                 size = thumbnailFile.length()
             ),
             blurhash = blurhash
         )
-        bitmapThumbnail?.recycle()
+        bitmapThumbnail.recycle()
         continuation.resume(thumbnailResult)
-
     }
 }
 
 data class ThumbnailResult(
     val file: File,
     val info: ThumbnailInfo,
-    val blurhash: String?,
+    val blurhash: String,
 )

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

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-package io.element.android.libraries.mediaupload
+package io.element.android.libraries.mediaupload.impl
 
 import android.content.Context
 import android.net.Uri

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

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-package io.element.android.libraries.mediaupload
+package io.element.android.libraries.mediaupload.impl
 
 import android.content.Context
 import android.os.Build
@@ -55,15 +55,11 @@ class AndroidMediaPreProcessorTest {
             deleteOriginal = false,
             compressIfPossible = true,
         )
-        // This is failing for now
-        val error = result.exceptionOrNull()
-        assertThat(error).isInstanceOf(MediaPreProcessor.Failure::class.java)
-        assertThat(error?.cause).isInstanceOf(NullPointerException::class.java)
-        /*
         val data = result.getOrThrow()
         assertThat(data.file.path).endsWith("image.png")
         val info = data as MediaUploadInfo.Image
-        assertThat(info.thumbnailFile).isNull() // TODO Check this
+        // Computing thumbnailFile is failing with Robolectric
+        assertThat(info.thumbnailFile).isNull()
         assertThat(info.imageInfo).isEqualTo(
             ImageInfo(
                 height = 1_178,
@@ -76,7 +72,6 @@ class AndroidMediaPreProcessorTest {
             )
         )
         assertThat(file.exists()).isTrue()
-         */
     }
 
     @Test
@@ -90,15 +85,11 @@ class AndroidMediaPreProcessorTest {
             deleteOriginal = false,
             compressIfPossible = true,
         )
-        // This is not working for now
-        val error = result.exceptionOrNull()
-        assertThat(error).isInstanceOf(MediaPreProcessor.Failure::class.java)
-        assertThat(error?.cause).isInstanceOf(NoSuchMethodError::class.java)
-        /*
         val data = result.getOrThrow()
         assertThat(data.file.path).endsWith("image.png")
         val info = data as MediaUploadInfo.Image
-        assertThat(info.thumbnailFile).isNull() // TODO Check this
+        // Computing thumbnailFile is failing with Robolectric
+        assertThat(info.thumbnailFile).isNull()
         assertThat(info.imageInfo).isEqualTo(
             ImageInfo(
                 height = 1_178,
@@ -111,7 +102,6 @@ class AndroidMediaPreProcessorTest {
             )
         )
         assertThat(file.exists()).isTrue()
-         */
     }
 
     @Test
@@ -266,7 +256,8 @@ class AndroidMediaPreProcessorTest {
         ).getOrThrow()
         assertThat(result.file.path).endsWith("video.mp4")
         val info = result as MediaUploadInfo.Video
-        assertThat(info.thumbnailFile).isNotNull()
+        // Computing thumbnailFile is failing with Robolectric
+        assertThat(info.thumbnailFile).isNull()
         assertThat(info.videoInfo).isEqualTo(
             VideoInfo(
                 duration = Duration.ZERO, // Not available with Robolectric?
@@ -274,7 +265,7 @@ class AndroidMediaPreProcessorTest {
                 width = 0, // Not available with Robolectric?
                 mimetype = MimeTypes.Mp4,
                 size = 1_673_712,
-                thumbnailInfo = ThumbnailInfo(height = null, width = null, mimetype = MimeTypes.Jpeg, size = 0), // Not available with Robolectric?
+                thumbnailInfo = null,
                 thumbnailSource = null,
                 blurhash = null,
             )