From b18542a839dab54b68fbf38c354fa4208c70ad57 Mon Sep 17 00:00:00 2001 From: Alex Hart Date: Wed, 13 Jul 2022 12:48:17 -0300 Subject: [PATCH] Ensure images sent to stories respect media quality settings. Stories should always use "Standard" quality, not L3 (high quality). This change ensures that we: 1. Always send stories at the appropriate quality 2. Do not corrupt or overwrite pre-existing image attachments 3. Close several streams when done (thanks StrictMode!) --- .../database/AttachmentDatabaseTest.kt | 106 ++++++++++++++++++ .../database/UriAttachmentBuilder.kt | 44 ++++++++ .../forward/MultiselectForwardFragmentArgs.kt | 2 +- .../database/AttachmentDatabase.java | 82 ++++++++++---- .../jobs/AttachmentCompressionJob.java | 1 - .../securesms/jobs/AttachmentUploadJob.java | 22 ++-- .../securesms/mediasend/Media.java | 19 ++++ .../mediasend/v2/MediaSelectionRepository.kt | 12 +- .../mediasend/v2/MediaSelectionViewModel.kt | 2 +- .../v2/review/MediaReviewFragment.kt | 2 +- .../securesms/mms/AttachmentManager.java | 25 +++-- .../securesms/mms/MediaConstraints.java | 4 + .../securesms/mms/PartAuthority.java | 11 ++ .../securesms/mms/PushMediaConstraints.java | 5 + .../securesms/mms/SlideFactory.java | 55 +++++---- .../securesms/sharing/MultiShareSender.java | 31 ++++- .../securesms/sms/MessageSender.java | 2 +- .../thoughtcrime/securesms/stories/Stories.kt | 17 +++ .../SignalServiceAttachmentStream.java | 9 +- .../internal/push/PushServiceSocket.java | 12 +- 20 files changed, 384 insertions(+), 79 deletions(-) create mode 100644 app/src/androidTest/java/org/thoughtcrime/securesms/database/AttachmentDatabaseTest.kt create mode 100644 app/src/androidTest/java/org/thoughtcrime/securesms/database/UriAttachmentBuilder.kt diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/database/AttachmentDatabaseTest.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/database/AttachmentDatabaseTest.kt new file mode 100644 index 000000000..704e6b346 --- /dev/null +++ b/app/src/androidTest/java/org/thoughtcrime/securesms/database/AttachmentDatabaseTest.kt @@ -0,0 +1,106 @@ +package org.thoughtcrime.securesms.database + +import android.net.Uri +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.thoughtcrime.securesms.attachments.UriAttachment +import org.thoughtcrime.securesms.mms.MediaStream +import org.thoughtcrime.securesms.mms.SentMediaQuality +import org.thoughtcrime.securesms.providers.BlobProvider +import org.thoughtcrime.securesms.util.MediaUtil +import java.util.Optional + +@RunWith(AndroidJUnit4::class) +class AttachmentDatabaseTest { + + @Before + fun setUp() { + SignalDatabase.attachments.deleteAllAttachments() + } + + @Test + fun givenABlob_whenIInsert2AttachmentsForPreUpload_thenIExpectDistinctIdsButSameFileName() { + val blob = BlobProvider.getInstance().forData(byteArrayOf(1, 2, 3, 4, 5)).createForSingleSessionInMemory() + val highQualityProperties = createHighQualityTransformProperties() + val highQualityImage = createAttachment(1, blob, highQualityProperties) + val attachment = SignalDatabase.attachments.insertAttachmentForPreUpload(highQualityImage) + val attachment2 = SignalDatabase.attachments.insertAttachmentForPreUpload(highQualityImage) + + assertNotEquals(attachment2.attachmentId, attachment.attachmentId) + assertEquals(attachment2.fileName, attachment.fileName) + } + + @Test + fun givenABlobAndDifferentTransformQuality_whenIInsert2AttachmentsForPreUpload_thenIExpectDifferentFileInfos() { + val blob = BlobProvider.getInstance().forData(byteArrayOf(1, 2, 3, 4, 5)).createForSingleSessionInMemory() + val highQualityProperties = createHighQualityTransformProperties() + val highQualityImage = createAttachment(1, blob, highQualityProperties) + val lowQualityImage = createAttachment(1, blob, AttachmentDatabase.TransformProperties.empty()) + val attachment = SignalDatabase.attachments.insertAttachmentForPreUpload(highQualityImage) + val attachment2 = SignalDatabase.attachments.insertAttachmentForPreUpload(lowQualityImage) + + SignalDatabase.attachments.updateAttachmentData( + attachment, + createMediaStream(byteArrayOf(1, 2, 3, 4, 5)), + false + ) + + SignalDatabase.attachments.updateAttachmentData( + attachment2, + createMediaStream(byteArrayOf(1, 2, 3)), + false + ) + + val attachment1Info = SignalDatabase.attachments.getAttachmentDataFileInfo(attachment.attachmentId, AttachmentDatabase.DATA) + val attachment2Info = SignalDatabase.attachments.getAttachmentDataFileInfo(attachment2.attachmentId, AttachmentDatabase.DATA) + + assertNotEquals(attachment1Info, attachment2Info) + } + + @Test + fun givenIdenticalAttachmentsInsertedForPreUpload_whenIUpdateAttachmentDataAndSpecifyOnlyModifyThisAttachment_thenIExpectDifferentFileInfos() { + val blob = BlobProvider.getInstance().forData(byteArrayOf(1, 2, 3, 4, 5)).createForSingleSessionInMemory() + val highQualityProperties = createHighQualityTransformProperties() + val highQualityImage = createAttachment(1, blob, highQualityProperties) + val attachment = SignalDatabase.attachments.insertAttachmentForPreUpload(highQualityImage) + val attachment2 = SignalDatabase.attachments.insertAttachmentForPreUpload(highQualityImage) + + SignalDatabase.attachments.updateAttachmentData( + attachment, + createMediaStream(byteArrayOf(1, 2, 3, 4, 5)), + true + ) + + SignalDatabase.attachments.updateAttachmentData( + attachment2, + createMediaStream(byteArrayOf(1, 2, 3, 4)), + true + ) + + val attachment1Info = SignalDatabase.attachments.getAttachmentDataFileInfo(attachment.attachmentId, AttachmentDatabase.DATA) + val attachment2Info = SignalDatabase.attachments.getAttachmentDataFileInfo(attachment2.attachmentId, AttachmentDatabase.DATA) + + assertNotEquals(attachment1Info, attachment2Info) + } + + private fun createAttachment(id: Long, uri: Uri, transformProperties: AttachmentDatabase.TransformProperties): UriAttachment { + return UriAttachmentBuilder.build( + id, + uri = uri, + contentType = MediaUtil.IMAGE_JPEG, + transformProperties = transformProperties + ) + } + + private fun createHighQualityTransformProperties(): AttachmentDatabase.TransformProperties { + return AttachmentDatabase.TransformProperties.forSentMediaQuality(Optional.empty(), SentMediaQuality.HIGH) + } + + private fun createMediaStream(byteArray: ByteArray): MediaStream { + return MediaStream(byteArray.inputStream(), MediaUtil.IMAGE_JPEG, 2, 2) + } +} diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/database/UriAttachmentBuilder.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/database/UriAttachmentBuilder.kt new file mode 100644 index 000000000..7068d5ef2 --- /dev/null +++ b/app/src/androidTest/java/org/thoughtcrime/securesms/database/UriAttachmentBuilder.kt @@ -0,0 +1,44 @@ +package org.thoughtcrime.securesms.database + +import android.net.Uri +import org.thoughtcrime.securesms.attachments.UriAttachment +import org.thoughtcrime.securesms.audio.AudioHash +import org.thoughtcrime.securesms.blurhash.BlurHash +import org.thoughtcrime.securesms.stickers.StickerLocator + +object UriAttachmentBuilder { + fun build( + id: Long, + uri: Uri = Uri.parse("content://$id"), + contentType: String, + transferState: Int = AttachmentDatabase.TRANSFER_PROGRESS_PENDING, + size: Long = 0L, + fileName: String = "file$id", + voiceNote: Boolean = false, + borderless: Boolean = false, + videoGif: Boolean = false, + quote: Boolean = false, + caption: String? = null, + stickerLocator: StickerLocator? = null, + blurHash: BlurHash? = null, + audioHash: AudioHash? = null, + transformProperties: AttachmentDatabase.TransformProperties? = null + ): UriAttachment { + return UriAttachment( + uri, + contentType, + transferState, + size, + fileName, + voiceNote, + borderless, + videoGif, + quote, + caption, + stickerLocator, + blurHash, + audioHash, + transformProperties + ) + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/conversation/mutiselect/forward/MultiselectForwardFragmentArgs.kt b/app/src/main/java/org/thoughtcrime/securesms/conversation/mutiselect/forward/MultiselectForwardFragmentArgs.kt index 6bf066a51..4f1984d80 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/conversation/mutiselect/forward/MultiselectForwardFragmentArgs.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/conversation/mutiselect/forward/MultiselectForwardFragmentArgs.kt @@ -170,7 +170,7 @@ data class MultiselectForwardFragmentArgs @JvmOverloads constructor( isVideoGif, Optional.empty(), Optional.ofNullable(caption), - Optional.empty() + Optional.of(transformProperties) ) } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentDatabase.java index dede16d1e..c81d78d5a 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentDatabase.java @@ -790,8 +790,8 @@ public class AttachmentDatabase extends Database { } /** - * @param onlyModifyThisAttachment If false and more than one attachment shares this file, they will all be updated. - * If true, then guarantees not to affect other attachments. + * @param onlyModifyThisAttachment If false and more than one attachment shares this file and quality, they will all + * be updated. If true, then guarantees not to affect other attachments. */ public void updateAttachmentData(@NonNull DatabaseAttachment databaseAttachment, @NonNull MediaStream mediaStream, @@ -807,7 +807,8 @@ public class AttachmentDatabase extends Database { File destination = oldDataInfo.file; - if (onlyModifyThisAttachment) { + boolean isSingleUseOfData = onlyModifyThisAttachment || oldDataInfo.hash == null; + if (isSingleUseOfData) { if (fileReferencedByMoreThanOneAttachment(destination)) { Log.i(TAG, "Creating a new file as this one is used by more than one attachment"); destination = newFile(); @@ -827,7 +828,10 @@ public class AttachmentDatabase extends Database { contentValues.put(DATA_RANDOM, dataInfo.random); contentValues.put(DATA_HASH, dataInfo.hash); - int updateCount = updateAttachmentAndMatchingHashes(database, databaseAttachment.getAttachmentId(), oldDataInfo.hash, contentValues); + int updateCount = updateAttachmentAndMatchingHashes(database, + databaseAttachment.getAttachmentId(), + isSingleUseOfData ? dataInfo.hash : oldDataInfo.hash, + contentValues); Log.i(TAG, "[updateAttachmentData] Updated " + updateCount + " rows."); } @@ -846,10 +850,32 @@ public class AttachmentDatabase extends Database { } public void markAttachmentAsTransformed(@NonNull AttachmentId attachmentId) { - updateAttachmentTransformProperties(attachmentId, TransformProperties.forSkipTransform()); + getWritableDatabase().beginTransaction(); + try { + updateAttachmentTransformProperties(attachmentId, getTransformProperties(attachmentId).withSkipTransform()); + getWritableDatabase().setTransactionSuccessful(); + } catch (Exception e) { + Log.w(TAG, "Could not mark attachment as transformed.", e); + } finally { + getWritableDatabase().endTransaction(); + } } - public void updateAttachmentTransformProperties(@NonNull AttachmentId attachmentId, @NonNull TransformProperties transformProperties) { + public @NonNull TransformProperties getTransformProperties(@NonNull AttachmentId attachmentId) { + String[] projection = SqlUtil.buildArgs(TRANSFORM_PROPERTIES); + String[] args = attachmentId.toStrings(); + + try (Cursor cursor = getWritableDatabase().query(TABLE_NAME, projection, PART_ID_WHERE, args, null, null, null, null)) { + if (cursor.moveToFirst()) { + String serializedProperties = CursorUtil.requireString(cursor, TRANSFORM_PROPERTIES); + return TransformProperties.parse(serializedProperties); + } else { + throw new AssertionError("No such attachment."); + } + } + } + + private void updateAttachmentTransformProperties(@NonNull AttachmentId attachmentId, @NonNull TransformProperties transformProperties) { DataInfo dataInfo = getAttachmentDataFileInfo(attachmentId, DATA); if (dataInfo == null) { @@ -1017,15 +1043,12 @@ public class AttachmentDatabase extends Database { } } - private @Nullable DataInfo getAttachmentDataFileInfo(@NonNull AttachmentId attachmentId, @NonNull String dataType) + @VisibleForTesting + @Nullable DataInfo getAttachmentDataFileInfo(@NonNull AttachmentId attachmentId, @NonNull String dataType) { SQLiteDatabase database = databaseHelper.getSignalReadableDatabase(); - Cursor cursor = null; - - try { - cursor = database.query(TABLE_NAME, new String[]{dataType, SIZE, DATA_RANDOM, DATA_HASH}, PART_ID_WHERE, attachmentId.toStrings(), - null, null, null); + try (Cursor cursor = database.query(TABLE_NAME, new String[] { dataType, SIZE, DATA_RANDOM, DATA_HASH }, PART_ID_WHERE, attachmentId.toStrings(), null, null, null)) { if (cursor != null && cursor.moveToFirst()) { if (cursor.isNull(cursor.getColumnIndexOrThrow(dataType))) { return null; @@ -1038,9 +1061,6 @@ public class AttachmentDatabase extends Database { } else { return null; } - } finally { - if (cursor != null) - cursor.close(); } } @@ -1127,7 +1147,7 @@ public class AttachmentDatabase extends Database { Pair selectorArgs = buildSharedFileSelectorArgs(hash, excludedAttachmentId); try (Cursor cursor = database.query(TABLE_NAME, - new String[]{DATA, DATA_RANDOM, SIZE}, + new String[]{DATA, DATA_RANDOM, SIZE, TRANSFORM_PROPERTIES}, selectorArgs.first, selectorArgs.second, null, @@ -1298,7 +1318,8 @@ public class AttachmentDatabase extends Database { template.getTransferState() == TRANSFER_PROGRESS_DONE && template.getTransformProperties().shouldSkipTransform() && template.getDigest() != null && - !attachment.getTransformProperties().isVideoEdited(); + !attachment.getTransformProperties().isVideoEdited() && + template.getTransformProperties().sentMediaQuality == attachment.getTransformProperties().getSentMediaQuality(); ContentValues contentValues = new ContentValues(); contentValues.put(MMS_ID, mmsId); @@ -1326,7 +1347,7 @@ public class AttachmentDatabase extends Database { contentValues.put(TRANSFORM_PROPERTIES, attachment.getTransformProperties().serialize()); } else { contentValues.put(VISUAL_HASH, getVisualHashStringOrNull(template)); - contentValues.put(TRANSFORM_PROPERTIES, template.getTransformProperties().serialize()); + contentValues.put(TRANSFORM_PROPERTIES, (useTemplateUpload ? template : attachment).getTransformProperties().serialize()); } if (attachment.isSticker()) { @@ -1340,7 +1361,7 @@ public class AttachmentDatabase extends Database { contentValues.put(DATA, dataInfo.file.getAbsolutePath()); contentValues.put(SIZE, dataInfo.length); contentValues.put(DATA_RANDOM, dataInfo.random); - if (attachment.getTransformProperties().isVideoEdited()) { + if (attachment.getTransformProperties().isVideoEdited() || attachment.getTransformProperties().sentMediaQuality != template.getTransformProperties().getSentMediaQuality()) { contentValues.putNull(DATA_HASH); } else { contentValues.put(DATA_HASH, dataInfo.hash); @@ -1408,7 +1429,8 @@ public class AttachmentDatabase extends Database { return EncryptedMediaDataSource.createFor(attachmentSecret, dataInfo.file, dataInfo.random, dataInfo.length); } - private static class DataInfo { + @VisibleForTesting + static class DataInfo { private final File file; private final long length; private final byte[] random; @@ -1420,6 +1442,22 @@ public class AttachmentDatabase extends Database { this.random = random; this.hash = hash; } + + @Override public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + final DataInfo dataInfo = (DataInfo) o; + return length == dataInfo.length && + Objects.equals(file, dataInfo.file) && + Arrays.equals(random, dataInfo.random) && + Objects.equals(hash, dataInfo.hash); + } + + @Override public int hashCode() { + int result = Objects.hash(file, length, hash); + result = 31 * result + Arrays.hashCode(random); + return result; + } } private static final class DataUsageResult { @@ -1520,6 +1558,10 @@ public class AttachmentDatabase extends Database { return sentMediaQuality; } + @NonNull TransformProperties withSkipTransform() { + return new TransformProperties(true, false, 0, 0, sentMediaQuality); + } + @NonNull String serialize() { return JsonUtil.toJson(this); } diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentCompressionJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentCompressionJob.java index dfe752b25..f56328680 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentCompressionJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentCompressionJob.java @@ -49,7 +49,6 @@ import org.thoughtcrime.securesms.video.videoconverter.EncodingException; import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; -import java.io.InputStream; import java.io.OutputStream; import java.util.Objects; import java.util.concurrent.TimeUnit; diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.java index 9ade1ef27..ee98db179 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.java @@ -31,6 +31,7 @@ import org.thoughtcrime.securesms.util.MediaUtil; import org.whispersystems.signalservice.api.SignalServiceMessageSender; import org.whispersystems.signalservice.api.messages.SignalServiceAttachment; import org.whispersystems.signalservice.api.messages.SignalServiceAttachmentPointer; +import org.whispersystems.signalservice.api.messages.SignalServiceAttachmentStream; import org.whispersystems.signalservice.api.push.exceptions.NonSuccessfulResumableUploadResponseCodeException; import org.whispersystems.signalservice.api.push.exceptions.ResumeLocationInvalidException; import org.whispersystems.signalservice.internal.push.http.ResumableUploadSpec; @@ -142,11 +143,12 @@ public final class AttachmentUploadJob extends BaseJob { Log.i(TAG, "Uploading attachment for message " + databaseAttachment.getMmsId() + " with ID " + databaseAttachment.getAttachmentId()); try (NotificationController notification = getNotificationForAttachment(databaseAttachment)) { - SignalServiceAttachment localAttachment = getAttachmentFor(databaseAttachment, notification, resumableUploadSpec); - SignalServiceAttachmentPointer remoteAttachment = messageSender.uploadAttachment(localAttachment.asStream()); - Attachment attachment = PointerAttachment.forPointer(Optional.of(remoteAttachment), null, databaseAttachment.getFastPreflightId()).get(); + try (SignalServiceAttachmentStream localAttachment = getAttachmentFor(databaseAttachment, notification, resumableUploadSpec)) { + SignalServiceAttachmentPointer remoteAttachment = messageSender.uploadAttachment(localAttachment); + Attachment attachment = PointerAttachment.forPointer(Optional.of(remoteAttachment), null, databaseAttachment.getFastPreflightId()).get(); - database.updateAttachmentAfterUpload(databaseAttachment.getAttachmentId(), attachment, remoteAttachment.getUploadTimestamp()); + database.updateAttachmentAfterUpload(databaseAttachment.getAttachmentId(), attachment, remoteAttachment.getUploadTimestamp()); + } } catch (NonSuccessfulResumableUploadResponseCodeException e) { if (e.getCode() == 400) { Log.w(TAG, "Failed to upload due to a 400 when getting resumable upload information. Downgrading to attachments v2", e); @@ -177,9 +179,12 @@ public final class AttachmentUploadJob extends BaseJob { return exception instanceof IOException && !(exception instanceof NotPushRegisteredException); } - private @NonNull SignalServiceAttachment getAttachmentFor(Attachment attachment, @Nullable NotificationController notification, @Nullable ResumableUploadSpec resumableUploadSpec) throws InvalidAttachmentException { + private @NonNull SignalServiceAttachmentStream getAttachmentFor(Attachment attachment, @Nullable NotificationController notification, @Nullable ResumableUploadSpec resumableUploadSpec) throws InvalidAttachmentException { + if (attachment.getUri() == null || attachment.getSize() == 0) { + throw new InvalidAttachmentException(new IOException("Assertion failed, outgoing attachment has no data!")); + } + try { - if (attachment.getUri() == null || attachment.getSize() == 0) throw new IOException("Assertion failed, outgoing attachment has no data!"); InputStream is = PartAuthority.getAttachmentStream(context, attachment.getUri()); SignalServiceAttachment.Builder builder = SignalServiceAttachment.newStreamBuilder() .withStream(is) @@ -208,7 +213,6 @@ public final class AttachmentUploadJob extends BaseJob { } else { return builder.build(); } - } catch (IOException ioe) { throw new InvalidAttachmentException(ioe); } @@ -218,7 +222,9 @@ public final class AttachmentUploadJob extends BaseJob { if (attachment.getBlurHash() != null) return attachment.getBlurHash().getHash(); if (attachment.getUri() == null) return null; - return BlurHashEncoder.encode(PartAuthority.getAttachmentStream(context, attachment.getUri())); + try (InputStream inputStream = PartAuthority.getAttachmentStream(context, attachment.getUri())) { + return BlurHashEncoder.encode(inputStream); + } } private @Nullable String getVideoBlurHash(@NonNull Attachment attachment) throws IOException { diff --git a/app/src/main/java/org/thoughtcrime/securesms/mediasend/Media.java b/app/src/main/java/org/thoughtcrime/securesms/mediasend/Media.java index 70fd23605..d7fee7014 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mediasend/Media.java +++ b/app/src/main/java/org/thoughtcrime/securesms/mediasend/Media.java @@ -7,6 +7,8 @@ import android.os.Parcelable; import androidx.annotation.NonNull; import org.thoughtcrime.securesms.database.AttachmentDatabase; +import org.thoughtcrime.securesms.util.MediaUtil; +import org.whispersystems.signalservice.api.util.Preconditions; import org.whispersystems.signalservice.internal.util.JsonUtil; import java.io.IOException; @@ -194,4 +196,21 @@ public class Media implements Parcelable { media.getCaption(), media.getTransformProperties()); } + + public static @NonNull Media stripTransform(@NonNull Media media) { + Preconditions.checkArgument(MediaUtil.isImageType(media.mimeType)); + + return new Media(media.getUri(), + media.getMimeType(), + media.getDate(), + media.getWidth(), + media.getHeight(), + media.getSize(), + media.getDuration(), + media.isBorderless(), + media.isVideoGif(), + media.getBucketId(), + media.getCaption(), + Optional.empty()); + } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/mediasend/v2/MediaSelectionRepository.kt b/app/src/main/java/org/thoughtcrime/securesms/mediasend/v2/MediaSelectionRepository.kt index cfd91b992..bee88d241 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mediasend/v2/MediaSelectionRepository.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/mediasend/v2/MediaSelectionRepository.kt @@ -127,7 +127,7 @@ class MediaSelectionRepository(context: Context) { ) } - val clippedMediaForStories = if (singleContact?.isStory == true || contacts.any { it.isStory }) { + val clippedVideosForStories: List = if (singleContact?.isStory == true || contacts.any { it.isStory }) { updatedMedia.filter { Stories.MediaTransform.getSendRequirements(it) == Stories.MediaTransform.SendRequirements.REQUIRES_CLIP }.map { media -> @@ -135,12 +135,20 @@ class MediaSelectionRepository(context: Context) { }.flatten() } else emptyList() + val lowResImagesForStories: List = if (singleContact?.isStory == true || contacts.any { it.isStory }) { + updatedMedia.filter { + Stories.MediaTransform.hasHighQualityTransform(it) + }.map { + Media.stripTransform(it) + } + } else emptyList() + uploadRepository.applyMediaUpdates(oldToNewMediaMap, singleRecipient) uploadRepository.updateCaptions(updatedMedia) uploadRepository.updateDisplayOrder(updatedMedia) uploadRepository.getPreUploadResults { uploadResults -> if (contacts.isNotEmpty()) { - sendMessages(contacts, splitBody, uploadResults, trimmedMentions, isViewOnce, clippedMediaForStories) + sendMessages(contacts, splitBody, uploadResults, trimmedMentions, isViewOnce, clippedVideosForStories + lowResImagesForStories) uploadRepository.deleteAbandonedAttachments() emitter.onComplete() } else if (uploadResults.isNotEmpty()) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/mediasend/v2/MediaSelectionViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/mediasend/v2/MediaSelectionViewModel.kt index 397553fb1..624fc3aab 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mediasend/v2/MediaSelectionViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/mediasend/v2/MediaSelectionViewModel.kt @@ -336,7 +336,7 @@ class MediaSelectionViewModel( } val filteredPreUploadMedia = if (Stories.isFeatureEnabled()) { - media.filterNot { Stories.MediaTransform.getSendRequirements(media) == Stories.MediaTransform.SendRequirements.REQUIRES_CLIP } + media.filter { Stories.MediaTransform.canPreUploadMedia(it) } } else { media } diff --git a/app/src/main/java/org/thoughtcrime/securesms/mediasend/v2/review/MediaReviewFragment.kt b/app/src/main/java/org/thoughtcrime/securesms/mediasend/v2/review/MediaReviewFragment.kt index 32a3e7506..a25bfe255 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mediasend/v2/review/MediaReviewFragment.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/mediasend/v2/review/MediaReviewFragment.kt @@ -448,7 +448,7 @@ class MediaReviewFragment : Fragment(R.layout.v2_media_review_fragment) { private fun computeQualityButtonAnimators(state: MediaSelectionState): List { val slide = listOf(MediaReviewAnimatorController.getSlideInAnimator(qualityButton)) - return slide + if (state.isTouchEnabled && state.selectedMedia.any { MediaUtil.isImageType(it.mimeType) }) { + return slide + if (state.isTouchEnabled && !state.isStory && state.selectedMedia.any { MediaUtil.isImageType(it.mimeType) }) { listOf(MediaReviewAnimatorController.getFadeInAnimator(qualityButton)) } else { listOf(MediaReviewAnimatorController.getFadeOutAnimator(qualityButton)) diff --git a/app/src/main/java/org/thoughtcrime/securesms/mms/AttachmentManager.java b/app/src/main/java/org/thoughtcrime/securesms/mms/AttachmentManager.java index 2c3653978..2d0ca6b31 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mms/AttachmentManager.java +++ b/app/src/main/java/org/thoughtcrime/securesms/mms/AttachmentManager.java @@ -51,6 +51,7 @@ import org.thoughtcrime.securesms.components.ThumbnailView; import org.thoughtcrime.securesms.components.location.SignalMapView; import org.thoughtcrime.securesms.components.location.SignalPlace; import org.thoughtcrime.securesms.conversation.MessageSendType; +import org.thoughtcrime.securesms.database.AttachmentDatabase; import org.thoughtcrime.securesms.giph.ui.GiphyActivity; import org.thoughtcrime.securesms.maps.PlacePickerActivity; import org.thoughtcrime.securesms.mediasend.v2.MediaSelectionActivity; @@ -316,7 +317,7 @@ public class AttachmentManager { } Log.d(TAG, "remote slide with size " + fileSize + " took " + (System.currentTimeMillis() - start) + "ms"); - return mediaType.createSlide(context, uri, fileName, mimeType, null, fileSize, width, height, false); + return mediaType.createSlide(context, uri, fileName, mimeType, null, fileSize, width, height, false, null); } } finally { if (cursor != null) cursor.close(); @@ -326,17 +327,19 @@ public class AttachmentManager { } private @NonNull Slide getManuallyCalculatedSlideInfo(Uri uri, int width, int height) throws IOException { - long start = System.currentTimeMillis(); - Long mediaSize = null; - String fileName = null; - String mimeType = null; - boolean gif = false; + long start = System.currentTimeMillis(); + Long mediaSize = null; + String fileName = null; + String mimeType = null; + boolean gif = false; + AttachmentDatabase.TransformProperties transformProperties = null; if (PartAuthority.isLocalUri(uri)) { - mediaSize = PartAuthority.getAttachmentSize(context, uri); - fileName = PartAuthority.getAttachmentFileName(context, uri); - mimeType = PartAuthority.getAttachmentContentType(context, uri); - gif = PartAuthority.getAttachmentIsVideoGif(context, uri); + mediaSize = PartAuthority.getAttachmentSize(context, uri); + fileName = PartAuthority.getAttachmentFileName(context, uri); + mimeType = PartAuthority.getAttachmentContentType(context, uri); + gif = PartAuthority.getAttachmentIsVideoGif(context, uri); + transformProperties = PartAuthority.getAttachmentTransformProperties(uri); } if (mediaSize == null) { @@ -354,7 +357,7 @@ public class AttachmentManager { } Log.d(TAG, "local slide with size " + mediaSize + " took " + (System.currentTimeMillis() - start) + "ms"); - return mediaType.createSlide(context, uri, fileName, mimeType, null, mediaSize, width, height, gif); + return mediaType.createSlide(context, uri, fileName, mimeType, null, mediaSize, width, height, gif, transformProperties); } }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR); diff --git a/app/src/main/java/org/thoughtcrime/securesms/mms/MediaConstraints.java b/app/src/main/java/org/thoughtcrime/securesms/mms/MediaConstraints.java index 9490ec05b..995d1705d 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mms/MediaConstraints.java +++ b/app/src/main/java/org/thoughtcrime/securesms/mms/MediaConstraints.java @@ -39,6 +39,10 @@ public abstract class MediaConstraints { public abstract int getImageMaxHeight(Context context); public abstract int getImageMaxSize(Context context); + public boolean isHighQuality() { + return false; + } + /** * Provide a list of dimensions that should be attempted during compression. We will keep moving * down the list until the image can be scaled to fit under {@link #getImageMaxSize(Context)}. diff --git a/app/src/main/java/org/thoughtcrime/securesms/mms/PartAuthority.java b/app/src/main/java/org/thoughtcrime/securesms/mms/PartAuthority.java index 2cb3cf6cc..a9493c7fe 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mms/PartAuthority.java +++ b/app/src/main/java/org/thoughtcrime/securesms/mms/PartAuthority.java @@ -12,6 +12,7 @@ import org.thoughtcrime.securesms.BuildConfig; import org.thoughtcrime.securesms.attachments.Attachment; import org.thoughtcrime.securesms.attachments.AttachmentId; import org.thoughtcrime.securesms.avatar.AvatarPickerStorage; +import org.thoughtcrime.securesms.database.AttachmentDatabase; import org.thoughtcrime.securesms.database.SignalDatabase; import org.thoughtcrime.securesms.emoji.EmojiFiles; import org.thoughtcrime.securesms.providers.BlobProvider; @@ -152,6 +153,16 @@ public class PartAuthority { } } + public static @Nullable AttachmentDatabase.TransformProperties getAttachmentTransformProperties(@NonNull Uri uri) { + int match = uriMatcher.match(uri); + switch (match) { + case PART_ROW: + return SignalDatabase.attachments().getTransformProperties(new PartUriParser(uri).getPartId()); + default: + return null; + } + } + public static Uri getAttachmentPublicUri(Uri uri) { PartUriParser partUri = new PartUriParser(uri); return PartProvider.getContentUri(partUri.getPartId()); diff --git a/app/src/main/java/org/thoughtcrime/securesms/mms/PushMediaConstraints.java b/app/src/main/java/org/thoughtcrime/securesms/mms/PushMediaConstraints.java index 53db4f486..973fb2b5b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mms/PushMediaConstraints.java +++ b/app/src/main/java/org/thoughtcrime/securesms/mms/PushMediaConstraints.java @@ -23,6 +23,11 @@ public class PushMediaConstraints extends MediaConstraints { currentConfig = getCurrentConfig(ApplicationDependencies.getApplication(), sentMediaQuality); } + @Override + public boolean isHighQuality() { + return currentConfig == MediaConfig.LEVEL_3; + } + @Override public int getImageMaxWidth(Context context) { return currentConfig.imageSizeTargets[0]; diff --git a/app/src/main/java/org/thoughtcrime/securesms/mms/SlideFactory.java b/app/src/main/java/org/thoughtcrime/securesms/mms/SlideFactory.java index ef6e366b3..889789a3f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/mms/SlideFactory.java +++ b/app/src/main/java/org/thoughtcrime/securesms/mms/SlideFactory.java @@ -13,6 +13,7 @@ import androidx.annotation.WorkerThread; import org.signal.core.util.logging.Log; import org.thoughtcrime.securesms.blurhash.BlurHash; +import org.thoughtcrime.securesms.database.AttachmentDatabase; import org.thoughtcrime.securesms.util.MediaUtil; import java.io.IOException; @@ -31,25 +32,25 @@ public final class SlideFactory { /** * Generates a slide from the given parameters. * - * @param context Application context - * @param contentType The contentType of the given Uri - * @param uri The Uri pointing to the resource to create a slide out of - * @param width (Optional) width, can be 0. - * @param height (Optional) height, can be 0. + * @param context Application context + * @param contentType The contentType of the given Uri + * @param uri The Uri pointing to the resource to create a slide out of + * @param width (Optional) width, can be 0. + * @param height (Optional) height, can be 0. + * @param transformProperties (Optional) transformProperties, can be 0. * * @return A Slide with all the information we can gather about it. */ - @WorkerThread - public static @Nullable Slide getSlide(@NonNull Context context, @Nullable String contentType, @NonNull Uri uri, int width, int height) { + public static @Nullable Slide getSlide(@NonNull Context context, @Nullable String contentType, @NonNull Uri uri, int width, int height, @Nullable AttachmentDatabase.TransformProperties transformProperties) { MediaType mediaType = MediaType.from(contentType); try { if (PartAuthority.isLocalUri(uri)) { - return getManuallyCalculatedSlideInfo(context, mediaType, uri, width, height); + return getManuallyCalculatedSlideInfo(context, mediaType, uri, width, height, transformProperties); } else { - Slide result = getContentResolverSlideInfo(context, mediaType, uri, width, height); + Slide result = getContentResolverSlideInfo(context, mediaType, uri, width, height, transformProperties); - if (result == null) return getManuallyCalculatedSlideInfo(context, mediaType, uri, width, height); + if (result == null) return getManuallyCalculatedSlideInfo(context, mediaType, uri, width, height, transformProperties); else return result; } } catch (IOException e) { @@ -58,7 +59,14 @@ public final class SlideFactory { } } - private static @Nullable Slide getContentResolverSlideInfo(@NonNull Context context, @Nullable MediaType mediaType, @NonNull Uri uri, int width, int height) { + private static @Nullable Slide getContentResolverSlideInfo( + @NonNull Context context, + @Nullable MediaType mediaType, + @NonNull Uri uri, + int width, + int height, + @Nullable AttachmentDatabase.TransformProperties transformProperties + ) { long start = System.currentTimeMillis(); try (Cursor cursor = context.getContentResolver().query(uri, null, null, null, null)) { @@ -78,14 +86,22 @@ public final class SlideFactory { } Log.d(TAG, "remote slide with size " + fileSize + " took " + (System.currentTimeMillis() - start) + "ms"); - return mediaType.createSlide(context, uri, fileName, mimeType, null, fileSize, width, height, false); + return mediaType.createSlide(context, uri, fileName, mimeType, null, fileSize, width, height, false, transformProperties); } } return null; } - private static @NonNull Slide getManuallyCalculatedSlideInfo(@NonNull Context context, @Nullable MediaType mediaType, @NonNull Uri uri, int width, int height) throws IOException { + private static @NonNull Slide getManuallyCalculatedSlideInfo( + @NonNull Context context, + @Nullable MediaType mediaType, + @NonNull Uri uri, + int width, + int height, + @Nullable AttachmentDatabase.TransformProperties transformProperties + ) throws IOException + { long start = System.currentTimeMillis(); Long mediaSize = null; String fileName = null; @@ -118,7 +134,7 @@ public final class SlideFactory { } Log.d(TAG, "local slide with size " + mediaSize + " took " + (System.currentTimeMillis() - start) + "ms"); - return mediaType.createSlide(context, uri, fileName, mimeType, null, mediaSize, width, height, gif); + return mediaType.createSlide(context, uri, fileName, mimeType, null, mediaSize, width, height, gif, transformProperties); } public enum MediaType { @@ -142,17 +158,18 @@ public final class SlideFactory { @Nullable String fileName, @Nullable String mimeType, @Nullable BlurHash blurHash, - long dataSize, - int width, - int height, - boolean gif) + long dataSize, + int width, + int height, + boolean gif, + @Nullable AttachmentDatabase.TransformProperties transformProperties) { if (mimeType == null) { mimeType = "application/octet-stream"; } switch (this) { - case IMAGE: return new ImageSlide(context, uri, dataSize, width, height, blurHash); + case IMAGE: return new ImageSlide(context, uri, mimeType, dataSize, width, height, false, null, blurHash, transformProperties); case GIF: return new GifSlide(context, uri, dataSize, width, height); case AUDIO: return new AudioSlide(context, uri, dataSize, false); case VIDEO: return new VideoSlide(context, uri, dataSize, gif); diff --git a/app/src/main/java/org/thoughtcrime/securesms/sharing/MultiShareSender.java b/app/src/main/java/org/thoughtcrime/securesms/sharing/MultiShareSender.java index 2233ef331..afe6f06e6 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/sharing/MultiShareSender.java +++ b/app/src/main/java/org/thoughtcrime/securesms/sharing/MultiShareSender.java @@ -21,6 +21,7 @@ import org.thoughtcrime.securesms.attachments.DatabaseAttachment; import org.thoughtcrime.securesms.contacts.paged.ContactSearchKey; import org.thoughtcrime.securesms.conversation.MessageSendType; import org.thoughtcrime.securesms.conversation.colors.ChatColors; +import org.thoughtcrime.securesms.database.AttachmentDatabase; import org.thoughtcrime.securesms.database.SignalDatabase; import org.thoughtcrime.securesms.database.ThreadDatabase; import org.thoughtcrime.securesms.database.model.Mention; @@ -32,9 +33,11 @@ import org.thoughtcrime.securesms.keyvalue.StorySend; import org.thoughtcrime.securesms.linkpreview.LinkPreview; import org.thoughtcrime.securesms.mediasend.Media; import org.thoughtcrime.securesms.mediasend.v2.text.TextStoryBackgroundColors; +import org.thoughtcrime.securesms.mms.ImageSlide; import org.thoughtcrime.securesms.mms.OutgoingMediaMessage; import org.thoughtcrime.securesms.mms.OutgoingSecureMediaMessage; import org.thoughtcrime.securesms.mms.PartAuthority; +import org.thoughtcrime.securesms.mms.SentMediaQuality; import org.thoughtcrime.securesms.mms.Slide; import org.thoughtcrime.securesms.mms.SlideDeck; import org.thoughtcrime.securesms.mms.SlideFactory; @@ -266,6 +269,8 @@ public final class MultiShareSender { .flatMap(slide -> { if (slide instanceof VideoSlide) { return expandToClips(context, (VideoSlide) slide).stream(); + } else if (slide instanceof ImageSlide) { + return java.util.stream.Stream.of(ensureDefaultQuality(context, (ImageSlide) slide)); } else { return java.util.stream.Stream.of(slide); } @@ -273,8 +278,6 @@ public final class MultiShareSender { .filter(it -> MediaUtil.isStorySupportedType(it.getContentType())) .collect(Collectors.toList()); - // For each video slide, we want to convert it into a media, then clip it, and then transform it BACK into a slide. - for (final Slide slide : storySupportedSlides) { SlideDeck singletonDeck = new SlideDeck(); singletonDeck.addSlide(slide); @@ -348,6 +351,26 @@ public final class MultiShareSender { } } + private static Slide ensureDefaultQuality(@NonNull Context context, @NonNull ImageSlide imageSlide) { + Attachment attachment = imageSlide.asAttachment(); + if (attachment.getTransformProperties().getSentMediaQuality() == SentMediaQuality.HIGH.getCode()) { + return new ImageSlide( + context, + attachment.getUri(), + attachment.getContentType(), + attachment.getSize(), + attachment.getWidth(), + attachment.getHeight(), + attachment.isBorderless(), + attachment.getCaption(), + attachment.getBlurHash(), + AttachmentDatabase.TransformProperties.empty() + ); + } else { + return imageSlide; + } + } + private static void sendTextMessage(@NonNull Context context, @NonNull MultiShareArgs multiShareArgs, @NonNull Recipient recipient, @@ -434,7 +457,7 @@ public final class MultiShareSender { slideDeck.addSlide(new StickerSlide(context, multiShareArgs.getDataUri(), 0, multiShareArgs.getStickerLocator(), multiShareArgs.getDataType())); } else if (!multiShareArgs.getMedia().isEmpty()) { for (Media media : multiShareArgs.getMedia()) { - Slide slide = SlideFactory.getSlide(context, media.getMimeType(), media.getUri(), media.getWidth(), media.getHeight()); + Slide slide = SlideFactory.getSlide(context, media.getMimeType(), media.getUri(), media.getWidth(), media.getHeight(), media.getTransformProperties().orElse(null)); if (slide != null) { slideDeck.addSlide(slide); } else { @@ -442,7 +465,7 @@ public final class MultiShareSender { } } } else if (multiShareArgs.getDataUri() != null) { - Slide slide = SlideFactory.getSlide(context, multiShareArgs.getDataType(), multiShareArgs.getDataUri(), 0, 0); + Slide slide = SlideFactory.getSlide(context, multiShareArgs.getDataType(), multiShareArgs.getDataUri(), 0, 0, null); if (slide != null) { slideDeck.addSlide(slide); } else { diff --git a/app/src/main/java/org/thoughtcrime/securesms/sms/MessageSender.java b/app/src/main/java/org/thoughtcrime/securesms/sms/MessageSender.java index 4caf4c5d9..9a74037d4 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/sms/MessageSender.java +++ b/app/src/main/java/org/thoughtcrime/securesms/sms/MessageSender.java @@ -148,7 +148,7 @@ public class MessageSender { MessageDatabase database = SignalDatabase.mms(); List messageIds = new ArrayList<>(messages.size()); List threads = new ArrayList<>(messages.size()); - UploadDependencyGraph dependencyGraph = UploadDependencyGraph.EMPTY; + UploadDependencyGraph dependencyGraph; try { database.beginTransaction(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/stories/Stories.kt b/app/src/main/java/org/thoughtcrime/securesms/stories/Stories.kt index b7af67990..0ef3cbac2 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/stories/Stories.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/stories/Stories.kt @@ -186,6 +186,23 @@ object Stories { object None : DurationResult() } + @JvmStatic + @WorkerThread + fun canPreUploadMedia(media: Media): Boolean { + return if (MediaUtil.isVideo(media.mimeType)) { + getSendRequirements(media) != SendRequirements.REQUIRES_CLIP + } else { + !hasHighQualityTransform(media) + } + } + + /** + * Checkst to see if the given media has the "High Quality" toggled in its transform properties. + */ + fun hasHighQualityTransform(media: Media): Boolean { + return MediaUtil.isImageType(media.mimeType) && media.transformProperties.map { it.sentMediaQuality == SentMediaQuality.HIGH.code }.orElse(false) + } + @JvmStatic @WorkerThread fun getSendRequirements(media: Media): SendRequirements { diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/messages/SignalServiceAttachmentStream.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/messages/SignalServiceAttachmentStream.java index 559b44cd8..c35f9a877 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/messages/SignalServiceAttachmentStream.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/messages/SignalServiceAttachmentStream.java @@ -10,13 +10,15 @@ package org.whispersystems.signalservice.api.messages; import org.whispersystems.signalservice.internal.push.http.CancelationSignal; import org.whispersystems.signalservice.internal.push.http.ResumableUploadSpec; +import java.io.Closeable; +import java.io.IOException; import java.io.InputStream; import java.util.Optional; /** * Represents a local SignalServiceAttachment to be sent. */ -public class SignalServiceAttachmentStream extends SignalServiceAttachment { +public class SignalServiceAttachmentStream extends SignalServiceAttachment implements Closeable { private final InputStream inputStream; private final long length; @@ -151,4 +153,9 @@ public class SignalServiceAttachmentStream extends SignalServiceAttachment { public Optional getResumableUploadSpec() { return resumableUploadSpec; } + + @Override + public void close() throws IOException { + inputStream.close(); + } } diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java index 06de2fd0e..d3200d272 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java @@ -1450,17 +1450,11 @@ public class PushServiceSocket { connections.add(call); } - try { - Response response; - - try { - response = call.execute(); - } catch (IOException e) { - throw new PushNetworkException(e); - } - + try (Response response = call.execute()) { if (response.isSuccessful()) return file.getTransmittedDigest(); else throw new NonSuccessfulResponseCodeException(response.code(), "Response: " + response); + } catch (IOException e) { + throw new PushNetworkException(e); } finally { synchronized (connections) { connections.remove(call);