LycheeOrg / Lychee

A great looking and easy-to-use photo-management-system you can run on your server, to manage and share photos.
https://lycheeorg.github.io/
MIT License
3.35k stars 299 forks source link

Checksum changes after import #645

Closed ikosa closed 2 years ago

ikosa commented 4 years ago

Detailed description of the problem

During the import process if a photo needs to be rotated file size and checksum changes. Because of that if you sync the same folder again all rotated photos will imported again and duplicated.

Another minor problem with this issue is it is not good that your files are changed especially if you select delete imported. I didnt dig deeper to understand how rotation changing the file size (i get smaller files after rotation in my tests. aprox 3.4MB->3.2MB) İt will be good to bind this behavior to a config.

inspected cause of this issue: In photoFunctions checksum is calculated before rotation.

Output of the diagnostics

Info: Latest version of PHP is 7.4
Warning: Dropbox import not working. dropbox_key is empty.

System Information
--------------
Lychee Version (git):       No git data found.
DB Version:                 4.0.6

composer install:           --no-dev
APP_ENV:                    production
APP_DEBUG:                  true

System:                     Linux
PHP Version:                7.3
MySQL Version:              10.4.13-MariaDB-1:10.4.13+maria~bionic-log

Imagick:                    1
Imagick Active:             1
Imagick Version:            1690
GD Version:                 2.2.5

Config Information
--------------
version:                    040006
check_for_updates:          0
sorting_Photos_col:         id
sorting_Photos_order:       DESC
sorting_Albums_col:         id
sorting_Albums_order:       DESC
imagick:                    1
skip_duplicates:            1
small_max_width:            0
small_max_height:           360
medium_max_width:           1920
medium_max_height:          1080
lang:                       en
layout:                     0
image_overlay:              0
image_overlay_type:         exif
default_license:            none
compression_quality:        90
full_photo:                 1
delete_imported:            0
Mod_Frame:                  1
Mod_Frame_refresh:          30
thumb_2x:                   1
small_2x:                   1
medium_2x:                  1
landing_page_enable:        0
landing_owner:              Erşan Kuneri
landing_title:              Erşan Kuneri
landing_subtitle:           Cats, Dogs & Humans Photography
landing_facebook:           https://www.facebook.com/JohnSmith
landing_flickr:             https://www.flickr.com/JohnSmith
landing_twitter:            https://www.twitter.com/JohnSmith
landing_instagram:          https://instagram.com/JohnSmith
landing_youtube:            https://www.youtube.com/JohnSmith
landing_background:         dist/cat.jpg
site_title:                 Lychee v4
site_copyright_enable:      1
site_copyright_begin:       1981
site_copyright_end:         2020
additional_footer_text:     
display_social_in_gallery:  0
public_search:              1
SL_enable:                  0
SL_for_admin:               0
public_recent:              0
recent_age:                 1
public_starred:             0
downloadable:               0
photos_wraparound:          1
map_display:                1
zip64:                      1
map_display_public:         0
map_provider:               Wikimedia
force_32bit_ids:            0
map_include_subalbums:      0
update_check_every_days:    3
has_exiftool:               1
share_button_visible:       0
import_via_symlink:         0
has_ffmpeg:                 1
location_decoding:          1
location_decoding_timeout:  30
location_show:              1
location_show_public:       0
rss_enable:                 0
rss_recent_days:            7
rss_max_items:              100
prefer_available_xmp_metadata: 0
tmp-hallenser commented 4 years ago

Can you provide a sample photo?

ikosa commented 4 years ago

https://drive.google.com/file/d/1CiWRQ8CbbZE8rwyzszn-IQs7cVMTlADI/view?usp=sharing

Before Import: c1f0c3c903091137f89284438d1218200e671ab1 (3.910.816 bytes) After Import: d325a48cef9d3dc8e37e5447709e5730af04782e (3.858.820 bytes)

I think this can be repeated with all portrait photos.

tmp-hallenser commented 4 years ago

I think I understand where it's coming from:

  1. We import a photo
  2. Lychee detects that it needs to be rotated
  3. It rotates the original file and creates thumbnails. Hash is calculated on the rotated picture.

If we then sync again, the following happens:

  1. Check sums are compared -> no match as original and rotated image have different check sums
  2. Photo is imported (and rotated) and a duplicate is created.

@ildyria @kamil4 Let's discuss how to approach this. I see two ways:

  1. Do not rotate original file Do not rotate the original file and only rotate the smaller versions Rational: If we rotate the original file, it's not the original file anymore (upload -> download results in a different file)

  2. Keep rotation of original file Do the rotation of the original file before checking for duplicate Rational: Keeps the logic as we have it now.

ildyria commented 4 years ago

How about compute the checksum before the rotation ? Rotate if necessary after the checksum. Sure, checksum of the rotated image won't match what is in the DB, but that solve the problem.

Additionally, this approach is also fine with the "rotation" photo editor which we added recently: we do not recompute the hash of the picture after rotation.

tmp-hallenser commented 4 years ago

I was a little to fast -> we do calculate the checksum before the rotation -> this can't be the issue.

ikosa commented 4 years ago

Hash is calculated before the rotation that is the main problem. The order of rotation and hash calculation has to be changed. Or a quick fix can be a recalculation after rotation which is a bit waste. I comment out the rotation line so far everything seems ok even if in IE.

ildyria commented 2 years ago

Status ?

nagmat84 commented 2 years ago

I have not been aware of this problem until now. As I rewrote most of the the import logic I feel as I would be in charge to fix this issue if it still exists. It probably does as I tried not to change any logic. The problem never occurred to me as it is not covered by our automatic tests.

I need to check with the current code what the best solution could be.

kamil4 commented 2 years ago

Yeah, I was not aware of this issue either, and in particular its possible connection with the interactive rotate functionality. I rewrote the interactive rotation support at one point and I remember wondering back then what to do about checksums, duplicates, and such. I implemented something but it wasn't clear-cut and I still have some doubts about what the right thing to do is.

It seems like this is something we should try to comprehensively address.

nagmat84 commented 2 years ago

It seems like this is something we should try to comprehensively address.

I just say, (m:n)-relationship ... :wink:

kamil4 commented 2 years ago

I'm closing this as I can't reproduce the problem on current master.

The file size update was definitely addressed as part of #888 (funny as I wasn't even aware of this issue at the time).

The checksum in the database is always the checksum of the original file and does not change when the image is rotated (be it during import or manually later). So it will match during the next import attempt and the file will be skipped, as expected.