Don't compress images sent through the Files attachment picker (#6755)
* Don't compress images sent through the Files attachment picker Images and videos picked through the "Attachment" picker are now uploaded without re-encoding, regardless of the "Optimize media quality" setting. The gallery and camera pickers keep the existing behaviour, matching what Element Web/Desktop and most other messengers do. Fixes #6365 Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Make sure we select the right video preset for sending as file Wait for the video size estimations to be calculated before preprocessing the video file --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Jorge Martín <jorgem@element.io>
This commit is contained in:
cizra
GitHub
parent
933b18f6c2
commit
a33d717aa0
12 changed files with 357 additions and 30 deletions
|
|
@ -16,5 +16,12 @@ import kotlinx.parcelize.Parcelize
|
|||
@Immutable
|
||||
sealed interface Attachment : Parcelable {
|
||||
@Parcelize
|
||||
data class Media(val localMedia: LocalMedia) : Attachment
|
||||
data class Media(
|
||||
val localMedia: LocalMedia,
|
||||
// When true, the media was picked through the "Files" picker and should be
|
||||
// uploaded without image recompression; videos still use the highest available
|
||||
// / best-fit preset rather than an additional size-reduction optimization pass.
|
||||
// See https://github.com/element-hq/element-x-android/issues/6365
|
||||
val sendAsFile: Boolean = false,
|
||||
) : Attachment
|
||||
}
|
||||
|
|
|
|||
|
|
@ -23,6 +23,8 @@ import dev.zacsweers.metro.AssistedFactory
|
|||
import dev.zacsweers.metro.AssistedInject
|
||||
import io.element.android.features.messages.impl.attachments.Attachment
|
||||
import io.element.android.features.messages.impl.attachments.video.MediaOptimizationSelectorPresenter
|
||||
import io.element.android.features.messages.impl.attachments.video.MediaOptimizationSelectorState
|
||||
import io.element.android.features.messages.impl.attachments.video.VideoCompressionPresetSelector
|
||||
import io.element.android.libraries.androidutils.file.TemporaryUriDeleter
|
||||
import io.element.android.libraries.androidutils.file.safeDelete
|
||||
import io.element.android.libraries.androidutils.hash.hash
|
||||
|
|
@ -61,6 +63,7 @@ class AttachmentsPreviewPresenter(
|
|||
private val permalinkBuilder: PermalinkBuilder,
|
||||
private val temporaryUriDeleter: TemporaryUriDeleter,
|
||||
private val mediaOptimizationSelectorPresenterFactory: MediaOptimizationSelectorPresenter.Factory,
|
||||
private val videoCompressionPresetSelector: VideoCompressionPresetSelector,
|
||||
@SessionCoroutineScope private val sessionCoroutineScope: CoroutineScope,
|
||||
private val dispatchers: CoroutineDispatchers,
|
||||
private val mediaOptimizationConfigProvider: MediaOptimizationConfigProvider,
|
||||
|
|
@ -96,7 +99,10 @@ class AttachmentsPreviewPresenter(
|
|||
|
||||
val mediaAttachment = attachment as Attachment.Media
|
||||
val mediaOptimizationSelectorPresenter = remember {
|
||||
mediaOptimizationSelectorPresenterFactory.create(mediaAttachment.localMedia)
|
||||
mediaOptimizationSelectorPresenterFactory.create(
|
||||
localMedia = mediaAttachment.localMedia,
|
||||
sendAsFile = mediaAttachment.sendAsFile,
|
||||
)
|
||||
}
|
||||
val mediaOptimizationSelectorState by rememberUpdatedState(mediaOptimizationSelectorPresenter.present())
|
||||
|
||||
|
|
@ -104,14 +110,25 @@ class AttachmentsPreviewPresenter(
|
|||
|
||||
var displayFileTooLargeError by remember { mutableStateOf(false) }
|
||||
|
||||
LaunchedEffect(mediaOptimizationSelectorState.displayMediaSelectorViews) {
|
||||
LaunchedEffect(
|
||||
mediaOptimizationSelectorState.displayMediaSelectorViews,
|
||||
mediaOptimizationSelectorState.videoSizeEstimations,
|
||||
) {
|
||||
// If the media optimization selector is not displayed, we can pre-process the media
|
||||
// to prepare it for sending. This is done to avoid blocking the UI thread when the
|
||||
// user clicks on the send button.
|
||||
if (mediaOptimizationSelectorState.displayMediaSelectorViews == false) {
|
||||
preprocessMediaJob = preProcessAttachment(
|
||||
if (mediaOptimizationSelectorState.displayMediaSelectorViews == false && preprocessMediaJob == null) {
|
||||
if (mediaAttachment.localMedia.info.mimeType.isMimeTypeVideo() && mediaOptimizationSelectorState.videoSizeEstimations.dataOrNull() == null) {
|
||||
Timber.d("Waiting for video size estimations to be able to select the best video compression preset before pre-processing the media")
|
||||
return@LaunchedEffect
|
||||
}
|
||||
val config = getAutoPreprocessMediaOptimizationConfig(
|
||||
mediaAttachment = mediaAttachment,
|
||||
mediaOptimizationSelectorState = mediaOptimizationSelectorState,
|
||||
) ?: return@LaunchedEffect
|
||||
preprocessMediaJob = coroutineScope.preProcessAttachment(
|
||||
attachment = attachment,
|
||||
mediaOptimizationConfig = mediaOptimizationConfigProvider.get(),
|
||||
mediaOptimizationConfig = config,
|
||||
displayProgress = false,
|
||||
sendActionState = sendActionState,
|
||||
)
|
||||
|
|
@ -233,6 +250,28 @@ class AttachmentsPreviewPresenter(
|
|||
)
|
||||
}
|
||||
|
||||
private suspend fun getAutoPreprocessMediaOptimizationConfig(
|
||||
mediaAttachment: Attachment.Media,
|
||||
mediaOptimizationSelectorState: MediaOptimizationSelectorState,
|
||||
): MediaOptimizationConfig? {
|
||||
return if (mediaAttachment.sendAsFile) {
|
||||
// If we're sending the media as a file, we can skip image compression and we should select the highest video compression preset that still fits
|
||||
// the upload limit (if the estimations are available)
|
||||
val videoCompressionPreset = videoCompressionPresetSelector.selectBestVideoPreset(
|
||||
expectedVideoPreset = VideoCompressionPreset.HIGH,
|
||||
videoSizeEstimations = mediaOptimizationSelectorState.videoSizeEstimations,
|
||||
).dataOrNull() ?: VideoCompressionPreset.HIGH
|
||||
|
||||
MediaOptimizationConfig(
|
||||
compressImages = false,
|
||||
videoCompressionPreset = videoCompressionPreset,
|
||||
)
|
||||
} else {
|
||||
// Otherwise, we just rely on the user preferences for media optimization
|
||||
mediaOptimizationConfigProvider.get()
|
||||
}
|
||||
}
|
||||
|
||||
private fun CoroutineScope.preProcessAttachment(
|
||||
attachment: Attachment,
|
||||
mediaOptimizationConfig: MediaOptimizationConfig,
|
||||
|
|
|
|||
|
|
@ -37,9 +37,11 @@ import kotlin.math.roundToLong
|
|||
@AssistedInject
|
||||
class DefaultMediaOptimizationSelectorPresenter(
|
||||
@Assisted private val localMedia: LocalMedia,
|
||||
@Assisted private val sendAsFile: Boolean,
|
||||
private val maxUploadSizeProvider: MaxUploadSizeProvider,
|
||||
private val featureFlagService: FeatureFlagService,
|
||||
private val mediaOptimizationConfigProvider: MediaOptimizationConfigProvider,
|
||||
private val videoCompressionPresetSelector: VideoCompressionPresetSelector,
|
||||
mediaExtractorFactory: VideoMetadataExtractor.Factory,
|
||||
) : MediaOptimizationSelectorPresenter {
|
||||
@ContributesBinding(SessionScope::class)
|
||||
|
|
@ -47,6 +49,7 @@ class DefaultMediaOptimizationSelectorPresenter(
|
|||
interface Factory : MediaOptimizationSelectorPresenter.Factory {
|
||||
override fun create(
|
||||
localMedia: LocalMedia,
|
||||
sendAsFile: Boolean,
|
||||
): DefaultMediaOptimizationSelectorPresenter
|
||||
}
|
||||
|
||||
|
|
@ -55,7 +58,9 @@ class DefaultMediaOptimizationSelectorPresenter(
|
|||
@Composable
|
||||
override fun present(): MediaOptimizationSelectorState {
|
||||
val displayMediaSelectorViews by produceState<Boolean?>(null) {
|
||||
value = featureFlagService.isFeatureEnabled(FeatureFlags.SelectableMediaQuality)
|
||||
// When sending as a raw file, never show the optimization selector: images skip
|
||||
// recompression, while videos use the highest available best-fit preset.
|
||||
value = !sendAsFile && featureFlagService.isFeatureEnabled(FeatureFlags.SelectableMediaQuality)
|
||||
}
|
||||
|
||||
var displayVideoPresetSelectorDialog by remember { mutableStateOf(false) }
|
||||
|
|
@ -123,12 +128,23 @@ class DefaultMediaOptimizationSelectorPresenter(
|
|||
var selectedVideoOptimizationPreset by remember { mutableStateOf<AsyncData<VideoCompressionPreset>>(AsyncData.Loading()) }
|
||||
|
||||
LaunchedEffect(videoSizeEstimations.dataOrNull()) {
|
||||
if (sendAsFile) {
|
||||
// Send-as-file path: pin to no image compression, and pick the highest-quality
|
||||
// video preset that still fits the upload limit (we have no true "do not re-encode
|
||||
// video" path in the pre-processor right now).
|
||||
selectedImageOptimization = AsyncData.Success(false)
|
||||
selectedVideoOptimizationPreset = videoCompressionPresetSelector.selectBestVideoPreset(
|
||||
expectedVideoPreset = VideoCompressionPreset.HIGH,
|
||||
videoSizeEstimations = videoSizeEstimations,
|
||||
)
|
||||
return@LaunchedEffect
|
||||
}
|
||||
val mediaOptimizationConfig = mediaOptimizationConfigProvider.get()
|
||||
selectedImageOptimization = AsyncData.Success(mediaOptimizationConfig.compressImages)
|
||||
// Find the best video preset based on the default preset and the video size estimations
|
||||
// Since the estimation for the current preset may be way too large to upload, we check the ones that provide lower file sizes
|
||||
selectedVideoOptimizationPreset = findBestVideoPreset(
|
||||
defaultVideoPreset = mediaOptimizationConfig.videoCompressionPreset,
|
||||
selectedVideoOptimizationPreset = videoCompressionPresetSelector.selectBestVideoPreset(
|
||||
expectedVideoPreset = mediaOptimizationConfig.videoCompressionPreset,
|
||||
videoSizeEstimations = videoSizeEstimations,
|
||||
)
|
||||
}
|
||||
|
|
@ -176,20 +192,4 @@ class DefaultMediaOptimizationSelectorPresenter(
|
|||
eventSink = ::handleEvent,
|
||||
)
|
||||
}
|
||||
|
||||
private fun findBestVideoPreset(
|
||||
defaultVideoPreset: VideoCompressionPreset,
|
||||
videoSizeEstimations: AsyncData<ImmutableList<VideoUploadEstimation>>,
|
||||
): AsyncData<VideoCompressionPreset> {
|
||||
val estimations = videoSizeEstimations.dataOrNull() ?: return AsyncData.Loading()
|
||||
// This will find the best video preset that can be used to produce a video that can be uploaded
|
||||
val bestEstimation = estimations.find { it.preset.ordinal >= defaultVideoPreset.ordinal && it.canUpload }?.preset
|
||||
return if (bestEstimation != null) {
|
||||
AsyncData.Success(bestEstimation)
|
||||
} else {
|
||||
AsyncData.Failure(
|
||||
IllegalStateException("No suitable video preset found for default preset: $defaultVideoPreset")
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ fun interface MediaOptimizationSelectorPresenter : Presenter<MediaOptimizationSe
|
|||
interface Factory {
|
||||
fun create(
|
||||
localMedia: LocalMedia,
|
||||
sendAsFile: Boolean,
|
||||
): MediaOptimizationSelectorPresenter
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,31 @@
|
|||
/*
|
||||
* Copyright (c) 2026 Element Creations 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.features.messages.impl.attachments.video
|
||||
|
||||
import dev.zacsweers.metro.Inject
|
||||
import io.element.android.libraries.architecture.AsyncData
|
||||
import io.element.android.libraries.preferences.api.store.VideoCompressionPreset
|
||||
import kotlinx.collections.immutable.ImmutableList
|
||||
|
||||
@Inject
|
||||
class VideoCompressionPresetSelector {
|
||||
fun selectBestVideoPreset(
|
||||
expectedVideoPreset: VideoCompressionPreset,
|
||||
videoSizeEstimations: AsyncData<ImmutableList<VideoUploadEstimation>>,
|
||||
): AsyncData<VideoCompressionPreset> {
|
||||
val estimations = videoSizeEstimations.dataOrNull() ?: return AsyncData.Loading()
|
||||
val bestEstimation = estimations.find { it.preset.ordinal >= expectedVideoPreset.ordinal && it.canUpload }?.preset
|
||||
return if (bestEstimation != null) {
|
||||
AsyncData.Success(bestEstimation)
|
||||
} else {
|
||||
AsyncData.Failure(
|
||||
IllegalStateException("No suitable video preset found for expected preset: $expectedVideoPreset")
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -179,7 +179,7 @@ class MessageComposerPresenter(
|
|||
handlePickedMedia(uri, mimeType)
|
||||
}
|
||||
val filesPicker = mediaPickerProvider.registerFilePicker(AnyMimeTypes) { uri, mimeType ->
|
||||
handlePickedMedia(uri, mimeType ?: MimeTypes.OctetStream)
|
||||
handlePickedMedia(uri, mimeType ?: MimeTypes.OctetStream, sendAsFile = true)
|
||||
}
|
||||
val cameraPhotoPicker = mediaPickerProvider.registerCameraPhotoPicker { uri ->
|
||||
handlePickedMedia(uri, MimeTypes.Jpeg)
|
||||
|
|
@ -605,6 +605,7 @@ class MessageComposerPresenter(
|
|||
private fun handlePickedMedia(
|
||||
uri: Uri?,
|
||||
mimeType: String? = null,
|
||||
sendAsFile: Boolean = false,
|
||||
) {
|
||||
uri ?: return
|
||||
val localMedia = localMediaFactory.createFromUri(
|
||||
|
|
@ -613,7 +614,7 @@ class MessageComposerPresenter(
|
|||
name = null,
|
||||
formattedFileSize = null
|
||||
)
|
||||
val mediaAttachment = Attachment.Media(localMedia)
|
||||
val mediaAttachment = Attachment.Media(localMedia, sendAsFile = sendAsFile)
|
||||
val inReplyToEventId = (messageComposerContext.composerMode as? MessageComposerMode.Reply)?.eventId
|
||||
navigator.navigateToPreviewAttachments(persistentListOf(mediaAttachment), inReplyToEventId)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue