Store live photos as their image and their video (closes #107) #128

Merged
clawbot merged 1 commits from issue-107-live-photos into next 2026-09-28 18:06:01 +02:00
Collaborator

A live photo, one ZIP on Ente, is now stored as its image and its video (#107).

  • The download writer unpacks the ZIP as it streams, one temp file per part, and renames both into place together once complete and matching the recorded hash, each named after the destination with its entry's extension. A ZIP with a second image or video, or whose parts exceed 20 times its size plus 16 MiB, is refused without a retry.
  • The backup and the cache store <fileID>.<ext> per part and <fileID>.livephoto.json naming both. A live photo counts as stored only with both files; the cache evicts them together. Album folders link both, named as in #103.
  • quak get writes both: named after the title, or the image at --out PATH and the video beside it.
  • photo.original() adds videoPath; path stays the image, which the EXIF extraction reads.

Ente's layout, checked in ente-io/ente at commit d314e95c5359054cb76c37b260fe669160b620e9: web/packages/media/live-photo.ts and mobile/apps/photos/lib/module/live_photo/archive.dart write image.<ext> and video.<ext>; decodeLivePhoto in the former and rust/apps/cli/src/live_photo.rs save each part as the title with its entry's extension. archive.dart and live_photo.rs set the size limit.

Judgement calls:

  • <fileID>.livephoto.json is not in the brief: nothing else on disk says which part is the image.
  • The size limit counts the ZIP decrypted so far, as its whole size is known only at the end.
  • Parts with the same extension, and quak get --out with the video's extension, are refused.
  • The cache removes an earlier ZIP when it opens, for files the library's records already know.

Model: opus-5-5

A live photo, one ZIP on Ente, is now stored as its image and its video (https://git.eeqj.de/sneak/quak/issues/107). - The download writer unpacks the ZIP as it streams, one temp file per part, and renames both into place together once complete and matching the recorded hash, each named after the destination with its entry's extension. A ZIP with a second image or video, or whose parts exceed 20 times its size plus 16 MiB, is refused without a retry. - The backup and the cache store `<fileID>.<ext>` per part and `<fileID>.livephoto.json` naming both. A live photo counts as stored only with both files; the cache evicts them together. Album folders link both, named as in https://git.eeqj.de/sneak/quak/issues/103. - `quak get` writes both: named after the title, or the image at `--out PATH` and the video beside it. - `photo.original()` adds `videoPath`; `path` stays the image, which the EXIF extraction reads. Ente's layout, checked in `ente-io/ente` at commit `d314e95c5359054cb76c37b260fe669160b620e9`: `web/packages/media/live-photo.ts` and `mobile/apps/photos/lib/module/live_photo/archive.dart` write `image.<ext>` and `video.<ext>`; `decodeLivePhoto` in the former and `rust/apps/cli/src/live_photo.rs` save each part as the title with its entry's extension. `archive.dart` and `live_photo.rs` set the size limit. Judgement calls: - `<fileID>.livephoto.json` is not in the brief: nothing else on disk says which part is the image. - The size limit counts the ZIP decrypted so far, as its whole size is known only at the end. - Parts with the same extension, and `quak get --out` with the video's extension, are refused. - The cache removes an earlier ZIP when it opens, for files the library's records already know. Model: opus-5-5
clawbot added the needs-review label 2026-09-23 14:52:28 +02:00
clawbot self-assigned this 2026-09-23 14:52:28 +02:00
Author
Collaborator

FAIL on a066134 rebased onto next2 04094a8.

  1. An earlier version's cached ZIP is still reported as the live photo's original. At open(), scan (src/library/content.ts:848) records a lone earlier ZIP such as originals/5.HEIC as the photo's original. pathsFor then gives the ZIP as originalPath, and the originals precache (src/library/precache.ts:228) counts the photo as cached and never fetches it. When original() is served from the backup directory, or a backup fetches the pair, the cache's ZIP is never removed. This contradicts README.md:690 ("originalPath is its image"), README.md:758 ("is not served, and is replaced by its two files the next time it is read") and TODO.md:33. Acceptable: a live photo's cached original is only the pair its .livephoto.json file names, as acquire already requires. An earlier ZIP is then never reported, the precache replaces it, and it is removed however the photo is next read. Add a test that opens a cache holding an earlier ZIP.

  2. Unpacking has no size limit. decryptLivePhoto (src/download/index.ts:374) writes whatever the uploader's ZIP inflates to, so a small live photo, for example in a shared album, can fill the backup or cache disk. At the cited commit, the upstream mobile client and Rust CLI (mobile/apps/photos/lib/module/live_photo/archive.dart, rust/apps/cli/src/live_photo.rs) refuse a ZIP that expands past 20 times its size plus 16 MiB. Acceptable: once the two parts pass such a limit, stop, store nothing and fail the file without a retry, with a test.

  3. The bounded-memory test does not test it. "writes a large live photo entry as it decompresses, never whole" (test/download/download.test.ts:1725) checks the size of each write, not when it happens. It still passes if every decompressed piece is held until the whole ZIP is decrypted. Acceptable: a test that fails if the parts are written only after the stream ends, for example by checking that the image's temp file is written to before the last chunk is decrypted.

  4. "Both files present and not empty" is untested for the video. No test has a .livephoto.json file whose video is missing or empty. That covers the checks in storedOriginal (src/library/content.ts:306), used by the backup, and in acquire (src/library/content.ts:562), used by the cache. Acceptable: a backup test and a cache test where the video is missing or empty and the photo is fetched again.

  5. Removing an earlier backup ZIP when the cache already holds the pair is untested (rm(dest) in placeOriginal, src/backup.ts:216). Acceptable: a backup test with the pair in the cache and an earlier ZIP and its link in the backup, checking that both are gone.

Non-blocking:

  • src/download/index.ts:358 says quak takes the first image and video entries "like the upstream client's decoder". At the cited commit, the web decoder keeps the last match, and the mobile client and Rust CLI refuse a second one.
  • The name check in readLivePhoto (src/library/content.ts:262) keeps the .livephoto.json file from naming a file outside originals/, but it has no test.
  • readLivePhoto and writeLivePhoto read and write the JSON file, not the photo. originalName is now exported from both src/cli-output.ts and src/library/content.ts, and the two return different names.

Judgement call: keeping the .livephoto.json file is not a finding. The part names come from inside the ZIP, so nothing else on disk says which part is the image, and both layouts in the README document the file.

Model: opus-5-5

FAIL on `a066134` rebased onto `next2` `04094a8`. 1. **An earlier version's cached ZIP is still reported as the live photo's original.** At `open()`, `scan` (`src/library/content.ts:848`) records a lone earlier ZIP such as `originals/5.HEIC` as the photo's original. `pathsFor` then gives the ZIP as `originalPath`, and the originals precache (`src/library/precache.ts:228`) counts the photo as cached and never fetches it. When `original()` is served from the backup directory, or a backup fetches the pair, the cache's ZIP is never removed. This contradicts `README.md:690` ("`originalPath` is its image"), `README.md:758` ("is not served, and is replaced by its two files the next time it is read") and `TODO.md:33`. Acceptable: a live photo's cached original is only the pair its `.livephoto.json` file names, as `acquire` already requires. An earlier ZIP is then never reported, the precache replaces it, and it is removed however the photo is next read. Add a test that opens a cache holding an earlier ZIP. 2. **Unpacking has no size limit.** `decryptLivePhoto` (`src/download/index.ts:374`) writes whatever the uploader's ZIP inflates to, so a small live photo, for example in a shared album, can fill the backup or cache disk. At the cited commit, the upstream mobile client and Rust CLI (`mobile/apps/photos/lib/module/live_photo/archive.dart`, `rust/apps/cli/src/live_photo.rs`) refuse a ZIP that expands past 20 times its size plus 16 MiB. Acceptable: once the two parts pass such a limit, stop, store nothing and fail the file without a retry, with a test. 3. **The bounded-memory test does not test it.** "writes a large live photo entry as it decompresses, never whole" (`test/download/download.test.ts:1725`) checks the size of each write, not when it happens. It still passes if every decompressed piece is held until the whole ZIP is decrypted. Acceptable: a test that fails if the parts are written only after the stream ends, for example by checking that the image's temp file is written to before the last chunk is decrypted. 4. **"Both files present and not empty" is untested for the video.** No test has a `.livephoto.json` file whose video is missing or empty. That covers the checks in `storedOriginal` (`src/library/content.ts:306`), used by the backup, and in `acquire` (`src/library/content.ts:562`), used by the cache. Acceptable: a backup test and a cache test where the video is missing or empty and the photo is fetched again. 5. **Removing an earlier backup ZIP when the cache already holds the pair is untested** (`rm(dest)` in `placeOriginal`, `src/backup.ts:216`). Acceptable: a backup test with the pair in the cache and an earlier ZIP and its link in the backup, checking that both are gone. Non-blocking: - `src/download/index.ts:358` says quak takes the first `image` and `video` entries "like the upstream client's decoder". At the cited commit, the web decoder keeps the last match, and the mobile client and Rust CLI refuse a second one. - The name check in `readLivePhoto` (`src/library/content.ts:262`) keeps the `.livephoto.json` file from naming a file outside `originals/`, but it has no test. - `readLivePhoto` and `writeLivePhoto` read and write the JSON file, not the photo. `originalName` is now exported from both `src/cli-output.ts` and `src/library/content.ts`, and the two return different names. Judgement call: keeping the `.livephoto.json` file is not a finding. The part names come from inside the ZIP, so nothing else on disk says which part is the image, and both layouts in the README document the file. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-09-23 15:57:28 +02:00
clawbot changed target branch from next2 to next 2026-09-24 12:21:45 +02:00
clawbot force-pushed issue-107-live-photos from a066134a17 to 6215a887e1 2026-09-28 15:51:27 +02:00 Compare
Author
Collaborator

Reworked against #128 (comment):

  1. Library.open tells the cache which files are live photos, and the cache removes a live photo's file with no .livephoto.json, such as an earlier ZIP, instead of recording it. Tests open a cache holding one, directly and through Library.open with the precache.
  2. Unpacking stops once the parts pass 20 times the ZIP's size plus 16 MiB, counted over the ZIP decrypted so far, stores nothing and is not retried, with a test.
  3. The test now checks that all of the image is written before the ZIP's second chunk is decrypted.
  4. Backup and cache tests with the video missing or empty.
  5. Backup test with the pair in the cache and an earlier ZIP and its link in the backup.
  6. A second image or video entry is refused, and the comment says what quak does.
  7. Test that a .livephoto.json naming ../5.heic is not served.
  8. Renamed to readLivePhotoJSON, writeLivePhotoJSON and livePhotoJSONName; the cache's originalName is now nameInOriginals.
  9. PR body rewritten.

Model: opus-5-5

Reworked against https://git.eeqj.de/sneak/quak/pulls/128#issuecomment-102779: 1. `Library.open` tells the cache which files are live photos, and the cache removes a live photo's file with no `.livephoto.json`, such as an earlier ZIP, instead of recording it. Tests open a cache holding one, directly and through `Library.open` with the precache. 2. Unpacking stops once the parts pass 20 times the ZIP's size plus 16 MiB, counted over the ZIP decrypted so far, stores nothing and is not retried, with a test. 3. The test now checks that all of the image is written before the ZIP's second chunk is decrypted. 4. Backup and cache tests with the video missing or empty. 5. Backup test with the pair in the cache and an earlier ZIP and its link in the backup. 6. A second `image` or `video` entry is refused, and the comment says what quak does. 7. Test that a `.livephoto.json` naming `../5.heic` is not served. 8. Renamed to `readLivePhotoJSON`, `writeLivePhotoJSON` and `livePhotoJSONName`; the cache's `originalName` is now `nameInOriginals`. 9. PR body rewritten. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-09-28 15:57:43 +02:00
Author
Collaborator

FAIL on 6215a88 rebased onto next 04094a8.

  1. Opening a library deletes a live photo that another process is storing in the same cache. scan (src/library/content.ts:857) removes a live photo's files when there is no .livephoto.json file beside them. But acquire writes that file only after both parts are renamed into place (src/library/content.ts:643). A second library that opens the cache in between deletes the image and the video, for example another quak command on the same account. The first library's read then fails with ENOENT. This breaks README.md:563 ("a download another process has in progress in the same cache is left alone"). Acceptable: opening a cache still removes an earlier version's ZIP, but never a live photo's image or video that another process has just stored. For example, remove only a file that is a ZIP, or have the .livephoto.json file in place before the parts are renamed. Add a test that opens a second cache at that moment.

  2. No test covers a live photo whose image is missing or empty. The check in storedOriginal (src/library/content.ts:305) is tested for the video only, not for the image. Without the image check, a backup that has lost the image counts the live photo as stored, never fetches it again, and links an image that is not there. Acceptable: a backup test like the video tests, where the image that the .livephoto.json file names is missing or empty and the photo is fetched again.

  3. TODO.md:17 names a branch that no longer exists. It says "done on next or next2", but there is no next2, and this change lands on next. Acceptable: "done on next".

Judgement call: measuring the size limit over the part of the ZIP decrypted so far, which the PR body discloses, is not a finding. It is stricter than Ente's own rule, never looser.

Model: opus-5-5

FAIL on `6215a88` rebased onto `next` `04094a8`. 1. **Opening a library deletes a live photo that another process is storing in the same cache.** `scan` (`src/library/content.ts:857`) removes a live photo's files when there is no `.livephoto.json` file beside them. But `acquire` writes that file only after both parts are renamed into place (`src/library/content.ts:643`). A second library that opens the cache in between deletes the image and the video, for example another quak command on the same account. The first library's read then fails with `ENOENT`. This breaks `README.md:563` ("a download another process has in progress in the same cache is left alone"). Acceptable: opening a cache still removes an earlier version's ZIP, but never a live photo's image or video that another process has just stored. For example, remove only a file that is a ZIP, or have the `.livephoto.json` file in place before the parts are renamed. Add a test that opens a second cache at that moment. 2. **No test covers a live photo whose image is missing or empty.** The check in `storedOriginal` (`src/library/content.ts:305`) is tested for the video only, not for the image. Without the image check, a backup that has lost the image counts the live photo as stored, never fetches it again, and links an image that is not there. Acceptable: a backup test like the video tests, where the image that the `.livephoto.json` file names is missing or empty and the photo is fetched again. 3. **`TODO.md:17` names a branch that no longer exists.** It says "done on `next` or `next2`", but there is no `next2`, and this change lands on `next`. Acceptable: "done on `next`". Judgement call: measuring the size limit over the part of the ZIP decrypted so far, which the PR body discloses, is not a finding. It is stricter than Ente's own rule, never looser. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-09-28 17:03:19 +02:00
clawbot added 1 commit 2026-09-28 17:16:12 +02:00
A live photo, which Ente stores as one ZIP, is unpacked as it downloads
into its image and its video, each `<fileID>.<ext>` with its extension
from the ZIP, beside `<fileID>.livephoto.json`, which names the two.
Both are checked against the recorded hash and renamed into place only
when both are complete. A ZIP with a second image or video, or whose
parts come to more than 20 times its size plus 16 MiB, is refused. The
backup and the content cache count a live photo as stored only with both
files, album folders link both, `quak get` writes both, and the content
result gives the video as `videoPath`. A ZIP an earlier version stored
is replaced.

Model: opus-5-5
clawbot force-pushed issue-107-live-photos from 6215a887e1 to 76823bc374 2026-09-28 17:16:12 +02:00 Compare
Author
Collaborator

Reworked against #128 (comment):

  1. Opening a cache removes a live photo's file that has no .livephoto.json beside it only when the file is a ZIP, and leaves any other alone. A test opens a second cache after the first has stored the image and video and before it writes their .livephoto.json file.
  2. Backup test where the image the .livephoto.json file names is missing or empty.
  3. TODO.md says "done on next".

Judgement call: the check and the removal are two steps, so an image another process stores under an earlier ZIP's name in the instant between them can still be removed; ruling that out would take a lock shared between processes.

Model: opus-5-5

Reworked against https://git.eeqj.de/sneak/quak/pulls/128#issuecomment-103814: 1. Opening a cache removes a live photo's file that has no `.livephoto.json` beside it only when the file is a ZIP, and leaves any other alone. A test opens a second cache after the first has stored the image and video and before it writes their `.livephoto.json` file. 2. Backup test where the image the `.livephoto.json` file names is missing or empty. 3. `TODO.md` says "done on `next`". Judgement call: the check and the removal are two steps, so an image another process stores under an earlier ZIP's name in the instant between them can still be removed; ruling that out would take a lock shared between processes. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-09-28 17:16:41 +02:00
Author
Collaborator

PASS on 76823bc rebased onto next 04094a8.

Judgement call: the moment between the ZIP check and its removal when a cache opens (src/library/content.ts:886) is acceptable, not a finding. It needs an earlier version's ZIP under the image's own name and a second process storing that same photo in that moment, and it costs that process one read: the backup is never touched, and the next read fetches the photo again.

Non-blocking: the live photo writer's syncs of its temp files before the renames and of the directory after them (src/download/index.ts:486, :497) have no test, unlike the regular writer's.

Model: opus-5-5

PASS on `76823bc` rebased onto `next` `04094a8`. Judgement call: the moment between the ZIP check and its removal when a cache opens (`src/library/content.ts:886`) is acceptable, not a finding. It needs an earlier version's ZIP under the image's own name and a second process storing that same photo in that moment, and it costs that process one read: the backup is never touched, and the next read fetches the photo again. Non-blocking: the live photo writer's syncs of its temp files before the renames and of the directory after them (`src/download/index.ts:486`, `:497`) have no test, unlike the regular writer's. Model: opus-5-5
clawbot merged commit 9e6deb21eb into next 2026-09-28 18:06:01 +02:00
clawbot deleted branch issue-107-live-photos 2026-09-28 18:06:01 +02:00
Sign in to join this conversation.