immich-app/immich
1
0
Fork 0
mirror of https://github.com/immich-app/immich.git synced 2026-02-27 19:08:16 +03:00
Code Issues 783 Packages Projects Releases 12 Wiki Activity

[PR #6337] [MERGED] fix(server): extraction of Samsung Motionphoto videos #11147

New Issue
Closed
opened 2026-02-05 14:34:19 +03:00 by OVERLORD · 0 comments
OVERLORD commented 2026-02-05 14:34:19 +03:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/immich-app/immich/pull/6337
Author: @kaysond
Created: 1/12/2024
Status: ✅ Merged
Merged: 1/22/2024
Merged by: @jrasm91

Base: main ← Head: main

📝 Commits (10+)

  • b1eb93c Fix extraction of samsung motionphoto videos
  • 58c7886 Refactor binary tag extraction to the repository to consolidate exiftool usage
  • 95b61d1 format
  • 6b4c85b fix linting and swap argument orders
  • 073ae39 Fix tag name and conditional order
  • 61f92c1 Add unit test
  • 0cd2a95 Update server test assets submodule
  • 8e76b55 Remove old motion photo video assets when a new one is extracted
  • 404d198 delete first, then write
  • b619c36 Include motion photo asset uuid's in the filename

📊 Changes

10 files changed (+176 additions, -46 deletions)

View changed files

📝 server/e2e/jobs/specs/metadata.e2e-spec.ts (+23 -1)
📝 server/src/domain/metadata/metadata.service.spec.ts (+95 -27)
📝 server/src/domain/metadata/metadata.service.ts (+43 -12)
📝 server/src/domain/repositories/metadata.repository.ts (+5 -1)
📝 server/src/domain/storage/storage.core.ts (+2 -2)
📝 server/src/infra/repositories/metadata.repository.ts (+4 -0)
📝 server/test/assets (+1 -1)
📝 server/test/fixtures/asset.stub.ts (+1 -1)
📝 server/test/fixtures/file.stub.ts (+1 -1)
📝 server/test/repositories/metadata.repository.mock.ts (+1 -0)

📄 Description

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.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/immich-app/immich/pull/6337 **Author:** [@kaysond](https://github.com/kaysond) **Created:** 1/12/2024 **Status:** ✅ Merged **Merged:** 1/22/2024 **Merged by:** [@jrasm91](https://github.com/jrasm91) **Base:** `main` ← **Head:** `main` --- ### 📝 Commits (10+) - [`b1eb93c`](https://github.com/immich-app/immich/commit/b1eb93c335a87a78114a212e0afb4896083bae1d) Fix extraction of samsung motionphoto videos - [`58c7886`](https://github.com/immich-app/immich/commit/58c7886b42b8f025d27ee12392b6c8d0eb9975c9) Refactor binary tag extraction to the repository to consolidate exiftool usage - [`95b61d1`](https://github.com/immich-app/immich/commit/95b61d17c9961f1637e327c0e4ebf6802eedc250) format - [`6b4c85b`](https://github.com/immich-app/immich/commit/6b4c85bf0ffb1c61c09a42092aa3bd13afd07d2d) fix linting and swap argument orders - [`073ae39`](https://github.com/immich-app/immich/commit/073ae396626ab84f66d4a1ad924ddb574b7f5c56) Fix tag name and conditional order - [`61f92c1`](https://github.com/immich-app/immich/commit/61f92c1e9f94ec8f963205aace76e916b9e283c4) Add unit test - [`0cd2a95`](https://github.com/immich-app/immich/commit/0cd2a959c33045c8d84946014e83921423f8f849) Update server test assets submodule - [`8e76b55`](https://github.com/immich-app/immich/commit/8e76b55bc6ec5409b240adc9825f36137fac6e3a) Remove old motion photo video assets when a new one is extracted - [`404d198`](https://github.com/immich-app/immich/commit/404d198f11ce54420adeb757768cf71d1592954b) delete first, then write - [`b619c36`](https://github.com/immich-app/immich/commit/b619c3623d293cc46b249104830fc21b08f952ac) Include motion photo asset uuid's in the filename ### 📊 Changes **10 files changed** (+176 additions, -46 deletions) <details> <summary>View changed files</summary> 📝 `server/e2e/jobs/specs/metadata.e2e-spec.ts` (+23 -1) 📝 `server/src/domain/metadata/metadata.service.spec.ts` (+95 -27) 📝 `server/src/domain/metadata/metadata.service.ts` (+43 -12) 📝 `server/src/domain/repositories/metadata.repository.ts` (+5 -1) 📝 `server/src/domain/storage/storage.core.ts` (+2 -2) 📝 `server/src/infra/repositories/metadata.repository.ts` (+4 -0) 📝 `server/test/assets` (+1 -1) 📝 `server/test/fixtures/asset.stub.ts` (+1 -1) 📝 `server/test/fixtures/file.stub.ts` (+1 -1) 📝 `server/test/repositories/metadata.repository.mock.ts` (+1 -0) </details> ### 📄 Description 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
OVERLORD added the pull-request label 2026-02-05 14:34:19 +03:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
accessibility
changelog:enhancement
changelog:security
changelog:skip
changelog:translation
cli
date-time
dependencies
documentation
external-library
format
good first issue
mobile-beta
mobile-beta
mobile-beta
needs-answer
nice to have
pull-request
Mirrored from GitHub Pull Request
 sharing
tech-debt
📱mobile
🖥️web
🗄️server
🧠machine-learning
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
OVERLORD
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: immich-app/immich#11147
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?