-
Notifications
You must be signed in to change notification settings - Fork 229
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
Changes from 4 commits
d45e6a2
9ad754f
bf4670f
560e463
ee94291
a7b5c52
dd1c594
bc7da98
453355a
112332a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,6 +47,19 @@ | |
this.__locked = false; | ||
}; | ||
|
||
Image.prototype.getMetadata = function() { | ||
if (!this.__metadata || this.__metadata === "") { | ||
return null; | ||
} else { | ||
return this.__metadata; | ||
} | ||
}; | ||
|
||
Image.prototype.setMetadata = function(data) { | ||
this.__metadata = data; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
return data; | ||
}; | ||
|
||
Image.prototype.width = function() { | ||
return this.__lwip.width(); | ||
}; | ||
|
@@ -550,6 +563,11 @@ | |
encoderCb | ||
); | ||
} else if (type === 'png') { | ||
var metadata = that.__metadata; | ||
if (!metadata) { | ||
metadata = ""; | ||
} | ||
|
||
util.normalizePngParams(params); | ||
return encoder.png( | ||
that.__lwip.buffer(), | ||
|
@@ -558,6 +576,7 @@ | |
params.compression, | ||
params.interlaced, | ||
params.transparency === 'auto' ? that.__trans : params.transparency, | ||
metadata, | ||
encoderCb | ||
); | ||
} else if (type === 'gif') { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
#include "decoder.h" | ||
|
||
string decode_jpeg_buffer(char * buffer, size_t size, CImg<unsigned char> ** cimg) { | ||
string decode_jpeg_buffer(char * buffer, size_t size, CImg<unsigned char> ** cimg, char ** metadata) { | ||
struct jpeg_decompress_struct cinfo; | ||
struct lwip_jpeg_error_mgr jerr; | ||
|
||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the comment should this: |
||
*metadata = (char *)malloc(sizeof(char)); | ||
*metadata[0] = '\0'; | ||
|
||
return ""; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,12 @@ NAN_METHOD(encodeToPngBuffer) { | |
int compression = args[3].As<Integer>()->Value(); | ||
bool interlaced = args[4]->BooleanValue(); | ||
bool trans = args[5]->BooleanValue(); | ||
NanCallback * callback = new NanCallback(args[6].As<Function>()); | ||
|
||
int metadata_len = args[6].As<String>()->Utf8Length(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this include the null termination character? I think it does, but I just want to make sure we allocate enough space for the string. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch! Looks like it actually doesn't include the null termination character, so I accounted for that in the new changes. |
||
char *metadata = (char *)malloc(metadata_len * sizeof(char)); | ||
args[6].As<String>()->WriteUtf8(metadata); | ||
|
||
NanCallback * callback = new NanCallback(args[7].As<Function>()); | ||
|
||
NanAsyncQueueWorker( | ||
new EncodeToPngBufferWorker( | ||
|
@@ -41,6 +46,7 @@ NAN_METHOD(encodeToPngBuffer) { | |
compression, | ||
interlaced, | ||
trans, | ||
metadata, | ||
callback | ||
) | ||
); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
#include "encoder.h" | ||
#include <iostream> | ||
|
||
#define RGB_N_CHANNELS 3 | ||
#define RGBA_N_CHANNELS 4 | ||
|
@@ -10,17 +11,23 @@ EncodeToPngBufferWorker::EncodeToPngBufferWorker( | |
int compression, | ||
bool interlaced, | ||
bool trans, | ||
char *metadata, | ||
// char *metadata, | ||
NanCallback * callback | ||
): NanAsyncWorker(callback), _width(width), _height(height), | ||
_compression(compression), _interlaced(interlaced), _trans(trans), | ||
_compression(compression), _interlaced(interlaced), _trans(trans), _metadata(metadata), | ||
_pngbuf(NULL), _pngbufsize(0) { | ||
SaveToPersistent("buff", buff); // make sure buff isn't GC'ed | ||
_pixbuf = (unsigned char *) Buffer::Data(buff); | ||
|
||
// SaveToPersistent("metadata", metadata); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess those comments were meant to be deleted? |
||
// _metadata = (char *)metadata; | ||
} | ||
|
||
EncodeToPngBufferWorker::~EncodeToPngBufferWorker() {} | ||
|
||
void EncodeToPngBufferWorker::Execute () { | ||
|
||
int n_chan = _trans ? RGBA_N_CHANNELS : RGB_N_CHANNELS; | ||
unsigned int rowBytes = _width * n_chan; | ||
int interlaceType; | ||
|
@@ -97,6 +104,17 @@ void EncodeToPngBufferWorker::Execute () { | |
PNG_COMPRESSION_TYPE_DEFAULT, | ||
PNG_FILTER_TYPE_DEFAULT | ||
); | ||
|
||
// set metadata in tEXt chunk | ||
if (strlen(_metadata) > 0) { | ||
png_text metadata; | ||
metadata.compression = PNG_TEXT_COMPRESSION_NONE; | ||
metadata.key = "lwip_data"; | ||
metadata.text = _metadata; | ||
|
||
png_set_text(png_ptr, info_ptr, &metadata, 1); | ||
} | ||
|
||
png_set_compression_level(png_ptr, compLevel); | ||
|
||
pngWriteCbData buffinf = {NULL, 0}; | ||
|
@@ -164,4 +182,4 @@ void pngWriteCB(png_structp png_ptr, png_bytep data, png_size_t length) { | |
|
||
memcpy(buffinf->buff + buffinf->buffsize, data, length); | ||
buffinf->buffsize += length; | ||
} | ||
} |
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.
We should probably allow users to set metadata to an empty string.
Also, since all the decoders now set 'metadata' to be at least an empty string, I think this check is redundant.
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.
That's probably a good idea, I changed it so that users can set metadata to an empty string (tEXt chunk with empty comment field) or null (no tEXt chunks).