search

RhetTbull / osxphotos

Python app to work with pictures and associated metadata from Apple Photos on macOS. Also includes a package to provide programmatic access to the Photos library, pictures, and metadata.
MIT License
2.17k stars 100 forks source link

--overwrite option doesn't apply to preview if `--preview` specified #526

Open spencerc99 opened 3 years ago

spencerc99 commented 3 years ago

Using --overwrite in combination with --preview doesn't overwrite existing preview images and instead re-exports them. Would expect it to have the same behavior as with exporting normal images.

Let me know if you need me to provide a smaller reproduction or an image to help!

RhetTbull commented 3 years ago

Yep, you're right. I was able to reproduce this. I'll take a look to see what's going on. I would expect the behavior to be the same as all the other images with --overwrite.

RhetTbull commented 3 years ago

See also, #516

RhetTbull commented 3 years ago

@all-contributors add @spencerc99 for bug

allcontributors[bot] commented 3 years ago

@RhetTbull

I've put up a pull request to add @spencerc99! :tada:

RhetTbull commented 3 years ago

This is fixed in v0.42.93. Thanks for the bug report!

spencerc99 commented 3 years ago

amazing thank you for the quick fix!!

spencerc99 commented 3 years ago

@RhetTbull hmm is it expected that --update also doesn't take into account previews? It seems like I'm getting dupes in the preview images on exporting with --update but not the originals. (let me know if i should open up another issue for this too)

RhetTbull commented 3 years ago

Yeah, that's another bug. I've reopened this issue.

RhetTbull commented 3 years ago

I think this is fixed in v0.42.94.

Because a preview image will likely exist even if the original image is missing on disk, the preview code can be reached even if the main export code has been skipped (because original image is missing) but that means a lot of the checks (e.g. for duplicate files) that happen in that part of the code also got skipped which is why these two issues happened. Let me know if you still see any issues!

spencerc99 commented 3 years ago

thanks @RhetTbull! It's mostly better (most photos don't dupe the previews) but i'm still experiencing a weird issue where sometimes it will make a preview of the preview.

this is the latest batch of my updates, where I would only expect the last 4. It seems like it might only be an issue for ones where the file name conflicts so it has to add a (1) counter? image

RhetTbull commented 3 years ago

@spencerc99 what's the full command line you're using that leads to this issue?

RhetTbull commented 3 years ago

I've not been able to replicate this. Will try again once I get your full command line as it may be interplay between certain options. In the process though, I did discover another bug #529 that will require some significant work to fix.

In the meantime, I suggest you name your files to avoid duplicate filenames which I believe will solve this as well as the new issue. (The code responsible for incrementing filenames needs some serious rework as it doesn't handle all possible cases well). One way to do this is to use the {id} template field in your filename which is unique to every single photo. For example:

--filename "{original_name}_{id}"

will append a unique id number to each photo. e.g. _1, _2, etc.

You can format the id to be zero padded like so:

--filename "{original_name}_{id:06d}"

Which would pad the id field with zeros to ensure the ID is always 6 digits, e.g._000001,_000002, etc.

spencerc99 commented 3 years ago

here's my full command: osxphotos export --download-missing --album fits\ 🧢 --use-photokit --skip-live --skip-original-if-edited --filename "{created.date}" --update --convert-to-jpeg --post-function scripts/transform_exported_fits_into_db.py::post_function --preview --verbose ./fits-export

I'll try avoiding duplicate names! Will the state DB will recognize that I've already exported all these photos even if the export name changes? (just want to make sure I won't re-export everything)

spencerc99 commented 3 years ago

So I'm switching over to the new file name scheme (looks like the DB unfortunately does not recognize that so I'll just clear my data and re-export everything), but I noticed one more strange thing where the first pass I exported 199 photos (100 original photos and 99 previews, strangely one preview missing). Running the same command again successfully exported the preview, but I would've expected it to get exported in the first round. Any ideas on this?

Here's the full command I just ran: osxphotos export --download-missing --album fits\ 🧢 --use-photokit --skip-live --skip-original-if-edited --filename "{created.date}/{original_name}" --update --convert-to-jpeg --post-function scripts/transform_exported_fits_into_db.py::post_function --preview --verbose ./fits-export

RhetTbull commented 3 years ago

looks like the DB unfortunately does not recognize that

Yes, if you change the file naming scheme, osxphotos won't be able to correlate previously exported photos to the new names. The export database uses the exported filename as the key for storing metadata about the photo. This is what allows --update to work.

strangely one preview missing

That is odd. It is possible the preview file itself was missing (would be unusual but not impossible) and Photos regenerated it. Because you're using --use-photokit, osxphotos will use the native PhotoKit API to do the export so it's possible PhotoKit requested the photo which was not available on the local machine and had to be downloaded from iCloud which then caused Photos to regenerate the missing preview.

Also, I noticed you're using a / in your filename. That's fine but you should be aware that, while the / shows in Finder, in the Terminal, the / character, which is the directory separator, will be represented as a :. This happens automatically but you'll need to know this if you are manipulating files in the command line. For this reason, I prefer a - or _ character in filenames.

RhetTbull commented 3 years ago

Thanks for the command line. I'll see if I can re-create the issue you're seeing. The export code is the most complicated bit of code in osxphotos and it's ripe for refactoring (#462, #529) -- unfortunately I'm in the midst of another project at the moment so this will have to wait a bit. I've already sketched out a new design though for the export code which should simplify things and make it easier to track down bugs like this issue.

spencerc99 commented 3 years ago

Also, I noticed you're using a / in your filename. That's fine but you should be aware that, while the / shows in Finder, in the Terminal, the / character, which is the directory separator, will be represented as a :. This happens automatically but you'll need to know this if you are manipulating files in the command line. For this reason, I prefer a - or _ character in filenames.

Oh interesting. I actually was using : as the separator there in my script but I noticed that it converted to / in Finder, so just naively switched it to / thinking that it would be interchangeable, but looks like I ran into that conversion.

That is odd. It is possible the preview file itself was missing (would be unusual but not impossible) and Photos regenerated it. Because you're using --use-photokit, osxphotos will use the native PhotoKit API to do the export so it's possible PhotoKit requested the photo which was not available on the local machine and had to be downloaded from iCloud which then caused Photos to regenerate the missing preview.

hmm I see and is the reason it missed it in the first round of exports then because --download-missing only applies to originals?

spencerc99 commented 3 years ago

Thanks for the command line. I'll see if I can re-create the issue you're seeing. The export code is the most complicated bit of code in osxphotos and it's ripe for refactoring (#462, #529) -- unfortunately I'm in the midst of another project at the moment so this will have to wait a bit. I've already sketched out a new design though for the export code which should simplify things and make it easier to track down bugs like this issue.

no worries appreciate the quick help! using the original filename in my export file name should work for now and makes the end export cleaner too :)

RhetTbull commented 3 years ago

hmm I see and is the reason it missed it in the first round of exports then because --download-missing only applies to originals?

--download-missing should download both the original and the edited version if there is one. I've never seen a case where a preview image wasn't available on the local machine but doesn't mean it couldn't happen. But you're using --skip-original-if-edited so if the edited version wasn't on the local machine perhaps the preview wouldn't have been as well as the preview gets updated when the image is edited.

RhetTbull commented 2 years ago

@spencerc99 I finally got some time to take a look at this issue. I have not been able to reproduce the "preview_preview" issue but I did discover a bug associated with --preview and --download-missing. Namely, if the original file is missing and thus the --download-missing code runs, the code that exports the preview will not be run and thus previews aren't exported for missing photos. If the export is run again with --update, chances are good the originals will now be in the library (they were downloaded in the previous run) and now the preview code is reached, exporting the previews. I'll open a new issue for this. Still looking into the duplicate preview and the --overwrite issue.