Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ A photo taken on a current Android phone shows its motion in Lychee: in the ligh
- Quality gate green: `php-cs-fixer`, `make phpstan`, scoped `php artisan test`, `npx vp check` (no `--fix`), `vp run typecheck`, `vp test`.

## Scope Alignment
- **In scope:** `App\Metadata\MotionPhoto` reader (FR-100-01); `Extractor` wiring (FR-100-01/02); backfill command (FR-100-04); v8 badge and hover playback for every live photo (FR-100-03); fixtures; docs.
- **Out of scope:** Spec Non-Goals (Samsung legacy trailer, HEIC motion photos, presentation timestamp, HEVC transcoding, lightbox player change, v7 grid, touch inline playback).
- **In scope:** `App\Metadata\MotionPhoto` reader (FR-100-01); `Extractor` wiring (FR-100-01/02); backfill command (FR-100-04); v8 badge and hover playback for every live photo (FR-100-03); main-lightbox play/stop dock button with a looping `<video>` overlay, v7 and v8 (FR-100-06, I14); fixtures; docs.
- **Out of scope:** Spec Non-Goals (Samsung legacy trailer, HEIC motion photos, presentation timestamp, HEVC transcoding, replacing LivePhotosKit (it still renders the lightbox still and keeps playing in Flow/Moderation, which have no dock), v7 grid, touch inline playback).

## Dependencies & Interfaces
- `App\Metadata\Extractor::createFromFile` (call site of the locator), `PanoramaDetector` (style precedent: final class, pure helpers, readonly record).
Expand Down Expand Up @@ -75,6 +75,9 @@ After T-100-15, during T-100-16, compare every FR/NFR/S-ID against the tests nam
- _Commands:_ `php artisan test --filter=PhotosAddHandlerImagickTest`.
10. **I10 – Docs + drift gate**
- _Steps:_ knowledge map, `1-concepts/photos.md`, `request-lifecycle-photo-upload.md`, roadmap; drift gate report.
11. **I14 – Lightbox live button in the dock, v8 then v7** (FR-100-06, S-100-25, S-100-26, UI-100-04)
- _Steps:_ failing Vitest `useLivePhotoToggle.test.ts` (registration, toggle, `stop()`, release, slide-transition ownership) → shared composable `resources/js/composables/photo/useLivePhotoToggle.ts`, which registers and releases `LivePhotoControls` in `PhotoState`; `PhotoBox` calls it, augments LivePhotosKit with `showsNativeControls: false` in the main lightbox and shows the looping `<video>` overlay while playing; `Dock` live button; `PhotoPanel` shows the dock to non-editors when controls are registered. (The first iteration drove LivePhotosKit `play()`/`stop()`; replaced in T-100-26.)
- _Commands:_ `npx vp test useLivePhotoToggle`, `npx vp check` (no `--fix`), `npx vp run typecheck`; manual check in the lightbox on an Apple and a Google live photo.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

## Scenario Tracking

Expand All @@ -93,6 +96,7 @@ After T-100-15, during T-100-16, compare every FR/NFR/S-ID against the tests nam
| S-100-11 … 14 | I6 / T-100-10 | `ExtractMotionPhotosTest` |
| S-100-15 … 18 | I8 / T-100-13, T-100-14 | `useMotionHover.test.ts` + manual |
| S-100-19, 20 | I8 / T-100-13, T-100-17 | `useMotionHover.test.ts` + manual per view |
| S-100-25, 26 | I14 / T-100-23 … T-100-29 | `useLivePhotoToggle.test.ts` + manual |

## Analysis Gate

Expand Down Expand Up @@ -120,6 +124,11 @@ Run 2026-10-17 by the agent (owner review pending).
- v7 grid playback, if requested.

## Intent Log
- 2026-10-31: owner asked to address CodeRabbit's second review of PR #4933: four doc-sync findings (plan scope, ownership in FR-100-06, UI-100-04 wording, knowledge-map contradiction) and translated `aria-label`s for every icon-only dock button, which needed new rotate keys in all 23 locales (T-100-29).
- 2026-10-31: owner tested the dock button in Firefox and Chrome (working) and asked to address the PR #4933 review. CodeRabbit asked to keep T-100-24/25 open until the manual checks passed (now recorded as passed) and to make the live control keyboard-operable: `DockButton` became a `<button type="button">` in v7 and v8, the live button got `aria-label`/`aria-pressed` (T-100-28).
- 2026-10-31: owner reported the motion did not loop until the button was turned off, and that Firefox highlighted the button without playing. Agent probed LivePhotosKit in Chromium with Playwright (Firefox would not start in the sandbox; running it outside was declined), found the gap-after-`ended` and the stall on non-Apple clips, and switched the dock path to a looping `<video>` overlay, the technique the grid hover already uses in Firefox (FR-100-06, T-100-26).
- 2026-10-31: owner asked for the same change in v7. Agent mirrored it in the v7 `PhotoBox`/`Dock`/`PhotoPanel` (new `is-live-dock-enabled` prop, as v7 `PhotoBox` has no zoom prop; PrimeIcons `pi-bullseye`, the closest glyph to the Live Photo rings) and moved `useLivePhotoToggle` to `resources/js/composables/photo/`, as v7 never imports from `@/v8`.
- 2026-10-31: owner asked to move the lightbox "LIVE" button into the dock next to highlight/move/delete, styled like the other dock buttons, click to play and click again to stop and show the still. Agent read `PhotoBox`, `Dock`, `DockButton`, `PhotoPanel`, the LivePhotosKit typings (`showsNativeControls`, `play`/`stop`, `ended` event); chose to keep playing (restart on `ended`) until the second click so the button state always matches what is shown, to show the dock to viewers without edit rights for live photos, and to keep the native controls in Flow/Moderation where there is no dock (FR-100-06).
- 2026-10-17: owner asked for a plan for Google motion photos (#753, #1283) with grid hover or play button. Agent surveyed the live photo pipeline (Explore agent + direct reads of `GoogleMotionPictureHandler`, `ExtractGoogleMotionPictures`, `Extractor`, php-exif 1.4.0 mapper via `gh api`), parsed the existing sample's XMP with a Python one-off (legacy format confirmed), drafted spec + Q-100-01 … 06. Owner chose Option A for all six. While planning FR-100-03 the agent found v3 tiles lack `live_photo_url` until tier-3 details load → Q-100-07; owner chose Option A (details on hover) → FR-100-05, T-100-17. Owner supplied a Pixel 11 Pro motion photo; agent stripped GPS, C2PA and the HDR+ maker note with exiftool (owner asked for GPS removal so they cannot be identified; C2PA/maker note removed as device fingerprints), verified trailer bytes identical and inspected the still and video frames for identifying content (none), placed it as FX-100-03 at full resolution; findings (Primary item without `Item:Length`, HEVC video) folded into the spec Appendix. Grep of v8 thumbnail hosts: `PhotoThumbPanel`, `PhotoThumbPanelVirtual`, `TimelineDatesV3`, `ResultPanel`, views `Timeline`, `Search`, `Flow`; Ken Burns hover is CSS-only, no conflict.

## Review Follow-up (PR #4900, 2026-10-18)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
| Field | Value |
|-------|-------|
| Status | Implemented |
| Last updated | 2026-10-17 |
| Last updated | 2026-10-31 |
| Owners | mlapaglia |
| Linked plan | [plan.md](plan.md) |
| Linked tasks | [tasks.md](tasks.md) |
Expand All @@ -27,7 +27,7 @@ Affected layers: metadata extraction (`app/Metadata`), one artisan command, v8 g
- HEIC/AVIF motion photos (primary image other than JPEG).
- Honouring `MotionPhotoPresentationTimestampUs`.
- Transcoding Apple Live Photo videos (separate `.mov` partners); only clips embedded in Google Motion Photos are transcoded (Q-100-09).
- Changing the lightbox player (LivePhotosKit stays).
- Replacing LivePhotosKit in the lightbox: it still renders the still and keeps playing in Flow/Moderation; only the main lightbox's play control moves to the dock and plays a plain `<video>` (FR-100-06).
- v7 grid playback (Q-100-05).
- Inline playback on touch devices (Q-100-01).

Expand All @@ -40,6 +40,7 @@ Affected layers: metadata extraction (`app/Metadata`), one artisan command, v8 g
| FR-100-03 | Grid motion playback (v8, every live photo). | `PhotoThumb.vue` and `PhotoThumbVirtual.vue`: a motion badge (`ThumbBadge` with `lucide:smartphone` on `bg-purple-700`, the icon and colour of the list-view "Live" label, FR-014-06; its new `label` prop sets `title` and `aria-label` to `gallery.motion_photo`) in the badge row whenever `precomputed.is_livephoto`. On devices matching `(hover: hover) and (pointer: fine)`, after 250 ms of continuous hover a muted, looping, `playsinline` `<video>` fades in over the thumbnail (`object-cover`, same box) and plays `live_photo_url`; on `mouseleave` it pauses, its `src` is removed and the still shows again. When `live_photo_url` is still `null` at `mouseenter`, the URL is fetched per FR-100-05; playback starts once both the 250 ms have passed and the URL is set, and never if the pointer left first. The overlay covers the still, so the Ken Burns hover zoom (when enabled) keeps running unseen underneath. | Only tiles with `is_livephoto === true`; nothing when `prefers-reduced-motion: reduce` (badge only); nothing on touch-only devices (badge only, tap opens the lightbox as today). | Video `error` event → overlay removed, still stays, no toast, no retry during that hover. | None (frontend only). | Owner request, Q-100-01, Q-100-03, Q-100-05 |
| FR-100-04 | Backfill already-imported photos. | `php artisan lychee:extract_motion_photos {limit=100} {tm=600} {--after=} {--album=} {--dry-run} {--reencode}` walks photos with `type = 'image/jpeg'` and `live_photo_short_path IS NULL`, ordered by id with an `--after` cursor (as `lychee:detect_360`). For each it uses the original in place when hosted locally, or a downloaded temporary copy otherwise (as `SizeVariantDefaultFactory`), runs `PHPExif\MotionPhoto\MotionPhotoLocator::locate()` on its path, and when the length > 0 extracts via `GoogleMotionPictureHandler` and writes `GoogleMotionPictureHandler::videoPathNextTo(<original path>)` on the original's disk (the helper the upload pipe `PlaceGoogleMotionVideo` now shares), then sets `live_photo_short_path` and dispatches `PhotoSaved` for the extracted ids. Output: `Motion photo: <id> (<title>)` per hit, then `Checked N photos: X motion photos extracted, Y failed.` (`found (dry run)` with `--dry-run`), and `More photos may remain: continue with --after=<last id>` when `N = limit`; nothing to scan → `No photos require motion photo extraction.` | `--reencode` switches the candidates to Google motion photos that already have a video (`live_photo_short_path` set, `live_photo_content_id` null) and re-extracts each clip from the original into its current path, transcoding as in FR-100-02; the summary says `re-extracted`. Every clip the command writes is staged at `<target>.part` on the same disk and then moved over the target, so a failed write leaves an existing clip intact (PR #4900 review). `--dry-run` writes nothing and does not need FFmpeg. `--album` restricts to that album's direct photos; unknown id → `Album <id> not found.`, exit 1. FFmpeg disabled without `--dry-run` → `FFmpeg is required to extract motion photo videos (or use --dry-run).`, exit 1, before the scan. | Missing original or any per-photo error → `Log::warning` and `Failed: <id> (<title>): <reason>` on the console, counted as failed, loop continues; exit 1 when any failed. When the batch is full and some photos failed, the cursor line is followed by `N photos failed in this batch and would be skipped by that cursor: run it again with --after=<same value> (or without --after) to retry them first.` (PR #4900 review). Plain JPEGs stay candidates, so a re-run checks them again (no "checked" marker; the cursor bounds each run). | Command output only. | Q-100-04 |
| FR-100-05 | Video URL for v3 grid tiles. | Each v8 view that renders photo thumbnails provides a `MotionUrlLoader` (`(photoIds: string[]) => Promise<void>`, Vue `provide`/`inject` key in `resources/js/v8/composables/photo/motionUrlLoader.ts`) wrapping its store's existing deduped details loader: `AlbumPanel.vue` → `AlbumState.loadPhotoDetails`, `Timeline.vue` → `TimelineState.loadPhotoDetailsV3`, `Search.vue` → `SearchState.loadPhotoDetailsV3`. `Flow.vue` renders its own cards, not these thumbnails, and the tag and favourites views use v2 `PhotoResource` payloads that already carry `live_photo_url`, so they provide none. The thumbnail calls it with its own id on `mouseenter` when `is_livephoto` and `live_photo_url === null`, on hover-capable devices only; the result lands in `live_photo_url` through `mergePhotoDetail()`. | No call when the URL is already set, the tile is not a live photo, or the device/motion gates of FR-100-03 fail. A view that provides no loader plays only tiles whose URL is already loaded. | Loader rejects or leaves the URL `null` → no playback for that hover, no toast; the next hover tries again (the store's dedupe makes a resolved id free). A request still in flight is reused when the pointer leaves and re-enters, because the stores mark an id resolved only once its response arrives (PR #4900 review). | None. | Q-100-07 |
| FR-100-06 | Lightbox play/stop from the dock (v7 and v8). | In the main lightbox (v8: `PhotoPanel` → `PhotoBox` with `is-zoom-enabled`; v7: `PhotoPanel` → `PhotoBox` with `is-live-dock-enabled`) LivePhotosKit only shows the still: it is augmented with `showsNativeControls: false`, so its "LIVE" badge and hover playback are gone. `PhotoBox` calls `resources/js/composables/photo/useLivePhotoToggle.ts`, which registers `LivePhotoControls` (`{ toggle }`) in `PhotoState.live_photo_controls` while the box shows a dock-controlled live photo, mirrors the playing state into `PhotoState.is_live_playing`, and releases both when the photo changes or the box unmounts, with the same ownership rule as the zoom controls (Feature 078). `Dock.vue` shows a `DockButton` (v8 `lucide:circle-dot-dashed`, v7 PrimeIcons `pi-bullseye`; tooltip and `aria-label` `gallery.motion_photo`, `aria-pressed` = playing; `DockButton` is a `<button type="button">`, so it is keyboard-operable, and every dock button carries a translated `aria-label`) first in the button row while controls are registered: white with the primary hover colour when idle, primary colour while playing. First click shows a muted, `loop`, `autoplay`, `playsinline` `<video>` of `live_photo_url` over the still (same box and size classes as the still, `object-contain`, `pointer-events-none`; the LivePhotosKit divs carry `isolate` so its z-indexed render layers stay below the video), so the motion loops without a gap until the next click removes it and the still shows again. The dock is shown to every viewer of a live photo, not only to editors: viewers without edit rights see the live button alone. | Flow (`LigtBox.vue`), Moderation and NSFW findings (v8), Flow and Moderation (v7) mount `PhotoBox` without that prop and keep the native LivePhotosKit controls. No button for photos that are not live photos or have no `live_photo_url`. Moving to another photo or closing the lightbox drops the controls; the next live photo starts stopped. Hidden with the rest of the dock during the slideshow. | Video `error` (e.g. HEVC in Firefox) → overlay removed, button back to idle, still visible, no toast. | None (frontend only). | Owner request 2026-10-31; LivePhotosKit playback was dropped from the dock path because its "full" style always ends (a restart on `ended` leaves a ~350 ms gap on the still), it played nothing in Firefox, and in Chromium it stalled at frame 0 on a Google clip without Apple timing metadata |
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

## Non-Functional Requirements

Expand Down Expand Up @@ -68,6 +69,21 @@ Affected layers: metadata extraction (`app/Metadata`), one artisan command, v8 g
Touch / reduced motion → badge only; tap opens the lightbox (LivePhotosKit plays it).
```

Lightbox dock (FR-100-06):

```
Live photo, stopped Live photo, playing (after a click on ◌)
┌───────────────────────────────────────────┐ ┌───────────────────────────────────────────┐
│ ◌ ⚑ 🖼 ⟲ ⟳ 📁 🗑 │ │ ◉ ⚑ 🖼 ⟲ ⟳ 📁 🗑 │
│ │ │ │
│ still photo │ │ <video loop> over the still, no gap │
│ (no LivePhotosKit "LIVE" badge) │ │ click ◉ again → stops, still shown │
└───────────────────────────────────────────┘ └───────────────────────────────────────────┘
◌ = live button (v8 lucide:circle-dot-dashed, v7 pi-bullseye; white, primary on hover; tooltip "Motion photo")
◉ = same button in the primary colour while playing
Viewer without edit rights: the dock holds only ◌.
```

## Branch & Scenario Matrix

| Scenario ID | Description / Expected outcome |
Expand Down Expand Up @@ -96,6 +112,8 @@ Affected layers: metadata extraction (`app/Metadata`), one artisan command, v8 g
| S-100-18 | Grid: video fails to load (e.g. HEVC in Firefox) → still stays, no toast. |
| S-100-19 | Grid on a v3 payload, URL not loaded: `mouseenter` triggers one details request for that id; playback starts after 250 ms or when the URL arrives, whichever is later; a second hover sends no request, including one that starts before the first request has answered. |
| S-100-20 | Details request fails or the pointer leaves before the URL arrives → no playback, no toast, no stray `<video>`. |
| S-100-25 | Lightbox, live photo: no native "LIVE" badge; the dock shows the live button; click → motion plays and keeps playing; click again → stops, still visible. |
| S-100-26 | Lightbox, live photo, viewer without edit rights → dock with the live button only. Non-live photo → no live button. |

## Test Strategy
- **Unit:** in php-exif since 1.5.0 (`tests/PHPExif/MotionPhoto/`: `JpegXmpReaderTest`, `MotionPhotoXmpTest`, `MotionPhotoLocatorTest`, `ReaderMotionPhotoTest`), over synthetic byte layouts and the v1/Ultra HDR samples (S-100-01, 03, 04, 05, 08, 09, 10; NFR-100-02). Lychee covers the integration through the upload and command tests below.
Expand Down Expand Up @@ -141,6 +159,7 @@ Provided by `lychee-org/php-exif` 1.5.0 (LycheeOrg/php-exif#83, ADR-100-01); Lyc
| UI-100-01 | Motion badge | `precomputed.is_livephoto` → badge in the thumbnail badge row. |
| UI-100-02 | Hover playback | FR-100-03 success path. |
| UI-100-03 | Playback failed | Video error → back to UI-100-01. |
| UI-100-04 | Lightbox live button | FR-100-06: idle (white, still visible) ↔ playing (primary colour, looping `<video>` overlay over the still). |

## Telemetry & Observability
No new events. Extraction keeps the `photo.extract_google_motion_pictures` span. Locator failures log `Log::warning` with the file basename (no path); FFmpeg failures go through `Handler::reportSafely` as today.
Expand Down Expand Up @@ -172,6 +191,7 @@ ui_states:
- { id: UI-100-01, description: Motion badge on live photo thumbnails }
- { id: UI-100-02, description: Muted looping hover playback }
- { id: UI-100-03, description: Playback failure falls back to the still }
- { id: UI-100-04, description: Lightbox dock live button toggles a looping video overlay over the still }
```

## Appendix – Motion Photo 1.0 layout
Expand Down
Loading
Loading