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

Support for reading and writing PNG metadata #165

Merged
merged 10 commits into from
Aug 7, 2015

Conversation

joannajw
Copy link
Contributor

This pull request adds the feature requested in issue #164. It adds support for writing and reading metadata in PNG files. setMetadata() sets a PNG tEXt chunk with the key lwip_data and comment as the input string. getMetadata() returns the contents of the first tEXt chunk found with the key lwip_data, or null if none exists.

Setting metadata:

lwip.create(width, height, function(err, img) {
    img.setMetadata("test");
    image.writeFile("test.png", function(writeErr) {});
});

Getting metadata:

lwip.open("test.png", function(err, img) {
    var metadata = img.getMetadata();
    console.log(metadata);
})

};

Image.prototype.setMetadata = function(data) {
this.__metadata = data;
Copy link
Owner

Choose a reason for hiding this comment

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

How are different types of 'data' handled?

Eventually this value will go all the way to here, at which time it will be casted to a string.

If users pass here an object, for example, do they expect us to serialize it for them? What if they pass in a function?

In the docs we say that 'data' should be a string. So we should probably enforce it here by doing type checks and throwing an exception if needed, as done in other methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I think it would make sense to only allow metadata to be set to a string, so I added that check.

I've also added the option to pass in null to setMetadata, which will cause the image to be saved without any tEXt chunks when writeFile is called.

@EyalAr
Copy link
Owner

EyalAr commented Jul 25, 2015

@joannajw It looks great. Please see the comments I've added.

Thanks!

P.S. please add the tests/.DS_Store to .gitignore.

@joannajw
Copy link
Contributor Author

Thanks for the comments! I've made the changes you mentioned and removed unnecessary comments. Let me know what you think.

EyalAr added a commit that referenced this pull request Aug 7, 2015
Support for reading and writing PNG metadata
@EyalAr EyalAr merged commit d0a2631 into EyalAr:master Aug 7, 2015
@EyalAr
Copy link
Owner

EyalAr commented Aug 7, 2015

@joannajw
Merged. Thanks for the great PR.

I'll release a new version of lwip with your changes shortly.

@joannajw
Copy link
Contributor Author

Thanks so much for the merge!

I was wondering if you have an idea of when a new release will be made, so we can update our project to incorporate the new feature as well?

@EyalAr
Copy link
Owner

EyalAr commented Aug 27, 2015

@joannajw I'll try to push it this weekend.

@joannajw
Copy link
Contributor Author

Awesome, thanks!

@@ -53,5 +53,9 @@ string decode_jpeg_buffer(char * buffer, size_t size, CImg<unsigned char> ** cim
jpeg_finish_decompress(&cinfo);
jpeg_destroy_decompress(&cinfo);

// TODO: implement getting metadata from GIFs; this is a placeholder

Choose a reason for hiding this comment

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

the comment should this:
// TODO: implement getting metadata from JPEGs; this is a placeholder

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.

3 participants