Image Compression Solution

[CowMuon]

Problem

Images added to the app (via a number of vectors) are not being compressed or otherwise handled in an efficient manner. This is happening in a number of places including the forum, & user profiles & adding community icons, that later of which was significantly reworked a while back by @kurtisassad.

Moreover we need to have a unified approach here that we extend for each use case, rather than a collection of one off tickets such as linked above.

Engineering Requirements

• Need an inventory of site images by type, such that we can set max image size, and hence compression, but image category across app.
  • eg. How big should a user avatar image file be? What is the maximum size images need to be saved off as? What is the multiplier for retina displays? For liquid retina displays? (arm64-m2) Do we need to or intend to support 4k displays? (product question)
• It should go without saying that image compression should be handled client side (to avoid sending all the extra data over the wire.)
• A single unified approach to image compression (as DRY as possible) that can be used/extended for each of the identified use cases.

Acceptance Criteria

1. Step one is to complete inventory of affected use cases.
2. Next, we need to to define max sizes for each of these categories.
3. Decide on actual compression
4. Make recommendation on whether to include converting uploaded images to .webp format in the current scope, or circle back later to make that change across app.
5. Meets Engineering Requirements as above. (so below)

Additional considerations

• Roadmap: Currently we save images in whatever format user submits. We would prefer to not just compress, but convert to a standard format -webp, for example- and it seems like it makes more sense to do that as part of this story scope, rather than defining a compression strategy to handle all the different formats.
• Max image size here implies minimum size for various UGC images across the site. TBC: Not necessarily included in current scope (viz. additional considerations.)

[CowMuon]

After some discussion, preference here is to go straight to .webp, unless there is an identified reason why not to.

[alexyoung23j]

Implementation:

1. Modify compressImage to always compress to .webp
2. Modify useImageUploader and useImageDropAndPaste hooks to incorporate compressImage.
3. Modify AvatarUpload to use compressImage.
4. Decide on maximum sizes for each use case.

[alexyoung23j]

Based on discussions with @NakulManchanda and @kurtisassad, here is the situation:

We have an open PR that is an unfinished implementation of the cloudflare solution discussed in the parking lot. If we pursue this option, and it works (needs to be fully tested), the following things are true:

• Images can be uploaded in whatever format the user wants, but they will be served to our site as .webp (cloudflare is handling the conversion)
• There will not be a size limit imposed on the images that are uploaded, but there will be a size limit when they are served on the site

The benefits of instead going forward with this ticket as written (changing the upload situation, not changing the previously uploaded content):

• We save some s3 costs because images are compressed client side before uploading
• Going forward, we have a standardized image format. (Unclear to me that this is actually worth much in terms of DevEx or performance; no matter the image format we are just fetching it and displaying to users.)

The benefits of also going back through previously uploaded content and converting to .webp:

• it's parsimonious and consistent? Again, unclear on what the actual win is here because I don't fully see why DevEx is improved at all.

So I guess my question for you @MuonShot was what specifically was the impetus for writing this ticket? What are the specific things about either DevEx or site performance or costs we want improved? Answers to those questions will help determine whether we should move forward with this ticket as written or defer finishing to Kurtis' PR, or neither.

[CowMuon]

What decisions need to be made by today / tomorrow, or initial work on this will be blocked? IOW, what is the order of operations for executing scope of work here?

[alexyoung23j]

Key decisions, in order:

1. Should be pursue Kurtis/Nakul solution instead of this ticket?
2. If kurtis/nakul solution does not work, should we still do this ticket and why?

[CowMuon]

Can we get an update how far we got on this, and an estimate how far out (ie how many hours) we are from delivery on this? @alexyoung23j

[viz. to get this out the door tomorrow]

[CowMuon]

Set this to In Progress on the new board; I did see it got sent back to Teed Up, but without an explanatory comment.

[alexyoung23j]

Do not have the appropriate AWS permissions (or was added to the wrong org) to view our current buckets. Need this access to proceed.

[alexyoung23j]

Update!!

Just worked with @kurtisassad to get this ready to go. We did the following:

1. Created a new bucket called assets.commonwealth.im and copied ALL contents of existing bucket commonwealth-uploads to this new bucket.
2. Set up a cloudflare to cache assets and compress them to .webp format. Because the cache needs to be renewed for this to take work, 12 hours or so will need to elapse before this takes effect.
3. Updated #2930 to reflect this new bucket. This is a migration that converts all image urls in the DB to the new bucket.

Next steps:

1. Run migration on a staging environment.
2. Ensure that the content is all still available and being fetched as webp.

@MuonShot

[CowMuon]

@alexyoung23j Moved to has PR but no PR added under Development?

[kurtisassad]

I have linked the PR now. Basically the CDN PR fixes this ticket as well as the CDN ticket.

It is finished but currently in draft while I wait for some staging environment to free up.

[CowMuon]

2930 is still marked as draft? What's the correct status there? @kurtisassad

[kurtisassad]

Still in draft because I would like to test it on staging before we review it. Unfortunately all staging envs are in use right now.

[CowMuon]

We will need an update to the team wiki documenting what changes were made in Cloudflare for compressed images along with any information how it was setup, expected ongoing concerns, troubleshooting, etc. @alexyoung23j @kurtisassad

[jnaviask]

Not closed until CDN settings are properly configured + documented.

[kurtisassad]

@alexyoung23j One more issue: So I think it won't resize too large of images as per the Cf-Polished response header due to input_too_large. In this case it will not compress the image according to: The Cf Polished statuses doc

I think we need to handle image resizing for images larger than 1000 x 1000 manually, as well as add a cap to it in the upload logic.

But after these images are forced to be smaller than 1k x 1k, cloudflare's image compression/conversion will kick in automatically.

[alexyoung23j]

https://www.npmjs.com/package/browser-image-compression Potential package to use for client side compression.

[jnaviask]

@alexyoung23j what is the scope of work here that you have in progress = what exact steps are you planning on taking?

[alexyoung23j]

@jnaviask Since the other PR went in that included the bucket changes + cloudflare polish updates, the scope now is simply to ensure that all images uploaded are compressed to ensure that Cloudflare Polish (which automatically tries to converts all images to webp) is able to succeed as often as possible. If images are already uploaded or optimized on upload, Polish does nothing, but based on this page in their docs, images should be less than 10mb and have a maximum height or width or 1000px in order for Polish to be used most often. So I will have a PR up in a few minutes that uses a package browser-image-compression to ensure those constraints before we upload to s3. I don't really understand what @CowMuon was getting at on the call today- hopefully this answers the question you had.

[CowMuon]

If images are already uploaded or optimized on upload,

What exactly does it mean for images to be "already uploaded on upload"? @alexyoung23j

[alexyoung23j]

@MuonShot ah just meant that Polish may not work on images that are already in the bucket, if they are larger than the constraints. After this PR goes in, newly uploaded images will all be compatible with Polish