From 0f1bae9bbdd7ea24c08eabd47daa634d40dc6989 Mon Sep 17 00:00:00 2001 From: automated-signal <37887102+automated-signal@users.noreply.github.com> Date: Wed, 6 Mar 2024 16:31:27 -0600 Subject: [PATCH] Remove autoOrientJPEG and consolidate downscaling logic Co-authored-by: trevor-signal <131492920+trevor-signal@users.noreply.github.com> --- ts/linkPreviews/linkPreviewFetch.ts | 10 +-- ts/models/conversations.ts | 30 ++++++- ts/services/LinkPreview.ts | 1 + ts/signal.ts | 7 +- .../util/scaleImageToLevel_test.ts | 17 ++-- ts/types/Message2.ts | 82 ++++--------------- ts/util/attachments.ts | 61 ++++---------- ts/util/handleEditMessage.ts | 5 +- ts/util/handleImageAttachment.ts | 14 ++-- ts/util/scaleImageToLevel.ts | 22 ++--- 10 files changed, 105 insertions(+), 144 deletions(-) diff --git a/ts/linkPreviews/linkPreviewFetch.ts b/ts/linkPreviews/linkPreviewFetch.ts index 1423c551a7..9dc038d30f 100644 --- a/ts/linkPreviews/linkPreviewFetch.ts +++ b/ts/linkPreviews/linkPreviewFetch.ts @@ -609,12 +609,12 @@ export async function fetchLinkPreviewImage( const dataBlob = new Blob([data], { type: contentType, }); - const { blob: xcodedDataBlob } = await scaleImageToLevel( - dataBlob, + const { blob: xcodedDataBlob } = await scaleImageToLevel({ + fileOrBlobOrURL: dataBlob, contentType, - dataBlob.size, - false - ); + size: dataBlob.size, + highQuality: false, + }); const xcodedDataArrayBuffer = await blobToArrayBuffer(xcodedDataBlob); data = new Uint8Array(xcodedDataArrayBuffer); diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index 5699374481..9503d93eef 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -161,6 +161,7 @@ import { deriveProfileKeyVersion } from '../util/zkgroup'; import { incrementMessageCounter } from '../util/incrementMessageCounter'; import OS from '../util/os/osMain'; import { getMessageAuthorText } from '../util/getMessageAuthorText'; +import { downscaleOutgoingAttachment } from '../util/attachments'; /* eslint-disable more/no-then */ window.Whisper = window.Whisper || {}; @@ -3818,7 +3819,7 @@ export class ConversationModel extends window.Backbone // If there are link previews present in the message we shouldn't include // any attachments as well. - const attachmentsToSend = preview && preview.length ? [] : attachments; + let attachmentsToSend = preview && preview.length ? [] : attachments; if (preview && preview.length) { attachments.forEach(attachment => { @@ -3828,6 +3829,33 @@ export class ConversationModel extends window.Backbone }); } + /** + * At this point, all attachments have been processed and written to disk as draft + * attachments, via processAttachments. All transcodable images have been re-encoded + * via canvas to remove EXIF data. Images above the high-quality threshold size have + * been scaled to high-quality JPEGs. + * + * If we choose to send images in standard quality, we need to scale them down + * (potentially for the second time). When we do so, we also delete the current + * draft attachment on disk for cleanup. + * + * All draft attachments (with a path or just in-memory) will be written to disk for + * real in `upgradeMessageSchema`. + */ + if (!sendHQImages) { + attachmentsToSend = await Promise.all( + attachmentsToSend.map(async attachment => { + const downscaledAttachment = await downscaleOutgoingAttachment( + attachment + ); + if (downscaledAttachment !== attachment && attachment.path) { + drop(deleteAttachmentData(attachment.path)); + } + return downscaledAttachment; + }) + ); + } + // Here we move attachments to disk const attributes = await upgradeMessageSchema({ id: generateGuid(), diff --git a/ts/services/LinkPreview.ts b/ts/services/LinkPreview.ts index 08aa4b9572..c179bafc5b 100644 --- a/ts/services/LinkPreview.ts +++ b/ts/services/LinkPreview.ts @@ -351,6 +351,7 @@ async function getPreview( type: fullSizeImage.contentType, }), fileName: title, + highQuality: true, }); const data = await fileToBytes(withBlob.file); diff --git a/ts/signal.ts b/ts/signal.ts index 873d356d32..2c8156ca16 100644 --- a/ts/signal.ts +++ b/ts/signal.ts @@ -111,7 +111,7 @@ type MigrationsModuleType = { }>; upgradeMessageSchema: ( attributes: MessageAttributesType, - options?: { maxVersion?: number; keepOnDisk?: boolean } + options?: { maxVersion?: number } ) => Promise; writeMessageAttachments: ( message: MessageAttributesType @@ -266,9 +266,9 @@ export function initializeMigrations({ }), upgradeMessageSchema: ( message: MessageAttributesType, - options: { maxVersion?: number; keepOnDisk?: boolean } = {} + options: { maxVersion?: number } = {} ) => { - const { maxVersion, keepOnDisk } = options; + const { maxVersion } = options; return MessageType.upgradeSchema(message, { deleteOnDisk, @@ -283,7 +283,6 @@ export function initializeMigrations({ writeNewAttachmentData, writeNewStickerData, - keepOnDisk, logger, maxVersion, }); diff --git a/ts/test-electron/util/scaleImageToLevel_test.ts b/ts/test-electron/util/scaleImageToLevel_test.ts index ed803df46c..3122984489 100644 --- a/ts/test-electron/util/scaleImageToLevel_test.ts +++ b/ts/test-electron/util/scaleImageToLevel_test.ts @@ -35,12 +35,12 @@ describe('scaleImageToLevel', () => { testCases.map( async ({ path, contentType, expectedWidth, expectedHeight }) => { const blob = await getBlob(path); - const scaled = await scaleImageToLevel( - blob, + const scaled = await scaleImageToLevel({ + fileOrBlobOrURL: blob, contentType, - blob.size, - true - ); + size: blob.size, + highQuality: true, + }); const data = await loadImage(scaled.blob, { orientation: true }); const { originalWidth: width, originalHeight: height } = data; @@ -61,7 +61,12 @@ describe('scaleImageToLevel', () => { 'Test setup failure: expected fixture to have EXIF data' ); - const scaled = await scaleImageToLevel(original, IMAGE_JPEG, original.size); + const scaled = await scaleImageToLevel({ + fileOrBlobOrURL: original, + contentType: IMAGE_JPEG, + size: original.size, + highQuality: true, + }); assert.isUndefined( (await loadImage(scaled.blob, { meta: true, orientation: true })).exif ); diff --git a/ts/types/Message2.ts b/ts/types/Message2.ts index d32ac0fcb1..25d4b934f1 100644 --- a/ts/types/Message2.ts +++ b/ts/types/Message2.ts @@ -5,7 +5,6 @@ import { isFunction, isObject, isString, omit } from 'lodash'; import * as Contact from './EmbeddedContact'; import type { AttachmentType, AttachmentWithHydratedData } from './Attachment'; -import { autoOrientJPEG } from '../util/attachments'; import { captureDimensionsAndScreenshot, hasData, @@ -52,7 +51,6 @@ export type ContextType = { height: number; }>; getRegionCode: () => string | undefined; - keepOnDisk?: boolean; logger: LoggerType; makeImageThumbnail: (params: { size: number; @@ -369,37 +367,18 @@ export const _mapPreviewAttachments = return { ...message, preview }; }; +const noopUpgrade = async (message: MessageAttributesType) => message; + const toVersion0 = async ( message: MessageAttributesType, context: ContextType ) => initializeSchemaVersion({ message, logger: context.logger }); + const toVersion1 = _withSchemaVersion({ schemaVersion: 1, - upgrade: _mapAttachments( - async ( - attachment: AttachmentType, - context, - options - ): Promise => { - const { deleteOnDisk, keepOnDisk } = context; - const rotatedAttachment = await autoOrientJPEG( - attachment, - context, - options - ); - - if ( - !keepOnDisk && - attachment !== rotatedAttachment && - rotatedAttachment.data && - attachment.path - ) { - await deleteOnDisk(attachment.path); - } - - return rotatedAttachment; - } - ), + // NOOP: We no longer need to run autoOrientJPEG on incoming JPEGs since Chromium + // respects the EXIF orientation for us when displaying the image + upgrade: noopUpgrade, }); const toVersion2 = _withSchemaVersion({ schemaVersion: 2, @@ -506,7 +485,6 @@ export const upgradeSchema = async ( makeVideoScreenshot, writeNewStickerData, deleteOnDisk, - keepOnDisk, logger, maxVersion = CURRENT_SCHEMA_VERSION, }: ContextType @@ -566,7 +544,6 @@ export const upgradeSchema = async ( getImageDimensions, makeImageThumbnail, makeVideoScreenshot, - keepOnDisk, logger, getAbsoluteStickerPath, getRegionCode, @@ -590,7 +567,6 @@ export const processNewAttachment = async ( getImageDimensions, makeImageThumbnail, makeVideoScreenshot, - deleteOnDisk, logger, }: Pick< ContextType, @@ -630,42 +606,16 @@ export const processNewAttachment = async ( throw new TypeError('context.logger is required'); } - const rotatedAttachment = await autoOrientJPEG( - attachment, - { logger }, - { - isIncoming: true, - } - ); - - let onDiskAttachment = rotatedAttachment; - - // If we rotated the attachment, then `data` will be the actual bytes of the attachment, - // in memory. We want that updated attachment to go back to disk. - if (rotatedAttachment.data) { - onDiskAttachment = await migrateDataToFileSystem(rotatedAttachment, { - writeNewAttachmentData, - logger, - }); - - if (rotatedAttachment !== attachment && attachment.path) { - await deleteOnDisk(attachment.path); - } - } - - const finalAttachment = await captureDimensionsAndScreenshot( - onDiskAttachment, - { - writeNewAttachmentData, - getAbsoluteAttachmentPath, - makeObjectUrl, - revokeObjectUrl, - getImageDimensions, - makeImageThumbnail, - makeVideoScreenshot, - logger, - } - ); + const finalAttachment = await captureDimensionsAndScreenshot(attachment, { + writeNewAttachmentData, + getAbsoluteAttachmentPath, + makeObjectUrl, + revokeObjectUrl, + getImageDimensions, + makeImageThumbnail, + makeVideoScreenshot, + logger, + }); return finalAttachment; }; diff --git a/ts/util/attachments.ts b/ts/util/attachments.ts index 0775cc8288..7f88417bef 100644 --- a/ts/util/attachments.ts +++ b/ts/util/attachments.ts @@ -3,6 +3,7 @@ import { blobToArrayBuffer } from 'blob-util'; +import * as log from '../logging/log'; import { scaleImageToLevel } from './scaleImageToLevel'; import { dropNull } from './dropNull'; import type { @@ -10,49 +11,23 @@ import type { UploadedAttachmentType, } from '../types/Attachment'; import { canBeTranscoded } from '../types/Attachment'; -import type { LoggerType } from '../types/Logging'; -import * as MIME from '../types/MIME'; import * as Errors from '../types/errors'; import * as Bytes from '../Bytes'; -// Upgrade steps NOTE: This step strips all EXIF metadata from JPEG images as part of -// re-encoding the image: - -// When sending an image: -// 1. During composition, images are passed through handleImageAttachment. If needed, this -// scales them down to high-quality (level 3). -// 2. Draft images are then written to disk as a draft image (so there is a `path`) -// 3. On send, the message schema is upgraded, triggering this function - -export async function autoOrientJPEG( - attachment: AttachmentType, - { logger }: { logger: LoggerType }, - { - sendHQImages = false, - isIncoming = false, - }: { - sendHQImages?: boolean; - isIncoming?: boolean; - } = {} -): Promise { - if (isIncoming && !MIME.isJPEG(attachment.contentType)) { - return attachment; - } - +// All outgoing images go through handleImageAttachment before being sent and thus have +// already been scaled to high-quality level, stripped of exif data, and saved. This +// should be called just before message send to downscale the attachment further if +// needed. +export const downscaleOutgoingAttachment = async ( + attachment: AttachmentType +): Promise => { if (!canBeTranscoded(attachment)) { return attachment; } - // If we haven't downloaded the attachment yet, we won't have the data. - // All images go through handleImageAttachment before being sent and thus have - // already been scaled to level, oriented, stripped of exif data, and saved - // in high quality format. If we want to send the image in HQ we can return - // the attachment as-is. Otherwise we'll have to further scale it down. + + let scaleTarget: string | Blob; const { data, path, size } = attachment; - if (sendHQImages) { - return attachment; - } - let scaleTarget: string | Blob; if (data) { scaleTarget = new Blob([data], { type: attachment.contentType, @@ -65,12 +40,12 @@ export async function autoOrientJPEG( } try { - const { blob: xcodedDataBlob } = await scaleImageToLevel( - scaleTarget, - attachment.contentType, + const { blob: xcodedDataBlob } = await scaleImageToLevel({ + fileOrBlobOrURL: scaleTarget, + contentType: attachment.contentType, size, - isIncoming - ); + highQuality: false, + }); const xcodedDataArrayBuffer = await blobToArrayBuffer(xcodedDataBlob); // IMPORTANT: We overwrite the existing `data` `Uint8Array` losing the original @@ -91,14 +66,14 @@ export async function autoOrientJPEG( return xcodedAttachment; } catch (error: unknown) { const errorString = Errors.toLogFormat(error); - logger.error( - 'autoOrientJPEG: Failed to rotate/scale attachment', + log.error( + 'downscaleOutgoingAttachment: Failed to scale attachment', errorString ); return attachment; } -} +}; export type CdnFieldsType = Pick< AttachmentType, diff --git a/ts/util/handleEditMessage.ts b/ts/util/handleEditMessage.ts index 586f3c1460..b4d61c287a 100644 --- a/ts/util/handleEditMessage.ts +++ b/ts/util/handleEditMessage.ts @@ -135,10 +135,7 @@ export async function handleEditMessage( } const upgradedEditedMessageData = - await window.Signal.Migrations.upgradeMessageSchema( - editAttributes.message, - { keepOnDisk: true } - ); + await window.Signal.Migrations.upgradeMessageSchema(editAttributes.message); // Copies over the attachments from the main message if they're the same // and they have already been downloaded. diff --git a/ts/util/handleImageAttachment.ts b/ts/util/handleImageAttachment.ts index 67f7cad6c2..96cc28bf62 100644 --- a/ts/util/handleImageAttachment.ts +++ b/ts/util/handleImageAttachment.ts @@ -46,6 +46,8 @@ export async function handleImageAttachment( : stringToMIMEType(file.type), fileName: file.name, file: processedFile, + // We always store draft attachments as HQ + highQuality: true, }); const data = await blobToArrayBuffer(resizedBlob); @@ -66,10 +68,12 @@ export async function autoScale({ contentType, file, fileName, + highQuality, }: { contentType: MIMEType; file: File | Blob; fileName: string; + highQuality: boolean; }): Promise<{ contentType: MIMEType; file: Blob; @@ -79,12 +83,12 @@ export async function autoScale({ return { contentType, file, fileName }; } - const { blob, contentType: newContentType } = await scaleImageToLevel( - file, + const { blob, contentType: newContentType } = await scaleImageToLevel({ + fileOrBlobOrURL: file, contentType, - file.size, - true - ); + size: file.size, + highQuality, + }); if (newContentType !== IMAGE_JPEG) { return { diff --git a/ts/util/scaleImageToLevel.ts b/ts/util/scaleImageToLevel.ts index 482d076f72..4c6ac51212 100644 --- a/ts/util/scaleImageToLevel.ts +++ b/ts/util/scaleImageToLevel.ts @@ -108,12 +108,17 @@ async function getCanvasBlobAsJPEG( return canvasToBlob(canvas, IMAGE_JPEG, quality); } -export async function scaleImageToLevel( - fileOrBlobOrURL: File | Blob | string, - contentType: MIMEType, - size: number, - sendAsHighQuality?: boolean -): Promise<{ +export async function scaleImageToLevel({ + fileOrBlobOrURL, + contentType, + size, + highQuality, +}: { + fileOrBlobOrURL: File | Blob | string; + contentType: MIMEType; + size: number; + highQuality: boolean | null; +}): Promise<{ blob: Blob; contentType: MIMEType; }> { @@ -134,16 +139,13 @@ export async function scaleImageToLevel( throw error; } - const level = sendAsHighQuality - ? MediaQualityLevels.Three - : getMediaQualityLevel(); + const level = highQuality ? MediaQualityLevels.Three : getMediaQualityLevel(); const { maxDimensions, quality, size: targetSize, thresholdSize, } = MEDIA_QUALITY_LEVEL_DATA.get(level) || DEFAULT_LEVEL_DATA; - if (size <= thresholdSize) { // Always encode through canvas as a temporary fix for a library bug const blob: Blob = await canvasToBlob(data.image, contentType);