posts: fix incorrect exif rotation for PNGs.
Fix a bug where where PNG images could be incorrectly detected as exif-rotated. This would happen when a PNG contained the IFD0:Orientation flag. It's technically possible for a PNG to contain this flag, but it's ignored by libvips and by browsers. post #3762340 (nsfw) is an example of a PNG like this. The fix is to use `autorot` to let libvips apply the rotation instead of trying to interpret the exif data ourselves. Note that libvips-8.9 has a bug where it doesn't strip the orientation flag after applying `autorot`, which leads to the image being incorrectly rotated a second time when generating the thumbnail. Use libvips-8.11 instead.
This commit is contained in:
@@ -11,23 +11,11 @@ class MediaFile::Image < MediaFile
|
|||||||
CROP_OPTIONS = { crop: :attention, linear: false }
|
CROP_OPTIONS = { crop: :attention, linear: false }
|
||||||
|
|
||||||
def dimensions
|
def dimensions
|
||||||
[width, height]
|
image.size
|
||||||
rescue Vips::Error
|
rescue Vips::Error
|
||||||
[0, 0]
|
[0, 0]
|
||||||
end
|
end
|
||||||
|
|
||||||
def width
|
|
||||||
is_rotated? ? image.height : image.width
|
|
||||||
rescue Vips::Error
|
|
||||||
0
|
|
||||||
end
|
|
||||||
|
|
||||||
def height
|
|
||||||
is_rotated? ? image.width : image.height
|
|
||||||
rescue Vips::Error
|
|
||||||
0
|
|
||||||
end
|
|
||||||
|
|
||||||
def is_corrupt?
|
def is_corrupt?
|
||||||
image.stats
|
image.stats
|
||||||
false
|
false
|
||||||
@@ -81,14 +69,9 @@ class MediaFile::Image < MediaFile
|
|||||||
file_ext == :png && metadata.fetch("PNG:AnimationFrames", 1) > 1
|
file_ext == :png && metadata.fetch("PNG:AnimationFrames", 1) > 1
|
||||||
end
|
end
|
||||||
|
|
||||||
# https://exiftool.org/TagNames/EXIF.html
|
|
||||||
def is_rotated?
|
|
||||||
metadata["IFD0:Orientation"].in?(["Rotate 90 CW", "Rotate 270 CW"])
|
|
||||||
end
|
|
||||||
|
|
||||||
# @return [Vips::Image] the Vips image object for the file
|
# @return [Vips::Image] the Vips image object for the file
|
||||||
def image
|
def image
|
||||||
Vips::Image.new_from_file(file.path, fail: true)
|
Vips::Image.new_from_file(file.path, fail: true).autorot
|
||||||
end
|
end
|
||||||
|
|
||||||
memoize :image, :dimensions, :is_corrupt?, :is_animated_gif?, :is_animated_png?
|
memoize :image, :dimensions, :is_corrupt?, :is_animated_gif?, :is_animated_png?
|
||||||
|
|||||||
@@ -49,7 +49,7 @@ class MediaAsset < ApplicationRecord
|
|||||||
|
|
||||||
# https://exiftool.org/TagNames/EXIF.html
|
# https://exiftool.org/TagNames/EXIF.html
|
||||||
def is_rotated?
|
def is_rotated?
|
||||||
metadata["IFD0:Orientation"].in?(["Rotate 90 CW", "Rotate 270 CW", "Rotate 180"])
|
file_ext == "jpg" && metadata["IFD0:Orientation"].in?(["Rotate 90 CW", "Rotate 270 CW", "Rotate 180"])
|
||||||
end
|
end
|
||||||
|
|
||||||
# Some animations technically have a finite loop count, but loop for hundreds
|
# Some animations technically have a finite loop count, but loop for hundreds
|
||||||
|
|||||||
BIN
test/files/test-rotation-90cw.png
Normal file
BIN
test/files/test-rotation-90cw.png
Normal file
Binary file not shown.
|
After Width: | Height: | Size: 26 KiB |
@@ -359,4 +359,16 @@ class MediaFileTest < ActiveSupport::TestCase
|
|||||||
assert_equal([33, 50], @file.preview(50, 50).dimensions)
|
assert_equal([33, 50], @file.preview(50, 50).dimensions)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context "a PNG with an exif orientation flag" do
|
||||||
|
should "not generate rotated dimensions" do
|
||||||
|
@file = MediaFile.open("test/files/test-rotation-90cw.png")
|
||||||
|
assert_equal([128, 96], @file.dimensions)
|
||||||
|
end
|
||||||
|
|
||||||
|
should "not generate a rotated thumbnail" do
|
||||||
|
@file = MediaFile.open("test/files/test-rotation-90cw.png")
|
||||||
|
assert_equal([64, 48], @file.preview(64, 64).dimensions)
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -1336,6 +1336,15 @@ class PostTest < ActiveSupport::TestCase
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context "a PNG with the exif orientation flag" do
|
||||||
|
should "not add the exif_rotation tag" do
|
||||||
|
@media_asset = MediaAsset.create!(file: "test/files/test-rotation-90cw.png")
|
||||||
|
@post.update!(md5: @media_asset.md5)
|
||||||
|
@post.reload.update!(tag_string: "tagme")
|
||||||
|
assert_equal("tagme", @post.tag_string)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
context "a non-repeating GIF missing the non-repeating_animation tag" do
|
context "a non-repeating GIF missing the non-repeating_animation tag" do
|
||||||
should "automatically add the non-repeating_animation tag" do
|
should "automatically add the non-repeating_animation tag" do
|
||||||
@media_asset = MediaAsset.create!(file: "test/files/test-animated-86x52-loop-1.gif")
|
@media_asset = MediaAsset.create!(file: "test/files/test-animated-86x52-loop-1.gif")
|
||||||
|
|||||||
Reference in New Issue
Block a user