[PATCH v3] windowscodecs: Fix data corruption for Adobe CMYK JPEGs

[Anton Romanov]

Fixes https://bugs.winehq.org/show_bug.cgi?id=43520

Signed-off-by: Anton Romanov 
---
 dlls/windowscodecs/jpegformat.c       |  13 ++--
 dlls/windowscodecs/tests/Makefile.in  |   1 +
 dlls/windowscodecs/tests/jpegformat.c | 123 ++++++++++++++++++++++++++++++++++
 3 files changed, 133 insertions(+), 4 deletions(-)
 create mode 100644 dlls/windowscodecs/tests/jpegformat.c

diff --git a/dlls/windowscodecs/jpegformat.c b/dlls/windowscodecs/jpegformat.c
index ef050ba688..12f6cf890b 100644
--- a/dlls/windowscodecs/jpegformat.c
+++ b/dlls/windowscodecs/jpegformat.c
@@ -678,11 +678,16 @@ static HRESULT WINAPI JpegDecoder_Frame_CopyPixels(IWICBitmapFrameDecode *iface,
                 This->cinfo.output_width, This->cinfo.output_scanline - first_scanline,
                 stride);
         }
+    }
+
+    if (This->cinfo.out_color_space == JCS_CMYK && This->cinfo.saw_Adobe_marker)
+    {
+        UINT i;
+        DWORD *pDwordData = (DWORD*) This->image_data;

-        if (This->cinfo.out_color_space == JCS_CMYK && This->cinfo.saw_Adobe_marker)
-            /* Adobe JPEG's have inverted CMYK data. */
-            for (i=0; i<data_size; i++)
-                This->image_data[i] ^= 0xff;
+        /* Adobe JPEG's have inverted CMYK data. */
+        for (i=0; i < data_size/4; i++)
+            *pDwordData++ ^= 0xffffffff;
     }

LeaveCriticalSection(&This->lock);
diff --git a/dlls/windowscodecs/tests/Makefile.in b/dlls/windowscodecs/tests/Makefile.in
index b1e78f995c..3d64210ba0 100644
--- a/dlls/windowscodecs/tests/Makefile.in
+++ b/dlls/windowscodecs/tests/Makefile.in
@@ -13,4 +13,5 @@ C_SRCS = \
    pngformat.c \
    propertybag.c \
    stream.c \
+	jpegformat.c \
    tiffformat.c
diff --git a/dlls/windowscodecs/tests/jpegformat.c b/dlls/windowscodecs/tests/jpegformat.c
new file mode 100644
index 0000000000..6830e38b45
--- /dev/null
+++ b/dlls/windowscodecs/tests/jpegformat.c
@@ -0,0 +1,123 @@
+#define COBJMACROS
+
+#include "objbase.h"
+#include "wincodec.h"
+#include "wine/test.h"
+
+static const char jpeg_adobe_cmyk_1x5[] =
+    "\xff\xd8\xff\xe0\x00\x10\x4a\x46\x49\x46\x00\x01\x01\x01\x01\x2c"
+	"\x01\x2c\x00\x00\xff\xee\x00\x0e\x41\x64\x6f\x62\x65\x00\x64\x00"
+	"\x00\x00\x00\x02\xff\xfe\x00\x13\x43\x72\x65\x61\x74\x65\x64\x20"
+	"\x77\x69\x74\x68\x20\x47\x49\x4d\x50\xff\xdb\x00\x43\x00\x01\x01"
+	"\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01"
+	"\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01"
+	"\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01"
+	"\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\xff\xdb"
+	"\x00\x43\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01"
+	"\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01"
+	"\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01"
+	"\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01"
+	"\x01\x01\x01\xff\xc0\x00\x14\x08\x00\x05\x00\x01\x04\x01\x11\x00"
+	"\x02\x11\x01\x03\x11\x01\x04\x11\x00\xff\xc4\x00\x15\x00\x01\x01"
+	"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x06\x08"
+	"\xff\xc4\x00\x14\x10\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+	"\x00\x00\x00\x00\x00\x00\xff\xc4\x00\x14\x01\x01\x00\x00\x00\x00"
+	"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x0a\xff\xc4\x00\x14"
+	"\x11\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
+	"\x00\x00\xff\xda\x00\x0e\x04\x01\x00\x02\x11\x03\x11\x04\x00\x00"
+	"\x3f\x00\x40\x44\x02\x1e\xa4\x1f\xff\xd9";
+
+static void test_decode_adobe_cmyk(void)
+{
+    IWICBitmapDecoder *decoder;
+    IWICBitmapFrameDecode *framedecode;
+    HRESULT hr;
+    HGLOBAL hjpegdata;
+    char *jpegdata;
+    IStream *jpegstream;
+    GUID guidresult;
+    UINT count=0, width=0, height=0;
+    BYTE imagedata[5 * 4] = {1};
+    const BYTE expected_imagedata[5 * 4] = {
+        0x00, 0xb0, 0xfc, 0x6d,
+        0x00, 0xb0, 0xfc, 0x6d,
+        0x00, 0xb0, 0xfc, 0x6d,
+        0x00, 0xb0, 0xfc, 0x6d,
+        0x00, 0xb0, 0xfc, 0x6d,
+    };
+
+    const BYTE expected_imagedata_24bpp[5 * 4] = {
+        0x0d, 0x4b, 0x94, 0x00,
+        0x0d, 0x4b, 0x94, 0x00,
+        0x0d, 0x4b, 0x94, 0x00,
+        0x0d, 0x4b, 0x94, 0x00,
+        0x0d, 0x4b, 0x94, 0x00,
+    };
+
+    hr = CoCreateInstance(&CLSID_WICJpegDecoder, NULL, CLSCTX_INPROC_SERVER,
+        &IID_IWICBitmapDecoder, (void**)&decoder);
+    ok(SUCCEEDED(hr), "CoCreateInstance failed, hr=%x\n", hr);
+    if (FAILED(hr)) return;
+
+    hjpegdata = GlobalAlloc(GMEM_MOVEABLE, sizeof(jpeg_adobe_cmyk_1x5));
+    ok(hjpegdata != 0, "GlobalAlloc failed\n");
+    if (hjpegdata)
+    {
+        jpegdata = GlobalLock(hjpegdata);
+        memcpy(jpegdata, jpeg_adobe_cmyk_1x5, sizeof(jpeg_adobe_cmyk_1x5));
+        GlobalUnlock(hjpegdata);
+
+        hr = CreateStreamOnHGlobal(hjpegdata, FALSE, &jpegstream);
+        ok(SUCCEEDED(hr), "CreateStreamOnHGlobal failed, hr=%x\n", hr);
+        if (SUCCEEDED(hr))
+        {
+            hr = IWICBitmapDecoder_Initialize(decoder, jpegstream, WICDecodeMetadataCacheOnLoad);
+            ok(hr == S_OK, "Initialize failed, hr=%x\n", hr);
+
+            hr = IWICBitmapDecoder_GetContainerFormat(decoder, &guidresult);
+            ok(SUCCEEDED(hr), "GetContainerFormat failed, hr=%x\n", hr);
+            ok(IsEqualGUID(&guidresult, &GUID_ContainerFormatJpeg), "unexpected container format\n");
+
+            hr = IWICBitmapDecoder_GetFrameCount(decoder, &count);
+            ok(SUCCEEDED(hr), "GetFrameCount failed, hr=%x\n", hr);
+            ok(count == 1, "unexpected count %u\n", count);
+
+            hr = IWICBitmapDecoder_GetFrame(decoder, 0, &framedecode);
+            ok(SUCCEEDED(hr), "GetFrame failed, hr=%x\n", hr);
+            if (SUCCEEDED(hr))
+            {
+                hr = IWICBitmapFrameDecode_GetSize(framedecode, &width, &height);
+                ok(SUCCEEDED(hr), "GetSize failed, hr=%x\n", hr);
+                ok(width == 1, "expected width=1, got %u\n", width);
+                ok(height == 5, "expected height=5, got %u\n", height);
+
+                hr = IWICBitmapFrameDecode_GetPixelFormat(framedecode, &guidresult);
+                ok(SUCCEEDED(hr), "GetPixelFormat failed, hr=%x\n", hr);
+                ok(IsEqualGUID(&guidresult, &GUID_WICPixelFormat32bppCMYK) ||
+                    broken(IsEqualGUID(&guidresult, &GUID_WICPixelFormat24bppBGR)), /* xp/2003 */
+                    "unexpected pixel format: %s\n", wine_dbgstr_guid(&guidresult));
+
+                hr = IWICBitmapFrameDecode_CopyPixels(framedecode, NULL, 4, sizeof(imagedata), imagedata);
+                ok(SUCCEEDED(hr), "CopyPixels failed, hr=%x\n", hr);
+                ok(!memcmp(imagedata, expected_imagedata, sizeof(imagedata)) ||
+                    broken(!memcmp(imagedata, expected_imagedata_24bpp, sizeof(expected_imagedata))), /* xp/2003 */
+                    "unexpected image data\n");
+
+                IWICBitmapFrameDecode_Release(framedecode);
+            }
+            IStream_Release(jpegstream);
+        }
+        GlobalFree(hjpegdata);
+    }
+    IWICBitmapDecoder_Release(decoder);
+}
+
+
+START_TEST(jpegformat)
+{
+    CoInitializeEx(NULL, COINIT_APARTMENTTHREADED);
+
+    test_decode_adobe_cmyk();
+
+    CoUninitialize();
+}

[Vincent Povirk]

I don't think this will work correctly if CopyPixels is called multiple times.

[Anton Romanov]

On Mon, Feb 5, 2018 at 9:27 AM, Vincent Povirk  wrote:
> I don't think this will work correctly if CopyPixels is called multiple times.

I think you are right, output scanline is being tracked. I'll move
inversion back inside the loop but make it not go all over the image
each time but only just decoded lines (and duplicate copypixels in
test to double check that failure case). Thanks!