Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initial support motion photos #813

Merged
merged 4 commits into from
Apr 14, 2023
Merged

Conversation

sickelap
Copy link
Contributor

@sickelap sickelap commented Apr 6, 2023

This adds support for parsing and extracting video from Google Pixel and Samsung motion photos.
Sample photo attached to #797 did not have XMP metadata and exiftool was unable to parse it. Parsing manually does pretty good job. Extracted videos are added as embedded_media property from Photo.main_file.

Adding support for HEIF/HEIC (a.k.a. Apple Live Photos) is more involved, but hopefully we will have it done as well in the future.

PR for frontend will follow

class Migration(migrations.Migration):

dependencies = [
("api", "0041_apply_user_enum_for_person"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now have a couple of other migrations, which means, that this line needs to be updated

@derneuere
Copy link
Member

I would prefer it if we could maintain consistency in how we save technical files for LibrePhotos. "embedded_media" files are extracted in the folder where the original file lives. This breaks the regular pattern where we save needed files in the protected_media folder. I think adding a folder named embedded_media and saving embedded_media there would make the most sense.

I think there are also a couple of changes needed for calculating the thumbnails. The thumbnail in the list is probably a video and the big thumbnail an image. We also would need a new type of live / motion photos, which you could then add to PigSerializer.

I am looking forward to your frontend pull request!

@sonarcloud
Copy link

sonarcloud bot commented Apr 13, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 3 Security Hotspots
Code Smell A 9 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@sickelap
Copy link
Contributor Author

I totally agree with embedded media location. As those are not primary files but rather extracted from the original, it makes sense to keep them in a private location. In fact, this was bugging me too. Now, extracted media files will be placed into the /protected_media/embedded_media/.

I don't think that messing with thumbnails is necessary because of what I have in mind for the frontend.
The idea I am working towards is not to display embedded media as standalone content. Instead, when opening the motion photo in light box, we will have a “Play” button (similar to how it's done in google photos). I also don't think we need to have a dedicated type for embedded media and the extracted content is a M2M relationship to the Photo.main_file. This makes implementation simpler (I think) and not as intrusive as it would have been otherwise.

@derneuere
Copy link
Member

Alright, looking forward to the frontend pr :)

@derneuere derneuere merged commit 3f7f6aa into LibrePhotos:dev Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants