From 18b0a376e33997a0cdcf32f112ac545ac8d41d70 Mon Sep 17 00:00:00 2001 From: wiredfool Date: Tue, 6 Dec 2016 21:48:37 +0000 Subject: [PATCH 1/9] initial take of proper tiff metadata support --- PIL/TiffImagePlugin.py | 48 ++++++++++--- PIL/TiffTags.py | 10 +++ encode.c | 153 +++++++++++++++++++++++++++++------------ 3 files changed, 158 insertions(+), 53 deletions(-) diff --git a/PIL/TiffImagePlugin.py b/PIL/TiffImagePlugin.py index ae98831d22f..ff354ae3176 100644 --- a/PIL/TiffImagePlugin.py +++ b/PIL/TiffImagePlugin.py @@ -1430,8 +1430,18 @@ def _save(im, fp, filename): # based on the data in the strip. blocklist = [STRIPOFFSETS, STRIPBYTECOUNTS] atts = {} + # atts is a dict of key: tuple(type, array, count, value) + # where type is the tifftype int + # array is 0/1 for single or array value + # count is the number of items + # value is the value, or a tuple of the items. + # Note that we've got some items where there's an unspecified length + # in the spec (or 0), and they may have 1 item, so they need to be + # passed in in the array interface as an array of one item. + # bits per sample is a single short in the tiff directory, not a list. - atts[BITSPERSAMPLE] = bits[0] + tag_info = TiffTags.lookup(BITSPERSAMPLE) + atts[BITSPERSAMPLE] = (tag_info.type, 0, 1, bits[0]) # Merge the ones that we have with (optional) more bits from # the original file, e.g x,y resolution so that we can # save(load('')) == original file. @@ -1448,16 +1458,38 @@ def _save(im, fp, filename): # UNDONE -- add code for the custom dictionary if tag not in TiffTags.LIBTIFF_CORE: continue - if tag not in atts and tag not in blocklist: - if isinstance(value, unicode if bytes is str else str): - atts[tag] = value.encode('ascii', 'replace') + b"\0" - elif isinstance(value, IFDRational): - atts[tag] = float(value) - else: - atts[tag] = value + if tag in atts: + continue + if tag in blocklist: + continue + tag_info = TiffTags.lookup(tag) + # numeric types + if tag_info.length == 1: + if tag_info.type in (3,4,6,8,9): + atts[tag] = (tag_info.type, 0, 1, int(value)) + elif tag_info.type in (5,10,11,12): + atts[tag] = (tag_info.type, 0, 1, float(value)) + elif tag_info.type == 2: + if isinstance(value, unicode if bytes is str else str): + atts[tag] = (tag_info.type, 0, 1, + value.encode('ascii', 'replace') + b"\0") + else: + atts[tag] = (tag_info.type, 0, 1, value) + # we're not sending bytes to libtiff, as they require custom fields. + #elif tag_info.type == 7: + # atts[tag] = (tag_info.type, 0, 1, value) + + else: # undefined or set length. + if tag_info.type in (3,4,6,8,9): + atts[tag] = (tag_info.type, 1, len(value), tuple(map(int,value))) + elif tag_info.type in (5,10,11,12): + atts[tag] = (tag_info.type, 1, len(value), tuple(map(float,value))) + # stringish types + if DEBUG: print("Converted items: %s" % sorted(atts.items())) + print("Length: %s" % len(atts)) # libtiff always expects the bytes in native order. # we're storing image byte order. So, if the rawmode diff --git a/PIL/TiffTags.py b/PIL/TiffTags.py index 6644b237e48..2e3c9bce420 100644 --- a/PIL/TiffTags.py +++ b/PIL/TiffTags.py @@ -130,6 +130,7 @@ def lookup(tag): 324: ("TileOffsets", LONG, 0), 325: ("TileByteCounts", LONG, 0), + 330: ("SubIFD", SHORT, 1), 332: ("InkSet", SHORT, 1), 333: ("InkNames", ASCII, 1), 334: ("NumberOfInks", SHORT, 1), @@ -158,6 +159,12 @@ def lookup(tag): 531: ("YCbCrPositioning", SHORT, 1), 532: ("ReferenceBlackWhite", LONG, 0), + # sgi, in core + 32995:("Matteing", SHORT, 1), + 32996:("DataType", SHORT, 0), + 32997:("ImageDepth", LONG, 1), + 32998:("TileDepth", LONG, 1), + 33432: ("Copyright", ASCII, 1), # FIXME add more tags here @@ -417,6 +424,7 @@ def _populate(): # 393: case TIFFTAG_INKNAMES: # some of these are not in our TAGS_V2 dict and were included from tiff.h +# Anything included here needs to have the correct type in TAGS_V2 above LIBTIFF_CORE = {255, 256, 257, 258, 259, 262, 263, 266, 274, 277, 278, 280, 281, 340, 341, 282, 283, 284, 286, 287, @@ -430,6 +438,8 @@ def _populate(): LIBTIFF_CORE.remove(301) # Array of short, crashes LIBTIFF_CORE.remove(532) # Array of long, crashes +LIBTIFF_CORE.remove(330) # subifd, requires extra support for uint64 payload + LIBTIFF_CORE.remove(255) # We don't have support for subfiletypes LIBTIFF_CORE.remove(322) # We don't have support for tiled images in libtiff LIBTIFF_CORE.remove(323) # Tiled images diff --git a/encode.c b/encode.c index 7e8f6b12585..b9dfb778bab 100644 --- a/encode.c +++ b/encode.c @@ -749,6 +749,19 @@ PyImaging_JpegEncoderNew(PyObject* self, PyObject* args) #include +#define PUSHMETAARRAY(_type, _PyFunction)\ + arrav = calloc(len, sizeof(_type));\ + if (arrav) {\ + for (i=0;istate,\ + (ttag_t) PyInt_AsLong(key),\ + len, arrav);\ + free(arrav);\ + } + + PyObject* PyImaging_LibTiffEncoderNew(PyObject* self, PyObject* args) { @@ -761,7 +774,8 @@ PyImaging_LibTiffEncoderNew(PyObject* self, PyObject* args) int fp; PyObject *dir; - PyObject *key, *value; + PyObject *key, *value, *valuetuple; + int type, length, flarray; Py_ssize_t pos = 0; int status; @@ -804,67 +818,116 @@ PyImaging_LibTiffEncoderNew(PyObject* self, PyObject* args) for (pos = 0; pos < d_size; pos++) { key = PyList_GetItem(keys, pos); - value = PyList_GetItem(values, pos); + valuetuple = PyList_GetItem(values, pos); status = 0; TRACE(("Attempting to set key: %d\n", (int)PyInt_AsLong(key))); - if (PyInt_Check(value)) { - TRACE(("Setting from Int: %d %ld \n", (int)PyInt_AsLong(key),PyInt_AsLong(value))); - status = ImagingLibTiffSetField(&encoder->state, - (ttag_t) PyInt_AsLong(key), - PyInt_AsLong(value)); - } else if (PyFloat_Check(value)) { - TRACE(("Setting from Float: %d, %f \n", (int)PyInt_AsLong(key),PyFloat_AsDouble(value))); - status = ImagingLibTiffSetField(&encoder->state, - (ttag_t) PyInt_AsLong(key), - (float)PyFloat_AsDouble(value)); - } else if (PyBytes_Check(value)) { - TRACE(("Setting from Bytes: %d, %s \n", (int)PyInt_AsLong(key),PyBytes_AsString(value))); - status = ImagingLibTiffSetField(&encoder->state, - (ttag_t) PyInt_AsLong(key), - PyBytes_AsString(value)); - } else if (PyTuple_Check(value)) { + + if (! PyTuple_Check(valuetuple)) { + PyErr_SetString(PyExc_ValueError, "Expected a tuple for tiff metadata"); + return NULL; + } + /* undone checks */ + type = PyInt_AsLong(PyTuple_GetItem(valuetuple,0)); + flarray = PyInt_AsLong(PyTuple_GetItem(valuetuple,1)); + length = PyInt_AsLong(PyTuple_GetItem(valuetuple,2)); + value = PyTuple_GetItem(valuetuple,3); + + if (flarray == 0) { + if (length != 1) { + PyErr_SetString(PyExc_ValueError, "Expected length == 1 for non-array item"); + return NULL; + } + switch (type) { + case 3: + case 4: + case 6: + case 8: + case 9: + if (PyInt_Check(value)) { + TRACE(("Setting %d from Int: %d %ld \n", + (int)PyInt_AsLong(key), + type, + PyInt_AsLong(value))); + status = ImagingLibTiffSetField(&encoder->state, + (ttag_t) PyInt_AsLong(key), + PyInt_AsLong(value)); + } else { + PyErr_SetString(PyExc_ValueError, "Expected int for metadata value"); + return NULL; + } + break; + case 5: + case 10: + case 11: + case 12: + if (PyFloat_Check(value)) { + TRACE(("Setting from Float: %d, %f \n", (int)PyInt_AsLong(key),PyFloat_AsDouble(value))); + status = ImagingLibTiffSetField(&encoder->state, + (ttag_t) PyInt_AsLong(key), + (float)PyFloat_AsDouble(value)); + } else { + PyErr_SetString(PyExc_ValueError, "Expected floatlike for metadata value"); + return NULL; + } + break; + default: + if (PyBytes_Check(value)) { + TRACE(("Setting from Bytes: %d, %s \n", (int)PyInt_AsLong(key),PyBytes_AsString(value))); + status = ImagingLibTiffSetField(&encoder->state, + (ttag_t) PyInt_AsLong(key), + PyBytes_AsString(value)); + } else { + PyErr_SetString(PyExc_ValueError, "Expected stringlike for metadata value"); + return NULL; + } + } + } else { Py_ssize_t len,i; - float *floatav; - int *intav; + void *arrav; + TRACE(("Setting from Tuple: %d \n", (int)PyInt_AsLong(key))); len = PyTuple_Size(value); - if (len) { - if (PyInt_Check(PyTuple_GetItem(value,0))) { + if ((int)len == length) { + switch (type) { + case 3: + TRACE((" %d elements, setting as short \n", (int)len)); + PUSHMETAARRAY(short, PyInt_AsLong); + break; + case 4: TRACE((" %d elements, setting as ints \n", (int)len)); - /* malloc check ok, calloc checks for overflow */ - intav = calloc(len, sizeof(int)); - if (intav) { + PUSHMETAARRAY(int, PyInt_AsLong); + /*arrav = calloc(len, sizeof(int)); + if (arrav) { for (i=0;istate, (ttag_t) PyInt_AsLong(key), - len, intav); - free(intav); - } - } else if (PyFloat_Check(PyTuple_GetItem(value,0))) { + len, arrav); + free(arrav); + }*/ + break; + case 5: + case 10: + case 11: TRACE((" %d elements, setting as floats \n", (int)len)); /* malloc check ok, calloc checks for overflow */ - floatav = calloc(len, sizeof(float)); - if (floatav) { - for (i=0;istate, - (ttag_t) PyInt_AsLong(key), - len, floatav); - free(floatav); - } - } else { - TRACE(("Unhandled type in tuple for key %d : %s \n", + PUSHMETAARRAY(float, PyFloat_AsDouble); + break; + case 12: + TRACE((" %d elements, setting as double \n", (int)len)); + PUSHMETAARRAY(double, PyFloat_AsDouble); + break; + default: + TRACE(("Unhandled type in tuple for key %d : %d, len: %d \n", (int)PyInt_AsLong(key), - PyBytes_AsString(PyObject_Str(value)))); + type, length)); } } - } else { + /* } else { TRACE(("Unhandled type for key %d : %s \n", (int)PyInt_AsLong(key), - PyBytes_AsString(PyObject_Str(value)))); + PyBytes_AsString(PyObject_Str(value))));*/ } if (!status) { TRACE(("Error setting Field\n")); From 1ad3dd72128ad6a6075fbc749a52f37c4de8c954 Mon Sep 17 00:00:00 2001 From: wiredfool Date: Fri, 9 Dec 2016 10:25:52 +0000 Subject: [PATCH 2/9] pulling out special cases, referenceblackwhite works --- PIL/TiffImagePlugin.py | 2 + PIL/TiffTags.py | 4 +- Tests/test_file_libtiff.py | 2 +- encode.c | 162 +++++++++++++++++++++++++------------ 4 files changed, 116 insertions(+), 54 deletions(-) diff --git a/PIL/TiffImagePlugin.py b/PIL/TiffImagePlugin.py index ff354ae3176..713878b1e5b 100644 --- a/PIL/TiffImagePlugin.py +++ b/PIL/TiffImagePlugin.py @@ -1448,6 +1448,8 @@ def _save(im, fp, filename): legacy_ifd = {} if hasattr(im, 'tag'): legacy_ifd = im.tag.to_v2() + if DEBUG: + print("Legacy IFD items: %s" % sorted(legacy_ifd.items())) for tag, value in itertools.chain(ifd.items(), getattr(im, 'tag_v2', {}).items(), legacy_ifd.items()): diff --git a/PIL/TiffTags.py b/PIL/TiffTags.py index 2e3c9bce420..76f00d6dd8c 100644 --- a/PIL/TiffTags.py +++ b/PIL/TiffTags.py @@ -157,7 +157,7 @@ def lookup(tag): 529: ("YCbCrCoefficients", RATIONAL, 3), 530: ("YCbCrSubSampling", SHORT, 2), 531: ("YCbCrPositioning", SHORT, 1), - 532: ("ReferenceBlackWhite", LONG, 0), + 532: ("ReferenceBlackWhite", RATIONAL, 6), # sgi, in core 32995:("Matteing", SHORT, 1), @@ -436,7 +436,7 @@ def _populate(): LIBTIFF_CORE.remove(320) # Array of short, crashes LIBTIFF_CORE.remove(301) # Array of short, crashes -LIBTIFF_CORE.remove(532) # Array of long, crashes +#LIBTIFF_CORE.remove(532) # Array of long, crashes LIBTIFF_CORE.remove(330) # subifd, requires extra support for uint64 payload diff --git a/Tests/test_file_libtiff.py b/Tests/test_file_libtiff.py index 48b74964fed..a0b43f8eb09 100644 --- a/Tests/test_file_libtiff.py +++ b/Tests/test_file_libtiff.py @@ -222,7 +222,7 @@ def test_additional_metadata(self): out = self.tempfile("temp.tif") TiffImagePlugin.WRITE_LIBTIFF = True - + im.save(out, tiffinfo=new_ifd) TiffImagePlugin.WRITE_LIBTIFF = False diff --git a/encode.c b/encode.c index b9dfb778bab..370f94a0c15 100644 --- a/encode.c +++ b/encode.c @@ -756,7 +756,7 @@ PyImaging_JpegEncoderNew(PyObject* self, PyObject* args) ((_type *)arrav)[i] = (_type)_PyFunction(PyTuple_GetItem(value,i)); \ }\ status = ImagingLibTiffSetField(&encoder->state,\ - (ttag_t) PyInt_AsLong(key),\ + tag,\ len, arrav);\ free(arrav);\ } @@ -775,6 +775,7 @@ PyImaging_LibTiffEncoderNew(PyObject* self, PyObject* args) PyObject *dir; PyObject *key, *value, *valuetuple; + ttag_t tag; int type, length, flarray; Py_ssize_t pos = 0; int status; @@ -818,6 +819,7 @@ PyImaging_LibTiffEncoderNew(PyObject* self, PyObject* args) for (pos = 0; pos < d_size; pos++) { key = PyList_GetItem(keys, pos); + tag = (ttag_t) PyInt_AsLong(key); valuetuple = PyList_GetItem(values, pos); status = 0; TRACE(("Attempting to set key: %d\n", (int)PyInt_AsLong(key))); @@ -838,19 +840,48 @@ PyImaging_LibTiffEncoderNew(PyObject* self, PyObject* args) return NULL; } switch (type) { - case 3: - case 4: - case 6: - case 8: - case 9: + case TIFF_BYTE: + case TIFF_UNDEFINED: + case TIFF_SBYTE: + case TIFF_SHORT: + case TIFF_SSHORT: if (PyInt_Check(value)) { TRACE(("Setting %d from Int: %d %ld \n", (int)PyInt_AsLong(key), type, PyInt_AsLong(value))); status = ImagingLibTiffSetField(&encoder->state, - (ttag_t) PyInt_AsLong(key), - PyInt_AsLong(value)); + tag, + (int)PyInt_AsLong(value)); + } else { + PyErr_SetString(PyExc_ValueError, "Expected int for metadata value"); + return NULL; + } + break; + case TIFF_LONG: + case TIFF_IFD: + if (PyInt_Check(value)) { + TRACE(("Setting %d from uInt: %d %ld \n", + (int)PyInt_AsLong(key), + type, + PyInt_AsLong(value))); + status = ImagingLibTiffSetField(&encoder->state, + tag, + (uint32)PyLong_AsUnsignedLong(value)); + } else { + PyErr_SetString(PyExc_ValueError, "Expected int for metadata value"); + return NULL; + } + break; + case TIFF_SLONG: + if (PyInt_Check(value)) { + TRACE(("Setting %d from Int: %d %ld \n", + (int)PyInt_AsLong(key), + type, + PyInt_AsLong(value))); + status = ImagingLibTiffSetField(&encoder->state, + tag, + (int32)PyInt_AsLong(value)); } else { PyErr_SetString(PyExc_ValueError, "Expected int for metadata value"); return NULL; @@ -863,8 +894,8 @@ PyImaging_LibTiffEncoderNew(PyObject* self, PyObject* args) if (PyFloat_Check(value)) { TRACE(("Setting from Float: %d, %f \n", (int)PyInt_AsLong(key),PyFloat_AsDouble(value))); status = ImagingLibTiffSetField(&encoder->state, - (ttag_t) PyInt_AsLong(key), - (float)PyFloat_AsDouble(value)); + tag, + (double)PyFloat_AsDouble(value)); } else { PyErr_SetString(PyExc_ValueError, "Expected floatlike for metadata value"); return NULL; @@ -874,7 +905,7 @@ PyImaging_LibTiffEncoderNew(PyObject* self, PyObject* args) if (PyBytes_Check(value)) { TRACE(("Setting from Bytes: %d, %s \n", (int)PyInt_AsLong(key),PyBytes_AsString(value))); status = ImagingLibTiffSetField(&encoder->state, - (ttag_t) PyInt_AsLong(key), + tag, PyBytes_AsString(value)); } else { PyErr_SetString(PyExc_ValueError, "Expected stringlike for metadata value"); @@ -885,49 +916,78 @@ PyImaging_LibTiffEncoderNew(PyObject* self, PyObject* args) Py_ssize_t len,i; void *arrav; - TRACE(("Setting from Tuple: %d \n", (int)PyInt_AsLong(key))); + TRACE(("Setting from Tuple: %d, type: %d \n", (int)PyInt_AsLong(key), type)); len = PyTuple_Size(value); - if ((int)len == length) { - switch (type) { - case 3: - TRACE((" %d elements, setting as short \n", (int)len)); - PUSHMETAARRAY(short, PyInt_AsLong); - break; - case 4: - TRACE((" %d elements, setting as ints \n", (int)len)); - PUSHMETAARRAY(int, PyInt_AsLong); - /*arrav = calloc(len, sizeof(int)); - if (arrav) { - for (i=0;istate, - (ttag_t) PyInt_AsLong(key), - len, arrav); - free(arrav); - }*/ - break; - case 5: - case 10: - case 11: - TRACE((" %d elements, setting as floats \n", (int)len)); - /* malloc check ok, calloc checks for overflow */ - PUSHMETAARRAY(float, PyFloat_AsDouble); - break; - case 12: - TRACE((" %d elements, setting as double \n", (int)len)); - PUSHMETAARRAY(double, PyFloat_AsDouble); - break; - default: - TRACE(("Unhandled type in tuple for key %d : %d, len: %d \n", - (int)PyInt_AsLong(key), - type, length)); + + switch (tag) { + /* special cases */ + case TIFFTAG_PAGENUMBER: + case TIFFTAG_HALFTONEHINTS: + case TIFFTAG_YCBCRSUBSAMPLING: + /* not an array, passing two int items */ + if (len != 2) { + PyErr_SetString(PyExc_ValueError, "Requiring 2 items for for tag"); + return NULL; } + status = ImagingLibTiffSetField(&encoder->state, + tag, + (int)PyInt_AsLong(PyTuple_GetItem(value,0)), + (int)PyInt_AsLong(PyTuple_GetItem(value,1))); + break; + case TIFFTAG_COLORMAP: + /* 3x uint16 * arrays of r,g,b palette values, len=2^^bpp */ + break; + case TIFFTAG_SUBIFD: + /* int short length, uint32* data */ + break; + case TIFFTAG_TRANSFERFUNCTION: + /* 1 or 3 uint16 * arrays len=2^^bpp */ + break; + case TIFFTAG_REFERENCEBLACKWHITE: + /* float *, array len==6 */ + if (len != 6) { + PyErr_SetString(PyExc_ValueError, "Requiring 6 items for for ReferenceBlackWhite"); + return NULL; + } + arrav = calloc(len, sizeof(float)); + if (arrav) { + for (i=0;istate, + tag, + arrav); + free(arrav); + } + break; + case TIFFTAG_INKNAMES: + /* int length, char * names */ + break; + default: + if ((int)len == length) { + switch (type) { + case 3: + TRACE((" %d elements, setting as short \n", (int)len)); + PUSHMETAARRAY(short, PyInt_AsLong); + break; + case 4: + TRACE((" %d elements, setting as ints \n", (int)len)); + PUSHMETAARRAY(int, PyInt_AsLong); + break; + case 5: + case 10: + case 11: + case 12: + TRACE((" %d elements, setting as double \n", (int)len)); + PUSHMETAARRAY(double, PyFloat_AsDouble); + break; + default: + TRACE(("Unhandled type in tuple for key %d : %d, len: %d \n", + (int)PyInt_AsLong(key), + type, length)); + } + } } - /* } else { - TRACE(("Unhandled type for key %d : %s \n", - (int)PyInt_AsLong(key), - PyBytes_AsString(PyObject_Str(value))));*/ } if (!status) { TRACE(("Error setting Field\n")); From 8a772759b120ecc61ada535c132644eb1f4eadba Mon Sep 17 00:00:00 2001 From: wiredfool Date: Mon, 12 Dec 2016 13:28:29 -0800 Subject: [PATCH 3/9] Added support for saving colormaps --- PIL/TiffTags.py | 5 +---- Tests/test_file_libtiff.py | 14 ++++++++++++++ encode.c | 20 ++++++++++++++++++++ 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/PIL/TiffTags.py b/PIL/TiffTags.py index 76f00d6dd8c..3bcf9b383b5 100644 --- a/PIL/TiffTags.py +++ b/PIL/TiffTags.py @@ -434,16 +434,13 @@ def _populate(): 269 # this has been in our tests forever, and works } -LIBTIFF_CORE.remove(320) # Array of short, crashes -LIBTIFF_CORE.remove(301) # Array of short, crashes -#LIBTIFF_CORE.remove(532) # Array of long, crashes - LIBTIFF_CORE.remove(330) # subifd, requires extra support for uint64 payload LIBTIFF_CORE.remove(255) # We don't have support for subfiletypes LIBTIFF_CORE.remove(322) # We don't have support for tiled images in libtiff LIBTIFF_CORE.remove(323) # Tiled images LIBTIFF_CORE.remove(333) # Ink Names either +LIBTIFF_CORE.remove(301) # Transfer Function. No support as of yet. # Note to advanced users: There may be combinations of these # parameters and values that when added properly, will work and diff --git a/Tests/test_file_libtiff.py b/Tests/test_file_libtiff.py index a0b43f8eb09..4d58a1d3fa7 100644 --- a/Tests/test_file_libtiff.py +++ b/Tests/test_file_libtiff.py @@ -194,6 +194,7 @@ def test_additional_metadata(self): del(core_items[tag]) except: pass + del(core_items[320]) # colormap is special, tested below # Type codes: # 2: "ascii", @@ -354,6 +355,19 @@ def test_cmyk_save(self): im2 = Image.open(out) self.assert_image_equal(im, im2) + def test_palette_save(self): + im = hopper('P') + out = self.tempfile('temp.tif') + TiffImagePlugin.WRITE_LIBTIFF = True + im.save(out) + TiffImagePlugin.WRITE_LIBTIFF = False + + reloaded = Image.open(out) + # colormap/palette tag + self.assertTrue(len(reloaded.tag_v2[320]), 768) + self.assert_image_equal(im, reloaded) + + def xtest_bw_compression_w_rgb(self): """ This test passes, but when running all tests causes a failure due to output on stderr from the error thrown by libtiff. We need to diff --git a/encode.c b/encode.c index 370f94a0c15..8baa002bfca 100644 --- a/encode.c +++ b/encode.c @@ -936,6 +936,26 @@ PyImaging_LibTiffEncoderNew(PyObject* self, PyObject* args) break; case TIFFTAG_COLORMAP: /* 3x uint16 * arrays of r,g,b palette values, len=2^^bpp */ + { + int stride = 256; + TRACE(("Setting ColorMap\n")); + if (len != 768) { + PyErr_SetString(PyExc_ValueError, "Requiring 768 items for for Colormap"); + return NULL; + } + arrav = calloc(len, sizeof(uint16)); + if (arrav) { + for (i=0;istate, + tag, + arrav, + ((uint16 *)arrav) + stride, + ((uint16 *)arrav) + 2 * stride); + free(arrav); + } + } break; case TIFFTAG_SUBIFD: /* int short length, uint32* data */ From 320884d8a220a7ea6d8bfe09350c5609b5c6ad67 Mon Sep 17 00:00:00 2001 From: wiredfool Date: Tue, 13 Dec 2016 12:49:47 -0800 Subject: [PATCH 4/9] Tests for issue #1765 --- Tests/test_file_libtiff.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Tests/test_file_libtiff.py b/Tests/test_file_libtiff.py index 4d58a1d3fa7..d74ce1420c9 100644 --- a/Tests/test_file_libtiff.py +++ b/Tests/test_file_libtiff.py @@ -228,6 +228,16 @@ def test_additional_metadata(self): TiffImagePlugin.WRITE_LIBTIFF = False + def test_int_dpi(self): + # issue #1765 + im = hopper('RGB') + out = self.tempfile('temp.tif') + TiffImagePlugin.WRITE_LIBTIFF = True + im.save(out, dpi=(72, 72)) + TiffImagePlugin.WRITE_LIBTIFF = False + reloaded = Image.open(out) + self.assertEqual(reloaded.info['dpi'], (72.0, 72.0)) + def test_g3_compression(self): i = Image.open('Tests/images/hopper_g4_500.tif') out = self.tempfile("temp.tif") From 713d383e00ca1c5734126b7d33049a8a2fb1d1e4 Mon Sep 17 00:00:00 2001 From: wiredfool Date: Tue, 13 Dec 2016 03:18:17 -0800 Subject: [PATCH 5/9] Docs --- docs/reference/internal_design.rst | 2 + docs/reference/tiff_metadata.rst | 135 +++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+) create mode 100644 docs/reference/tiff_metadata.rst diff --git a/docs/reference/internal_design.rst b/docs/reference/internal_design.rst index a8d6e2284b4..0a2b60cb402 100644 --- a/docs/reference/internal_design.rst +++ b/docs/reference/internal_design.rst @@ -6,3 +6,5 @@ Internal Reference Docs open_files limits + tiff_metadata + diff --git a/docs/reference/tiff_metadata.rst b/docs/reference/tiff_metadata.rst new file mode 100644 index 00000000000..daf1227f02a --- /dev/null +++ b/docs/reference/tiff_metadata.rst @@ -0,0 +1,135 @@ +Tiff Metadata +============= + +Pillow currently reads TIFF metadata in pure python and writes either +through its own writer (if writing an uncompressed tiff) or libtiff. + +Basic overview +++++++++++++++ + +TIFF is Tagged Image File Format -- the metadata is stored as a well +known tag number, a type, a quantity, and the data. Many tags are +defined in the spec and implemented in libtiff, but there's also the +possibility to add custom tags that aren't specified in the spec. The +metadata is packed into a structure known as the Image File Directory, +or IFD. We define many of these tags in :py:mod:`~PIL.TiffTags`. + +The Tiff IFD is also used in other file formats (like JPEG) to store +EXIF information or other metadata. + +Writing Metadata in Libtiff ++++++++++++++++++++++++++++ + +There are three categories of metadata:: + +* Built in, but not special cases +* Special cases, built in +* Custom tags + +These categories aren't listed in the docs anywhere, it's from a dive +through libtif's tif_dir.c and the various headers. + +Metadata is set using a single api call to ``TIFFVSetField(tiff, tag, +*args)`` which has a var args setup, so the function signature changes +based on the tag passed in. The count and data sizes are defined in +the libtiff directory headers. There are many different memcopys that +are performed with **no** validation of the input parameters, either +in type or quantity. These lead to a segfault in the best case. + +.. Warning:: + This is a security nightmare. + +Because of this security nightmare, we're whitelisting and testing +individual tiff tags for writing. The complexity of this simple +interface means that we have to essentially duplicate the logic of the +libtiff interface to put the parameters in the right configuration. We +are whitelisting these tags in :py:mod:`~PIL.TiffTags.LIBTIFF_CORE`. + + +Built In +-------- + +There is a long list (in theory, you have to go through the code for +them) of built in items that regular. These have one of three call +signatures: + +* Individual: ``TIFFVSetField(tiff, tag, item)`` +* Multiple: ``TIFFVSetField(tiff, tag, ct, items* )`` +* Alternate Multiple: ``TIFFVSetField(tiff, tag, items* )`` + +In libtiff4, the individual integer like numeric items are passed as +32 bit ints (signed or unsigned as appropriate) even if the actual +item is a short or char. The individual rational and floating point +types are all passed as a double. + +The multipile value items are passed as pointers to packed arrays of +the correct type, short for short. + +UNDONE -- This isn't quite true: The count is only used for items +where field_passcount is true. Then if ``field->writecount`` == +``TIFF_VARIABLE2``, then it's a ``uint32``, otherwise count is an int. +Otherwise, if ``field_writecount`` is ``TIFF_VARIABLE`` or +``TIFF_VARIABLE2``, then the count is not used and set to 1. If it's +``TIFF_SPP``, then it's set to samplesperpixel. Otherwise, it's set to +the ``field_writecount``. + + +Special Cases +------------- + +There are a set of special cases in the ``tif_dir.c:_TIFFVSetField`` +function where tag by tag, the individual items are pulled. These are +mainly items that are specifically used by the tiff decoder. The +individual items all follow the pattern of the built ins above, but +the array based items are special, each in their own way. + +* Where there are two shorts passed in, they are passed as separate + parameters rather than a packed array. (e.g. ``TIFFTAG_PAGENUMBER``) + +* ``TIFFTAG_REFERENCEBLACKWHITE`` is just passed as an array of 6 + ``float``, there's no count of items. + +* ``TIFFTAG_COLORMAP`` is passed as three pointers to arrays of ``short`` + +* ``TIFFTAG_TRANSFERFUNCTION`` is passed as 1 or 3 pointers to arrays + of ``short`` + +* ``TIFFTAG_INKNAMES`` is passed as a length and a ``char*``. + +* ``TIFFTAG_SUBIFD`` is passed as a length and pointer to ``uint32`` + UNDONE -- is this length in bytes, or in integers, and does this + change in libtiff5? + +Custom Tags +----------- + +These are tags that are not defined in libtiff. To use these, we would +need to define the tag for that image by passing in the appropriate +definition. + +.. Note:: + Custom tags are currently unimplemented. + + +Writing Metadata in Python +++++++++++++++++++++++++++ + +UNDONE -- review/expand this on down. + +When writing a TIFF file using python, the IFD is written using the +code at +:py:mod:`~PIL.TiffImagePlugin.ImageFileDirectory_v2.save`. This uses +the types from the IFD to control the writing. This leads to safe but +possibly out of spec writing. + +Metadata Storage in Pillow +++++++++++++++++++++++++++ + +* See TiffImagePlugin +* tags_v2 vs tags + +Reading Metadata in TiffImagePlugin ++++++++++++++++++++++++++++++++++++ + +* Type confusion between file and spec. + From e1110b5a9c5d601aab4f947eb70de82aff71aa0a Mon Sep 17 00:00:00 2001 From: wiredfool Date: Wed, 14 Dec 2016 03:50:06 -0800 Subject: [PATCH 6/9] Whitepoint, Primary Chromaticities working --- PIL/TiffTags.py | 18 +++++----- Tests/test_file_libtiff.py | 10 ++++++ docs/reference/tiff_metadata.rst | 4 +-- encode.c | 57 +++++++++++++++++++++++--------- libImaging/TiffDecode.c | 7 ++++ libImaging/TiffDecode.h | 1 + 6 files changed, 72 insertions(+), 25 deletions(-) diff --git a/PIL/TiffTags.py b/PIL/TiffTags.py index 3bcf9b383b5..c5c301e9724 100644 --- a/PIL/TiffTags.py +++ b/PIL/TiffTags.py @@ -95,8 +95,8 @@ def lookup(tag): 278: ("RowsPerStrip", LONG, 1), 279: ("StripByteCounts", LONG, 0), - 280: ("MinSampleValue", LONG, 0), - 281: ("MaxSampleValue", SHORT, 0), + 280: ("MinSampleValue", LONG, 1), + 281: ("MaxSampleValue", SHORT, 1), 282: ("XResolution", RATIONAL, 1), 283: ("YResolution", RATIONAL, 1), 284: ("PlanarConfiguration", SHORT, 1, {"Contiguous": 1, "Separate": 2}), @@ -121,7 +121,7 @@ def lookup(tag): 316: ("HostComputer", ASCII, 1), 317: ("Predictor", SHORT, 1, {"none": 1, "Horizontal Differencing": 2}), 318: ("WhitePoint", RATIONAL, 2), - 319: ("PrimaryChromaticities", SHORT, 6), + 319: ("PrimaryChromaticities", RATIONAL, 6), 320: ("ColorMap", SHORT, 0), 321: ("HalftoneHints", SHORT, 2), @@ -137,10 +137,10 @@ def lookup(tag): 336: ("DotRange", SHORT, 0), 337: ("TargetPrinter", ASCII, 1), 338: ("ExtraSamples", SHORT, 0), - 339: ("SampleFormat", SHORT, 0), + 339: ("SampleFormat", SHORT, 1), - 340: ("SMinSampleValue", DOUBLE, 0), - 341: ("SMaxSampleValue", DOUBLE, 0), + 340: ("SMinSampleValue", DOUBLE, 1), + 341: ("SMaxSampleValue", DOUBLE, 1), 342: ("TransferRange", SHORT, 6), # obsolete JPEG tags @@ -161,7 +161,7 @@ def lookup(tag): # sgi, in core 32995:("Matteing", SHORT, 1), - 32996:("DataType", SHORT, 0), + 32996:("DataType", SHORT, 1), 32997:("ImageDepth", LONG, 1), 32998:("TileDepth", LONG, 1), @@ -431,7 +431,9 @@ def _populate(): 296, 297, 321, 320, 338, 32995, 322, 323, 32998, 32996, 339, 32997, 330, 531, 530, 301, 532, 333, # as above - 269 # this has been in our tests forever, and works + 269, # this has been in our tests forever, and works + 318, # Whitepoint, Specific test for it + 319, # Primary Chromaticities } LIBTIFF_CORE.remove(330) # subifd, requires extra support for uint64 payload diff --git a/Tests/test_file_libtiff.py b/Tests/test_file_libtiff.py index d74ce1420c9..1c4b67624d1 100644 --- a/Tests/test_file_libtiff.py +++ b/Tests/test_file_libtiff.py @@ -517,6 +517,16 @@ def test_crashing_metadata(self): im.save(out, format='TIFF') TiffImagePlugin.WRITE_LIBTIFF = False + reloaded = Image.open(out) + + self.assert_image_equal(im, reloaded) + for tag in (318, 319): + for ix in range(len(im.tag_v2[tag])): + self.assertAlmostEqual(float(im.tag_v2[tag][ix]), + float(reloaded.tag_v2[tag][ix]), + places=5) + + def test_page_number_x_0(self): # Issue 973 # Test TIFF with tag 297 (Page Number) having value of 0 0. diff --git a/docs/reference/tiff_metadata.rst b/docs/reference/tiff_metadata.rst index daf1227f02a..5f420d96c2b 100644 --- a/docs/reference/tiff_metadata.rst +++ b/docs/reference/tiff_metadata.rst @@ -54,8 +54,8 @@ them) of built in items that regular. These have one of three call signatures: * Individual: ``TIFFVSetField(tiff, tag, item)`` -* Multiple: ``TIFFVSetField(tiff, tag, ct, items* )`` -* Alternate Multiple: ``TIFFVSetField(tiff, tag, items* )`` +* Multiple, passcount=1: ``TIFFVSetField(tiff, tag, ct, items* )`` +* Multiple, passcount=0: ``TIFFVSetField(tiff, tag, items* )`` In libtiff4, the individual integer like numeric items are passed as 32 bit ints (signed or unsigned as appropriate) even if the actual diff --git a/encode.c b/encode.c index 8baa002bfca..e0f05917e69 100644 --- a/encode.c +++ b/encode.c @@ -754,11 +754,17 @@ PyImaging_JpegEncoderNew(PyObject* self, PyObject* args) if (arrav) {\ for (i=0;istate,\ - tag,\ - len, arrav);\ - free(arrav);\ + } \ + if (fi->field_passcount) { \ + status = ImagingLibTiffSetField(&encoder->state, \ + tag, \ + len, arrav); \ + } else { \ + status = ImagingLibTiffSetField(&encoder->state, \ + tag, \ + arrav); \ + } \ + free(arrav); \ } @@ -783,6 +789,7 @@ PyImaging_LibTiffEncoderNew(PyObject* self, PyObject* args) Py_ssize_t d_size; PyObject *keys, *values; + const TIFFFieldInfo* fi; if (! PyArg_ParseTuple(args, "sssisO", &mode, &rawmode, &compname, &fp, &filename, &dir)) { return NULL; @@ -834,6 +841,13 @@ PyImaging_LibTiffEncoderNew(PyObject* self, PyObject* args) length = PyInt_AsLong(PyTuple_GetItem(valuetuple,2)); value = PyTuple_GetItem(valuetuple,3); + fi = ImagingLibTiffGetFieldInfo(&encoder->state, tag); + if (!fi) { + /* undone, custom */ + PyErr_SetString(PyExc_ValueError, "Couldn't find field info for tag."); + return NULL; + } + if (flarray == 0) { if (length != 1) { PyErr_SetString(PyExc_ValueError, "Expected length == 1 for non-array item"); @@ -969,21 +983,31 @@ PyImaging_LibTiffEncoderNew(PyObject* self, PyObject* args) PyErr_SetString(PyExc_ValueError, "Requiring 6 items for for ReferenceBlackWhite"); return NULL; } - arrav = calloc(len, sizeof(float)); - if (arrav) { - for (i=0;istate, - tag, - arrav); - free(arrav); - } + PUSHMETAARRAY(float, PyFloat_AsDouble); break; case TIFFTAG_INKNAMES: /* int length, char * names */ break; default: + // Check for the right length for default case items + if (len > INT_MAX) { + PyErr_SetString(PyExc_MemoryError, "Metadata size error - int overflow"); + return NULL; + } + + if (fi->field_writecount == TIFF_VARIABLE || + fi->field_writecount == TIFF_VARIABLE2) { + if (len != 1) { + PyErr_SetString(PyExc_ValueError, "Expected 1 item for tag"); + return NULL; + } + } else if (fi->field_writecount == TIFF_SPP) { + /* need to check for samples per pixel here */ + } else if (len != fi->field_writecount) { + PyErr_SetString(PyExc_ValueError, "Incorrect number of items for tag"); + return NULL; + } + if ((int)len == length) { switch (type) { case 3: @@ -997,6 +1021,9 @@ PyImaging_LibTiffEncoderNew(PyObject* self, PyObject* args) case 5: case 10: case 11: + TRACE((" %d elements, setting as float \n", (int)len)); + PUSHMETAARRAY(float, PyFloat_AsDouble); + break; case 12: TRACE((" %d elements, setting as double \n", (int)len)); PUSHMETAARRAY(double, PyFloat_AsDouble); diff --git a/libImaging/TiffDecode.c b/libImaging/TiffDecode.c index f292da3883d..afdf99b14a2 100644 --- a/libImaging/TiffDecode.c +++ b/libImaging/TiffDecode.c @@ -336,6 +336,13 @@ int ImagingLibTiffEncodeInit(ImagingCodecState state, char *filename, int fp) { } + +const TIFFFieldInfo* +ImagingLibTiffGetFieldInfo(ImagingCodecState state, ttag_t tag) { + TIFFSTATE *clientstate = (TIFFSTATE *)state->context; + return TIFFFieldWithTag(clientstate->tiff, tag); +} + int ImagingLibTiffSetField(ImagingCodecState state, ttag_t tag, ...){ // after tif_dir.c->TIFFSetField. TIFFSTATE *clientstate = (TIFFSTATE *)state->context; diff --git a/libImaging/TiffDecode.h b/libImaging/TiffDecode.h index e14a0932922..8c0f39dd8a1 100644 --- a/libImaging/TiffDecode.h +++ b/libImaging/TiffDecode.h @@ -46,6 +46,7 @@ typedef struct { extern int ImagingLibTiffInit(ImagingCodecState state, int fp, int offset); extern int ImagingLibTiffEncodeInit(ImagingCodecState state, char *filename, int fp); extern int ImagingLibTiffSetField(ImagingCodecState state, ttag_t tag, ...); +extern const TIFFFieldInfo* ImagingLibTiffGetFieldInfo(ImagingCodecState state, ttag_t tag); /* From c69500e026414a4f066e0d3fd16e9d009f2c33fe Mon Sep 17 00:00:00 2001 From: wiredfool Date: Wed, 14 Dec 2016 04:28:27 -0800 Subject: [PATCH 7/9] doc updates --- docs/reference/tiff_metadata.rst | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/reference/tiff_metadata.rst b/docs/reference/tiff_metadata.rst index 5f420d96c2b..25faef51edc 100644 --- a/docs/reference/tiff_metadata.rst +++ b/docs/reference/tiff_metadata.rst @@ -54,22 +54,23 @@ them) of built in items that regular. These have one of three call signatures: * Individual: ``TIFFVSetField(tiff, tag, item)`` -* Multiple, passcount=1: ``TIFFVSetField(tiff, tag, ct, items* )`` * Multiple, passcount=0: ``TIFFVSetField(tiff, tag, items* )`` +* Multiple, passcount=1: ``TIFFVSetField(tiff, tag, ct, items* )`` + In libtiff4, the individual integer like numeric items are passed as 32 bit ints (signed or unsigned as appropriate) even if the actual item is a short or char. The individual rational and floating point types are all passed as a double. -The multipile value items are passed as pointers to packed arrays of +The multiple value items are passed as pointers to packed arrays of the correct type, short for short. UNDONE -- This isn't quite true: The count is only used for items where field_passcount is true. Then if ``field->writecount`` == ``TIFF_VARIABLE2``, then it's a ``uint32``, otherwise count is an int. Otherwise, if ``field_writecount`` is ``TIFF_VARIABLE`` or -``TIFF_VARIABLE2``, then the count is not used and set to 1. If it's +``TIFF_VARIABLE2``, then the count is not passed and 1 item is read. If it's ``TIFF_SPP``, then it's set to samplesperpixel. Otherwise, it's set to the ``field_writecount``. From d79b1d4a042095d1c4ad6d54f67239822bdf1c81 Mon Sep 17 00:00:00 2001 From: wiredfool Date: Wed, 14 Dec 2016 07:42:03 -0800 Subject: [PATCH 8/9] Warning on count or type mismatch on exif data --- PIL/TiffImagePlugin.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/PIL/TiffImagePlugin.py b/PIL/TiffImagePlugin.py index 713878b1e5b..1a03d59fe2e 100644 --- a/PIL/TiffImagePlugin.py +++ b/PIL/TiffImagePlugin.py @@ -666,11 +666,22 @@ def load(self, fp): try: for i in range(self._unpack("H", self._ensure_read(fp, 2))[0]): tag, typ, count, data = self._unpack("HHL4s", self._ensure_read(fp, 12)) + taginfo = TiffTags.lookup(tag) if DEBUG: - tagname = TiffTags.lookup(tag).name typname = TYPES.get(typ, "unknown") - print("tag: %s (%d) - type: %s (%d)" % - (tagname, tag, typname, typ), end=" ") + print("tag: %s (%d) - type: %s (%d) - spec type: %s - count: %d, spec ct: %d" % + (taginfo.name, tag, typname, typ, + taginfo.type, count, taginfo.length), end=" ") + + if (taginfo.type and typ != taginfo.type): + warnings.warn(("Possibly corrupt EXIF data. " + + "File tag type %s does not match spec %s for tag %s" + ) % (typ, taginfo.type, tag)) + + if (taginfo.length and count != taginfo.length): + warnings.warn(("Possibly corrupt EXIF data. " + + "File tag count %s does not match spec %s for tag %s" + ) % (count, taginfo.length, tag)) try: unit_size, handler = self._load_dispatch[typ] From 5c9de81f9f0e3155495ea4de2482d06a9487173a Mon Sep 17 00:00:00 2001 From: wiredfool Date: Wed, 14 Dec 2016 07:42:58 -0800 Subject: [PATCH 9/9] Mismatch on count of items, sampleformat is troublesome UNDONE--approach? --- PIL/TiffImagePlugin.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/PIL/TiffImagePlugin.py b/PIL/TiffImagePlugin.py index 1a03d59fe2e..b228a8670ff 100644 --- a/PIL/TiffImagePlugin.py +++ b/PIL/TiffImagePlugin.py @@ -549,7 +549,11 @@ def _setitem(self, tag, value, legacy_api): if info.length == 1: if legacy_api and self.tagtype[tag] in [5, 10]: values = values, - dest[tag], = values + try: + dest[tag], = values + except ValueError: + # there's a mismatch between the spec and the item from the file. + dest[tag] = values else: dest[tag] = values @@ -1145,6 +1149,8 @@ def _setup(self): print("- size:", self.size) sampleFormat = self.tag_v2.get(SAMPLEFORMAT, (1,)) + if not isinstance(sampleFormat, tuple): + sampleFormat = (sampleFormat,) if (len(sampleFormat) > 1 and max(sampleFormat) == min(sampleFormat) == 1): # SAMPLEFORMAT is properly per band, so an RGB image will