[PATCH 0/8] MR7350: windowscodecs/tests: Add some tests for default metadata item set.

[Nikolay Sivov (＠nsivov)]

Signed-off-by: Nikolay Sivov

[PATCH 1/8] windowscodecs/tests: Add some tests for default metadata item set.

[Nikolay Sivov]

From: Nikolay Sivov

Signed-off-by: Nikolay Sivov 
---
 dlls/windowscodecs/tests/metadata.c | 166 ++++++++++++++++++++++++++--
 1 file changed, 158 insertions(+), 8 deletions(-)

diff --git a/dlls/windowscodecs/tests/metadata.c b/dlls/windowscodecs/tests/metadata.c
index 42d14f28ad5..97fa186cf00 100644
--- a/dlls/windowscodecs/tests/metadata.c
+++ b/dlls/windowscodecs/tests/metadata.c
@@ -316,10 +316,10 @@ static const char metadata_gAMA[] = {
 static const char metadata_cHRM[] = {
     0,0,0,32, /* chunk length */
     'c','H','R','M', /* chunk type */
-    0,0,122,38, 0,0,128,132, /* white point */
-    0,0,250,0, 0,0,128,232, /* red */
-    0,0,117,48, 0,0,234,96, /* green */
-    0,0,58,152, 0,0,23,112, /* blue */
+    0,0,122,40, 0,0,120,132, /* white point */
+    0,0,240,0, 0,0,128,220, /* red */
+    0,0,100,48, 0,0,234,100, /* green */
+    0,0,58,140, 0,0,23,100, /* blue */
     0xff,0xff,0xff,0xff /* chunk CRC */
 };

@@ -813,6 +813,21 @@ static void test_metadata_unknown(void)
     todo_wine
     ok(count == 1, "Unexpected count %u.\n", count);

+    PropVariantInit(&schema);
+    PropVariantInit(&id);
+    PropVariantInit(&value);
+    hr = IWICMetadataWriter_GetValueByIndex(writer, 0, &schema, &id, &value);
+    todo_wine
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    if (hr == S_OK)
+    {
+        ok(schema.vt == VT_EMPTY, "Unexpected type %d.\n", schema.vt);
+        ok(id.vt == VT_EMPTY, "Unexpected type %d.\n", id.vt);
+        ok(value.vt == VT_BLOB, "Unexpected type %d.\n", id.vt);
+        ok(!value.blob.cbSize, "Unexpected size %lu.\n", value.blob.cbSize);
+        ok(!value.blob.pBlobData, "Unexpected data pointer %p.\n", value.blob.pBlobData);
+    }
+
     PropVariantInit(&schema);
     PropVariantInit(&value);

@@ -1085,6 +1100,28 @@ static void test_metadata_gAMA(void)
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ok(count == 1, "Unexpected count %u.\n", count);

+    hr = IWICMetadataWriter_GetValueByIndex(writer, 0, &schema, &id, &value);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    ok(schema.vt == VT_EMPTY, "Unexpected type %d.\n", schema.vt);
+    ok(id.vt == VT_LPWSTR, "Unexpected type %d.\n", id.vt);
+    ok(!lstrcmpW(id.pwszVal, L"ImageGamma"), "Unexpected id %s.\n", wine_dbgstr_w(id.pwszVal));
+    PropVariantClear(&id);
+
+    ok(value.vt == VT_UI4, "Unexpected type %d.\n", value.vt);
+    ok(value.ulVal == 45455, "unexpected value: %lu\n", value.ulVal);
+
+    hr = IWICMetadataWriter_RemoveValueByIndex(writer, 0);
+    ok(hr == WINCODEC_ERR_UNSUPPORTEDOPERATION, "Unexpected hr %#lx.\n", hr);
+
+    hr = IWICMetadataWriter_RemoveValueByIndex(writer, count);
+    ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr);
+
+    id.vt = VT_EMPTY;
+    V_UI4(&id) = 123;
+    hr = IWICMetadataWriter_SetValue(writer, &schema, &id, &value);
+    ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr);
+
     load_stream(writer, metadata_gAMA, sizeof(metadata_gAMA), 0);
     load_stream(writer, metadata_gAMA, sizeof(metadata_gAMA), WICPersistOptionNoCacheStream);

@@ -1112,7 +1149,10 @@ static void test_metadata_cHRM(void)
         L"BlueY",
     };
     static const ULONG expected_vals[8] = {
-        31270,32900, 64000,33000, 30000,60000, 15000,6000
+        31272, 30852, 61440, 32988, 25648, 60004, 14988, 5988
+    };
+    static const ULONG default_values[8] = {
+        31270,32900,64000, 33000,30000, 60000,15000, 6000
     };

PropVariantInit(&schema);
@@ -1176,6 +1216,32 @@ static void test_metadata_cHRM(void)
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ok(count == 8, "Unexpected count %u.\n", count);

+    for (i = 0; i < count && count == ARRAY_SIZE(expected_names); ++i)
+    {
+        hr = IWICMetadataWriter_GetValueByIndex(writer, i, &schema, &id, &value);
+        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+        ok(schema.vt == VT_EMPTY, "Unexpected type %d.\n", schema.vt);
+        PropVariantClear(&schema);
+
+        ok(id.vt == VT_LPWSTR, "Unexpected type %d.\n", id.vt);
+        ok(!lstrcmpW(id.pwszVal, expected_names[i]), "Unexpected id %s, expected %s.\n",
+                wine_dbgstr_w(id.pwszVal), wine_dbgstr_w(expected_names[i]));
+        PropVariantClear(&id);
+
+        ok(value.vt == VT_UI4, "Unexpected type %d.\n", value.vt);
+        ok(value.ulVal == default_values[i], "Got %lu, expected %lu.\n", value.ulVal, default_values[i]);
+        PropVariantClear(&value);
+    }
+
+    for (i = 0; i < count; ++i)
+    {
+        hr = IWICMetadataWriter_RemoveValueByIndex(writer, i);
+        ok(hr == WINCODEC_ERR_UNSUPPORTEDOPERATION, "Unexpected hr %#lx.\n", hr);
+    }
+    hr = IWICMetadataWriter_RemoveValueByIndex(writer, count);
+    ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr);
+
     load_stream(writer, metadata_cHRM, sizeof(metadata_cHRM), 0);
     load_stream(writer, metadata_cHRM, sizeof(metadata_cHRM), WICPersistOptionNoCacheStream);

@@ -1251,6 +1317,24 @@ static void test_metadata_hIST(void)
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ok(count == 1, "Unexpected count %u.\n", count);

+    hr = IWICMetadataWriter_GetValueByIndex(writer, 0, &schema, &id, &value);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    ok(schema.vt == VT_EMPTY, "Unexpected type %d.\n", schema.vt);
+
+    ok(id.vt == VT_LPWSTR, "Unexpected type %d.\n", id.vt);
+    ok(!lstrcmpW(id.pwszVal, L"Frequencies"), "Unexpected id %s.\n", wine_dbgstr_w(id.pwszVal));
+    PropVariantClear(&id);
+
+    ok(value.vt == (VT_UI2|VT_VECTOR), "Unexpected type %#x.\n", value.vt);
+    ok(!value.caui.cElems, "Unexpected cElems %ld.\n", value.caub.cElems);
+    ok(!value.caui.pElems, "Unexpected value %p.\n", value.caui.pElems);
+
+    hr = IWICMetadataWriter_RemoveValueByIndex(writer, 0);
+    ok(hr == WINCODEC_ERR_UNSUPPORTEDOPERATION, "Unexpected hr %#lx.\n", hr);
+    hr = IWICMetadataWriter_RemoveValueByIndex(writer, 1);
+    ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr);
+
     load_stream(writer, metadata_hIST, sizeof(metadata_hIST), 0);
     load_stream(writer, metadata_hIST, sizeof(metadata_hIST), WICPersistOptionNoCacheStream);

@@ -1262,7 +1346,7 @@ static void test_metadata_tIME(void)
     HRESULT hr;
     IWICMetadataReader *reader;
     IWICMetadataWriter *writer;
-    UINT count;
+    UINT i, count;
     GUID format;
     static const struct test_data td[] =
     {
@@ -1273,6 +1357,15 @@ static void test_metadata_tIME(void)
         { VT_UI1, 0, 0, { 34 }, NULL, L"Minute" },
         { VT_UI1, 0, 0, { 56 }, NULL, L"Second" },
     };
+    static const struct test_data default_data[] =
+    {
+        { VT_UI2, 0, 0, { 0 }, NULL, L"Year" },
+        { VT_UI1, 0, 0, { 1 }, NULL, L"Month" },
+        { VT_UI1, 0, 0, { 1 }, NULL, L"Day" },
+        { VT_UI1, 0, 0, { 0 }, NULL, L"Hour" },
+        { VT_UI1, 0, 0, { 0 }, NULL, L"Minute" },
+        { VT_UI1, 0, 0, { 0 }, NULL, L"Second" },
+    };

hr = CoCreateInstance(&CLSID_WICPngTimeMetadataReader, NULL, CLSCTX_INPROC_SERVER,
         &IID_IWICMetadataReader, (void**)&reader);
@@ -1316,6 +1409,16 @@ static void test_metadata_tIME(void)
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ok(count == 6, "Unexpected count %u.\n", count);

+    compare_metadata((IWICMetadataReader *)writer, default_data, count);
+
+    for (i = 0; i < count; ++i)
+    {
+        hr = IWICMetadataWriter_RemoveValueByIndex(writer, i);
+        ok(hr == WINCODEC_ERR_UNSUPPORTEDOPERATION, "Unexpected hr %#lx.\n", hr);
+    }
+    hr = IWICMetadataWriter_RemoveValueByIndex(writer, count);
+    ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr);
+
     load_stream(writer, metadata_tIME, sizeof(metadata_tIME), 0);
     load_stream(writer, metadata_tIME, sizeof(metadata_tIME), WICPersistOptionNoCacheStream);

@@ -2977,6 +3080,14 @@ static void test_metadata_GCE(void)
         { VT_UI2, 0, 0, { 0x302 }, NULL, L"Delay" },
         { VT_UI1, 0, 0, { 4 }, NULL, L"TransparentColorIndex" }
     };
+    static const struct test_data default_data[5] =
+    {
+        { VT_UI1, 0, 0, { 0 }, NULL, L"Disposal" },
+        { VT_BOOL, 0, 0, { 0 }, NULL, L"UserInputFlag" },
+        { VT_BOOL, 0, 0, { 0 }, NULL, L"TransparencyFlag" },
+        { VT_UI2, 0, 0, { 0 }, NULL, L"Delay" },
+        { VT_UI1, 0, 0, { 0 }, NULL, L"TransparentColorIndex" }
+    };
     LARGE_INTEGER pos;
     HRESULT hr;
     IStream *stream;
@@ -2985,7 +3096,7 @@ static void test_metadata_GCE(void)
     IWICMetadataWriter *writer;
     IWICMetadataHandlerInfo *info;
     WCHAR name[64];
-    UINT count, dummy;
+    UINT i, count, dummy;
     GUID format;
     CLSID id;

@@ -3061,6 +3172,16 @@ static void test_metadata_GCE(void)
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ok(count == 5, "Unexpected count %u.\n", count);

+    compare_metadata((IWICMetadataReader *)writer, default_data, count);
+
+    for (i = 0; i < count; ++i)
+    {
+        hr = IWICMetadataWriter_RemoveValueByIndex(writer, i);
+        ok(hr == WINCODEC_ERR_UNSUPPORTEDOPERATION, "Unexpected hr %#lx.\n", hr);
+    }
+    hr = IWICMetadataWriter_RemoveValueByIndex(writer, count);
+    ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr);
+
     load_stream(writer, GCE_data, sizeof(GCE_data), 0);
     load_stream(writer, GCE_data, sizeof(GCE_data), WICPersistOptionNoCacheStream);

@@ -3079,6 +3200,11 @@ static void test_metadata_APE(void)
         { VT_UI1|VT_VECTOR, 0, 11, { 'H','e','l','l','o',' ','W','o','r','l','d' }, NULL, L"Application" },
         { VT_UI1|VT_VECTOR, 0, 10, { 1,0x11,2,0x22,0x33,4,0x44,0x55,0x66,0x77 }, NULL, L"Data" }
     };
+    static const struct test_data default_data[2] =
+    {
+        { VT_EMPTY, 0, 0, { 0 }, NULL, L"Application" },
+        { VT_UI1|VT_VECTOR, 0, 0, { 0 }, NULL, L"Data" }
+    };
     WCHAR dataW[] = { 'd','a','t','a',0 };
     HRESULT hr;
     IStream *stream;
@@ -3172,6 +3298,16 @@ static void test_metadata_APE(void)
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ok(count == 2, "Unexpected count %u.\n", count);

+    compare_metadata((IWICMetadataReader *)writer, default_data, count);
+
+    for (i = 0; i < count; ++i)
+    {
+        hr = IWICMetadataWriter_RemoveValueByIndex(writer, i);
+        ok(hr == WINCODEC_ERR_UNSUPPORTEDOPERATION, "Unexpected hr %#lx.\n", hr);
+    }
+    hr = IWICMetadataWriter_RemoveValueByIndex(writer, count);
+    ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr);
+
     load_stream(writer, APE_data, sizeof(APE_data), 0);
     load_stream(writer, APE_data, sizeof(APE_data), WICPersistOptionNoCacheStream);

@@ -3189,6 +3325,10 @@ static void test_metadata_GIF_comment(void)
     {
         { VT_LPSTR, 0, 12, { 0 }, "Hello World!", L"TextEntry" }
     };
+    static const struct test_data default_data[1] =
+    {
+        { VT_LPSTR, 0, 0, { 0 }, "", L"TextEntry" }
+    };
     WCHAR text_entryW[] = L"TEXTENTRY";
     HRESULT hr;
     IStream *stream;
@@ -3197,7 +3337,7 @@ static void test_metadata_GIF_comment(void)
     IWICMetadataWriter *writer;
     IWICMetadataHandlerInfo *info;
     WCHAR name[64];
-    UINT count, dummy;
+    UINT i, count, dummy;
     GUID format;
     CLSID clsid;
     PROPVARIANT id, value;
@@ -3280,6 +3420,16 @@ static void test_metadata_GIF_comment(void)
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ok(count == 1, "Unexpected count %u.\n", count);

+    compare_metadata((IWICMetadataReader *)writer, default_data, count);
+
+    for (i = 0; i < count; ++i)
+    {
+        hr = IWICMetadataWriter_RemoveValueByIndex(writer, i);
+        ok(hr == WINCODEC_ERR_UNSUPPORTEDOPERATION, "Unexpected hr %#lx.\n", hr);
+    }
+    hr = IWICMetadataWriter_RemoveValueByIndex(writer, count);
+    ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr);
+
     load_stream(writer, GIF_comment_data, sizeof(GIF_comment_data), 0);
     load_stream(writer, GIF_comment_data, sizeof(GIF_comment_data), WICPersistOptionNoCacheStream);

[PATCH 2/8] windowscodecs/metadata: Add flags mask to configure builtin handlers.

[Nikolay Sivov]

From: Nikolay Sivov

Signed-off-by: Nikolay Sivov 
---
 dlls/windowscodecs/metadatahandler.c   | 12 ++++++------
 dlls/windowscodecs/wincodecs_private.h |  7 ++++++-
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/dlls/windowscodecs/metadatahandler.c b/dlls/windowscodecs/metadatahandler.c
index 1fd2730390f..b68b6656980 100644
--- a/dlls/windowscodecs/metadatahandler.c
+++ b/dlls/windowscodecs/metadatahandler.c
@@ -93,7 +93,7 @@ static HRESULT WINAPI MetadataHandler_QueryInterface(IWICMetadataWriter *iface,

if (IsEqualIID(&IID_IUnknown, iid) ||
         IsEqualIID(&IID_IWICMetadataReader, iid) ||
-        (IsEqualIID(&IID_IWICMetadataWriter, iid) && This->vtable->is_writer))
+        (IsEqualIID(&IID_IWICMetadataWriter, iid) && This->vtable->flags & METADATAHANDLER_IS_WRITER))
     {
         *ppv = &This->IWICMetadataWriter_iface;
     }
@@ -882,7 +882,7 @@ HRESULT UnknownMetadataReader_CreateInstance(REFIID iid, void** ppv)

static const MetadataHandlerVtbl UnknownMetadataWriter_Vtbl =
 {
-    .is_writer = true,
+    .flags = METADATAHANDLER_IS_WRITER,
     .clsid = &CLSID_WICUnknownMetadataWriter,
     .fnLoad = LoadUnknownMetadata
 };
@@ -1583,7 +1583,7 @@ HRESULT IfdMetadataReader_CreateInstance(REFIID iid, void **ppv)

static const MetadataHandlerVtbl IfdMetadataWriter_Vtbl =
 {
-    .is_writer = true,
+    .flags = METADATAHANDLER_IS_WRITER,
     &CLSID_WICIfdMetadataWriter,
     LoadIfdMetadataWriter
 };
@@ -1607,7 +1607,7 @@ HRESULT GpsMetadataReader_CreateInstance(REFIID iid, void **ppv)

static const MetadataHandlerVtbl GpsMetadataWriter_Vtbl =
 {
-    .is_writer = true,
+    .flags = METADATAHANDLER_IS_WRITER,
     &CLSID_WICGpsMetadataWriter,
     LoadGpsMetadataWriter
 };
@@ -1631,7 +1631,7 @@ HRESULT ExifMetadataReader_CreateInstance(REFIID iid, void **ppv)

static const MetadataHandlerVtbl ExifMetadataWriter_Vtbl =
 {
-    .is_writer = true,
+    .flags = METADATAHANDLER_IS_WRITER,
     &CLSID_WICExifMetadataWriter,
     LoadExifMetadataWriter
 };
@@ -1655,7 +1655,7 @@ HRESULT App1MetadataReader_CreateInstance(REFIID iid, void **ppv)

static const MetadataHandlerVtbl App1MetadataWriter_Vtbl =
 {
-    .is_writer = true,
+    .flags = METADATAHANDLER_IS_WRITER,
     &CLSID_WICApp1MetadataWriter,
     LoadApp1MetadataWriter
 };
diff --git a/dlls/windowscodecs/wincodecs_private.h b/dlls/windowscodecs/wincodecs_private.h
index 5b193bb1b26..fb3ef312851 100644
--- a/dlls/windowscodecs/wincodecs_private.h
+++ b/dlls/windowscodecs/wincodecs_private.h
@@ -205,9 +205,14 @@ typedef struct _MetadataItem
     PROPVARIANT value;
 } MetadataItem;

+enum metadatahandler_flags
+{
+    METADATAHANDLER_IS_WRITER = 0x1,
+};
+
 typedef struct _MetadataHandlerVtbl
 {
-    bool is_writer;
+    DWORD flags;
     const CLSID *clsid;
     HRESULT (*fnLoad)(IStream *stream, const GUID *preferred_vendor,
         DWORD persist_options, MetadataItem **items, DWORD *item_count);

[PATCH 3/8] windowscodecs/metadata: Implement removing items with IWICMetadataWriter.

[Nikolay Sivov]

From: Nikolay Sivov

Signed-off-by: Nikolay Sivov 
---
 dlls/windowscodecs/metadatahandler.c   | 74 +++++++++++++++++++++-----
 dlls/windowscodecs/tests/metadata.c    | 45 +++++++++++++++-
 dlls/windowscodecs/wincodecs_private.h |  1 +
 3 files changed, 107 insertions(+), 13 deletions(-)

diff --git a/dlls/windowscodecs/metadatahandler.c b/dlls/windowscodecs/metadatahandler.c
index b68b6656980..eb69903d589 100644
--- a/dlls/windowscodecs/metadatahandler.c
+++ b/dlls/windowscodecs/metadatahandler.c
@@ -228,13 +228,14 @@ static HRESULT WINAPI MetadataHandler_GetValueByIndex(IWICMetadataWriter *iface,
 }

static MetadataItem *metadatahandler_get_item(MetadataHandler *handler, const PROPVARIANT *schema,
-        const PROPVARIANT *id)
+        const PROPVARIANT *id, unsigned int *item_index)
 {
     PROPVARIANT index;
     GUID format;
     HRESULT hr;
     UINT i;

+    if (item_index) *item_index = 0;
     PropVariantInit(&index);
     if (id->vt == VT_CLSID && SUCCEEDED(PropVariantChangeType(&index, schema, 0, VT_UI4)))
     {
@@ -254,7 +255,11 @@ static MetadataItem *metadatahandler_get_item(MetadataHandler *handler, const PR
                 {
                     if (IsEqualGUID(&format, id->puuid))
                     {
-                        if (!index.ulVal) return &handler->items[i];
+                        if (!index.ulVal)
+                        {
+                            if (item_index) *item_index = i;
+                            return &handler->items[i];
+                        }
                         --index.ulVal;
                     }
                 }
@@ -271,12 +276,22 @@ static MetadataItem *metadatahandler_get_item(MetadataHandler *handler, const PR

if (PropVariantCompareEx(id, &handler->items[i].id, 0, PVCF_USESTRCMPI) != 0) continue;

+        if (item_index) *item_index = i;
         return &handler->items[i];
     }

return NULL;
 }

+static void metadata_handler_remove_item(MetadataHandler *handler, unsigned int index)
+{
+    clear_metadata_item(&handler->items[index]);
+    handler->item_count--;
+    if (index != handler->item_count)
+        memmove(&handler->items[index], &handler->items[index + 1],
+                (handler->item_count - index) * sizeof(*handler->items));
+}
+
 static HRESULT WINAPI MetadataHandler_GetValue(IWICMetadataWriter *iface,
     const PROPVARIANT *schema, const PROPVARIANT *id, PROPVARIANT *value)
 {
@@ -290,7 +305,7 @@ static HRESULT WINAPI MetadataHandler_GetValue(IWICMetadataWriter *iface,

EnterCriticalSection(&This->lock);

-    if ((item = metadatahandler_get_item(This, schema, id)))
+    if ((item = metadatahandler_get_item(This, schema, id, NULL)))
     {
         hr = value ? PropVariantCopy(value, &item->value) : S_OK;
     }
@@ -323,7 +338,7 @@ static HRESULT WINAPI MetadataHandler_SetValue(IWICMetadataWriter *iface,

EnterCriticalSection(&This->lock);

-    if ((item = metadatahandler_get_item(This, schema, id)))
+    if ((item = metadatahandler_get_item(This, schema, id, NULL)))
     {
         PropVariantClear(&item->value);
         hr = PropVariantCopy(&item->value, value);
@@ -370,17 +385,52 @@ static HRESULT WINAPI MetadataHandler_SetValueByIndex(IWICMetadataWriter *iface,
 }

static HRESULT WINAPI MetadataHandler_RemoveValue(IWICMetadataWriter *iface,
-    const PROPVARIANT *pvarSchema, const PROPVARIANT *pvarId)
+        const PROPVARIANT *schema, const PROPVARIANT *id)
 {
-    FIXME("(%p,%p,%p): stub\n", iface, pvarSchema, pvarId);
-    return E_NOTIMPL;
+    MetadataHandler *handler = impl_from_IWICMetadataWriter(iface);
+    unsigned int index;
+    HRESULT hr = S_OK;
+
+    TRACE("(%p,%p,%p)\n", iface, schema, id);
+
+    if (handler->vtable->flags & METADATAHANDLER_FIXED_ITEMS)
+        return WINCODEC_ERR_UNSUPPORTEDOPERATION;
+
+    EnterCriticalSection(&handler->lock);
+
+    if (metadatahandler_get_item(handler, schema, id, &index))
+    {
+        metadata_handler_remove_item(handler, index);
+    }
+    else
+    {
+        hr = WINCODEC_ERR_PROPERTYNOTFOUND;
+    }
+
+    LeaveCriticalSection(&handler->lock);
+
+    return hr;
 }

-static HRESULT WINAPI MetadataHandler_RemoveValueByIndex(IWICMetadataWriter *iface,
-    UINT nIndex)
+static HRESULT WINAPI MetadataHandler_RemoveValueByIndex(IWICMetadataWriter *iface, UINT index)
 {
-    FIXME("(%p,%u): stub\n", iface, nIndex);
-    return E_NOTIMPL;
+    MetadataHandler *handler = impl_from_IWICMetadataWriter(iface);
+    HRESULT hr = S_OK;
+
+    TRACE("(%p,%u)\n", iface, index);
+
+    EnterCriticalSection(&handler->lock);
+
+    if (index >= handler->item_count)
+        hr = E_INVALIDARG;
+    else if (handler->vtable->flags & METADATAHANDLER_FIXED_ITEMS)
+        hr = WINCODEC_ERR_UNSUPPORTEDOPERATION;
+    else
+        metadata_handler_remove_item(handler, index);
+
+    LeaveCriticalSection(&handler->lock);
+
+    return hr;
 }

static const IWICMetadataWriterVtbl MetadataHandler_Vtbl = {
@@ -882,7 +932,7 @@ HRESULT UnknownMetadataReader_CreateInstance(REFIID iid, void** ppv)

static const MetadataHandlerVtbl UnknownMetadataWriter_Vtbl =
 {
-    .flags = METADATAHANDLER_IS_WRITER,
+    .flags = METADATAHANDLER_IS_WRITER | METADATAHANDLER_FIXED_ITEMS,
     .clsid = &CLSID_WICUnknownMetadataWriter,
     .fnLoad = LoadUnknownMetadata
 };
diff --git a/dlls/windowscodecs/tests/metadata.c b/dlls/windowscodecs/tests/metadata.c
index 97fa186cf00..fb73451524e 100644
--- a/dlls/windowscodecs/tests/metadata.c
+++ b/dlls/windowscodecs/tests/metadata.c
@@ -837,8 +837,16 @@ static void test_metadata_unknown(void)
     todo_wine
     ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr);

+    PropVariantInit(&id);
+    PropVariantInit(&schema);
+    hr = IWICMetadataWriter_RemoveValue(writer, &schema, &id);
+    ok(hr == WINCODEC_ERR_UNSUPPORTEDOPERATION, "Unexpected hr %#lx.\n", hr);
+
+    id.vt = VT_UI4;
+    hr = IWICMetadataWriter_RemoveValue(writer, &schema, &id);
+    ok(hr == WINCODEC_ERR_UNSUPPORTEDOPERATION, "Unexpected hr %#lx.\n", hr);
+
     hr = IWICMetadataWriter_RemoveValueByIndex(writer, 0);
-    todo_wine
     ok(hr == WINCODEC_ERR_UNSUPPORTEDOPERATION, "Unexpected hr %#lx.\n", hr);

hr = IWICMetadataWriter_GetCount(writer, &count);
@@ -1829,6 +1837,7 @@ static void test_metadata_Exif(void)

static void test_metadata_Gps(void)
 {
+    PROPVARIANT schema, id, value;
     IWICMetadataReader *reader;
     IWICMetadataWriter *writer;
     UINT count=0;
@@ -1877,6 +1886,31 @@ static void test_metadata_Gps(void)
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ok(!count, "Unexpected count %u.\n", count);

+    id.vt = VT_UI2;
+    id.uiVal = 0x300;
+    value.vt = VT_UI2;
+    value.ulVal = 555;
+    PropVariantInit(&schema);
+    hr = IWICMetadataWriter_SetValue(writer, &schema, &id, &value);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = IWICMetadataWriter_GetCount(writer, &count);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(count == 1, "Unexpected count %u.\n", count);
+
+    id.uiVal = 1;
+    hr = IWICMetadataWriter_RemoveValue(writer, &schema, &id);
+    ok(hr == WINCODEC_ERR_PROPERTYNOTFOUND, "Unexpected hr %#lx.\n", hr);
+
+    schema.vt = VT_UI4;
+    id.uiVal = 0x300;
+    hr = IWICMetadataWriter_RemoveValue(writer, &schema, &id);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = IWICMetadataWriter_GetCount(writer, &count);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(!count, "Unexpected count %u.\n", count);
+
     load_stream(writer, (const char *)&IFD_data, sizeof(IFD_data), 0);
     load_stream(writer, (const char *)&IFD_data, sizeof(IFD_data), WICPersistOptionNoCacheStream);

@@ -5377,6 +5411,15 @@ static void test_metadata_App1(void)
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ok(!count, "Unexpected count %u.\n", count);

+    id.vt = VT_UI2;
+    id.uiVal = 0x300;
+    value.vt = VT_UI2;
+    value.ulVal = 555;
+    PropVariantInit(&schema);
+    hr = IWICMetadataWriter_SetValue(writer, &schema, &id, &value);
+    todo_wine
+    ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr);
+
     IWICMetadataWriter_Release(writer);

IWICComponentFactory_Release(factory);
diff --git a/dlls/windowscodecs/wincodecs_private.h b/dlls/windowscodecs/wincodecs_private.h
index fb3ef312851..c255df06f84 100644
--- a/dlls/windowscodecs/wincodecs_private.h
+++ b/dlls/windowscodecs/wincodecs_private.h
@@ -208,6 +208,7 @@ typedef struct _MetadataItem
 enum metadatahandler_flags
 {
     METADATAHANDLER_IS_WRITER = 0x1,
+    METADATAHANDLER_FIXED_ITEMS = 0x2, /* Items cannot be added or removed. */
 };

typedef struct _MetadataHandlerVtbl

[PATCH 4/8] windowscodecs/tests: Add some tests for RemoveMetadataByName().

[Nikolay Sivov]

From: Nikolay Sivov

Signed-off-by: Nikolay Sivov 
---
 dlls/windowscodecs/tests/metadata.c | 90 ++++++++++++++++++++++++++++-
 1 file changed, 88 insertions(+), 2 deletions(-)

diff --git a/dlls/windowscodecs/tests/metadata.c b/dlls/windowscodecs/tests/metadata.c
index fb73451524e..6265e7814e2 100644
--- a/dlls/windowscodecs/tests/metadata.c
+++ b/dlls/windowscodecs/tests/metadata.c
@@ -4274,8 +4274,18 @@ static HRESULT WINAPI test_block_writer_SetWriterByIndex(IWICMetadataBlockWriter

static HRESULT WINAPI test_block_writer_RemoveWriterByIndex(IWICMetadataBlockWriter *iface, UINT index)
 {
-    ok(0, "not implemented\n");
-    return E_NOTIMPL;
+    struct test_block_writer *writer = impl_from_IWICMetadataBlockWriter(iface);
+
+    if (index >= writer->count)
+        return E_INVALIDARG;
+
+    IWICMetadataWriter_Release(writer->writers[index]);
+    writer->writers[index] = NULL;
+    writer->count--;
+    if (index != writer->count)
+        memcpy(&writer->writers[index], &writer->writers[index + 1], (writer->count - index) * sizeof(*writer->writers));
+
+    return S_OK;
 }

static const IWICMetadataBlockWriterVtbl test_block_writer_vtbl =
@@ -6050,6 +6060,81 @@ if (hr == S_OK)
     IWICComponentFactory_Release(factory);
 }

+static void test_RemoveMetadataByName(void)
+{
+    IWICMetadataQueryWriter *query_writer;
+    IWICComponentFactory *factory;
+    IWICMetadataReader *reader;
+    PROPVARIANT value;
+    HRESULT hr;
+
+    hr = CoCreateInstance(&CLSID_WICImagingFactory, NULL, CLSCTX_INPROC_SERVER,
+            &IID_IWICComponentFactory, (void **)&factory);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = CoCreateInstance(&CLSID_WICApp1MetadataReader, NULL, CLSCTX_INPROC_SERVER,
+            &IID_IWICMetadataReader, (void **)&reader);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    load_stream(reader, (const char *)&app1_data, sizeof(app1_data), 0);
+
+    hr = create_query_writer_from_metadata_reader(factory, reader, &GUID_ContainerFormatJpeg, &query_writer);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    PropVariantInit(&value);
+    hr = IWICMetadataQueryWriter_GetMetadataByName(query_writer, L"/app1/ifd/exif/{ushort=512}", &value);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(value.vt == VT_UI2, "Unexpected value type: %u.\n", value.vt);
+    ok(value.ulVal == 444, "Unexpected value %lu.\n", value.ulVal);
+
+    /* Item does not exist. */
+    hr = IWICMetadataQueryWriter_RemoveMetadataByName(query_writer, NULL);
+    todo_wine
+    ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr);
+    hr = IWICMetadataQueryWriter_RemoveMetadataByName(query_writer, L"/app1/ifd/exif/{ushort=513}");
+    todo_wine
+    ok(hr == WINCODEC_ERR_PROPERTYNOTFOUND, "Unexpected hr %#lx.\n", hr);
+    hr = IWICMetadataQueryWriter_RemoveMetadataByName(query_writer, L"/app1/ifd/exif/ifd");
+    todo_wine
+    ok(hr == WINCODEC_ERR_PROPERTYNOTFOUND, "Unexpected hr %#lx.\n", hr);
+    hr = IWICMetadataQueryWriter_RemoveMetadataByName(query_writer, L"/app1/ifd/exif/invalid");
+    todo_wine
+    ok(hr == WINCODEC_ERR_PROPERTYNOTFOUND, "Unexpected hr %#lx.\n", hr);
+    hr = IWICMetadataQueryWriter_RemoveMetadataByName(query_writer, L"/app1/invalid/exif");
+    todo_wine
+    ok(hr == WINCODEC_ERR_PROPERTYNOTFOUND, "Unexpected hr %#lx.\n", hr);
+
+    /* Removing terminal item. */
+    hr = IWICMetadataQueryWriter_RemoveMetadataByName(query_writer, L"/app1/ifd/exif/{ushort=512}");
+    todo_wine
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    PropVariantInit(&value);
+    hr = IWICMetadataQueryWriter_GetMetadataByName(query_writer, L"/app1/ifd/exif/{ushort=512}", &value);
+    todo_wine
+    ok(hr == WINCODEC_ERR_PROPERTYNOTFOUND, "Unexpected hr %#lx.\n", hr);
+
+    /* Remove at handler level. */
+    hr = IWICMetadataQueryWriter_RemoveMetadataByName(query_writer, L"/app1/ifd");
+    todo_wine
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    PropVariantInit(&value);
+    hr = IWICMetadataQueryWriter_GetMetadataByName(query_writer, L"/app1/ifd", &value);
+    todo_wine
+    ok(hr == WINCODEC_ERR_PROPERTYNOTFOUND, "Unexpected hr %#lx.\n", hr);
+    /* At root */
+    hr = IWICMetadataQueryWriter_RemoveMetadataByName(query_writer, L"/");
+    todo_wine
+    ok(hr == WINCODEC_ERR_INVALIDQUERYREQUEST, "Unexpected hr %#lx.\n", hr);
+    hr = IWICMetadataQueryWriter_RemoveMetadataByName(query_writer, L"/app1");
+    todo_wine
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    IWICMetadataQueryWriter_Release(query_writer);
+    IWICMetadataReader_Release(reader);
+    IWICComponentFactory_Release(factory);
+}
+
 START_TEST(metadata)
 {
     CoInitializeEx(NULL, COINIT_APARTMENTTHREADED);
@@ -6083,6 +6168,7 @@ START_TEST(metadata)
     test_metadata_writer();
     test_CreateQueryWriter();
     test_CreateQueryWriterFromReader();
+    test_RemoveMetadataByName();

CoUninitialize();
 }

[PATCH 5/8] windowscodecs/metadata: Implement RemoveMetadataByName().

[Nikolay Sivov]

From: Nikolay Sivov

Signed-off-by: Nikolay Sivov 
---
 dlls/windowscodecs/metadataquery.c  | 38 ++++++++++++++++++++---------
 dlls/windowscodecs/tests/metadata.c | 11 ---------
 2 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/dlls/windowscodecs/metadataquery.c b/dlls/windowscodecs/metadataquery.c
index e3225d8d361..1fd75bbff00 100644
--- a/dlls/windowscodecs/metadataquery.c
+++ b/dlls/windowscodecs/metadataquery.c
@@ -533,7 +533,7 @@ static HRESULT parser_set_top_level_metadata_handler(struct query_handler *query

/* Root component has to be an object within block collection, it's located using {CLSID, index} pair. */
     if (comp->id.vt != VT_CLSID)
-        return E_UNEXPECTED;
+        return WINCODEC_ERR_INVALIDQUERYREQUEST;

hr = IWICMetadataBlockReader_GetCount(query_handler->object.block_reader, &count);
     if (hr != S_OK) return hr;
@@ -669,14 +669,9 @@ static HRESULT parse_query(struct query_handler *query_handler, const WCHAR *que
     if (SUCCEEDED(parser->hr))
     {
         parser->last = &parser->components[parser->count - 1];
-        if (parser->last->handler)
-            return S_OK;
-
-        if (parser->count < 2)
-            return parser->hr = WINCODEC_ERR_INVALIDQUERYREQUEST;
+        parser->prev = parser->count > 1 ? &parser->components[parser->count - 2] : NULL;

-        parser->prev = &parser->components[parser->count - 2];
-        if (!parser->prev->handler)
+        if (!parser->last->handler && !(parser->prev && parser->prev->handler))
             return parser->hr = WINCODEC_ERR_INVALIDQUERYREQUEST;
     }

@@ -873,11 +868,32 @@ static HRESULT WINAPI query_handler_SetMetadataByName(IWICMetadataQueryWriter *i
     return S_OK;
 }

-static HRESULT WINAPI query_handler_RemoveMetadataByName(IWICMetadataQueryWriter *iface, LPCWSTR name)
+static HRESULT WINAPI query_handler_RemoveMetadataByName(IWICMetadataQueryWriter *iface, const WCHAR *query)
 {
-    FIXME("iface %p, name %s stub.\n", iface, debugstr_w(name));
+    struct query_handler *handler = impl_from_IWICMetadataQueryWriter(iface);
+    struct query_component *last, *prev;
+    struct query_parser parser;
+    HRESULT hr;

-    return E_NOTIMPL;
+    TRACE("iface %p, name %s.\n", iface, debugstr_w(query));
+
+    if (!query)
+        return E_INVALIDARG;
+
+    if (SUCCEEDED(hr = parse_query(handler, query, &parser)))
+    {
+        last = parser.last;
+        prev = parser.prev;
+
+        if (is_block_handler(handler) && parser.count == 1)
+            hr = IWICMetadataBlockWriter_RemoveWriterByIndex(handler->object.block_writer, last->index);
+        else
+            hr = IWICMetadataWriter_RemoveValue(prev->writer, &last->schema, &last->id);
+    }
+
+    parser_cleanup(&parser);
+
+    return hr;
 }

static IWICMetadataQueryWriterVtbl query_handler_vtbl =
diff --git a/dlls/windowscodecs/tests/metadata.c b/dlls/windowscodecs/tests/metadata.c
index 6265e7814e2..ba8f1771875 100644
--- a/dlls/windowscodecs/tests/metadata.c
+++ b/dlls/windowscodecs/tests/metadata.c
@@ -6089,45 +6089,34 @@ static void test_RemoveMetadataByName(void)

/* Item does not exist. */
     hr = IWICMetadataQueryWriter_RemoveMetadataByName(query_writer, NULL);
-    todo_wine
     ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr);
     hr = IWICMetadataQueryWriter_RemoveMetadataByName(query_writer, L"/app1/ifd/exif/{ushort=513}");
-    todo_wine
     ok(hr == WINCODEC_ERR_PROPERTYNOTFOUND, "Unexpected hr %#lx.\n", hr);
     hr = IWICMetadataQueryWriter_RemoveMetadataByName(query_writer, L"/app1/ifd/exif/ifd");
-    todo_wine
     ok(hr == WINCODEC_ERR_PROPERTYNOTFOUND, "Unexpected hr %#lx.\n", hr);
     hr = IWICMetadataQueryWriter_RemoveMetadataByName(query_writer, L"/app1/ifd/exif/invalid");
-    todo_wine
     ok(hr == WINCODEC_ERR_PROPERTYNOTFOUND, "Unexpected hr %#lx.\n", hr);
     hr = IWICMetadataQueryWriter_RemoveMetadataByName(query_writer, L"/app1/invalid/exif");
-    todo_wine
     ok(hr == WINCODEC_ERR_PROPERTYNOTFOUND, "Unexpected hr %#lx.\n", hr);

/* Removing terminal item. */
     hr = IWICMetadataQueryWriter_RemoveMetadataByName(query_writer, L"/app1/ifd/exif/{ushort=512}");
-    todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     PropVariantInit(&value);
     hr = IWICMetadataQueryWriter_GetMetadataByName(query_writer, L"/app1/ifd/exif/{ushort=512}", &value);
-    todo_wine
     ok(hr == WINCODEC_ERR_PROPERTYNOTFOUND, "Unexpected hr %#lx.\n", hr);

/* Remove at handler level. */
     hr = IWICMetadataQueryWriter_RemoveMetadataByName(query_writer, L"/app1/ifd");
-    todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);

PropVariantInit(&value);
     hr = IWICMetadataQueryWriter_GetMetadataByName(query_writer, L"/app1/ifd", &value);
-    todo_wine
     ok(hr == WINCODEC_ERR_PROPERTYNOTFOUND, "Unexpected hr %#lx.\n", hr);
     /* At root */
     hr = IWICMetadataQueryWriter_RemoveMetadataByName(query_writer, L"/");
-    todo_wine
     ok(hr == WINCODEC_ERR_INVALIDQUERYREQUEST, "Unexpected hr %#lx.\n", hr);
     hr = IWICMetadataQueryWriter_RemoveMetadataByName(query_writer, L"/app1");
-    todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);

IWICMetadataQueryWriter_Release(query_writer);

[PATCH 6/8] windowscodecs/tests: Remove WinXP workarounds from png metadata test.

[Nikolay Sivov]

From: Nikolay Sivov

Signed-off-by: Nikolay Sivov 
---
 dlls/windowscodecs/tests/metadata.c | 101 ++++++++++++----------------
 1 file changed, 43 insertions(+), 58 deletions(-)

diff --git a/dlls/windowscodecs/tests/metadata.c b/dlls/windowscodecs/tests/metadata.c
index ba8f1771875..6a2c9cd6e0c 100644
--- a/dlls/windowscodecs/tests/metadata.c
+++ b/dlls/windowscodecs/tests/metadata.c
@@ -2188,85 +2188,70 @@ static void test_metadata_png(void)
     hr = IWICBitmapFrameDecode_QueryInterface(frame, &IID_IWICMetadataBlockReader, (void**)&blockreader);
     ok(hr == S_OK, "QueryInterface failed, hr=%lx\n", hr);

-    if (SUCCEEDED(hr))
-    {
-        winetest_push_context("png");
-        test_block_reader_enumerator(blockreader);
-        winetest_pop_context();
+    winetest_push_context("png");
+    test_block_reader_enumerator(blockreader);
+    winetest_pop_context();

-        hr = IWICMetadataBlockReader_GetContainerFormat(blockreader, NULL);
-        ok(hr == E_INVALIDARG, "GetContainerFormat failed, hr=%lx\n", hr);
+    hr = IWICMetadataBlockReader_GetContainerFormat(blockreader, NULL);
+    ok(hr == E_INVALIDARG, "GetContainerFormat failed, hr=%lx\n", hr);

-        hr = IWICMetadataBlockReader_GetContainerFormat(blockreader, &containerformat);
-        ok(hr == S_OK, "GetContainerFormat failed, hr=%lx\n", hr);
-        ok(IsEqualGUID(&containerformat, &GUID_ContainerFormatPng), "unexpected container format\n");
+    hr = IWICMetadataBlockReader_GetContainerFormat(blockreader, &containerformat);
+    ok(hr == S_OK, "GetContainerFormat failed, hr=%lx\n", hr);
+    ok(IsEqualGUID(&containerformat, &GUID_ContainerFormatPng), "unexpected container format\n");

-        hr = IWICMetadataBlockReader_GetCount(blockreader, NULL);
-        ok(hr == E_INVALIDARG, "GetCount failed, hr=%lx\n", hr);
+    hr = IWICMetadataBlockReader_GetCount(blockreader, NULL);
+    ok(hr == E_INVALIDARG, "GetCount failed, hr=%lx\n", hr);

-        hr = IWICMetadataBlockReader_GetCount(blockreader, &count);
-        ok(hr == S_OK, "GetCount failed, hr=%lx\n", hr);
-        ok(count == 1, "unexpected count %d\n", count);
+    hr = IWICMetadataBlockReader_GetCount(blockreader, &count);
+    ok(hr == S_OK, "GetCount failed, hr=%lx\n", hr);
+    ok(count == 1, "unexpected count %d\n", count);

-        if (0)
-        {
-            /* Crashes on Windows XP */
-            hr = IWICMetadataBlockReader_GetReaderByIndex(blockreader, 0, NULL);
-            ok(hr == E_INVALIDARG, "GetReaderByIndex failed, hr=%lx\n", hr);
-        }
+    hr = IWICMetadataBlockReader_GetReaderByIndex(blockreader, 0, NULL);
+    ok(hr == E_INVALIDARG, "GetReaderByIndex failed, hr=%lx\n", hr);

-        hr = IWICMetadataBlockReader_GetReaderByIndex(blockreader, 0, &reader);
-        ok(hr == S_OK, "GetReaderByIndex failed, hr=%lx\n", hr);
+    hr = IWICMetadataBlockReader_GetReaderByIndex(blockreader, 0, &reader);
+    ok(hr == S_OK, "GetReaderByIndex failed, hr=%lx\n", hr);

-        if (SUCCEEDED(hr))
-        {
-            hr = IWICMetadataReader_GetMetadataFormat(reader, &containerformat);
-            ok(hr == S_OK, "GetMetadataFormat failed, hr=%#lx\n", hr);
-            ok(IsEqualGUID(&containerformat, &GUID_MetadataFormatChunktIME) ||
-               broken(IsEqualGUID(&containerformat, &GUID_MetadataFormatUnknown)) /* Windows XP */,
-               "unexpected container format\n");
+    hr = IWICMetadataReader_GetMetadataFormat(reader, &containerformat);
+    ok(hr == S_OK, "GetMetadataFormat failed, hr=%#lx\n", hr);
+    ok(IsEqualGUID(&containerformat, &GUID_MetadataFormatChunktIME), "Unexpected format %s.\n",
+            wine_dbgstr_guid(&containerformat));

-            test_reader_container_format(reader, &GUID_ContainerFormatPng);
+    test_reader_container_format(reader, &GUID_ContainerFormatPng);

-            hr = IWICMetadataReader_GetCount(reader, &count);
-            ok(hr == S_OK, "GetCount error %#lx\n", hr);
-            ok(count == 6 || broken(count == 1) /* XP */, "expected 6, got %u\n", count);
-            if (count == 6)
-                compare_metadata(reader, td, count);
+    hr = IWICMetadataReader_GetCount(reader, &count);
+    ok(hr == S_OK, "GetCount error %#lx\n", hr);
+    ok(count == 6, "Unexpected item count %u.\n", count);
+    if (count == 6)
+        compare_metadata(reader, td, count);

-            IWICMetadataReader_Release(reader);
-        }
+    IWICMetadataReader_Release(reader);

-        hr = IWICMetadataBlockReader_GetReaderByIndex(blockreader, 1, &reader);
-        todo_wine ok(hr == WINCODEC_ERR_VALUEOUTOFRANGE, "GetReaderByIndex failed, hr=%lx\n", hr);
+    hr = IWICMetadataBlockReader_GetReaderByIndex(blockreader, 1, &reader);
+    todo_wine ok(hr == WINCODEC_ERR_VALUEOUTOFRANGE, "GetReaderByIndex failed, hr=%lx\n", hr);

-        hr = CoCreateInstance(&CLSID_WICImagingFactory, NULL, CLSCTX_INPROC_SERVER,
-            &IID_IWICComponentFactory, (void**)&factory);
-        ok(hr == S_OK, "CoCreateInstance failed, hr=%lx\n", hr);
+    hr = CoCreateInstance(&CLSID_WICImagingFactory, NULL, CLSCTX_INPROC_SERVER,
+        &IID_IWICComponentFactory, (void**)&factory);
+    ok(hr == S_OK, "CoCreateInstance failed, hr=%lx\n", hr);

-        hr = IWICComponentFactory_CreateQueryReaderFromBlockReader(factory, NULL, &queryreader);
-        ok(hr == E_INVALIDARG, "CreateQueryReaderFromBlockReader should have failed: %08lx\n", hr);
+    hr = IWICComponentFactory_CreateQueryReaderFromBlockReader(factory, NULL, &queryreader);
+    ok(hr == E_INVALIDARG, "CreateQueryReaderFromBlockReader should have failed: %08lx\n", hr);

-        hr = IWICComponentFactory_CreateQueryReaderFromBlockReader(factory, blockreader, NULL);
-        ok(hr == E_INVALIDARG, "CreateQueryReaderFromBlockReader should have failed: %08lx\n", hr);
+    hr = IWICComponentFactory_CreateQueryReaderFromBlockReader(factory, blockreader, NULL);
+    ok(hr == E_INVALIDARG, "CreateQueryReaderFromBlockReader should have failed: %08lx\n", hr);

-        hr = IWICComponentFactory_CreateQueryReaderFromBlockReader(factory, blockreader, &queryreader);
-        ok(hr == S_OK, "CreateQueryReaderFromBlockReader failed: %08lx\n", hr);
+    hr = IWICComponentFactory_CreateQueryReaderFromBlockReader(factory, blockreader, &queryreader);
+    ok(hr == S_OK, "CreateQueryReaderFromBlockReader failed: %08lx\n", hr);

-        IWICMetadataQueryReader_Release(queryreader);
+    IWICMetadataQueryReader_Release(queryreader);

-        IWICComponentFactory_Release(factory);
+    IWICComponentFactory_Release(factory);

-        IWICMetadataBlockReader_Release(blockreader);
-    }
+    IWICMetadataBlockReader_Release(blockreader);

hr = IWICBitmapFrameDecode_GetMetadataQueryReader(frame, &queryreader);
     ok(hr == S_OK, "GetMetadataQueryReader failed: %08lx\n", hr);
-
-    if (SUCCEEDED(hr))
-    {
-        IWICMetadataQueryReader_Release(queryreader);
-    }
+    IWICMetadataQueryReader_Release(queryreader);

IWICBitmapFrameDecode_Release(frame);

[PATCH 7/8] windowscodecs/tests: Move non-specific CreateQueryReaderFromBlockReader() tests to a separate function.

[Nikolay Sivov]

From: Nikolay Sivov

Signed-off-by: Nikolay Sivov 
---
 dlls/windowscodecs/tests/metadata.c | 32 +++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/dlls/windowscodecs/tests/metadata.c b/dlls/windowscodecs/tests/metadata.c
index 6a2c9cd6e0c..a075ce7c362 100644
--- a/dlls/windowscodecs/tests/metadata.c
+++ b/dlls/windowscodecs/tests/metadata.c
@@ -2234,12 +2234,6 @@ static void test_metadata_png(void)
         &IID_IWICComponentFactory, (void**)&factory);
     ok(hr == S_OK, "CoCreateInstance failed, hr=%lx\n", hr);

-    hr = IWICComponentFactory_CreateQueryReaderFromBlockReader(factory, NULL, &queryreader);
-    ok(hr == E_INVALIDARG, "CreateQueryReaderFromBlockReader should have failed: %08lx\n", hr);
-
-    hr = IWICComponentFactory_CreateQueryReaderFromBlockReader(factory, blockreader, NULL);
-    ok(hr == E_INVALIDARG, "CreateQueryReaderFromBlockReader should have failed: %08lx\n", hr);
-
     hr = IWICComponentFactory_CreateQueryReaderFromBlockReader(factory, blockreader, &queryreader);
     ok(hr == S_OK, "CreateQueryReaderFromBlockReader failed: %08lx\n", hr);

@@ -6109,6 +6103,31 @@ static void test_RemoveMetadataByName(void)
     IWICComponentFactory_Release(factory);
 }

+static void test_CreateQueryReaderFromBlockReader(void)
+{
+    IWICMetadataQueryReader *queryreader;
+    IWICMetadataBlockWriter *writer;
+    IWICComponentFactory *factory;
+    HRESULT hr;
+
+    hr = CoCreateInstance(&CLSID_WICImagingFactory, NULL, CLSCTX_INPROC_SERVER,
+            &IID_IWICComponentFactory, (void **)&factory);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = create_test_block_writer(&GUID_ContainerFormatPng, &writer);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = IWICComponentFactory_CreateQueryReaderFromBlockReader(factory, NULL, &queryreader);
+    ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr);
+
+    hr = IWICComponentFactory_CreateQueryReaderFromBlockReader(factory, (IWICMetadataBlockReader *)writer, NULL);
+    ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr);
+
+    IWICMetadataBlockWriter_Release(writer);
+
+    IWICComponentFactory_Release(factory);
+}
+
 START_TEST(metadata)
 {
     CoInitializeEx(NULL, COINIT_APARTMENTTHREADED);
@@ -6143,6 +6162,7 @@ START_TEST(metadata)
     test_CreateQueryWriter();
     test_CreateQueryWriterFromReader();
     test_RemoveMetadataByName();
+    test_CreateQueryReaderFromBlockReader();

CoUninitialize();
 }

[PATCH 8/8] windowscodecs/tests: Add a test for reader instances returned from the decoder.

[Nikolay Sivov]

From: Nikolay Sivov

Signed-off-by: Nikolay Sivov 
---
 dlls/windowscodecs/tests/metadata.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/dlls/windowscodecs/tests/metadata.c b/dlls/windowscodecs/tests/metadata.c
index a075ce7c362..98b59a2cdb9 100644
--- a/dlls/windowscodecs/tests/metadata.c
+++ b/dlls/windowscodecs/tests/metadata.c
@@ -2162,7 +2162,7 @@ static void test_metadata_png(void)
     IWICBitmapDecoder *decoder;
     IWICBitmapFrameDecode *frame;
     IWICMetadataBlockReader *blockreader;
-    IWICMetadataReader *reader;
+    IWICMetadataReader *reader, *reader2;
     IWICMetadataQueryReader *queryreader;
     IWICComponentFactory *factory;
     GUID containerformat;
@@ -2211,6 +2211,11 @@ static void test_metadata_png(void)

hr = IWICMetadataBlockReader_GetReaderByIndex(blockreader, 0, &reader);
     ok(hr == S_OK, "GetReaderByIndex failed, hr=%lx\n", hr);
+    hr = IWICMetadataBlockReader_GetReaderByIndex(blockreader, 0, &reader2);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    todo_wine
+    ok(reader == reader2, "Unexpected instance.\n");
+    IWICMetadataReader_Release(reader2);

hr = IWICMetadataReader_GetMetadataFormat(reader, &containerformat);
     ok(hr == S_OK, "GetMetadataFormat failed, hr=%#lx\n", hr);
@@ -2335,7 +2340,7 @@ static void test_metadata_gif(void)
     IWICBitmapDecoder *decoder;
     IWICBitmapFrameDecode *frame;
     IWICMetadataBlockReader *blockreader;
-    IWICMetadataReader *reader;
+    IWICMetadataReader *reader, *reader2;
     IWICMetadataQueryReader *queryreader;
     GUID format;
     HRESULT hr;
@@ -2374,6 +2379,12 @@ static void test_metadata_gif(void)
         hr = IWICMetadataBlockReader_GetReaderByIndex(blockreader, 0, &reader);
         ok(hr == S_OK, "GetReaderByIndex error %#lx\n", hr);

+        hr = IWICMetadataBlockReader_GetReaderByIndex(blockreader, 0, &reader2);
+        ok(hr == S_OK, "GetReaderByIndex error %#lx\n", hr);
+        todo_wine
+        ok(reader == reader2, "Unexpected instance.\n");
+        IWICMetadataReader_Release(reader2);
+
         if (SUCCEEDED(hr))
         {
             hr = IWICMetadataReader_GetMetadataFormat(reader, &format);
@@ -2424,6 +2435,11 @@ static void test_metadata_gif(void)

hr = IWICMetadataBlockReader_GetReaderByIndex(blockreader, 0, &reader);
         ok(hr == S_OK, "GetReaderByIndex error %#lx\n", hr);
+        hr = IWICMetadataBlockReader_GetReaderByIndex(blockreader, 0, &reader2);
+        ok(hr == S_OK, "GetReaderByIndex error %#lx\n", hr);
+        todo_wine
+        ok(reader == reader2, "Unexpected instance.\n");
+        IWICMetadataReader_Release(reader2);

if (SUCCEEDED(hr))
         {

[Esme Povirk (＠madewokherd)]

Esme Povirk (@madewokherd) commented about dlls/windowscodecs/tests/metadata.c:
>  ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
>  ok(count == 1, "Unexpected count %u.\n", count);
>
> - hr = IWICMetadataWriter_GetValueByIndex(writer, 0, &schema, &id, &value);

I think `value` needs to be freed?

[Nikolay Sivov (＠nsivov)]

On Wed Feb 19 00:43:17 2025 +0000, Esme Povirk wrote:
> I think `value` needs to be freed?

The value here is an empty vector, so nothing is leaked technically. If you think it's better to Clear regardless, I'll add it.

[Esme Povirk (＠madewokherd)]

On Wed Feb 19 01:20:53 2025 +0000, Nikolay Sivov wrote:
> The value here is an empty vector, so nothing is leaked technically. If
> you think it's better to Clear regardless, I'll add it.

Oh, I missed that. OK.

[PATCH 0/8] MR7350: windowscodecs/tests: Add some tests for default metadata item set. - approved

[Esme Povirk (＠madewokherd)]

This merge request was approved by Esme Povirk.