-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: Add withXmp() method for XMP metadata injection #4416
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
Conversation
|
||
- <code>Error</code> Invalid parameters | ||
|
||
**Since**: 0.33.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the version will need to be updated depending on when this is merged
/** | ||
* Set XMP metadata in the output image. | ||
* | ||
* @since 0.33.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this version string will need an update too
Bonjour, thank you very much for the PR Thibaut, its always a pleasure to see tests. I think it might be easier both for external API usage as well as internal implementation, if this new method accepts a string rather than a Buffer. withXmp(xmp: string) I realise this would differ from the existing If we can, given we're in control of the input, let's enforce a standard JavaScript string (UTF-16 sous le capot), which the C++ internals can then access as a UTF-8 (As a separate discussion, it might be worth exposing the XMP metadata as a standardised JavaScript string instead of, or as well as, a Buffer.) |
I took a quick look at exposing XMP metadata as a string. Commit 6f4ce5b out on the |
@lovell thanks for the feedback, it does make it easier to provide a string. I've edited my code, doing the I've chosen not to venture on adding XMP metadata parsing as string for now, as it could be a good thing to do in a separate PR. Let me know what you think! |
partially resolves lovell#4143 - Add withXmp(xmpBuffer) method to embed custom XMP metadata in output images - Include parameter validation for non-Buffer inputs - Add tests - Update API documentation
- this will be more user-friendly for folks who want to edit the XMP metadata
19c0cae
to
1d93e2e
Compare
note: I've just force pushed to rebase on top of main |
Landed via commit 4e3f379 I removed the manual memory copying as the use of a string means we can pass it all the way through from JS to libvips, plus added a partnering (I also added the XMP-as-a-string feature via commit 8ee8d27) Thank you very much Thibaut for your work on this, it will be part of the forthcoming 0.34.3 release. |
partially resolves #4143