Large image resize - why only 64% scaling at most?

support
1

I am happy that large image uploads are now being resized, but its not taking a 4.6MB image down to 1MB - I recently set the max image size to 1024K and tried uploading a 4.6MB image and got a too-large error.

Looking at the code it is doing a 80%-resize up to 2 times and if its not small enough by then it fails (I am looking at https://github.com/discourse/discourse/blob/42a523a3beae3fa646cff5bd4a7a822497a1a06a/app/controllers/uploads_controller.rb) - ?? That will do at most a 64% resize which will not get 6MB to 1MB - ? Why is it so limited?

3 Likes
2

Because there’s no way to 1) predict how long a downsize operation will take and 2) know what % we need to use to downsize a xMB image to 1MB image.

If you know the answer to either one of those points, please share :raised_hands:

So we do it the hard way. We indeed only do 2 80-%-downsizes. From my testing, these values seem to be a good balance between successfully resizing large images and not taking too much time trying to…

I might make these values ("# of attemps" and “% of downsize”) new site settings, but I also don’t want to give users an easy way to :gun: themselves in the :shoe:.

3

Yes, in thinking about it its harder than I was expecting. Here is a Stack Overflow post with some good ideas:

Check out answer #2, it seems like with some tweaking something that could be made to work. The idea is calculate bytes per pixel in the original image and use that to scale the dimensions. But, smaller images often will use slightly more bytes per pixel so a fudge factor is needed. With a good formula the hope is only one iteration will be needed, bounding the time.

3 Likes
4

I’d happily merge a PR replacing the hardcoded-80-% with a decent heuristic :wink:

3 Likes
5

Following up on my old post here… I noticed after a recent update we were not getting any resizing to work. In looking at the code, there are a couple of recent commits to uploads_controller.rb that look like it broke the resizing.

One is loop exit condition break if downsized_size >= uploaded_size || downsized_size < max_image_size_kb in fact always holds the first time through the loop since the first time through downsized_size is equal to uploaded_size (this condition can fire before the first resize). So, the resize code is currently unreachable and no resizing will ever happen.

Another issue is the 10mb hard coded constant was turned into

maximum_upload_size = [SiteSetting.max_image_size_kb, SiteSetting.max_attachment_size_kb].max.kilobytes

but the problem is a catch-22 arises with the line

if 0 < uploaded_size && uploaded_size < maximum_upload_size && ...

– if a resize is needed, uploaded_size (size of actual upload initially) is probably bigger than SiteSetting.max_image_size_kb (i.e. its too big and needs shrinking), but the above condition will often cause resizing to abort (it always does with the default settings for example).

To get around these glitches, I set max_attachment_size_kb to 10mb in the admin panel, and also removed the condition downsized_size >= uploaded_size since it should never arise (or, maybe it should be just > to fix the initial loop bug). Since the maximum upload size is the bigger of max image/attachment size, this hack allows maximum_upload_size to be big compared to the image and allow the resize code to run.

Here is also some suggested code to do a bit more resizing, it uses a mini-table to figure out how much to resize. I tested this code on a development server with max_image_size_kb at both 1000 and 3000, and max_attachment_size_kb at 10000, and it worked OK and got big images down to 1MB ones. This code also has the faulty condition removed.

 break if downsized_size < max_image_size_kb # removed faulty >= condition here
            image_info = FastImage.new(tempfile.path) rescue nil
            w, h = *(image_info.try(:size) || [0, 0])
            break if w == 0 ||h == 0
            if downsized_size / 9 > max_image_size_kb
              downsize_ratio = 0.3
            elsif downsized_size / 3 > max_image_size_kb
              downsize_ratio = 0.6
            else
              downsize_ratio = 0.8
            end
            dimensions = "#{(w * downsize_ratio).floor}x#{(h * downsize_ratio).floor}"

In theory I could try to put in a PR on this, but first I don’t know Ruby and am Googling for every bit of syntax, and second I don’t know what the goal is here - the hard-coded 10MB was a useful limit it seems in terms of what size uploaded was even allowed by the browser, but I’m not sure where it went in the design space.

3 Likes
6

Sure @zogstrip can have a look.

7

Thanks for all your suggestions! They’re now implemented :wink:

https://github.com/discourse/discourse/commit/7fca6f502ffaaccbeda7453ec743625690fdb051

2 Likes
8

Cool! Code looks great now. I will give it some tests on my dev server at some point. The ratios I picked in the table were based on eyeball calculations so they may not be perfect. But they should not be too far off.

2 Likes
9

It’s fine, we can iterate on it :wink:

2 Likes
10

Update: I patched my production site with the revised code and all is working good for a day now :smile: I set the max size to 2mb and many images are getting resized.

Now that I worked on this I belatedly noticed you un-deprecated the Amazon S3 image storage option. So, its much less of a life-or-death issue for my site now. But it still good to not be wasting space, theres no need for 5mb+ images.

3 Likes