Repository navigation
Google Motion Photos: Android 11+ format and grid hover playback - #4900
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 Walkthrough
Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 35.21% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 142 functions across 46 files. (6 skipped… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 35.21% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 142 functions across 46 files. (6 skipped: 6 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit watched the still frame glow,
Then saw its hidden cliplet flow.
A badge appeared; the hover stirred,
The tiny movie hopped, undeterred.
“Motion photos!” the rabbit cried,
And tucked a moving frame inside.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
a0531c06-273f-4d1d-8f7a-18c7209fe887
⛔ Files ignored due to path filters: 3
tests/Samples/google_motion_photo_pixel.jpgis excluded by!**/*.jpgtests/Samples/google_motion_photo_ultrahdr.jpgis excluded by!**/*.jpgtests/Samples/google_motion_photo_v1.jpgis excluded by!**/*.jpg
📒 Files selected for processing: 59
app/Actions/Photo/Pipes/Standalone/PlaceGoogleMotionVideo.phpapp/Console/Commands/ImageProcessing/ExtractMotionPhotos.phpapp/Image/Handlers/GoogleMotionPictureHandler.phpapp/Metadata/Extractor.phpapp/Metadata/MotionPhoto/FtypBox.phpapp/Metadata/MotionPhoto/JpegHeader.phpapp/Metadata/MotionPhoto/JpegXmpReader.phpapp/Metadata/MotionPhoto/MotionPhotoLocator.phpapp/Metadata/MotionPhoto/MotionPhotoXmp.phpapp/Metadata/MotionPhoto/MotionPhotoXmpHandler.phpdocs/specs/1-concepts/photos.mddocs/specs/4-architecture/features/098-google-motion-photos/open-questions.mddocs/specs/4-architecture/features/098-google-motion-photos/plan.mddocs/specs/4-architecture/features/098-google-motion-photos/spec.mddocs/specs/4-architecture/features/098-google-motion-photos/tasks.mddocs/specs/4-architecture/knowledge-map.mddocs/specs/4-architecture/request-lifecycle-photo-upload.mddocs/specs/4-architecture/roadmap.mddocs/specs/6-decisions/ADR-098-01-in-house-motion-photo-xmp-reader.mddocs/specs/6-decisions/README.mdlang/ar/gallery.phplang/bg/gallery.phplang/cz/gallery.phplang/de/gallery.phplang/el/gallery.phplang/en/gallery.phplang/es/gallery.phplang/fa/gallery.phplang/fr/gallery.phplang/hu/gallery.phplang/it/gallery.phplang/ja/gallery.phplang/nl/gallery.phplang/no/gallery.phplang/pl/gallery.phplang/pt/gallery.phplang/ru/gallery.phplang/sk/gallery.phplang/sv/gallery.phplang/tr/gallery.phplang/vi/gallery.phplang/zh_CN/gallery.phplang/zh_TW/gallery.phpresources/js/v8/components/gallery/albumModule/AlbumPanel.vueresources/js/v8/components/gallery/albumModule/Virtualized/PhotoThumbVirtual.vueresources/js/v8/components/gallery/albumModule/thumbs/PhotoThumb.vueresources/js/v8/components/gallery/albumModule/thumbs/ThumbBadge.vueresources/js/v8/composables/photo/motionUrlLoader.tsresources/js/v8/composables/photo/useMotionHover.test.tsresources/js/v8/composables/photo/useMotionHover.tsresources/js/v8/views/gallery-panels/Search.vueresources/js/v8/views/gallery-panels/Timeline.vuetests/Constants/TestConstants.phptests/ImageProcessing/Commands/ExtractMotionPhotosTest.phptests/ImageProcessing/Image/Handlers/BaseImageHandler.phptests/Samples/build_motion_photo_fixtures.phptests/Unit/Metadata/MotionPhoto/JpegXmpReaderTest.phptests/Unit/Metadata/MotionPhoto/MotionPhotoLocatorTest.phptests/Unit/Metadata/MotionPhoto/MotionPhotoXmpTest.php
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
…detail requests Feature 098 was taken upstream by Tag Albums Category, so the spec folder, ADR and all identifiers move to 100. Repeated hover now reuses the in-flight details request instead of sending a duplicate. Spec impact: Feature 100 spec FR-100-05 and S-100-19 describe the reused request, and the plan records the review follow-up. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
fb011b8e-47dc-4de8-96ed-366ceb8a0c2e
📒 Files selected for processing: 30
app/Console/Commands/ImageProcessing/ExtractMotionPhotos.phpapp/Metadata/Extractor.phpapp/Metadata/MotionPhoto/FtypBox.phpapp/Metadata/MotionPhoto/JpegXmpReader.phpapp/Metadata/MotionPhoto/MotionPhotoLocator.phpapp/Metadata/MotionPhoto/MotionPhotoXmp.phpdocs/specs/1-concepts/photos.mddocs/specs/4-architecture/features/100-google-motion-photos/open-questions.mddocs/specs/4-architecture/features/100-google-motion-photos/plan.mddocs/specs/4-architecture/features/100-google-motion-photos/spec.mddocs/specs/4-architecture/features/100-google-motion-photos/tasks.mddocs/specs/4-architecture/knowledge-map.mddocs/specs/4-architecture/request-lifecycle-photo-upload.mddocs/specs/4-architecture/roadmap.mddocs/specs/6-decisions/ADR-100-01-in-house-motion-photo-xmp-reader.mddocs/specs/6-decisions/README.mdresources/js/v8/components/gallery/albumModule/AlbumPanel.vueresources/js/v8/components/gallery/albumModule/Virtualized/PhotoThumbVirtual.vueresources/js/v8/components/gallery/albumModule/thumbs/PhotoThumb.vueresources/js/v8/composables/photo/motionUrlLoader.tsresources/js/v8/composables/photo/useMotionHover.test.tsresources/js/v8/composables/photo/useMotionHover.tsresources/js/v8/views/gallery-panels/Search.vueresources/js/v8/views/gallery-panels/Timeline.vuetests/ImageProcessing/Commands/ExtractMotionPhotosTest.phptests/ImageProcessing/Image/Handlers/BaseImageHandler.phptests/Samples/build_motion_photo_fixtures.phptests/Unit/Metadata/MotionPhoto/JpegXmpReaderTest.phptests/Unit/Metadata/MotionPhoto/MotionPhotoLocatorTest.phptests/Unit/Metadata/MotionPhoto/MotionPhotoXmpTest.php
🚧 Files skipped from review as they are similar to previous changes: 25
- resources/js/v8/components/gallery/albumModule/AlbumPanel.vue
- docs/specs/4-architecture/roadmap.md
- resources/js/v8/views/gallery-panels/Timeline.vue
- docs/specs/6-decisions/README.md
- resources/js/v8/views/gallery-panels/Search.vue
- docs/specs/1-concepts/photos.md
- app/Metadata/Extractor.php
- resources/js/v8/components/gallery/albumModule/Virtualized/PhotoThumbVirtual.vue
- resources/js/v8/components/gallery/albumModule/thumbs/PhotoThumb.vue
- resources/js/v8/composables/photo/motionUrlLoader.ts
- docs/specs/4-architecture/knowledge-map.md
- docs/specs/4-architecture/request-lifecycle-photo-upload.md
- app/Metadata/MotionPhoto/MotionPhotoXmp.php
- resources/js/v8/composables/photo/useMotionHover.ts
- resources/js/v8/composables/photo/useMotionHover.test.ts
- tests/Unit/Metadata/MotionPhoto/JpegXmpReaderTest.php
- tests/ImageProcessing/Image/Handlers/BaseImageHandler.php
- app/Metadata/MotionPhoto/FtypBox.php
- app/Metadata/MotionPhoto/JpegXmpReader.php
- app/Console/Commands/ImageProcessing/ExtractMotionPhotos.php
- app/Metadata/MotionPhoto/MotionPhotoLocator.php
- tests/Samples/build_motion_photo_fixtures.php
- tests/ImageProcessing/Commands/ExtractMotionPhotosTest.php
- tests/Unit/Metadata/MotionPhoto/MotionPhotoXmpTest.php
- tests/Unit/Metadata/MotionPhoto/MotionPhotoLocatorTest.php
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
The fallback scan started at the start-of-scan marker, so compressed image data that looked like an MP4 header could be taken for the video. It now starts after the end-of-image marker of the primary image. The plan and tasks no longer prescribe vp check --fix and no longer list Flow.vue as a loader provider. Spec impact: FR-100-01, NFR-100-02, SVC-100-04, ADR-100-01 and the knowledge map describe the new scan start, and the plan records the second review round. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Hi, thank you for your contribution! You say:
Why not make a PR on https://github.com/LycheeOrg/php-exif to add the support for it ? |
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
Should we transcode them with ffmpeg then? |
php-exif 1.5.0 now locates the video of Google Motion Photos for every reader (LycheeOrg/php-exif#83). Bump the dependency, drop Lychee's own reader and its unit tests, and let the backfill command call the php-exif locator. Spec impact: ADR-100-02 supersedes ADR-100-01, Q-100-08 records the decision, and FR-100-01, the services catalogue, test strategy, knowledge map and upload lifecycle now point at php-exif. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Good call, done. The reader now lives in php-exif (LycheeOrg/php-exif#83, released in 1.5.0). This PR now bumps |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
56ce5f28-1c05-470e-a25d-d4d1081c87de
⛔ Files ignored due to path filters: 4
composer.lockis excluded by!**/*.locktests/Samples/google_motion_photo_pixel.jpgis excluded by!**/*.jpgtests/Samples/google_motion_photo_ultrahdr.jpgis excluded by!**/*.jpgtests/Samples/google_motion_photo_v1.jpgis excluded by!**/*.jpg
📒 Files selected for processing: 13
app/Console/Commands/ImageProcessing/ExtractMotionPhotos.phpapp/Metadata/Extractor.phpcomposer.jsondocs/specs/4-architecture/features/100-google-motion-photos/open-questions.mddocs/specs/4-architecture/features/100-google-motion-photos/plan.mddocs/specs/4-architecture/features/100-google-motion-photos/spec.mddocs/specs/4-architecture/features/100-google-motion-photos/tasks.mddocs/specs/4-architecture/knowledge-map.mddocs/specs/4-architecture/request-lifecycle-photo-upload.mddocs/specs/4-architecture/roadmap.mddocs/specs/6-decisions/ADR-100-01-in-house-motion-photo-xmp-reader.mddocs/specs/6-decisions/ADR-100-02-motion-photo-reader-in-php-exif.mddocs/specs/6-decisions/README.md
🚧 Files skipped from review as they are similar to previous changes: 7
- docs/specs/4-architecture/request-lifecycle-photo-upload.md
- app/Metadata/Extractor.php
- docs/specs/4-architecture/roadmap.md
- docs/specs/4-architecture/knowledge-map.md
- docs/specs/6-decisions/ADR-100-01-in-house-motion-photo-xmp-reader.md
- docs/specs/4-architecture/features/100-google-motion-photos/plan.md
- docs/specs/4-architecture/features/100-google-motion-photos/tasks.md
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
Current Pixel phones record motion photo clips in HEVC, which several browsers cannot play. Copy H.264 clips as before and transcode any other codec to H.264 (libx264, CRF 23) in the same .mov container, with the rotation applied. lychee:extract_motion_photos --reencode re-extracts clips stored before this change. Spec impact: Q-100-09 resolved, the Non-Goals, FR-100-02, FR-100-04 and S-100-21 to S-100-23 describe the transcoding and --reencode. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A full batch with a failed photo printed an --after cursor that skips it. Print each failure and, after the cursor, ask to run the same batch again first. Spec impact: FR-100-04 describes the failure lines and the retry hint. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
2f6af084-3946-468b-9e9f-c2866cd53a59
📒 Files selected for processing: 12
app/Console/Commands/ImageProcessing/ExtractMotionPhotos.phpapp/Image/Handlers/GoogleMotionPictureHandler.phpapp/ModelFunctions/H264MOVFormat.phpdocs/specs/4-architecture/features/100-google-motion-photos/open-questions.mddocs/specs/4-architecture/features/100-google-motion-photos/plan.mddocs/specs/4-architecture/features/100-google-motion-photos/spec.mddocs/specs/4-architecture/features/100-google-motion-photos/tasks.mddocs/specs/4-architecture/knowledge-map.mddocs/specs/4-architecture/roadmap.mdtests/ImageProcessing/Commands/ExtractMotionPhotosTest.phptests/ImageProcessing/Image/Handlers/BaseImageHandler.phptests/Traits/ProbesStoredVideo.php
🚧 Files skipped from review as they are similar to previous changes: 5
- docs/specs/4-architecture/features/100-google-motion-photos/tasks.md
- docs/specs/4-architecture/knowledge-map.md
- docs/specs/4-architecture/features/100-google-motion-photos/plan.md
- docs/specs/4-architecture/features/100-google-motion-photos/open-questions.md
- docs/specs/4-architecture/roadmap.md
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
Yes, done in 43af3bb. Embedded clips in H.264 are still copied as is; any other codec (HEVC from current Pixels) is transcoded to H.264 with libx264 at CRF 23 in the same Clips stored before this change can be converted with |
…ment Remove the in-house XMP reader ADR and renumber the php-exif reader ADR from ADR-100-02 to ADR-100-01, folding the context and alternatives it still needs into it. Drop the micro_video_offset doc comment in Extractor, left over from the removed in-house locator call. Spec impact: ADR index, Feature 100 spec (DO-100-01), plan, tasks and open questions, roadmap and knowledge map now cite ADR-100-01 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@coderabbitai approve please |
|
@ildyria, submitting your approval request for this PR. 🧠 Learnings used✅ Action performedChanges approved. Review threads were not changed. |
fixes #753, fixes #1283
What this enables
MicroVideoOffsetformat, so these photos lost their video. The video is cut out of the JPEG and stored next to the original, like Apple Live Photos. The existing LivePhotosKit player in the lightbox then plays it with no change.MicroVideoOffset.php artisan lychee:extract_motion_photosrecovers the video of photos uploaded before this change. The original file still contains the video. Options:--dry-run,--album=<id>,--after=<id>to continue in batches, and--reencodeto convert clips already stored as HEVC to H.264..mov, with the rotation applied. On the Pixel sample that took 0.17 s and shrank the clip from 3.69 MB to 1.15 MB.How it works
lychee-org/php-exif(feat: Google Motion Photo support for every adapter php-exif#83, released in 1.5.0), and this PR bumps php-exif to^1.5.0.Reader::read()now reports the video asMicroVideoOffsetfor every reader (exiftool, Imagick or Native), in both the legacy and the Motion Photo 1.0 format (ADR-100-01).Container:Directoryinto lists that misalign on real Pixel files, whose primary item has noItem:Length. It takes the length of theMotionPhotocontainer item, or the legacy offset, and trusts a length only if an MP4ftypbox starts where it points. If the file saysMotionPhoto="1"with no usable length, it scans after the primary image's end-of-image marker, in 64 KiB chunks.ExtractorcopiesgetMicroVideoOffset()into the existingmicro_video_offset, so the upload pipes are unchanged.GoogleMotionPictureHandler::saveVideoStream()now picks the output format by codec:MOVFormat(stream copy) for H.264, the newH264MOVFormatotherwise.lychee:extract_motion_photoscalls php-exif'sMotionPhotoLocatordirectly.live_photo_urluntil their details are loaded. On hover, a thumbnail asks the view's existing details loader for its own photo. That's one request the first time, cached after that, and needs no API change.Why it doesn't break existing features
lychee-org/php-exif^1.4.0→^1.5.0; no new composer or npm package.PlaceGoogleMotionVideonow uses a shared path helper,GoogleMotionPictureHandler::videoPathNextTo(). The stored path is the same as before.<video>element exists until hover intent, so pages load no video.ThumbBadgegains an optionallabelprop. v7 is untouched.Tests
New:
Readerwith every adapter) are in php-exif undertests/PHPExif/MotionPhoto/.BaseImageHandler, run by every handler test class: uploads of the new format, Ultra HDR, a real Pixel photo, no FFmpeg, and an old-format file without exiftool.ExtractMotionPhotosTest(13), including--reencode, failed photos and the resume cursor, and a failed write that must leave the stored clip intact.useMotionHover.test.ts(18, Vitest).Samples:
google_motion_photo_pixel.jpgis a real Pixel 11 Pro motion photo. GPS, the C2PA manifest and the HDR+ debug maker note were removed. The gain map and video bytes are unchanged.google_motion_photo_v1.jpgandgoogle_motion_photo_ultrahdr.jpgare built bytests/Samples/build_motion_photo_fixtures.php.Run locally:
make phpstan: no errors.rector process --dry-run: no changes (its two internal errors are in the existing telemetry files).npx vp check: no errors.npx vp run typecheck,npx vp test: pass.ExtractMotionPhotosTest,PhotoAddTest,DownloadTest,Detect360Test, and the otherExtractorusers.Manual: Chrome on a scratch instance with v8 and the v3 grid enabled. The badges show, nothing loads before hover, one details request on the first hover, playback, unload on leave, badges only with reduced motion, and the still is kept if the clip can't play. The lightbox shows the LIVE button for the Pixel photo.
Notes for reviewers
<path>.partand moves it into place, so an interrupted write never truncates a clip already in use.docs/specs/4-architecture/features/100-google-motion-photos/.🤖 Generated with Claude Code
Summary by CodeRabbit