forked/osxphotos
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix download missing when not needed 1086 (#1088)

Browse Source 
* Fixed staging to not call download missing if not needed

* Optimizations for #1086

* Memoize compiled XMP template, #1086
This commit is contained in:
Rhet Turnbull
2023-06-17 07:14:24 -07:00
committed by GitHub
parent 986010ec5f
commit 269a2dfa9e
2 changed files with 144 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files

127
osxphotos/photoexporter.py
View File

@@ -1,15 +1,15 @@
""" PhotoExport class to export photos """ PhotoExport class to export photos
""" """
from __future__ import annotations
import dataclasses import dataclasses
import json import json
import logging import logging
import os import os
import pathlib import pathlib
import re import re
import sys
import typing as t import typing as t
from collections import namedtuple # pylint: disable=syntax-error
from dataclasses import asdict, dataclass from dataclasses import asdict, dataclass
from datetime import datetime from datetime import datetime
from enum import Enum from enum import Enum
@@ -39,10 +39,10 @@ from .phototemplate import RenderOptions
from .rich_utils import add_rich_markup_tag from .rich_utils import add_rich_markup_tag
from .uti import get_preferred_uti_extension from .uti import get_preferred_uti_extension
from .utils import ( from .utils import (
is_macos,
hexdigest, hexdigest,
increment_filename, increment_filename,
increment_filename_with_count, increment_filename_with_count,
is_macos,
lineno, lineno,
list_directory, list_directory,
lock_filename, lock_filename,
@@ -76,6 +76,10 @@ if t.TYPE_CHECKING:
# retry if download_missing/use_photos_export fails the first time (which sometimes it does) # retry if download_missing/use_photos_export fails the first time (which sometimes it does)
MAX_PHOTOSCRIPT_RETRIES = 3 MAX_PHOTOSCRIPT_RETRIES = 3
# Global to hold the compiled XMP template
# This is expensive to compile so we only want to do it once
_global_xmp_template: Template | None = None
# return values for _should_update_photo # return values for _should_update_photo
class ShouldUpdate(Enum): class ShouldUpdate(Enum):
@@ -244,6 +248,9 @@ class StagedFiles:
def __str__(self): def __str__(self):
return str(self.asdict()) return str(self.asdict())
def __repr__(self):
return f"StagedFiles({self.asdict()})"
def asdict(self): def asdict(self):
return { return {
"original": self.original, "original": self.original,
@@ -479,6 +486,11 @@ class PhotoExporter:
dest, options = self._should_convert_to_jpeg(dest, options) dest, options = self._should_convert_to_jpeg(dest, options)
# stage files for export by finding path in local library or downloading from iCloud as appropriate # stage files for export by finding path in local library or downloading from iCloud as appropriate
# for `--download-missing` and `--update` case, this may cause unnecessary downloads
# as it will download the file even if it's not needed (won't be checked until the _should_update_photo() call from _export_photo()
# fixing this will require major refactoring of the export code, see #1086
# leaving it for now as this should not be a common use case
# (if using `--update` it is much better to be using "Download originals to this Mac" in Photos)
staged_files = self._stage_photos_for_export(options) staged_files = self._stage_photos_for_export(options)
src = staged_files.edited if options.edited else staged_files.original src = staged_files.edited if options.edited else staged_files.original
@@ -665,8 +677,16 @@ class PhotoExporter:
return lock_filename(filename) if lock else filename return lock_filename(filename) if lock else filename
# if overwrite==False and #increment==False, export should fail if file exists # if overwrite==False and #increment==False, export should fail if file exists
if dest.exists() and not any( if (
[options.increment, options.update, options.force_update, options.overwrite] not any(
[
options.increment,
options.update,
options.force_update,
options.overwrite,
]
)
and dest.exists()
): ):
raise FileExistsError( raise FileExistsError(
f"destination exists ({dest}); overwrite={options.overwrite}, increment={options.increment}" f"destination exists ({dest}); overwrite={options.overwrite}, increment={options.increment}"
@@ -725,9 +745,19 @@ class PhotoExporter:
return dest return dest
def _should_update_photo( def _should_update_photo(
self, src: pathlib.Path, dest: pathlib.Path, options: ExportOptions self, src: pathlib.Path | None, dest: pathlib.Path, options: ExportOptions
) -> t.Literal[True, False]: ) -> bool | ShouldUpdate:
"""Return True if photo should be updated, else False""" """Return True if photo should be updated, else False
Args:
src (pathlib.Path | None): source path; if None, photo is missing and
any checks that require src will return True
dest (pathlib.Path): destination path
Returns:
False if photo should not be updated otherwise a truthy ShouldUpdate value
"""
# NOTE: The order of certain checks is important # NOTE: The order of certain checks is important
# read the comments below to understand why before changing # read the comments below to understand why before changing
@@ -740,11 +770,11 @@ class PhotoExporter:
# photo doesn't exist in database, should update # photo doesn't exist in database, should update
return ShouldUpdate.NOT_IN_DATABASE return ShouldUpdate.NOT_IN_DATABASE
if options.export_as_hardlink and not dest.samefile(src): if options.export_as_hardlink and (not src or not dest.samefile(src)):
# different files, should update # different files, should update
return ShouldUpdate.HARDLINK_DIFFERENT_FILES return ShouldUpdate.HARDLINK_DIFFERENT_FILES
if not options.export_as_hardlink and dest.samefile(src): if not options.export_as_hardlink and (not src or dest.samefile(src)):
# same file but not exporting as hardlink, should update # same file but not exporting as hardlink, should update
return ShouldUpdate.NOT_HARDLINK_SAME_FILES return ShouldUpdate.NOT_HARDLINK_SAME_FILES
@@ -776,7 +806,9 @@ class PhotoExporter:
# as exiftool will be used to update edited file # as exiftool will be used to update edited file
return ShouldUpdate.EXIFTOOL_DIFFERENT if rv else False return ShouldUpdate.EXIFTOOL_DIFFERENT if rv else False
if options.edited and not fileutil.cmp_file_sig(src, file_record.src_sig): if options.edited and (
not src or not fileutil.cmp_file_sig(src, file_record.src_sig)
):
# edited file in Photos doesn't match what was last exported # edited file in Photos doesn't match what was last exported
return ShouldUpdate.EDITED_SIG_DIFFERENT return ShouldUpdate.EDITED_SIG_DIFFERENT
@@ -827,22 +859,46 @@ class PhotoExporter:
# download any missing files # download any missing files
if options.download_missing: if options.download_missing:
live_photo = staged.edited_live if options.edited else staged.original_live staged |= self._stage_missing_photos_for_export(
missing_options = ExportOptions( staged=staged, options=options
edited=options.edited,
preview=options.preview and not staged.preview,
raw_photo=options.raw_photo and not staged.raw,
live_photo=options.live_photo and not live_photo,
) )
if options.use_photokit:
missing_staged = self._stage_photo_for_export_with_photokit( return staged
options=missing_options
) def _stage_missing_photos_for_export(
else: self, staged: StagedFiles, options: ExportOptions
missing_staged = self._stage_photo_for_export_with_applescript( ) -> StagedFiles:
options=missing_options """Download and stage any missing files for export"""
)
staged |= missing_staged # if live photo and requesting edited version need the edited live photo
live_photo = staged.edited_live if options.edited else staged.original_live
# is there actually a missing file? (#1086)
something_to_download = (
(self.photo.hasadjustments and options.edited and not staged.edited)
or (self.photo.live_photo and options.live_photo and not live_photo)
or (self.photo.has_raw and options.raw_photo and not staged.raw)
or (options.preview and not staged.preview)
or (not options.edited and not staged.original)
)
if not something_to_download:
return staged
missing_options = ExportOptions(
edited=options.edited,
preview=options.preview and not staged.preview,
raw_photo=self.photo.has_raw and options.raw_photo and not staged.raw,
live_photo=self.photo.live_photo and options.live_photo and not live_photo,
)
if options.use_photokit:
missing_staged = self._stage_photo_for_export_with_photokit(
options=missing_options
)
else:
missing_staged = self._stage_photo_for_export_with_applescript(
options=missing_options
)
staged |= missing_staged
return staged return staged
def _stage_photo_for_export_with_photokit( def _stage_photo_for_export_with_photokit(
@@ -1959,10 +2015,7 @@ class PhotoExporter:
options = options or ExportOptions() options = options or ExportOptions()
xmp_template_file = ( xmp_template = self._xmp_template()
_XMP_TEMPLATE_NAME if not self.photo._db._beta else _XMP_TEMPLATE_NAME_BETA
)
xmp_template = Template(filename=os.path.join(_TEMPLATE_DIR, xmp_template_file))
if extension is None: if extension is None:
extension = pathlib.Path(self.photo.original_filename) extension = pathlib.Path(self.photo.original_filename)
@@ -2069,6 +2122,20 @@ class PhotoExporter:
xmp_str = "\n".join(line for line in xmp_str.split("\n") if line.strip() != "") xmp_str = "\n".join(line for line in xmp_str.split("\n") if line.strip() != "")
return xmp_str return xmp_str
def _xmp_template(self):
"""Return the mako template for XMP sidecar, creating it if necessary"""
global _global_xmp_template
if _global_xmp_template is not None:
return _global_xmp_template
xmp_template_file = (
_XMP_TEMPLATE_NAME_BETA if self.photo._db._beta else _XMP_TEMPLATE_NAME
)
_global_xmp_template = Template(
filename=os.path.join(_TEMPLATE_DIR, xmp_template_file)
)
return _global_xmp_template
def _write_sidecar(self, filename, sidecar_str): def _write_sidecar(self, filename, sidecar_str):
"""write sidecar_str to filename """write sidecar_str to filename
used for exporting sidecar info""" used for exporting sidecar info"""

73
osxphotos/photoinfo.py
View File

@@ -877,33 +877,10 @@ class PhotoInfo:
photopath = None photopath = None
if self._db._db_version <= _PHOTOS_4_VERSION: if self._db._db_version <= _PHOTOS_4_VERSION:
if self.live_photo and not self.ismissing: return self._path_live_photo_4()
live_model_id = self._info["live_model_id"]
if live_model_id is None:
logger.debug(f"missing live_model_id: {self._uuid}")
photopath = None
else:
folder_id, file_id, nn_id = _get_resource_loc(live_model_id)
library_path = self._db.library_path
photopath = os.path.join(
library_path,
"resources",
"media",
"master",
folder_id,
nn_id,
f"jpegvideocomplement_{file_id}.mov",
)
if not os.path.isfile(photopath):
# In testing, I've seen occasional missing movie for live photo
# These appear to be valid -- e.g. live component hasn't been downloaded from iCloud
# photos 4 has "isOnDisk" column we could check
# or could do the actual check with "isfile"
# TODO: should this be a warning or debug?
photopath = None
else:
photopath = None
elif self.live_photo and self.path and not self.ismissing: elif self.live_photo and self.path and not self.ismissing:
if self.shared:
return self._path_live_photo_shared_5()
filename = pathlib.Path(self.path) filename = pathlib.Path(self.path)
photopath = filename.parent.joinpath(f"{filename.stem}_3.mov") photopath = filename.parent.joinpath(f"{filename.stem}_3.mov")
photopath = str(photopath) photopath = str(photopath)
@@ -917,6 +894,50 @@ class PhotoInfo:
return photopath return photopath
def _path_live_photo_shared_5(self):
"""Return path for live photo for shared photos"""
if not self.shared:
raise ValueError(f"photo {self.uuid} is not a shared photo")
if not self.live_photo:
raise ValueError(f"photo {self.uuid} is not a live photo")
photopath = self._path_5_shared()
if photopath:
photopath = pathlib.Path(photopath).with_suffix(".MOV")
if not photopath.exists():
photopath = None
return photopath
def _path_live_photo_4(self):
"""Return path for live edited photo for Photos <= 4"""
if self.live_photo and not self.ismissing:
live_model_id = self._info["live_model_id"]
if live_model_id is None:
logger.debug(f"missing live_model_id: {self._uuid}")
photopath = None
else:
folder_id, file_id, nn_id = _get_resource_loc(live_model_id)
library_path = self._db.library_path
photopath = os.path.join(
library_path,
"resources",
"media",
"master",
folder_id,
nn_id,
f"jpegvideocomplement_{file_id}.mov",
)
if not os.path.isfile(photopath):
# In testing, I've seen occasional missing movie for live photo
# These appear to be valid -- e.g. live component hasn't been downloaded from iCloud
# photos 4 has "isOnDisk" column we could check
# or could do the actual check with "isfile"
# TODO: should this be a warning or debug?
photopath = None
else:
photopath = None
return photopath
@cached_property @cached_property
def path_derivatives(self): def path_derivatives(self):
"""Return any derivative (preview) images associated with the photo as a list of paths, sorted by file size (largest first)""" """Return any derivative (preview) images associated with the photo as a list of paths, sorted by file size (largest first)"""