icloud-photos-downloader / icloud_photos_downloader

A command-line tool to download photos from iCloud
MIT License
6.61k stars 536 forks source link

When multiple sizes and --force-size are specified, some sizes can be skipped even if exist. #955

Open iowk opened 3 days ago

iowk commented 3 days ago

Overview

When both --size medium and --size thumb are specified with --force-size, but only the thumb-size asset exists for a photo, both sizes will be skipped.

Steps to Reproduce

  1. Upload a small image to iCloud Photos, such that the medium-size asset exists but thumb-size does not. (e.g. a 200*250 heic file)
  2. Execute icloudpd -d syncdir --size medium --size thumb --force-size.

Expected Behavior

The medium-size asset should be skipped and the thumb-size asset should be downloaded.

Actual Behavior

Both medium-size and thumb-size assets are skipped.

Context

Changing this to continue should be a simple fix. https://github.com/icloud-photos-downloader/icloud_photos_downloader/blob/6e9c6c69996a39d0e7f77c9fd4f9b160b4b8bcdd/src/icloudpd/base.py#L896 I would be glad to issue a PR if you consider this appropriate.

AndreyNikiforov commented 3 days ago

Thanks for reporting the issue. Yes, you can submit PR. Please include tests.

I am curious how are you using medium & thumb sized assets?

iowk commented 2 days ago

I have created a PR with tests.

I am developing an application that utilizes the assets to preview the photos.

It would be helpful to me if --live-photo-size also supports multiple sizes. I can work on that if you think it's a good idea.

AndreyNikiforov commented 2 days ago

It would be helpful to me if --live-photo-size also supports multiple sizes. I can work on that if you think it's a good idea.

I did not see requests for live photo and main assets sizes, so I suspect that with the increase of the internet speed, ppl are using original size. From that point of view, I do not see much value in implementing multiple size support for live photos.