-
-
Notifications
You must be signed in to change notification settings - Fork 853
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
Support for XMP metadata #1918
Support for XMP metadata #1918
Conversation
Will do a proper review later on but wasn’t specifically planning on supporting this for V2 so depending on the design we might have to make this internal to ensure we have the time to get it right. Regarding Metadata, we cannot use the allocator since we require the ability to explicitly set profile instances to null. |
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.
Lovely bit of work so far but I'm not sure about where we're going with this. Needs more discussion.
The main challenge here is that metadata profiles shall be |
I had to apply the fix of #1907 to |
Codecov Report
@@ Coverage Diff @@
## master #1918 +/- ##
=======================================
Coverage 88% 88%
=======================================
Files 966 968 +2
Lines 51321 51564 +243
Branches 6397 6428 +31
=======================================
+ Hits 45185 45403 +218
- Misses 5076 5091 +15
- Partials 1060 1070 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
Some memory optimization suggestions. Commented on Jpeg codec, but applicable to all.
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.
Just a single missed allocation optimization opportunity and our gif check remaining and this looks good to go in. I've gotta say, I'm super impressed with how quickly you've added this across multiple formats. Really nice 👍
Let's get this merged 😄 Thanks @ynse01 for the code and everyone else for the excellent reviews. Much appreciated! |
Prerequisites
Description
This PR fixes #858.
Adding the ability to read and write XMP metadata from the following image formats:
XMP metadata is placed in a dedicated XmpProfile class, which exposes the raw XML data.
Some highlights on the changes:
Open points, not part of this review:
IDisposable
and require the constructors to have aConfiguration
argument.