[PATCH 0/1] MR2321: windowscodecs: Don't reorder gif extension blocks in GetReaderByIndex.

[Piotr Caban (＠piotr)]

The code that reorders extension blocks is broken and causes some extensions to be not reported (when GCE is not the last extension block).

This fixes GCE blocks reading from gifs attached in [bug 54563](https://bugs.winehq.org/show_bug.cgi?id=54563).

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2321

[PATCH 1/1] windowscodecs: Don't reorder gif extension blocks in GetReaderByIndex.

[Piotr Caban]

From: Piotr Caban <piotr(a)codeweavers.com>

---
 dlls/windowscodecs/gifformat.c      | 54 ++++++++++++-----------------
 dlls/windowscodecs/tests/metadata.c | 28 +++++++--------
 2 files changed, 36 insertions(+), 46 deletions(-)

diff --git a/dlls/windowscodecs/gifformat.c b/dlls/windowscodecs/gifformat.c
index 4bd0772810d..f0048d12da9 100644
--- a/dlls/windowscodecs/gifformat.c
+++ b/dlls/windowscodecs/gifformat.c
@@ -947,7 +947,10 @@ static HRESULT WINAPI GifFrameDecode_Block_GetReaderByIndex(IWICMetadataBlockRea
     UINT index, IWICMetadataReader **reader)
 {
     GifFrameDecode *This = frame_from_IWICMetadataBlockReader(iface);
-    int i, gce_index = -1, gce_skipped = 0;
+    class_constructor constructor;
+    ExtensionBlock *ext;
+    const void *data;
+    int data_size;
 
     TRACE("(%p,%u,%p)\n", iface, index, reader);
 
@@ -959,40 +962,27 @@ static HRESULT WINAPI GifFrameDecode_Block_GetReaderByIndex(IWICMetadataBlockRea
     if (index >= This->frame->Extensions.ExtensionBlockCount + 1)
         return E_INVALIDARG;
 
-    for (i = 0; i < This->frame->Extensions.ExtensionBlockCount; i++)
+    ext = This->frame->Extensions.ExtensionBlocks + index - 1;
+    if (ext->Function == GRAPHICS_EXT_FUNC_CODE)
     {
-        class_constructor constructor;
-        const void *data;
-        int data_size;
-
-        if (index != i + 1 - gce_skipped) continue;
-
-        if (This->frame->Extensions.ExtensionBlocks[i].Function == GRAPHICS_EXT_FUNC_CODE)
-        {
-            gce_index = i;
-            gce_skipped = 1;
-            continue;
-        }
-        else if (This->frame->Extensions.ExtensionBlocks[i].Function == COMMENT_EXT_FUNC_CODE)
-        {
-            constructor = GifCommentReader_CreateInstance;
-            data = This->frame->Extensions.ExtensionBlocks[i].Bytes;
-            data_size = This->frame->Extensions.ExtensionBlocks[i].ByteCount;
-        }
-        else
-        {
-            constructor = UnknownMetadataReader_CreateInstance;
-            data = This->frame->Extensions.ExtensionBlocks[i].Bytes;
-            data_size = This->frame->Extensions.ExtensionBlocks[i].ByteCount;
-        }
-        return create_metadata_reader(data, data_size, constructor, reader);
+        constructor = GCEReader_CreateInstance;
+        data = ext->Bytes + 3;
+        data_size = ext->ByteCount - 4;
+    }
+    else if (ext->Function == COMMENT_EXT_FUNC_CODE)
+    {
+        constructor = GifCommentReader_CreateInstance;
+        data = ext->Bytes;
+        data_size = ext->ByteCount;
+    }
+    else
+    {
+        constructor = UnknownMetadataReader_CreateInstance;
+        data = ext->Bytes;
+        data_size = ext->ByteCount;
     }
 
-    if (gce_index == -1) return E_INVALIDARG;
-
-    return create_metadata_reader(This->frame->Extensions.ExtensionBlocks[gce_index].Bytes + 3,
-                                  This->frame->Extensions.ExtensionBlocks[gce_index].ByteCount - 4,
-                                  GCEReader_CreateInstance, reader);
+    return create_metadata_reader(data, data_size, constructor, reader);
 }
 
 static HRESULT WINAPI GifFrameDecode_Block_GetEnumerator(IWICMetadataBlockReader *iface,
diff --git a/dlls/windowscodecs/tests/metadata.c b/dlls/windowscodecs/tests/metadata.c
index 7902a5fcb8d..1deb09cfd19 100644
--- a/dlls/windowscodecs/tests/metadata.c
+++ b/dlls/windowscodecs/tests/metadata.c
@@ -188,10 +188,10 @@ static const char animatedgif[] = {
 0x00,0x00,0x00,0x00,0x01,0x00,0x01,0x00,0x81,
 0xDE,0xDE,0xDE,0x00,0x00,0x00,
 0x00,0x00,0x00,0x00,0x00,0x00,0x02,0x02,0x4C,0x01,0x00,
+0x21,0xF9,0x04,0x01,0x0A,0x00,0x01,0x00,
 0x21,0xFE,0x08,'i','m','a','g','e',' ','#','1',0x00,
 0x21,0x01,0x0C,'p','l','a','i','n','t','e','x','t',' ','#','1',0x00,
-0x21,0xF9,0x04,0x01,0x0A,0x00,0x01,0x00,0x2C,
-0x00,0x00,0x00,0x00,0x01,0x00,0x01,0x00,0x81,
+0x2C,0x00,0x00,0x00,0x00,0x01,0x00,0x01,0x00,0x81,
 0x4D,0x4D,0x4D,0x00,0x00,0x00,
 0x00,0x00,0x00,0x00,0x00,0x00,0x02,0x02,0x44,0x01,0x00,
 0x21,0xFE,0x08,'i','m','a','g','e',' ','#','2',0x00,
@@ -1517,15 +1517,14 @@ static void test_metadata_gif(void)
         {
             hr = IWICMetadataReader_GetMetadataFormat(reader, &format);
             ok(hr == S_OK, "GetMetadataFormat failed, hr=%#lx\n", hr);
-            ok(IsEqualGUID(&format, &GUID_MetadataFormatGifComment), /* Comment Extension */
-                "wrong metadata format %s\n", wine_dbgstr_guid(&format));
+            ok(IsEqualGUID(&format, &GUID_MetadataFormatGCE), /* Graphic Control Extension */
+               "wrong metadata format %s\n", wine_dbgstr_guid(&format));
 
             hr = IWICMetadataReader_GetCount(reader, &count);
             ok(hr == S_OK, "GetCount error %#lx\n", hr);
-            ok(count == ARRAY_SIZE(animated_gif_comment_2), "unexpected count %u\n", count);
+            ok(count == ARRAY_SIZE(animated_gif_GCE), "unexpected count %u\n", count);
 
-            if (count == 1)
-            compare_metadata(reader, animated_gif_comment_2, count);
+            compare_metadata(reader, animated_gif_GCE, count);
 
             IWICMetadataReader_Release(reader);
         }
@@ -1537,14 +1536,15 @@ static void test_metadata_gif(void)
         {
             hr = IWICMetadataReader_GetMetadataFormat(reader, &format);
             ok(hr == S_OK, "GetMetadataFormat failed, hr=%#lx\n", hr);
-            ok(IsEqualGUID(&format, &GUID_MetadataFormatUnknown),
-               "wrong metadata format %s\n", wine_dbgstr_guid(&format));
+            ok(IsEqualGUID(&format, &GUID_MetadataFormatGifComment), /* Comment Extension */
+                "wrong metadata format %s\n", wine_dbgstr_guid(&format));
 
             hr = IWICMetadataReader_GetCount(reader, &count);
             ok(hr == S_OK, "GetCount error %#lx\n", hr);
-            ok(count == ARRAY_SIZE(animated_gif_plain_2), "unexpected count %u\n", count);
+            ok(count == ARRAY_SIZE(animated_gif_comment_2), "unexpected count %u\n", count);
 
-            compare_metadata(reader, animated_gif_plain_2, count);
+            if (count == 1)
+            compare_metadata(reader, animated_gif_comment_2, count);
 
             IWICMetadataReader_Release(reader);
         }
@@ -1556,14 +1556,14 @@ static void test_metadata_gif(void)
         {
             hr = IWICMetadataReader_GetMetadataFormat(reader, &format);
             ok(hr == S_OK, "GetMetadataFormat failed, hr=%#lx\n", hr);
-            ok(IsEqualGUID(&format, &GUID_MetadataFormatGCE), /* Graphic Control Extension */
+            ok(IsEqualGUID(&format, &GUID_MetadataFormatUnknown),
                "wrong metadata format %s\n", wine_dbgstr_guid(&format));
 
             hr = IWICMetadataReader_GetCount(reader, &count);
             ok(hr == S_OK, "GetCount error %#lx\n", hr);
-            ok(count == ARRAY_SIZE(animated_gif_GCE), "unexpected count %u\n", count);
+            ok(count == ARRAY_SIZE(animated_gif_plain_2), "unexpected count %u\n", count);
 
-            compare_metadata(reader, animated_gif_GCE, count);
+            compare_metadata(reader, animated_gif_plain_2, count);
 
             IWICMetadataReader_Release(reader);
         }
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/2321

[PATCH 0/1] MR2321: windowscodecs: Don't reorder gif extension blocks in GetReaderByIndex. - approved

[Esme Povirk (＠madewokherd)]

This merge request was approved by Esme Povirk.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2321