From 2bcca46cc5b938a7c06e603e6c479d8f2024751c Mon Sep 17 00:00:00 2001 From: jbrw Date: Fri, 23 Oct 2020 12:38:28 -0400 Subject: [PATCH] FEATURE - ImageMagick jpeg quality (#11004) * FEATURE - Add SiteSettings to control JPEG image quality `recompress_original_jpg_quality` - the maximum quality of a newly uploaded file. `image_preview_jpg_quality` - the maximum quality of OptimizedImages --- app/models/optimized_image.rb | 18 ++++++++++++++---- app/models/upload.rb | 8 ++++++++ config/locales/server.en.yml | 2 ++ config/site_settings.yml | 8 ++++++++ lib/upload_creator.rb | 25 +++++++++++++++++-------- spec/lib/upload_creator_spec.rb | 20 ++++++++++++++++++++ 6 files changed, 69 insertions(+), 12 deletions(-) diff --git a/app/models/optimized_image.rb b/app/models/optimized_image.rb index 42a62b98f7e..0c33d7d1629 100644 --- a/app/models/optimized_image.rb +++ b/app/models/optimized_image.rb @@ -85,6 +85,9 @@ class OptimizedImage < ActiveRecord::Base temp_file = Tempfile.new(["discourse-thumbnail", extension]) temp_path = temp_file.path + target_quality = upload.target_image_quality(original_path, SiteSetting.image_preview_jpg_quality) + opts = opts.merge(quality: target_quality) if target_quality + if upload.extension == "svg" FileUtils.cp(original_path, temp_path) resized = true @@ -218,6 +221,10 @@ class OptimizedImage < ActiveRecord::Base instructions << "-colors" << opts[:colors].to_s end + if opts[:quality] + instructions << "-quality" << opts[:quality].to_s + end + # NOTE: ORDER is important! instructions.concat(%W{ -auto-orient @@ -228,7 +235,6 @@ class OptimizedImage < ActiveRecord::Base -interpolate catrom -unsharp 2x0.5+0.7+0 -interlace none - -quality 98 -profile #{File.join(Rails.root, 'vendor', 'data', 'RT_sRGB.icm')} #{to} }) @@ -240,7 +246,7 @@ class OptimizedImage < ActiveRecord::Base from = prepend_decoder!(from, to, opts) to = prepend_decoder!(to, to, opts) - %W{ + instructions = %W{ convert #{from}[0] -auto-orient @@ -250,10 +256,14 @@ class OptimizedImage < ActiveRecord::Base -crop #{dimensions}+0+0 -unsharp 2x0.5+0.7+0 -interlace none - -quality 98 -profile #{File.join(Rails.root, 'vendor', 'data', 'RT_sRGB.icm')} - #{to} } + + if opts[:quality] + instructions << "-quality" << opts[:quality].to_s + end + + instructions << to end def self.downsize_instructions(from, to, dimensions, opts = {}) diff --git a/app/models/upload.rb b/app/models/upload.rb index 27d8825fdd2..dac34616af3 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -269,6 +269,14 @@ class Upload < ActiveRecord::Base get_dimension(:thumbnail_height) end + def target_image_quality(local_path, test_quality) + @file_quality ||= Discourse::Utils.execute_command("identify", "-format", "%Q", local_path).to_i rescue 0 + + if @file_quality == 0 || @file_quality > test_quality + test_quality + end + end + def self.sha1_from_short_path(path) if path =~ /(\/uploads\/short-url\/)([a-zA-Z0-9]+)(\..*)?/ self.sha1_from_base62_encoded($2) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index b6a95a4e0c1..444b85d9c8f 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1758,6 +1758,8 @@ en: allow_all_attachments_for_group_messages: "Allow all email attachments for group messages." png_to_jpg_quality: "Quality of the converted JPG file (1 is lowest quality, 99 is best quality, 100 to disable)." + recompress_original_jpg_quality: "Quality of uploaded image files (1 is lowest quality, 99 is best quality, 100 to disable)." + image_preview_jpg_quality: "Quality of resized image files (1 is lowest quality, 99 is best quality, 100 to disable)." allow_staff_to_upload_any_file_in_pm: "Allow staff members to upload any files in PM." diff --git a/config/site_settings.yml b/config/site_settings.yml index 4d23fcbba7c..f3cabc72506 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1339,6 +1339,14 @@ files: default: 95 min: 1 max: 100 + recompress_original_jpg_quality: + default: 90 + min: 1 + max: 100 + image_preview_jpg_quality: + default: 90 + min: 1 + max: 100 allow_staff_to_upload_any_file_in_pm: default: true client: true diff --git a/lib/upload_creator.rb b/lib/upload_creator.rb index bdfb26cdf10..cf6f87aadba 100644 --- a/lib/upload_creator.rb +++ b/lib/upload_creator.rb @@ -56,7 +56,7 @@ class UploadCreator if @image_info.type.to_s == "svg" clean_svg! elsif !Rails.env.test? || @opts[:force_optimize] - convert_to_jpeg! if convert_png_to_jpeg? + convert_to_jpeg! if convert_png_to_jpeg? || should_alter_quality? downsize! if should_downsize? return @upload if is_still_too_big? @@ -202,11 +202,16 @@ class UploadCreator from = OptimizedImage.prepend_decoder!(from, nil, filename: "image.#{@image_info.type}") to = OptimizedImage.prepend_decoder!(to) + opts = {} + desired_quality = [SiteSetting.png_to_jpg_quality, SiteSetting.recompress_original_jpg_quality].compact.min + target_quality = @upload.target_image_quality(from, desired_quality) + opts = { quality: target_quality } if target_quality + begin - execute_convert(from, to) + execute_convert(from, to, opts) rescue # retry with debugging enabled - execute_convert(from, to, true) + execute_convert(from, to, opts.merge(debug: true)) end new_size = File.size(jpeg_tempfile.path) @@ -237,7 +242,7 @@ class UploadCreator execute_convert(from, to) rescue # retry with debugging enabled - execute_convert(from, to, true) + execute_convert(from, to, { debug: true }) end @file.respond_to?(:close!) ? @file.close! : @file.close @@ -245,22 +250,26 @@ class UploadCreator extract_image_info! end - def execute_convert(from, to, debug = false) + def execute_convert(from, to, opts = {}) command = [ "convert", from, "-auto-orient", "-background", "white", "-interlace", "none", - "-flatten", - "-quality", SiteSetting.png_to_jpg_quality.to_s + "-flatten" ] - command << "-debug" << "all" if debug + command << "-debug" << "all" if opts[:debug] + command << "-quality" << opts[:quality].to_s if opts[:quality] command << to Discourse::Utils.execute_command(*command, failure_message: I18n.t("upload.png_to_jpg_conversion_failure_message")) end + def should_alter_quality? + @upload.target_image_quality(@file.path, SiteSetting.recompress_original_jpg_quality).present? + end + def should_downsize? max_image_size > 0 && filesize >= max_image_size end diff --git a/spec/lib/upload_creator_spec.rb b/spec/lib/upload_creator_spec.rb index 49fbbab8381..471c37ffc9b 100644 --- a/spec/lib/upload_creator_spec.rb +++ b/spec/lib/upload_creator_spec.rb @@ -123,6 +123,11 @@ RSpec.describe UploadCreator do end describe 'converting to jpeg' do + def image_quality(path) + local_path = File.join(Rails.root, 'public', path) + Discourse::Utils.execute_command("identify", "-format", "%Q", local_path).to_i + end + let(:filename) { "should_be_jpeg.png" } let(:file) { file_from_fixtures(filename) } @@ -168,6 +173,21 @@ RSpec.describe UploadCreator do expect(File.extname(upload.url)).to eq('.jpeg') expect(upload.original_filename).to eq('should_be_jpeg.jpg') end + + it 'should alter the image quality' do + SiteSetting.png_to_jpg_quality = 75 + SiteSetting.recompress_original_jpg_quality = 40 + SiteSetting.image_preview_jpg_quality = 10 + + upload = UploadCreator.new(file, filename, force_optimize: true).create_for(user.id) + + expect(image_quality(upload.url)).to eq(SiteSetting.recompress_original_jpg_quality) + + upload.create_thumbnail!(100, 100) + upload.reload + + expect(image_quality(upload.optimized_images.first.url)).to eq(SiteSetting.image_preview_jpg_quality) + end end describe 'converting HEIF to jpeg' do