Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use in-memory thumbnail APIs when possible #3817

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ import io.element.android.libraries.designsystem.components.blurhash.blurHashBac
import io.element.android.libraries.designsystem.preview.ElementPreview
import io.element.android.libraries.designsystem.preview.PreviewsDayNight
import io.element.android.libraries.matrix.api.timeline.item.event.MessageFormat
import io.element.android.libraries.matrix.ui.media.MediaRequestData
import io.element.android.libraries.textcomposer.ElementRichTextEditorStyle
import io.element.android.libraries.ui.strings.CommonStrings
import io.element.android.wysiwyg.compose.EditorStyledText
Expand Down Expand Up @@ -86,13 +85,7 @@ fun TimelineItemImageView(
modifier = Modifier
.fillMaxWidth()
.then(if (isLoaded) Modifier.background(Color.White) else Modifier),
model = MediaRequestData(
source = content.preferredMediaSource,
kind = MediaRequestData.Kind.File(
fileName = content.filename,
mimeType = content.mimeType,
),
),
model = content.thumbnailMediaRequestData,
contentScale = ContentScale.Fit,
alignment = Alignment.Center,
contentDescription = description,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ import io.element.android.libraries.designsystem.modifiers.roundedBackground
import io.element.android.libraries.designsystem.preview.ElementPreview
import io.element.android.libraries.designsystem.preview.PreviewsDayNight
import io.element.android.libraries.matrix.api.timeline.item.event.MessageFormat
import io.element.android.libraries.matrix.ui.media.MAX_THUMBNAIL_HEIGHT
import io.element.android.libraries.matrix.ui.media.MAX_THUMBNAIL_WIDTH
import io.element.android.libraries.matrix.ui.media.MediaRequestData
import io.element.android.libraries.textcomposer.ElementRichTextEditorStyle
import io.element.android.libraries.ui.strings.CommonStrings
Expand Down Expand Up @@ -97,9 +99,9 @@ fun TimelineItemVideoView(
.then(if (isLoaded) Modifier.background(Color.White) else Modifier),
model = MediaRequestData(
source = content.thumbnailSource,
kind = MediaRequestData.Kind.File(
fileName = content.filename,
mimeType = content.mimeType
kind = MediaRequestData.Kind.Thumbnail(
width = content.thumbnailWidth?.toLong() ?: MAX_THUMBNAIL_WIDTH,
height = content.thumbnailHeight?.toLong() ?: MAX_THUMBNAIL_HEIGHT,
)
),
contentScale = ContentScale.Fit,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ class TimelineItemContentMessageFactory @Inject constructor(
blurhash = messageType.info?.blurhash,
width = messageType.info?.width?.toInt(),
height = messageType.info?.height?.toInt(),
thumbnailWidth = messageType.info?.thumbnailInfo?.width?.toInt(),
thumbnailHeight = messageType.info?.thumbnailInfo?.height?.toInt(),
aspectRatio = aspectRatio,
formattedFileSize = fileSizeFormatter.format(messageType.info?.size ?: 0),
fileExtension = fileExtensionExtractor.extractFromName(messageType.filename)
Expand Down Expand Up @@ -146,6 +148,8 @@ class TimelineItemContentMessageFactory @Inject constructor(
mimeType = messageType.info?.mimetype ?: MimeTypes.OctetStream,
width = messageType.info?.width?.toInt(),
height = messageType.info?.height?.toInt(),
thumbnailWidth = messageType.info?.thumbnailInfo?.width?.toInt(),
thumbnailHeight = messageType.info?.thumbnailInfo?.height?.toInt(),
duration = messageType.info?.duration ?: Duration.ZERO,
blurHash = messageType.info?.blurhash,
aspectRatio = aspectRatio,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@

package io.element.android.features.messages.impl.timeline.model.event

import io.element.android.libraries.core.mimetype.MimeTypes
import io.element.android.libraries.core.mimetype.MimeTypes.isMimeTypeAnimatedImage
import io.element.android.libraries.matrix.api.media.MediaSource
import io.element.android.libraries.matrix.api.timeline.item.event.FormattedBody
import io.element.android.libraries.matrix.ui.media.MAX_THUMBNAIL_HEIGHT
import io.element.android.libraries.matrix.ui.media.MAX_THUMBNAIL_WIDTH
import io.element.android.libraries.matrix.ui.media.MediaRequestData

data class TimelineItemImageContent(
override val filename: String,
Expand All @@ -23,15 +26,31 @@
val blurhash: String?,
val width: Int?,
val height: Int?,
val thumbnailWidth: Int?,
val thumbnailHeight: Int?,
val aspectRatio: Float?
) : TimelineItemEventContentWithAttachment {
override val type: String = "TimelineItemImageContent"

val showCaption = caption != null

val preferredMediaSource = if (mimeType == MimeTypes.Gif) {
mediaSource
} else {
thumbnailSource ?: mediaSource
val thumbnailMediaRequestData: MediaRequestData by lazy {
if (mimeType.isMimeTypeAnimatedImage()) {
MediaRequestData(
source = mediaSource,
kind = MediaRequestData.Kind.File(
fileName = filename,
mimeType = mimeType

Check warning on line 43 in features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemImageContent.kt

View check run for this annotation

Codecov / codecov/patch

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/event/TimelineItemImageContent.kt#L39-L43

Added lines #L39 - L43 were not covered by tests
)
)
} else {
MediaRequestData(
source = thumbnailSource ?: mediaSource,
kind = MediaRequestData.Kind.Thumbnail(
width = thumbnailWidth?.toLong() ?: MAX_THUMBNAIL_WIDTH,
height = thumbnailHeight?.toLong() ?: MAX_THUMBNAIL_HEIGHT
),
)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ fun aTimelineItemImageContent(
blurhash = blurhash,
width = null,
height = 300,
thumbnailWidth = null,
thumbnailHeight = 150,
aspectRatio = aspectRatio,
formattedFileSize = "4MB",
fileExtension = "jpg"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ data class TimelineItemVideoContent(
val blurHash: String?,
val height: Int?,
val width: Int?,
val thumbnailWidth: Int?,
val thumbnailHeight: Int?,
val mimeType: String,
val formattedFileSize: String,
val fileExtension: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@ fun aTimelineItemVideoContent(
aspectRatio = aspectRatio,
duration = 100.milliseconds,
videoSource = MediaSource(""),
height = 300,
width = 150,
height = 300,
thumbnailWidth = 150,
thumbnailHeight = 300,
mimeType = MimeTypes.Mp4,
formattedFileSize = "14MB",
fileExtension = "mp4"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,8 @@ class MessagesPresenterTest {
blurhash = null,
width = 20,
height = 20,
thumbnailWidth = null,
thumbnailHeight = null,
aspectRatio = 1.0f,
fileExtension = "jpg",
formattedFileSize = "4MB"
Expand Down Expand Up @@ -384,6 +386,8 @@ class MessagesPresenterTest {
blurHash = null,
width = 20,
height = 20,
thumbnailWidth = 20,
thumbnailHeight = 20,
aspectRatio = 1.0f,
fileExtension = "mp4",
formattedFileSize = "50MB"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,8 @@ class TimelineItemContentMessageFactoryTest {
width = null,
mimeType = MimeTypes.OctetStream,
formattedFileSize = "0 Bytes",
thumbnailWidth = null,
thumbnailHeight = null,
fileExtension = "",
)
assertThat(result).isEqualTo(expected)
Expand Down Expand Up @@ -294,6 +296,8 @@ class TimelineItemContentMessageFactoryTest {
width = 300,
mimeType = MimeTypes.Mp4,
formattedFileSize = "555 Bytes",
thumbnailWidth = 5,
thumbnailHeight = 10,
fileExtension = "mp4",
)
assertThat(result).isEqualTo(expected)
Expand Down Expand Up @@ -458,6 +462,8 @@ class TimelineItemContentMessageFactoryTest {
blurhash = null,
width = null,
height = null,
thumbnailWidth = null,
thumbnailHeight = null,
aspectRatio = null
)
assertThat(result).isEqualTo(expected)
Expand Down Expand Up @@ -531,6 +537,8 @@ class TimelineItemContentMessageFactoryTest {
blurhash = A_BLUR_HASH,
width = 5,
height = 10,
thumbnailWidth = 5,
thumbnailHeight = 10,
aspectRatio = 0.5f,
)
assertThat(result).isEqualTo(expected)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ object MimeTypes {
fun String?.normalizeMimeType() = if (this == BadJpg) Jpeg else this

fun String?.isMimeTypeImage() = this?.startsWith("image/").orFalse()
fun String?.isMimeTypeAnimatedImage() = this == Gif || this == WebP
fun String?.isMimeTypeVideo() = this?.startsWith("video/").orFalse()
fun String?.isMimeTypeAudio() = this?.startsWith("audio/").orFalse()
fun String?.isMimeTypeApplication() = this?.startsWith("application/").orFalse()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class RustMediaLoader(
withContext(mediaDispatcher) {
runCatching {
source.toRustMediaSource().use { source ->
innerClient.getMediaContent(source).toUByteArray().toByteArray()
innerClient.getMediaContent(source)
}
}
}
Expand All @@ -55,7 +55,7 @@ class RustMediaLoader(
mediaSource = mediaSource,
width = width.toULong(),
height = height.toULong()
).toUByteArray().toByteArray()
)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,9 @@ data class MediaRequestData(
}
}
}

/** Max width a thumbnail can have according to [the spec](https://spec.matrix.org/v1.10/client-server-api/#thumbnails). */
const val MAX_THUMBNAIL_WIDTH = 800L

/** Max height a thumbnail can have according to [the spec](https://spec.matrix.org/v1.10/client-server-api/#thumbnails). */
const val MAX_THUMBNAIL_HEIGHT = 600L
Loading