fix(server): extraction of Samsung Motionphoto videos

[kaysond]

Fixes #4873.

The issue is that the current video extraction relies on data length and padding from the XMP inside the EXIF. This happened to work on One UI 5 (the resulting mp4 file was technically invalid, but still worked because the extra bytes were at the end), but stopped working on One UI 6. In any case, the author of exiftool recommends not using the XMP because it doesn't have any fundamental connection to the "Trailer" of the jpeg where the video data actually resides. See https://exiftool.org/forum/index.php?topic=15565.0

Fortunately, exiftool is already able to parse the jpeg trailer data structure, and exiftool-vendored has methods to leverage that. The data structure is somewhat complicated, (see https://github.com/exiftool/exiftool/blob/master/lib/Image/ExifTool/Samsung.pm#L1532), so I think it's unwise to attempt to parse the structure ourselves.

This also adds support for extracting the video from an heic-encoded motionphoto, since it's just a different tag name. (see https://exiftool.org/forum/index.php?topic=7161)

This is a minimally-invasive but naive implementation. Creating a dedicated exiftool singleton for each video that has to be extracted is inefficient and the performance hit would probably be noticeable if you upload a lot of motionphotos at once. Fixing this pretty straightforward; it would just mean refactoring exiftool up from metadata.repository into metadata.service.

I also didn't touch any of the existing extraction code. I'm not sure if it's still needed for iOS or other motionphoto implementations (Google?), but it could potentially be removed if not.

[jrasm91]

Looks good, minus the linting.

[kaysond]

Done with the draft. Just need to actually test it now and it should be good to go.

Do we want to add unit tests for live/motionphoto video extraction?

[jrasm91]

If you could add a unit test or two that would be great.

[danieldietzler]

LGTM! :)

[kaysond]

LGTM! :)

Thanks. Going to make one more change so that running the metadata extraction job will fix broken videos.

[kaysond]

Ok this is finally ready for merging. Thanks to everyone who helped!

A few notes:

• Users can run the Metadata Extraction (All) job to fix corrupt motionphoto videos. Maybe we can mention this in the release notes @alextran1502 ?
• Because we fix corrupt videos, the above job has slowed down from before. It could be moved into an entirely separate job, but that would be a much bigger change.
• It seems that the metadata extraction job does not run on trashed assets, but it does on the trashed asset's motionphoto videos. This means errors can still show up if an image with corrupt motionphoto video is trashed. (NB: it may be worth updating the "trashing" job to also trash the associated video)

[jrasm91]

I'm surprised this works because motionPath is the same for both the old and new motion assets. Saving the new video would overwrite the old motion file, but then you queue the old one for deletion.

[kaysond]

I'm surprised this works because motionPath is the same for both the old and new motion assets. Saving the new video would overwrite the old motion file, but then you queue the old one for deletion.

@jrasm91 - that's a really good catch. I'm guessing that by the time the code looked like this, my immich instance had all the right assets, so it never triggered that code.

I moved the delete before the save. There's no race condition here, right? Meaning await this.jobRepository.queue({ name: JobName.ASSET_DELETION, data: { id: asset.livePhotoVideoId } }); will block until the job completes?

[jrasm91]

No, that only queues it. Who knows when it will be deleted. Although, we can also solve this by simply changing how the original path is generated - it doesn't have to be the parent asset's id. I think we could generate a UUID ahead of time, and use that to create the motion asset and motion asset original path.

[kaysond]

No, that only queues it. Who knows when it will be deleted. Although, we can also solve this by simply changing how the original path is generated - it doesn't have to be the parent asset's id. I think we could generate a UUID ahead of time, and use that to create the motion asset and motion asset original path.

Ok this should take care of things. I've been trying to test it but I can't get the dev docker compose to start up because of some weird docker issue. Tried two different machines and same bug on both...

If you have a chance to test it please lmk.

[kaysond]

This should finally be ready for merge @jrasm91
Tested by:

1. Ran immich v1.93.2 dev instance
2. Uploaded 3 test motionphotos
3. Note that OneUI 5 works, but 6 is broken
4. Switch to PR dev instance
5. Note that OneUI 5 still works, but 6 is broken
6. Run metadata extraction job
7. Note that old video assets are deleted and new video assets are extracted correctly
8. Run job again, nothing happens as expected

[kaysond]

I'm going to hold my tongue until this is actually merged :D

Formatting fixed and tests fixed and updated to have more

[jrasm91]

I tested this and it worked well. The only thing I noticed was that the broken video could be cached in the browser and that could prevent it from playing successfully immediately afterwards.