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

Editing with Photomator breaks query command JSON output #1518

Closed kitschpatrol closed 7 months ago

kitschpatrol commented 7 months ago

Thanks for this fantastic tool!

This is a bit of an edge case, and it's likely Photomator is doing something funky here, but I noticed that Photos libraries containing images edited in specific ways by Photomator can break osxphotos query --json output.

I've attached a single-photo test library that triggers the bug.

The error seems to come from parsing adjustments metadata added to the library when the "Super Resolution" feature is used in Photomator.

Sepcifically, adjustmentsinfo.py:115:

return self._adjustments["adjustments"] if self._adjustments else []

Throws:

TypeError: list indices must be integers or slices, not str

Inspecting self._adjustments of the offending image shows the following:

[
  1,
  {
    "s": {
      "v": 2,
      "c": {
        "o": "14.4.1",
        "v": "3.3.5",
        "h": "ea636c8",
        "u": "public.jpeg",
        "r": "Photomator",
        "d": "Mac15,10",
        "t": 735096853.71553
      }
    },
    "c": [
      3,
      {
        "c": "73DF27EA-0F00-4AB1-AEB1-407E802BFB3E",
        "super": [
          3,
          {
            "n": "404D022B-7D4C-45C5-977B-8B74E7E93E4D_4_5005_c",
            "i": "85C8AA14-8877-4C2C-8683-FDE7BF8B32B4",
            "u": "com.pixelmatorteam.pixelmator-photo-edit.document.binary",
            "e": true,
            "h": "85C8AA14",
            "v": 2,
            "w": [
              1,
              {
                "v": "3.3.5",
                "o": "14.4.1",
                "d": "Mac15,10",
                "a": "photomator",
                "b": "ea636c8",
                "r": 16230,
                "p": "macOS"
              }
            ],
            "l": "local"
          }
        ]
      }
    ]
  }
]

Seems like the expected "adjustments" key is missing.

To Reproduce from scratch:

  1. Open your library with Photomator
  2. Select an image
  3. Tools → Open Edit Tools
  4. Tools → Super Resolution
  5. Tools → Close Edit Tools
  6. Close Photomator
  7. Run osxphotos query --json

To Reproduce with the attached sample library:

  1. osxphotos query --library "Path/To/PhotomatorTest.photoslibrary" --json

Expected behavior: Normal JSON photo info output.

Running the same command without the --json flag does not trigger the error and produces normal output.

Desktop Mac: osxphotos, version 0.67.10 Python 3.12.3 (main, Apr 9 2024, 08:09:14) [Clang 15.0.0 (clang-1500.3.9.4)] macOS 14.4.1, arm64

RhetTbull commented 7 months ago

Thanks for the detailed report and test library. This will be helpful for debugging this! I'll take a look as soon as I have chance.

@all-contributors please add @kitschpatrol for bug

RhetTbull commented 7 months ago

@all-contributors please add @kitschpatrol for bug

allcontributors[bot] commented 7 months ago

@RhetTbull

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

I couldn't determine any contributions to add, did you specify any contributions? Please make sure to use valid contribution names.

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

allcontributors[bot] commented 7 months ago

@RhetTbull

@kitschpatrol already contributed before to bug

kitschpatrol commented 7 months ago

Thank you.

As a work-around for now I've patched extra TypeError checks in adjustmentsinfo.py, so that the (currently) un-parsable Photomator adjustment metadata is ignored.

I can see how supporting every possible adjustment data format would be out of scope for this tool, so maybe this approach makes sense? Or maybe it would better to indicate that data was present but unparsable by some means other than empty values?

RhetTbull commented 7 months ago

I can see how supporting every possible adjustment data format would be out of scope for this tool, so maybe this approach makes sense? Or maybe it would better to indicate that data was present but unparsable by some means other than empty values?

Yes, it's not possible to parse every format that an external editor might use. It might be best just to log a warning and return None or an empty dictionary. I'll take a look at the code and see what makes sense.

kitschpatrol commented 7 months ago

Thanks, sounds good. I just created a fork with the minimal changes required to (silently) ignore the missing fields when querying the example library I provided.

RhetTbull commented 7 months ago

This is fixed in main and will be included in next release