From 7872501c5b3a5c64e84efad6d8128a9a5c11f3b7 Mon Sep 17 00:00:00 2001 From: Stephen Arthur Date: Sun, 22 May 2016 17:53:26 -0700 Subject: [PATCH 1/4] Added test cases to exhibit issues with custom qtables --- Tests/test_file_jpeg.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Tests/test_file_jpeg.py b/Tests/test_file_jpeg.py index 3b2e6cb20..dba16d346 100644 --- a/Tests/test_file_jpeg.py +++ b/Tests/test_file_jpeg.py @@ -303,6 +303,15 @@ class TestFileJpeg(PillowTestCase): filename = "Tests/images/junk_jpeg_header.jpg" Image.open(filename) + def _n_qtables_helper(self, n, test_file): + im = Image.open(test_file) + f = self.tempfile('temp.jpg') + im.save(f, qtables=[[n]*64]*n) + im = Image.open(f) + self.assertEqual(len(im.quantization), n) + reloaded = self.roundtrip(im, qtables="keep") + self.assertEqual(im.quantization, reloaded.quantization) + def test_qtables(self): im = Image.open("Tests/images/hopper.jpg") qtables = im.quantization @@ -359,6 +368,15 @@ class TestFileJpeg(PillowTestCase): 1: standard_chrominance_qtable }), 30) + self._n_qtables_helper(1, "Tests/images/hopper_gray.jpg") + self._n_qtables_helper(1, "Tests/images/pil_sample_rgb.jpg") + self._n_qtables_helper(2, "Tests/images/pil_sample_rgb.jpg") + self._n_qtables_helper(3, "Tests/images/pil_sample_rgb.jpg") + self._n_qtables_helper(1, "Tests/images/pil_sample_cmyk.jpg") + self._n_qtables_helper(2, "Tests/images/pil_sample_cmyk.jpg") + self._n_qtables_helper(3, "Tests/images/pil_sample_cmyk.jpg") + self._n_qtables_helper(4, "Tests/images/pil_sample_cmyk.jpg") + # not a sequence self.assertRaises(Exception, lambda: self.roundtrip(im, qtables='a')) # sequence wrong length From 8b572ade81d569e7a06233acdc18eee64f834612 Mon Sep 17 00:00:00 2001 From: Stephen Arthur Date: Sun, 22 May 2016 17:43:57 -0700 Subject: [PATCH 2/4] Found edge cases with custom qtables And there was a lingering bug since the previous qtable unsigned char fix (#1814) since the call to array.array was in another place, the roundtrip was no longer equivalent. That was a minor change, but the revised test wouldn't pass because saving an image with one custom qtable automatically adds a second to it by the call to jpeg_set_defaults. With 1 or >2 custom tables, there is extra work we have to do due to that call. --- PIL/JpegImagePlugin.py | 2 +- libImaging/JpegEncode.c | 21 ++++++++++++++++----- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/PIL/JpegImagePlugin.py b/PIL/JpegImagePlugin.py index 00026c4f7..4a818fe89 100644 --- a/PIL/JpegImagePlugin.py +++ b/PIL/JpegImagePlugin.py @@ -195,7 +195,7 @@ def DQT(self, marker): raise SyntaxError("bad quantization table marker") v = i8(s[0]) if v//16 == 0: - self.quantization[v & 15] = array.array("b", s[1:65]) + self.quantization[v & 15] = array.array("B", s[1:65]) s = s[65:] else: return # FIXME: add code to read 16-bit tables! diff --git a/libImaging/JpegEncode.c b/libImaging/JpegEncode.c index 5bcba2aa4..75237d3df 100644 --- a/libImaging/JpegEncode.c +++ b/libImaging/JpegEncode.c @@ -151,11 +151,22 @@ ImagingJpegEncode(Imaging im, ImagingCodecState state, UINT8* buf, int bytes) if (context->quality > 0) { quality = context->quality; } - for (i = 0; i < context->qtablesLen; i++) { - // TODO: Should add support for none baseline - jpeg_add_quant_table(&context->cinfo, i, &context->qtables[i * DCTSIZE2], - quality, TRUE); - } + int last_q = 0; + for (i = 0; i < context->qtablesLen; i++) { + // TODO: Should add support for none baseline + jpeg_add_quant_table(&context->cinfo, i, &context->qtables[i * DCTSIZE2], + quality, TRUE); + context->cinfo.comp_info[i].quant_tbl_no = i; + last_q = i; + } + if (context->qtablesLen == 1) { + // jpeg_set_defaults created two qtables internally, but we only wanted one. + jpeg_add_quant_table(&context->cinfo, 1, &context->qtables[0], + quality, TRUE); + } + for (i = last_q; i < context->cinfo.num_components; i++) { + context->cinfo.comp_info[i].quant_tbl_no = last_q; + } } else if (context->quality > 0) { jpeg_set_quality(&context->cinfo, context->quality, 1); } From 67a243d82dacfbca5da2d0da9910b29028e50e3f Mon Sep 17 00:00:00 2001 From: Stephen Arthur Date: Sun, 22 May 2016 18:50:42 -0700 Subject: [PATCH 3/4] Removing the one-line variable declare / assign to make appveyor happy --- libImaging/JpegEncode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libImaging/JpegEncode.c b/libImaging/JpegEncode.c index 75237d3df..50a8e7255 100644 --- a/libImaging/JpegEncode.c +++ b/libImaging/JpegEncode.c @@ -151,7 +151,7 @@ ImagingJpegEncode(Imaging im, ImagingCodecState state, UINT8* buf, int bytes) if (context->quality > 0) { quality = context->quality; } - int last_q = 0; + int last_q; for (i = 0; i < context->qtablesLen; i++) { // TODO: Should add support for none baseline jpeg_add_quant_table(&context->cinfo, i, &context->qtables[i * DCTSIZE2], From 0bc4423487913925f1eb1f2b5f120bee9666ff13 Mon Sep 17 00:00:00 2001 From: Stephen Arthur Date: Sun, 22 May 2016 19:13:19 -0700 Subject: [PATCH 4/4] Move the declaration to the beginning of scope to make vs happy --- libImaging/JpegEncode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libImaging/JpegEncode.c b/libImaging/JpegEncode.c index 50a8e7255..23f8a9a13 100644 --- a/libImaging/JpegEncode.c +++ b/libImaging/JpegEncode.c @@ -148,10 +148,10 @@ ImagingJpegEncode(Imaging im, ImagingCodecState state, UINT8* buf, int bytes) if (context->qtables) { int i; int quality = 100; + int last_q = 0; if (context->quality > 0) { quality = context->quality; } - int last_q; for (i = 0; i < context->qtablesLen; i++) { // TODO: Should add support for none baseline jpeg_add_quant_table(&context->cinfo, i, &context->qtables[i * DCTSIZE2],