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

feat: Added blurhash to image metadata #13009

Merged
merged 3 commits into from
Aug 20, 2024

Conversation

skalidindi53
Copy link
Contributor

☑️ Resolves

🛠️ API Checklist

🚧 Tasks

  • Added blurhash to image metadata

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not possible
  • 📘 API documentation in docs/ has been updated or is not required
  • 🔖 Capability is added or not needed

@skalidindi53 skalidindi53 force-pushed the skalidindi53/12351/Add-blurhash-to-image-metadata branch 3 times, most recently from aa74c4f to 5e811e0 Compare August 15, 2024 17:38
@skalidindi53 skalidindi53 marked this pull request as ready for review August 15, 2024 18:00
@skalidindi53 skalidindi53 force-pushed the skalidindi53/12351/Add-blurhash-to-image-metadata branch from 5e811e0 to 52955da Compare August 16, 2024 18:38
@skalidindi53 skalidindi53 force-pushed the skalidindi53/12351/Add-blurhash-to-image-metadata branch from e351992 to 6d5fb17 Compare August 16, 2024 19:11
Signed-off-by: skalidindi53 <[email protected]>
@skalidindi53 skalidindi53 added feature: api 🛠️ OCS API for conversations, chats and participants 3. to review labels Aug 19, 2024
@skalidindi53 skalidindi53 self-assigned this Aug 19, 2024
@skalidindi53 skalidindi53 added this to the 💙 Next RC (30) milestone Aug 19, 2024
@nickvergessen
Copy link
Member

@SystemKeeper can/want to give this a try?

@nickvergessen
Copy link
Member

/backport to stable30

@SystemKeeper
Copy link
Contributor

@SystemKeeper can/want to give this a try?

The PR works fine for me and correctly returns the blurhash.

Would it make sense to rename size-related methods/variables like getMetadataPhotosSizeForFileId to getImageMetadataForFileId or the like to make it clearer that this is not only about size?

One thing I noticed is that blurhash generation has since been moved to a background job: https://github.com/nextcloud/server/blob/560282a47bf3f9dca4653f89d530a6e1dd144f5f/lib/private/Blurhash/Listener/GenerateBlurhashMetadata.php#L57-L61

So when we upload images, a blurhash will only be available after the next background run, which might limit the usefulness a bit...

Example from ios:

@nickvergessen
Copy link
Member

One thing I noticed is that blurhash generation has since been moved to a background job

Hmpf, makes sense performancewise, but yeah kind of kills the feature

@skalidindi53 skalidindi53 force-pushed the skalidindi53/12351/Add-blurhash-to-image-metadata branch from 0c728ea to 51d419b Compare August 20, 2024 16:44
@skalidindi53 skalidindi53 merged commit 73a0aeb into main Aug 20, 2024
87 of 133 checks passed
@skalidindi53 skalidindi53 deleted the skalidindi53/12351/Add-blurhash-to-image-metadata branch August 20, 2024 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review feature: api 🛠️ OCS API for conversations, chats and participants
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add blurhash to image metadata
3 participants