[PATCH v2 0/10] MR7329: windowscodecs/tests: Add some query tests with the Unknown reader.

[Nikolay Sivov (＠nsivov)]

Signed-off-by: Nikolay Sivov

--
  v2: windowscodecs: Implement CreateQueryWriter().
      windowscodecs/metadata: Support creating query handlers over bare metadata handlers.
      windowscodecs/metadata: Handle empty items in queries.
      windowscodecs/metadata: Split up query parsing into helpers according to the query grammar.
      windowscodecs/tests: Add some tests for the query reader container format.
      windowscodecs/tests: Add some tests for CreateQueryWriterFromReader().
      windowscodecs/tests: Add query reader tests for live block reader updates.
      windowscodecs/tests: Add some tests for CreateQueryWriter().
      windowscodecs/metadata: Share implementation between query reader and writer objects.
      windowscodecs/tests: Add some query tests with the Unknown reader.

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

[PATCH v2 01/10] windowscodecs/tests: Add some query tests with the Unknown reader.

[Nikolay Sivov]

From: Nikolay Sivov

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

diff --git a/dlls/windowscodecs/tests/metadata.c b/dlls/windowscodecs/tests/metadata.c
index ee9750eb96c..54c725b09d3 100644
--- a/dlls/windowscodecs/tests/metadata.c
+++ b/dlls/windowscodecs/tests/metadata.c
@@ -90,6 +90,9 @@ static HRESULT get_persist_stream(void *iface_ptr, IStream **stream)
     return hr;
 }

+static HRESULT create_query_reader_from_metadata_reader(IWICComponentFactory *factory, IWICMetadataReader *metadata_reader,
+        const GUID *container_format, IWICMetadataQueryReader **query_reader);
+
 #define compare_blob(a,b,c) compare_blob_(__LINE__,a,b,c)
 static void compare_blob_(unsigned int line, const PROPVARIANT *propvar, const char *data, ULONG length)
 {
@@ -655,23 +658,29 @@ static void _test_reader_container_format(IWICMetadataReader *reader, const GUID
     IWICMetadataHandlerInfo_Release(info);
 }

-
 static void test_metadata_unknown(void)
 {
+    IWICMetadataQueryReader *query_reader, *query_reader2;
     HRESULT hr;
     IWICMetadataReader *reader;
     IWICEnumMetadataItem *enumerator;
+    IWICComponentFactory *factory;
     PROPVARIANT schema, id, value;
     IWICMetadataWriter *writer;
     IWICPersistStream *persist;
-    ULONG items_returned;
+    IEnumString *enum_string;
     IStream *stream;
+    ULONG fetched;
+    WCHAR *str;
     UINT count;

hr = CoCreateInstance(&CLSID_WICUnknownMetadataReader, NULL, CLSCTX_INPROC_SERVER,
         &IID_IWICMetadataReader, (void**)&reader);
     ok(hr == S_OK, "CoCreateInstance failed, hr=%lx\n", hr);
-    if (FAILED(hr)) return;
+
+    hr = CoCreateInstance(&CLSID_WICImagingFactory, NULL, CLSCTX_INPROC_SERVER,
+            &IID_IWICComponentFactory, (void **)&factory);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);

check_interface(reader, &IID_IWICMetadataReader, TRUE);
     check_interface(reader, &IID_IPersist, TRUE);
@@ -682,51 +691,41 @@ static void test_metadata_unknown(void)

load_stream(reader, metadata_unknown, sizeof(metadata_unknown), WICPersistOptionDefault);

+    /* Item enumerator. */
     hr = IWICMetadataReader_GetEnumerator(reader, &enumerator);
     ok(hr == S_OK, "GetEnumerator failed, hr=%lx\n", hr);

-    if (SUCCEEDED(hr))
-    {
-        PropVariantInit(&schema);
-        PropVariantInit(&id);
-        PropVariantInit(&value);
-
-        hr = IWICEnumMetadataItem_Next(enumerator, 1, &schema, &id, &value, &items_returned);
-        ok(hr == S_OK, "Next failed, hr=%lx\n", hr);
-        ok(items_returned == 1, "unexpected item count %li\n", items_returned);
-
-        if (hr == S_OK && items_returned == 1)
-        {
-            ok(schema.vt == VT_EMPTY, "unexpected vt: %i\n", schema.vt);
-            ok(id.vt == VT_EMPTY, "unexpected vt: %i\n", id.vt);
-            compare_blob(&value, metadata_unknown, sizeof(metadata_unknown));
+    PropVariantInit(&schema);
+    PropVariantInit(&id);
+    PropVariantInit(&value);

-            PropVariantClear(&schema);
-            PropVariantClear(&id);
-            PropVariantClear(&value);
-        }
+    hr = IWICEnumMetadataItem_Next(enumerator, 1, &schema, &id, &value, &fetched);
+    ok(hr == S_OK, "Next failed, hr=%lx\n", hr);
+    ok(fetched == 1, "Unexpected count %lu.\n", fetched);

-        hr = IWICEnumMetadataItem_Next(enumerator, 1, &schema, &id, &value, &items_returned);
-        ok(hr == S_FALSE, "Next failed, hr=%lx\n", hr);
-        ok(items_returned == 0, "unexpected item count %li\n", items_returned);
+    ok(schema.vt == VT_EMPTY, "unexpected vt: %i\n", schema.vt);
+    ok(id.vt == VT_EMPTY, "unexpected vt: %i\n", id.vt);
+    compare_blob(&value, metadata_unknown, sizeof(metadata_unknown));

-        hr = IWICEnumMetadataItem_Reset(enumerator);
-        ok(hr == S_OK, "Reset failed, hr=%lx\n", hr);
+    PropVariantClear(&schema);
+    PropVariantClear(&id);
+    PropVariantClear(&value);

-        hr = IWICEnumMetadataItem_Next(enumerator, 1, &schema, &id, NULL, NULL);
-        ok(hr == S_OK, "Next failed, hr=%lx\n", hr);
+    hr = IWICEnumMetadataItem_Next(enumerator, 1, &schema, &id, &value, &fetched);
+    ok(hr == S_FALSE, "Next failed, hr=%lx\n", hr);
+    ok(!fetched, "Unexpected count %lu.\n", fetched);

-        if (hr == S_OK)
-        {
-            ok(schema.vt == VT_EMPTY, "unexpected vt: %i\n", schema.vt);
-            ok(id.vt == VT_EMPTY, "unexpected vt: %i\n", id.vt);
+    hr = IWICEnumMetadataItem_Reset(enumerator);
+    ok(hr == S_OK, "Reset failed, hr=%lx\n", hr);

-            PropVariantClear(&schema);
-            PropVariantClear(&id);
-        }
+    hr = IWICEnumMetadataItem_Next(enumerator, 1, &schema, &id, NULL, NULL);
+    ok(hr == S_OK, "Next failed, hr=%lx\n", hr);
+    ok(schema.vt == VT_EMPTY, "unexpected vt: %i\n", schema.vt);
+    ok(id.vt == VT_EMPTY, "unexpected vt: %i\n", id.vt);
+    PropVariantClear(&schema);
+    PropVariantClear(&id);

-        IWICEnumMetadataItem_Release(enumerator);
-    }
+    IWICEnumMetadataItem_Release(enumerator);

hr = IWICMetadataReader_QueryInterface(reader, &IID_IWICPersistStream, (void **)&persist);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
@@ -751,6 +750,58 @@ static void test_metadata_unknown(void)
     ok(count == 1, "Unexpected count %u.\n", count);
     IWICPersistStream_Release(persist);

+    /* Query reader. */
+    load_stream(reader, metadata_unknown, sizeof(metadata_unknown), 0);
+
+    hr = create_query_reader_from_metadata_reader(factory, reader, &GUID_NULL, &query_reader);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    PropVariantInit(&value);
+    hr = IWICMetadataQueryReader_GetMetadataByName(query_reader, L"/unknown", &value);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(value.vt == VT_UNKNOWN, "Unexpected value type %d.\n", value.vt);
+    hr = IUnknown_QueryInterface(value.punkVal, &IID_IWICMetadataQueryReader, (void **)&query_reader2);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    PropVariantClear(&value);
+
+    hr = IWICMetadataQueryReader_GetEnumerator(query_reader2, &enum_string);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    str = NULL;
+    hr = IEnumString_Next(enum_string, 1, &str, &fetched);
+    todo_wine
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    todo_wine
+    ok(fetched == 1, "Unexpected count %lu.\n", fetched);
+    if (hr == S_OK)
+    {
+        ok(!wcscmp(str, L"/{}"), "Unexpected query %s.\n", wine_dbgstr_w(str));
+        CoTaskMemFree(str);
+    }
+    IEnumString_Release(enum_string);
+
+    PropVariantInit(&value);
+    hr = IWICMetadataQueryReader_GetMetadataByName(query_reader2, L"/{}", &value);
+    todo_wine
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    if (hr == S_OK)
+    {
+        compare_blob(&value, metadata_unknown, sizeof(metadata_unknown));
+        PropVariantClear(&value);
+    }
+
+    PropVariantInit(&value);
+    hr = IWICMetadataQueryReader_GetMetadataByName(query_reader, L"/unknown/{}", &value);
+    todo_wine
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    if (hr == S_OK)
+    {
+        compare_blob(&value, metadata_unknown, sizeof(metadata_unknown));
+        PropVariantClear(&value);
+    }
+
+    IWICMetadataQueryReader_Release(query_reader2);
+    IWICMetadataQueryReader_Release(query_reader);
+
     IWICMetadataReader_Release(reader);

hr = CoCreateInstance(&CLSID_WICUnknownMetadataWriter, NULL, CLSCTX_INPROC_SERVER,
@@ -787,6 +838,7 @@ static void test_metadata_unknown(void)
     load_stream(writer, metadata_unknown, sizeof(metadata_unknown), WICPersistOptionNoCacheStream);

IWICMetadataWriter_Release(writer);
+    IWICComponentFactory_Release(factory);
 }

static void test_metadata_tEXt(void)

[PATCH v2 02/10] windowscodecs/metadata: Share implementation between query reader and writer objects.

[Nikolay Sivov]

From: Nikolay Sivov

Signed-off-by: Nikolay Sivov 
---
 dlls/windowscodecs/metadataquery.c  | 264 +++++++++-------------------
 dlls/windowscodecs/tests/metadata.c |  79 ++++-----
 2 files changed, 119 insertions(+), 224 deletions(-)

diff --git a/dlls/windowscodecs/metadataquery.c b/dlls/windowscodecs/metadataquery.c
index d0d27bdd0fb..b1235dbda6f 100644
--- a/dlls/windowscodecs/metadataquery.c
+++ b/dlls/windowscodecs/metadataquery.c
@@ -34,28 +34,47 @@ WINE_DEFAULT_DEBUG_CHANNEL(wincodecs);

static const WCHAR *map_shortname_to_schema(const GUID *format, const WCHAR *name);

-typedef struct {
-    IWICMetadataQueryReader IWICMetadataQueryReader_iface;
+enum metadata_object_type
+{
+    BLOCK_READER,
+    BLOCK_WRITER,
+};
+
+struct query_handler
+{
+    IWICMetadataQueryWriter IWICMetadataQueryWriter_iface;
     LONG ref;
-    IWICMetadataBlockReader *block;
+    union
+    {
+        IUnknown *handler;
+        IWICMetadataBlockReader *block_reader;
+        IWICMetadataBlockWriter *block_writer;
+    } object;
+    enum metadata_object_type object_type;
     WCHAR *root;
-} QueryReader;
+};
+
+static bool is_writer_handler(const struct query_handler *handler)
+{
+    return handler->object_type == BLOCK_WRITER;
+}

-static inline QueryReader *impl_from_IWICMetadataQueryReader(IWICMetadataQueryReader *iface)
+static inline struct query_handler *impl_from_IWICMetadataQueryWriter(IWICMetadataQueryWriter *iface)
 {
-    return CONTAINING_RECORD(iface, QueryReader, IWICMetadataQueryReader_iface);
+    return CONTAINING_RECORD(iface, struct query_handler, IWICMetadataQueryWriter_iface);
 }

-static HRESULT WINAPI mqr_QueryInterface(IWICMetadataQueryReader *iface, REFIID riid,
+static HRESULT WINAPI query_handler_QueryInterface(IWICMetadataQueryWriter *iface, REFIID riid,
         void **ppvObject)
 {
-    QueryReader *This = impl_from_IWICMetadataQueryReader(iface);
+    struct query_handler *handler = impl_from_IWICMetadataQueryWriter(iface);

-    TRACE("(%p,%s,%p)\n", This, debugstr_guid(riid), ppvObject);
+    TRACE("(%p,%s,%p)\n", iface, debugstr_guid(riid), ppvObject);

if (IsEqualGUID(riid, &IID_IUnknown) ||
-            IsEqualGUID(riid, &IID_IWICMetadataQueryReader))
-        *ppvObject = &This->IWICMetadataQueryReader_iface;
+            IsEqualGUID(riid, &IID_IWICMetadataQueryReader) ||
+            (is_writer_handler(handler) && IsEqualGUID(riid, &IID_IWICMetadataQueryWriter)))
+        *ppvObject = &handler->IWICMetadataQueryWriter_iface;
     else
         *ppvObject = NULL;

@@ -68,48 +87,48 @@ static HRESULT WINAPI mqr_QueryInterface(IWICMetadataQueryReader *iface, REFIID
     return E_NOINTERFACE;
 }

-static ULONG WINAPI mqr_AddRef(IWICMetadataQueryReader *iface)
+static ULONG WINAPI query_handler_AddRef(IWICMetadataQueryWriter *iface)
 {
-    QueryReader *This = impl_from_IWICMetadataQueryReader(iface);
-    ULONG ref = InterlockedIncrement(&This->ref);
-    TRACE("(%p) refcount=%lu\n", This, ref);
+    struct query_handler *handler = impl_from_IWICMetadataQueryWriter(iface);
+    ULONG ref = InterlockedIncrement(&handler->ref);
+    TRACE("(%p) refcount=%lu\n", iface, ref);
     return ref;
 }

-static ULONG WINAPI mqr_Release(IWICMetadataQueryReader *iface)
+static ULONG WINAPI query_handler_Release(IWICMetadataQueryWriter *iface)
 {
-    QueryReader *This = impl_from_IWICMetadataQueryReader(iface);
-    ULONG ref = InterlockedDecrement(&This->ref);
-    TRACE("(%p) refcount=%lu\n", This, ref);
+    struct query_handler *handler = impl_from_IWICMetadataQueryWriter(iface);
+    ULONG ref = InterlockedDecrement(&handler->ref);
+    TRACE("(%p) refcount=%lu\n", iface, ref);
     if (!ref)
     {
-        IWICMetadataBlockReader_Release(This->block);
-        free(This->root);
-        free(This);
+        IUnknown_Release(handler->object.handler);
+        free(handler->root);
+        free(handler);
     }
     return ref;
 }

-static HRESULT WINAPI mqr_GetContainerFormat(IWICMetadataQueryReader *iface, GUID *format)
+static HRESULT WINAPI query_handler_GetContainerFormat(IWICMetadataQueryWriter *iface, GUID *format)
 {
-    QueryReader *This = impl_from_IWICMetadataQueryReader(iface);
+    struct query_handler *handler = impl_from_IWICMetadataQueryWriter(iface);

-    TRACE("(%p,%p)\n", This, format);
+    TRACE("(%p,%p)\n", iface, format);

-    return IWICMetadataBlockReader_GetContainerFormat(This->block, format);
+    return IWICMetadataBlockReader_GetContainerFormat(handler->object.block_reader, format);
 }

-static HRESULT WINAPI mqr_GetLocation(IWICMetadataQueryReader *iface, UINT len, WCHAR *location, UINT *ret_len)
+static HRESULT WINAPI query_handler_GetLocation(IWICMetadataQueryWriter *iface, UINT len, WCHAR *location, UINT *ret_len)
 {
-    QueryReader *This = impl_from_IWICMetadataQueryReader(iface);
+    struct query_handler *handler = impl_from_IWICMetadataQueryWriter(iface);
     const WCHAR *root;
     UINT actual_len;

-    TRACE("(%p,%u,%p,%p)\n", This, len, location, ret_len);
+    TRACE("(%p,%u,%p,%p)\n", iface, len, location, ret_len);

if (!ret_len) return E_INVALIDARG;

-    root = This->root ? This->root : L"/";
+    root = handler->root ? handler->root : L"/";
     actual_len = lstrlenW(root) + 1;

if (location)
@@ -443,9 +462,9 @@ static HRESULT get_next_reader(IWICMetadataReader *reader, UINT index,
     return hr;
 }

-static HRESULT WINAPI mqr_GetMetadataByName(IWICMetadataQueryReader *iface, LPCWSTR query, PROPVARIANT *value)
+static HRESULT WINAPI query_handler_GetMetadataByName(IWICMetadataQueryWriter *iface, LPCWSTR query, PROPVARIANT *value)
 {
-    QueryReader *This = impl_from_IWICMetadataQueryReader(iface);
+    struct query_handler *This = impl_from_IWICMetadataQueryWriter(iface);
     struct string_t elem;
     WCHAR *full_query;
     const WCHAR *p;
@@ -518,7 +537,7 @@ static HRESULT WINAPI mqr_GetMetadataByName(IWICMetadataQueryReader *iface, LPCW
                 reader = new_reader;
             }
             else
-                hr = find_reader_from_block(This->block, index, &guid, &reader);
+                hr = find_reader_from_block(This->object.block_reader, index, &guid, &reader);

if (hr != S_OK) break;

@@ -532,7 +551,7 @@ static HRESULT WINAPI mqr_GetMetadataByName(IWICMetadataQueryReader *iface, LPCW

PropVariantClear(&new_value);
             new_value.vt = VT_UNKNOWN;
-            hr = MetadataQueryReader_CreateInstance(This->block, root, (IWICMetadataQueryReader **)&new_value.punkVal);
+            hr = MetadataQueryReader_CreateInstance(This->object.block_reader, root, (IWICMetadataQueryReader **)&new_value.punkVal);
             SysFreeString(root);
             if (hr != S_OK) break;
         }
@@ -706,7 +725,7 @@ static HRESULT string_enumerator_create(IEnumString **enum_string)
     return S_OK;
 }

-static HRESULT WINAPI mqr_GetEnumerator(IWICMetadataQueryReader *iface,
+static HRESULT WINAPI query_handler_GetEnumerator(IWICMetadataQueryWriter *iface,
         IEnumString **enum_string)
 {
     TRACE("iface %p, enum_string %p.\n", iface, enum_string);
@@ -714,177 +733,66 @@ static HRESULT WINAPI mqr_GetEnumerator(IWICMetadataQueryReader *iface,
     return string_enumerator_create(enum_string);
 }

-static IWICMetadataQueryReaderVtbl mqr_vtbl = {
-    mqr_QueryInterface,
-    mqr_AddRef,
-    mqr_Release,
-    mqr_GetContainerFormat,
-    mqr_GetLocation,
-    mqr_GetMetadataByName,
-    mqr_GetEnumerator
-};
-
-HRESULT MetadataQueryReader_CreateInstance(IWICMetadataBlockReader *mbr, const WCHAR *root, IWICMetadataQueryReader **out)
-{
-    QueryReader *obj;
-
-    obj = calloc(1, sizeof(*obj));
-    if (!obj)
-        return E_OUTOFMEMORY;
-
-    obj->IWICMetadataQueryReader_iface.lpVtbl = &mqr_vtbl;
-    obj->ref = 1;
-
-    IWICMetadataBlockReader_AddRef(mbr);
-    obj->block = mbr;
-
-    obj->root = wcsdup(root);
-
-    *out = &obj->IWICMetadataQueryReader_iface;
-
-    return S_OK;
-}
-
-typedef struct
-{
-    IWICMetadataQueryWriter IWICMetadataQueryWriter_iface;
-    LONG ref;
-    IWICMetadataBlockWriter *block;
-    WCHAR *root;
-}
-QueryWriter;
-
-static inline QueryWriter *impl_from_IWICMetadataQueryWriter(IWICMetadataQueryWriter *iface)
-{
-    return CONTAINING_RECORD(iface, QueryWriter, IWICMetadataQueryWriter_iface);
-}
-
-static HRESULT WINAPI mqw_QueryInterface(IWICMetadataQueryWriter *iface, REFIID riid,
-        void **object)
-{
-    QueryWriter *writer = impl_from_IWICMetadataQueryWriter(iface);
-
-    TRACE("writer %p, riid %s, object %p.\n", writer, debugstr_guid(riid), object);
-
-    if (IsEqualGUID(riid, &IID_IUnknown)
-            || IsEqualGUID(riid, &IID_IWICMetadataQueryWriter)
-            || IsEqualGUID(riid, &IID_IWICMetadataQueryReader))
-        *object = &writer->IWICMetadataQueryWriter_iface;
-    else
-        *object = NULL;
-
-    if (*object)
-    {
-        IUnknown_AddRef((IUnknown *)*object);
-        return S_OK;
-    }
-
-    return E_NOINTERFACE;
-}
-
-static ULONG WINAPI mqw_AddRef(IWICMetadataQueryWriter *iface)
-{
-    QueryWriter *writer = impl_from_IWICMetadataQueryWriter(iface);
-    ULONG ref = InterlockedIncrement(&writer->ref);
-
-    TRACE("writer %p, refcount=%lu\n", writer, ref);
-
-    return ref;
-}
-
-static ULONG WINAPI mqw_Release(IWICMetadataQueryWriter *iface)
-{
-    QueryWriter *writer = impl_from_IWICMetadataQueryWriter(iface);
-    ULONG ref = InterlockedDecrement(&writer->ref);
-
-    TRACE("writer %p, refcount=%lu.\n", writer, ref);
-
-    if (!ref)
-    {
-        IWICMetadataBlockWriter_Release(writer->block);
-        free(writer->root);
-        free(writer);
-    }
-    return ref;
-}
-
-static HRESULT WINAPI mqw_GetContainerFormat(IWICMetadataQueryWriter *iface, GUID *container_format)
-{
-    FIXME("iface %p, container_format %p stub.\n", iface, container_format);
-
-    return E_NOTIMPL;
-}
-
-static HRESULT WINAPI mqw_GetEnumerator(IWICMetadataQueryWriter *iface, IEnumString **enum_string)
-{
-    TRACE("iface %p, enum_string %p.\n", iface, enum_string);
-
-    return string_enumerator_create(enum_string);
-}
-
-static HRESULT WINAPI mqw_GetLocation(IWICMetadataQueryWriter *iface, UINT max_length, WCHAR *namespace, UINT *actual_length)
-{
-    FIXME("iface %p, max_length %u, namespace %s, actual_length %p stub.\n",
-            iface, max_length, debugstr_w(namespace), actual_length);
-
-    return E_NOTIMPL;
-}
-
-static HRESULT WINAPI mqw_GetMetadataByName(IWICMetadataQueryWriter *iface, LPCWSTR name, PROPVARIANT *value)
-{
-    FIXME("name %s, value %p stub.\n", debugstr_w(name), value);
-
-    return E_NOTIMPL;
-}
-
-static HRESULT WINAPI mqw_SetMetadataByName(IWICMetadataQueryWriter *iface, LPCWSTR name, const PROPVARIANT *value)
+static HRESULT WINAPI query_handler_SetMetadataByName(IWICMetadataQueryWriter *iface, LPCWSTR name, const PROPVARIANT *value)
 {
     FIXME("iface %p, name %s, value %p stub.\n", iface, debugstr_w(name), value);

return S_OK;
 }

-static HRESULT WINAPI mqw_RemoveMetadataByName(IWICMetadataQueryWriter *iface, LPCWSTR name)
+static HRESULT WINAPI query_handler_RemoveMetadataByName(IWICMetadataQueryWriter *iface, LPCWSTR name)
 {
     FIXME("iface %p, name %s stub.\n", iface, debugstr_w(name));

return E_NOTIMPL;
 }

-static const IWICMetadataQueryWriterVtbl mqw_vtbl =
+static IWICMetadataQueryWriterVtbl query_handler_vtbl =
 {
-    mqw_QueryInterface,
-    mqw_AddRef,
-    mqw_Release,
-    mqw_GetContainerFormat,
-    mqw_GetLocation,
-    mqw_GetMetadataByName,
-    mqw_GetEnumerator,
-    mqw_SetMetadataByName,
-    mqw_RemoveMetadataByName,
+    query_handler_QueryInterface,
+    query_handler_AddRef,
+    query_handler_Release,
+    query_handler_GetContainerFormat,
+    query_handler_GetLocation,
+    query_handler_GetMetadataByName,
+    query_handler_GetEnumerator,
+    query_handler_SetMetadataByName,
+    query_handler_RemoveMetadataByName,
 };

-HRESULT MetadataQueryWriter_CreateInstance(IWICMetadataBlockWriter *mbw, const WCHAR *root, IWICMetadataQueryWriter **out)
+static HRESULT create_query_handler(IUnknown *block_handler, enum metadata_object_type object_type,
+        const WCHAR *root, IWICMetadataQueryWriter **ret)
 {
-    QueryWriter *obj;
+    struct query_handler *obj;

obj = calloc(1, sizeof(*obj));
     if (!obj)
         return E_OUTOFMEMORY;

-    obj->IWICMetadataQueryWriter_iface.lpVtbl = &mqw_vtbl;
+    obj->IWICMetadataQueryWriter_iface.lpVtbl = &query_handler_vtbl;
     obj->ref = 1;
-
-    IWICMetadataBlockWriter_AddRef(mbw);
-    obj->block = mbw;
-
+    IUnknown_AddRef(block_handler);
+    obj->object.handler = block_handler;
+    obj->object_type = object_type;
     obj->root = wcsdup(root);

-    *out = &obj->IWICMetadataQueryWriter_iface;
+    *ret = &obj->IWICMetadataQueryWriter_iface;

return S_OK;
 }

+HRESULT MetadataQueryReader_CreateInstance(IWICMetadataBlockReader *block_reader, const WCHAR *root,
+        IWICMetadataQueryReader **out)
+{
+    return create_query_handler((IUnknown *)block_reader, BLOCK_READER, root, (IWICMetadataQueryWriter **)out);
+}
+
+HRESULT MetadataQueryWriter_CreateInstance(IWICMetadataBlockWriter *block_writer, const WCHAR *root,
+        IWICMetadataQueryWriter **out)
+{
+    return create_query_handler((IUnknown *)block_writer, BLOCK_WRITER, root, out);
+}
+
 static const struct
 {
     const GUID *guid;
diff --git a/dlls/windowscodecs/tests/metadata.c b/dlls/windowscodecs/tests/metadata.c
index 54c725b09d3..dcdc47f931f 100644
--- a/dlls/windowscodecs/tests/metadata.c
+++ b/dlls/windowscodecs/tests/metadata.c
@@ -4965,113 +4965,100 @@ static void test_metadata_App1(void)

PropVariantInit(&value);
     hr = IWICMetadataQueryWriter_GetMetadataByName(query_writer, L"/app1", &value);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(value.vt == VT_UNKNOWN, "Unexpected value type: %u.\n", value.vt);
+
+    check_interface(value.punkVal, &IID_IWICMetadataQueryReader, TRUE);
+    todo_wine
+    check_interface(value.punkVal, &IID_IWICMetadataQueryWriter, TRUE);
+
+    hr = IUnknown_QueryInterface(value.punkVal, &IID_IWICMetadataQueryWriter, (void **)&query_writer2);
     todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     if (hr == S_OK)
     {
-        ok(value.vt == VT_UNKNOWN, "Unexpected value type: %u.\n", value.vt);
-
-        check_interface(value.punkVal, &IID_IWICMetadataQueryReader, TRUE);
-        check_interface(value.punkVal, &IID_IWICMetadataQueryWriter, TRUE);
-
-        hr = IUnknown_QueryInterface(value.punkVal, &IID_IWICMetadataQueryWriter, (void **)&query_writer2);
-        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
         hr = IWICMetadataQueryWriter_GetLocation(query_writer2, ARRAY_SIZE(path), path, &length);
         ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
         ok(!lstrcmpW(path, L"/app1"), "Unexpected path %s.\n", wine_dbgstr_w(path));
         IWICMetadataQueryWriter_Release(query_writer2);
-        PropVariantClear(&value);
     }
+    PropVariantClear(&value);

PropVariantInit(&value);
     hr = IWICMetadataQueryWriter_GetMetadataByName(query_writer, L"/app1/ifd", &value);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(value.vt == VT_UNKNOWN, "Unexpected value type: %u.\n", value.vt);
+    hr = IUnknown_QueryInterface(value.punkVal, &IID_IWICMetadataQueryWriter, (void **)&query_writer2);
     todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     if (hr == S_OK)
     {
-        ok(value.vt == VT_UNKNOWN, "Unexpected value type: %u.\n", value.vt);
-        hr = IUnknown_QueryInterface(value.punkVal, &IID_IWICMetadataQueryWriter, (void **)&query_writer2);
-        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
         hr = IWICMetadataQueryWriter_GetLocation(query_writer2, ARRAY_SIZE(path), path, &length);
         ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
         ok(!lstrcmpW(path, L"/app1/ifd"), "Unexpected path %s.\n", wine_dbgstr_w(path));
         IWICMetadataQueryWriter_Release(query_writer2);
-        PropVariantClear(&value);
     }
+    PropVariantClear(&value);

PropVariantInit(&value);
     hr = IWICMetadataQueryWriter_GetMetadataByName(query_writer, L"/app1/ifd/gps", &value);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(value.vt == VT_UNKNOWN, "Unexpected value type: %u.\n", value.vt);
+    hr = IUnknown_QueryInterface(value.punkVal, &IID_IWICMetadataQueryWriter, (void **)&query_writer2);
     todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     if (hr == S_OK)
     {
-        ok(value.vt == VT_UNKNOWN, "Unexpected value type: %u.\n", value.vt);
-        hr = IUnknown_QueryInterface(value.punkVal, &IID_IWICMetadataQueryWriter, (void **)&query_writer2);
-        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
         hr = IWICMetadataQueryWriter_GetLocation(query_writer2, ARRAY_SIZE(path), path, &length);
         ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
         ok(!lstrcmpW(path, L"/app1/ifd/gps"), "Unexpected path %s.\n", wine_dbgstr_w(path));
         IWICMetadataQueryWriter_Release(query_writer2);
-        PropVariantClear(&value);
     }
+    PropVariantClear(&value);

PropVariantInit(&value);
     hr = IWICMetadataQueryWriter_GetMetadataByName(query_writer, L"/app1/ifd/exif", &value);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(value.vt == VT_UNKNOWN, "Unexpected value type: %u.\n", value.vt);
+    hr = IUnknown_QueryInterface(value.punkVal, &IID_IWICMetadataQueryWriter, (void **)&query_writer2);
     todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     if (hr == S_OK)
     {
-        ok(value.vt == VT_UNKNOWN, "Unexpected value type: %u.\n", value.vt);
-        hr = IUnknown_QueryInterface(value.punkVal, &IID_IWICMetadataQueryWriter, (void **)&query_writer2);
-        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
         hr = IWICMetadataQueryWriter_GetLocation(query_writer2, ARRAY_SIZE(path), path, &length);
         ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
         ok(!lstrcmpW(path, L"/app1/ifd/exif"), "Unexpected path %s.\n", wine_dbgstr_w(path));
         IWICMetadataQueryWriter_Release(query_writer2);
-        PropVariantClear(&value);
     }
+    PropVariantClear(&value);

PropVariantInit(&value);
     hr = IWICMetadataQueryWriter_GetMetadataByName(query_writer, L"/app1/ifd/exif/{ushort=512}", &value);
-    todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-    if (hr == S_OK)
-    {
-        ok(value.vt == VT_UI2, "Unexpected value type: %u.\n", value.vt);
-        ok(value.ulVal == 444, "Unexpected value %lu.\n", value.ulVal);
-    }
+    ok(value.vt == VT_UI2, "Unexpected value type: %u.\n", value.vt);
+    ok(value.ulVal == 444, "Unexpected value %lu.\n", value.ulVal);

PropVariantInit(&value);
     hr = IWICMetadataQueryWriter_GetMetadataByName(query_writer, L"/app1/ifd/gps/{ushort=768}", &value);
-    todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-    if (hr == S_OK)
-    {
-        ok(value.vt == VT_UI2, "Unexpected value type: %u.\n", value.vt);
-        ok(value.ulVal == 555, "Unexpected value %lu.\n", value.ulVal);
-    }
+    ok(value.vt == VT_UI2, "Unexpected value type: %u.\n", value.vt);
+    ok(value.ulVal == 555, "Unexpected value %lu.\n", value.ulVal);

PropVariantInit(&value);
     hr = IWICMetadataQueryWriter_GetMetadataByName(query_writer, L"/app1/ifd/{ushort=256}", &value);
-    todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-    if (hr == S_OK)
-    {
-        ok(value.vt == VT_UI4, "Unexpected value type: %u.\n", value.vt);
-        ok(value.ulVal == 222, "Unexpected value %lu.\n", value.ulVal);
-    }
+    ok(value.vt == VT_UI4, "Unexpected value type: %u.\n", value.vt);
+    ok(value.ulVal == 222, "Unexpected value %lu.\n", value.ulVal);

PropVariantInit(&value);
     hr = IWICMetadataQueryWriter_GetMetadataByName(query_writer, L"/app1/ifd/{ushort=34665}", &value);
-    todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-    if (hr == S_OK)
-    {
-        ok(value.vt == VT_UNKNOWN, "Unexpected value type: %u.\n", value.vt);
-        check_interface(value.punkVal, &IID_IWICMetadataQueryReader, TRUE);
-        check_interface(value.punkVal, &IID_IWICMetadataQueryWriter, TRUE);
-        PropVariantClear(&value);
-    }
+    ok(value.vt == VT_UNKNOWN, "Unexpected value type: %u.\n", value.vt);
+    todo_wine
+    check_interface(value.punkVal, &IID_IWICMetadataQueryReader, TRUE);
+    todo_wine
+    check_interface(value.punkVal, &IID_IWICMetadataQueryWriter, TRUE);
+    PropVariantClear(&value);

IWICMetadataQueryWriter_Release(query_writer);

[PATCH v2 03/10] windowscodecs/tests: Add some tests for CreateQueryWriter().

[Nikolay Sivov]

From: Nikolay Sivov

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

diff --git a/dlls/windowscodecs/tests/metadata.c b/dlls/windowscodecs/tests/metadata.c
index dcdc47f931f..307caf267bc 100644
--- a/dlls/windowscodecs/tests/metadata.c
+++ b/dlls/windowscodecs/tests/metadata.c
@@ -804,6 +804,7 @@ static void test_metadata_unknown(void)

IWICMetadataReader_Release(reader);

+    /* "Unknown" writer. */
     hr = CoCreateInstance(&CLSID_WICUnknownMetadataWriter, NULL, CLSCTX_INPROC_SERVER,
             &IID_IWICMetadataWriter, (void **)&writer);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
@@ -815,6 +816,28 @@ static void test_metadata_unknown(void)
     check_interface(writer, &IID_IWICPersistStream, TRUE);
     check_interface(writer, &IID_IWICStreamProvider, TRUE);

+    hr = IWICMetadataWriter_GetCount(writer, &count);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    todo_wine
+    ok(count == 1, "Unexpected count %u.\n", count);
+
+    PropVariantInit(&schema);
+    PropVariantInit(&value);
+
+    id.vt = VT_EMPTY;
+    V_UI4(&id) = 123;
+    hr = IWICMetadataWriter_SetValue(writer, &schema, &id, &value);
+    todo_wine
+    ok(hr == E_INVALIDARG, "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);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(count == 1, "Unexpected count %u.\n", count);
+
     load_stream(writer, metadata_unknown, sizeof(metadata_unknown), 0);

hr = IWICMetadataWriter_GetCount(writer, &count);
@@ -979,6 +1002,10 @@ static void test_metadata_tEXt(void)
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     if (FAILED(hr)) return;

+    hr = IWICMetadataWriter_GetCount(writer, &count);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(!count, "Unexpected count %u.\n", count);
+
     check_interface(writer, &IID_IWICMetadataWriter, TRUE);
     check_interface(writer, &IID_IWICMetadataReader, TRUE);
     check_interface(writer, &IID_IPersist, TRUE);
@@ -1062,6 +1089,10 @@ static void test_metadata_gAMA(void)
     check_interface(writer, &IID_IWICPersistStream, TRUE);
     check_interface(writer, &IID_IWICStreamProvider, TRUE);

+    hr = IWICMetadataWriter_GetCount(writer, &count);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(count == 1, "Unexpected count %u.\n", count);
+
     load_stream(writer, metadata_gAMA, sizeof(metadata_gAMA), 0);
     load_stream(writer, metadata_gAMA, sizeof(metadata_gAMA), WICPersistOptionNoCacheStream);

@@ -1149,6 +1180,10 @@ static void test_metadata_cHRM(void)
     check_interface(writer, &IID_IWICPersistStream, TRUE);
     check_interface(writer, &IID_IWICStreamProvider, TRUE);

+    hr = IWICMetadataWriter_GetCount(writer, &count);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(count == 8, "Unexpected count %u.\n", count);
+
     load_stream(writer, metadata_cHRM, sizeof(metadata_cHRM), 0);
     load_stream(writer, metadata_cHRM, sizeof(metadata_cHRM), WICPersistOptionNoCacheStream);

@@ -1220,6 +1255,10 @@ static void test_metadata_hIST(void)
     check_interface(writer, &IID_IWICPersistStream, TRUE);
     check_interface(writer, &IID_IWICStreamProvider, TRUE);

+    hr = IWICMetadataWriter_GetCount(writer, &count);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(count == 1, "Unexpected count %u.\n", count);
+
     load_stream(writer, metadata_hIST, sizeof(metadata_hIST), 0);
     load_stream(writer, metadata_hIST, sizeof(metadata_hIST), WICPersistOptionNoCacheStream);

@@ -1281,6 +1320,10 @@ static void test_metadata_tIME(void)
     check_interface(writer, &IID_IWICPersistStream, TRUE);
     check_interface(writer, &IID_IWICStreamProvider, TRUE);

+    hr = IWICMetadataWriter_GetCount(writer, &count);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(count == 6, "Unexpected count %u.\n", count);
+
     load_stream(writer, metadata_tIME, sizeof(metadata_tIME), 0);
     load_stream(writer, metadata_tIME, sizeof(metadata_tIME), WICPersistOptionNoCacheStream);

@@ -1623,6 +1666,10 @@ static void test_metadata_Ifd(void)
     check_interface(writer, &IID_IWICPersistStream, TRUE);
     check_interface(writer, &IID_IWICStreamProvider, TRUE);

+    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);

@@ -1675,6 +1722,10 @@ static void test_metadata_Exif(void)
     check_interface(writer, &IID_IWICPersistStream, TRUE);
     check_interface(writer, &IID_IWICStreamProvider, TRUE);

+    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);

@@ -1727,6 +1778,10 @@ static void test_metadata_Gps(void)
     check_interface(writer, &IID_IWICPersistStream, TRUE);
     check_interface(writer, &IID_IWICStreamProvider, TRUE);

+    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);

@@ -2805,6 +2860,10 @@ static void test_metadata_LSD(void)
     check_interface(writer, &IID_IWICPersistStream, TRUE);
     check_interface(writer, &IID_IWICStreamProvider, TRUE);

+    hr = IWICMetadataWriter_GetCount(writer, &count);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(!count, "Unexpected count %u.\n", count);
+
     load_stream(writer, LSD_data, sizeof(LSD_data), 0);
     load_stream(writer, LSD_data, sizeof(LSD_data), WICPersistOptionNoCacheStream);

@@ -2905,6 +2964,10 @@ static void test_metadata_IMD(void)
     check_interface(writer, &IID_IWICPersistStream, TRUE);
     check_interface(writer, &IID_IWICStreamProvider, TRUE);

+    hr = IWICMetadataWriter_GetCount(writer, &count);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(!count, "Unexpected count %u.\n", count);
+
     load_stream(writer, IMD_data, sizeof(IMD_data), 0);
     load_stream(writer, IMD_data, sizeof(IMD_data), WICPersistOptionNoCacheStream);

@@ -3002,6 +3065,10 @@ static void test_metadata_GCE(void)
     check_interface(writer, &IID_IWICPersistStream, TRUE);
     check_interface(writer, &IID_IWICStreamProvider, TRUE);

+    hr = IWICMetadataWriter_GetCount(writer, &count);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(count == 5, "Unexpected count %u.\n", count);
+
     load_stream(writer, GCE_data, sizeof(GCE_data), 0);
     load_stream(writer, GCE_data, sizeof(GCE_data), WICPersistOptionNoCacheStream);

@@ -3109,6 +3176,10 @@ static void test_metadata_APE(void)
     check_interface(writer, &IID_IWICPersistStream, TRUE);
     check_interface(writer, &IID_IWICStreamProvider, TRUE);

+    hr = IWICMetadataWriter_GetCount(writer, &count);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(count == 2, "Unexpected count %u.\n", count);
+
     load_stream(writer, APE_data, sizeof(APE_data), 0);
     load_stream(writer, APE_data, sizeof(APE_data), WICPersistOptionNoCacheStream);

@@ -3213,6 +3284,10 @@ static void test_metadata_GIF_comment(void)
     check_interface(writer, &IID_IWICPersistStream, TRUE);
     check_interface(writer, &IID_IWICStreamProvider, TRUE);

+    hr = IWICMetadataWriter_GetCount(writer, &count);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(count == 1, "Unexpected count %u.\n", count);
+
     load_stream(writer, GIF_comment_data, sizeof(GIF_comment_data), 0);
     load_stream(writer, GIF_comment_data, sizeof(GIF_comment_data), WICPersistOptionNoCacheStream);

@@ -5075,6 +5150,10 @@ static void test_metadata_App1(void)
     check_interface(writer, &IID_IWICPersistStream, TRUE);
     check_interface(writer, &IID_IWICStreamProvider, TRUE);

+    hr = IWICMetadataWriter_GetCount(writer, &count);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(!count, "Unexpected count %u.\n", count);
+
     IWICMetadataWriter_Release(writer);

IWICComponentFactory_Release(factory);
@@ -5496,6 +5575,87 @@ static void test_metadata_writer(void)
     IWICComponentFactory_Release(factory);
 }

+static void test_CreateQueryWriter(void)
+{
+    IWICMetadataQueryWriter *query_writer;
+    IWICImagingFactory *factory;
+    IEnumString *enum_string;
+    WCHAR buff[64], *str;
+    PROPVARIANT value;
+    ULONG fetched;
+    GUID format;
+    HRESULT hr;
+    UINT len;
+
+    hr = CoCreateInstance(&CLSID_WICImagingFactory, NULL, CLSCTX_INPROC_SERVER,
+            &IID_IWICImagingFactory, (void **)&factory);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    /* "Unknown" format. */
+    hr = IWICImagingFactory_CreateQueryWriter(factory, &GUID_MetadataFormatUnknown, NULL, &query_writer);
+    todo_wine
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    if (FAILED(hr))
+    {
+        IWICImagingFactory_Release(factory);
+        return;
+    }
+
+    hr = IWICMetadataQueryWriter_GetLocation(query_writer, ARRAY_SIZE(buff), buff, &len);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(!wcscmp(buff, L"/unknown"), "Unexpected location %s.\n", wine_dbgstr_w(buff));
+
+    hr = IWICMetadataQueryWriter_GetContainerFormat(query_writer, &format);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(IsEqualGUID(&format, &GUID_MetadataFormatUnknown), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
+
+    PropVariantInit(&value);
+    hr = IWICMetadataQueryWriter_GetMetadataByName(query_writer, L"/", &value);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(value.vt == VT_BLOB, "Unexpected value type %u.\n", value.vt);
+    ok(!value.blob.cbSize, "Unexpected size %lu.\n", value.blob.cbSize);
+    ok(!value.blob.pBlobData, "Unexpected data pointer %p.\n", value.blob.pBlobData);
+    PropVariantClear(&value);
+
+    hr = IWICMetadataQueryWriter_GetEnumerator(query_writer, &enum_string);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    hr = IEnumString_Next(enum_string, 1, &str, &fetched);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(!wcscmp(str, L"/{}"), "Unexpected string %s.\n", wine_dbgstr_w(str));
+    CoTaskMemFree(str);
+    hr = IEnumString_Next(enum_string, 1, &str, &fetched);
+    ok(hr == S_FALSE, "Unexpected hr %#lx.\n", hr);
+    IEnumString_Release(enum_string);
+
+    IWICMetadataQueryWriter_Release(query_writer);
+
+    /* App1 */
+    hr = IWICImagingFactory_CreateQueryWriter(factory, &GUID_MetadataFormatApp1, NULL, &query_writer);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = IWICMetadataQueryWriter_GetLocation(query_writer, ARRAY_SIZE(buff), buff, &len);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(!wcscmp(buff, L"/app1"), "Unexpected location %s.\n", wine_dbgstr_w(buff));
+
+    hr = IWICMetadataQueryWriter_GetContainerFormat(query_writer, &format);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(IsEqualGUID(&format, &GUID_MetadataFormatApp1), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
+
+    PropVariantInit(&value);
+    hr = IWICMetadataQueryWriter_GetMetadataByName(query_writer, L"/", &value);
+    ok(hr == WINCODEC_ERR_PROPERTYNOTFOUND, "Unexpected hr %#lx.\n", hr);
+
+    hr = IWICMetadataQueryWriter_GetEnumerator(query_writer, &enum_string);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    hr = IEnumString_Next(enum_string, 1, &str, &fetched);
+    ok(hr == S_FALSE, "Unexpected hr %#lx.\n", hr);
+    IEnumString_Release(enum_string);
+
+    IWICMetadataQueryWriter_Release(query_writer);
+
+    IWICImagingFactory_Release(factory);
+}
+
 START_TEST(metadata)
 {
     CoInitializeEx(NULL, COINIT_APARTMENTTHREADED);
@@ -5527,6 +5687,7 @@ START_TEST(metadata)
     test_CreateMetadataWriterFromReader();
     test_CreateMetadataWriter();
     test_metadata_writer();
+    test_CreateQueryWriter();

CoUninitialize();
 }

[PATCH v2 04/10] windowscodecs/tests: Add query reader tests for live block reader updates.

[Nikolay Sivov]

From: Nikolay Sivov

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

diff --git a/dlls/windowscodecs/tests/metadata.c b/dlls/windowscodecs/tests/metadata.c
index 307caf267bc..3cf48ad0996 100644
--- a/dlls/windowscodecs/tests/metadata.c
+++ b/dlls/windowscodecs/tests/metadata.c
@@ -4258,27 +4258,36 @@ static HRESULT create_query_writer_from_metadata_reader(IWICComponentFactory *fa
     return hr;
 }

-static HRESULT create_query_reader(IWICComponentFactory *factory, const struct metadata *data,
-        IWICMetadataQueryReader **reader)
+static HRESULT create_test_block_writer_init(const struct metadata *data, IWICMetadataBlockWriter **block_writer)
 {
-    IWICMetadataBlockWriter *block_writer;
     IWICMetadataWriter *writer;
     unsigned int i;
     HRESULT hr;

-    hr = create_test_block_writer(data->container_format, &block_writer);
+    hr = create_test_block_writer(data->container_format, block_writer);
     if (SUCCEEDED(hr))
     {
         for (i = 0; i < data->count; ++i)
         {
             writer = create_test_writer(&data->block[i]);
             ok(!!writer, "Failed to create a writer.\n");
-            hr = IWICMetadataBlockWriter_AddWriter(block_writer, writer);
+            hr = IWICMetadataBlockWriter_AddWriter(*block_writer, writer);
             ok(hr == S_OK, "Failed to add a writer, hr %#lx.\n", hr);
             IWICMetadataWriter_Release(writer);
         }
     }

+    return hr;
+}
+
+static HRESULT create_query_reader(IWICComponentFactory *factory, const struct metadata *data,
+        IWICMetadataQueryReader **reader)
+{
+    IWICMetadataBlockWriter *block_writer;
+    HRESULT hr;
+
+    hr = create_test_block_writer_init(data, &block_writer);
+
     if (SUCCEEDED(hr))
     {
         hr = IWICComponentFactory_CreateQueryReaderFromBlockReader(factory,
@@ -4337,12 +4346,16 @@ static void test_queryreader(void)
         { TRUE, &data1, L"/ifd/[0]Rating", E_INVALIDARG },
         { TRUE, &data1, L"/ifd/[*]Rating", WINCODEC_ERR_REQUESTONLYVALIDATMETADATAROOT },
     };
+    IWICMetadataBlockWriter *block_writer;
     HRESULT hr;
     IWICComponentFactory *factory;
     IWICMetadataQueryReader *reader;
+    IWICMetadataWriter *writer;
+    IEnumString *enum_string;
     GUID format;
     PROPVARIANT value;
-    UINT i;
+    UINT i, count;
+    WCHAR *str;

hr = CoCreateInstance(&CLSID_WICImagingFactory, NULL, CLSCTX_INPROC_SERVER,
             &IID_IWICComponentFactory, (void **)&factory);
@@ -4433,6 +4446,71 @@ static void test_queryreader(void)
         winetest_pop_context();
     }

+    /* Modify block set after query reader was created. */
+    hr = create_test_block_writer_init(&data1, &block_writer);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    hr = IWICMetadataBlockWriter_GetCount(block_writer, &count);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(count == 1, "Unexpected count %u.\n", count);
+
+    hr = IWICComponentFactory_CreateQueryReaderFromBlockReader(factory,
+            (IWICMetadataBlockReader *)block_writer, &reader);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = IWICMetadataQueryReader_GetEnumerator(reader, &enum_string);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    hr = IEnumString_Next(enum_string, 1, &str, NULL);
+    todo_wine
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    if (hr == S_OK)
+    {
+        ok(!wcscmp(str, L"/ifd"), "Unexpected query %s.\n", wine_dbgstr_w(str));
+        CoTaskMemFree(str);
+    }
+    hr = IEnumString_Next(enum_string, 1, &str, NULL);
+    todo_wine
+    ok(hr == S_FALSE, "Unexpected hr %#lx.\n", hr);
+
+    hr = IWICComponentFactory_CreateMetadataWriter(factory, &GUID_MetadataFormatApp1, NULL, 0, &writer);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    hr = IWICMetadataBlockWriter_AddWriter(block_writer, writer);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    IWICMetadataWriter_Release(writer);
+    hr = IWICMetadataBlockWriter_GetCount(block_writer, &count);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(count == 2, "Unexpected count %u.\n", count);
+
+    /* Existing enumerator does not pick it up. */
+    hr = IEnumString_Next(enum_string, 1, &str, NULL);
+    todo_wine
+    ok(hr == S_FALSE, "Unexpected hr %#lx.\n", hr);
+    IEnumString_Release(enum_string);
+
+    hr = IWICMetadataQueryReader_GetEnumerator(reader, &enum_string);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    hr = IEnumString_Next(enum_string, 1, &str, NULL);
+    todo_wine
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    if (hr == S_OK)
+    {
+        ok(!wcscmp(str, L"/ifd"), "Unexpected query %s.\n", wine_dbgstr_w(str));
+        CoTaskMemFree(str);
+    }
+    hr = IEnumString_Next(enum_string, 1, &str, NULL);
+    todo_wine
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    if (hr == S_OK)
+    {
+        ok(!wcscmp(str, L"/app1"), "Unexpected query %s.\n", wine_dbgstr_w(str));
+        CoTaskMemFree(str);
+    }
+
+    IEnumString_Release(enum_string);
+
+    IWICMetadataQueryReader_Release(reader);
+
+    IWICMetadataBlockWriter_Release(block_writer);
+
     IWICComponentFactory_Release(factory);
 }

[PATCH v2 05/10] windowscodecs/tests: Add some tests for CreateQueryWriterFromReader().

[Nikolay Sivov]

From: Nikolay Sivov

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

diff --git a/dlls/windowscodecs/tests/metadata.c b/dlls/windowscodecs/tests/metadata.c
index 3cf48ad0996..b8c095b04f3 100644
--- a/dlls/windowscodecs/tests/metadata.c
+++ b/dlls/windowscodecs/tests/metadata.c
@@ -5734,6 +5734,143 @@ static void test_CreateQueryWriter(void)
     IWICImagingFactory_Release(factory);
 }

+static HRESULT create_query_writer(IWICComponentFactory *factory, const struct metadata *data,
+        IWICMetadataQueryWriter **writer)
+{
+    IWICMetadataBlockWriter *block_writer;
+    HRESULT hr;
+
+    hr = create_test_block_writer_init(data, &block_writer);
+
+    if (SUCCEEDED(hr))
+    {
+        hr = IWICComponentFactory_CreateQueryWriterFromBlockWriter(factory, block_writer, writer);
+        IWICMetadataBlockWriter_Release(block_writer);
+    }
+
+    return hr;
+}
+
+static void test_CreateQueryWriterFromReader(void)
+{
+    IWICMetadataBlockReader *block_reader;
+    IWICMetadataQueryWriter *query_writer;
+    IWICMetadataQueryWriter *query_writer2;
+    IWICMetadataQueryReader *query_reader, *query_reader2;
+    IWICBitmapFrameDecode *decoder_frame;
+    IWICComponentFactory *factory;
+    IWICBitmapDecoder *decoder;
+    IWICMetadataReader *reader;
+    PROPVARIANT value;
+    IStream *stream;
+    WCHAR buff[64];
+    UINT count, len;
+    HRESULT hr;
+
+    hr = CoCreateInstance(&CLSID_WICImagingFactory, NULL, CLSCTX_INPROC_SERVER,
+            &IID_IWICComponentFactory, (void **)&factory);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    /* Using query reader created with CreateQueryReaderFromBlockReader().
+
+       It's not allowed to create the writer for top level reader. */
+    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_reader_from_metadata_reader(factory, reader, &GUID_ContainerFormatJpeg, &query_reader);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = IWICComponentFactory_CreateQueryWriterFromReader(factory, query_reader, NULL, &query_writer);
+    todo_wine
+    ok(hr == WINCODEC_ERR_UNEXPECTEDMETADATATYPE, "Unexpected hr %#lx.\n", hr);
+
+    PropVariantInit(&value);
+    hr = IWICMetadataQueryReader_GetMetadataByName(query_reader, L"/app1/ifd", &value);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(value.vt == VT_UNKNOWN, "Unexpected type %d.\n", value.vt);
+
+    hr = IUnknown_QueryInterface(value.punkVal, &IID_IWICMetadataQueryReader, (void **)&query_reader2);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = IWICComponentFactory_CreateQueryWriterFromReader(factory, query_reader2, NULL, &query_writer);
+    todo_wine
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+if (hr == S_OK)
+{
+    hr = IWICMetadataQueryWriter_GetLocation(query_writer, ARRAY_SIZE(buff), buff, &len);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(!wcscmp(buff, L"/ifd"), "Unexpected location %s.\n", wine_dbgstr_w(buff));
+    IWICMetadataQueryWriter_Release(query_writer);
+}
+
+    IWICMetadataQueryReader_Release(query_reader);
+    IWICMetadataQueryReader_Release(query_reader2);
+
+    PropVariantClear(&value);
+
+    /* IWICMetadataReader -> IWICMetadataWriter -> IWICMetadataQueryWriter -> CreateQueryWriterFromReader(). */
+    hr = create_query_writer_from_metadata_reader(factory, reader, &GUID_ContainerFormatJpeg, &query_writer);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = IWICComponentFactory_CreateQueryWriter(factory, &GUID_MetadataFormatChunktIME, NULL, &query_writer);
+    todo_wine
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = IWICComponentFactory_CreateQueryWriterFromReader(factory, (IWICMetadataQueryReader *)query_writer,
+            NULL, &query_writer2);
+    todo_wine
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = create_query_writer(factory, &data1, &query_writer);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    if (hr == S_OK)
+        IWICMetadataQueryWriter_Release(query_writer);
+
+    /* Using decoder frame. */
+    hr = CoCreateInstance(&CLSID_WICPngDecoder, NULL, CLSCTX_INPROC_SERVER,
+            &IID_IWICBitmapDecoder, (void **)&decoder);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    stream = create_stream(pngimage, sizeof(pngimage));
+    hr = IWICBitmapDecoder_Initialize(decoder, stream, WICDecodeMetadataCacheOnLoad);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    IStream_Release(stream);
+
+    hr = IWICBitmapDecoder_GetFrame(decoder, 0, &decoder_frame);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    hr = IWICBitmapFrameDecode_QueryInterface(decoder_frame, &IID_IWICMetadataBlockReader, (void **)&block_reader);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = IWICBitmapFrameDecode_GetMetadataQueryReader(decoder_frame, &query_reader);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = IWICComponentFactory_CreateQueryWriterFromReader(factory, query_reader, &GUID_VendorMicrosoft, &query_writer);
+    todo_wine
+    ok(hr == WINCODEC_ERR_UNEXPECTEDMETADATATYPE, "Unexpected hr %#lx.\n", hr);
+
+    IWICMetadataQueryReader_Release(query_reader);
+    IWICBitmapFrameDecode_Release(decoder_frame);
+
+    hr = IWICMetadataBlockReader_GetCount(block_reader, &count);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(count == 1, "Unexpected count %u.\n", count);
+
+    hr = IWICComponentFactory_CreateQueryReaderFromBlockReader(factory, block_reader, &query_reader);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+
+    hr = IWICComponentFactory_CreateQueryWriterFromReader(factory, query_reader, &GUID_VendorMicrosoft, &query_writer);
+    todo_wine
+    ok(hr == WINCODEC_ERR_UNEXPECTEDMETADATATYPE, "Unexpected hr %#lx.\n", hr);
+    IWICMetadataQueryReader_Release(query_reader);
+
+    IWICMetadataReader_Release(reader);
+
+    IWICComponentFactory_Release(factory);
+}
+
 START_TEST(metadata)
 {
     CoInitializeEx(NULL, COINIT_APARTMENTTHREADED);
@@ -5766,6 +5903,7 @@ START_TEST(metadata)
     test_CreateMetadataWriter();
     test_metadata_writer();
     test_CreateQueryWriter();
+    test_CreateQueryWriterFromReader();

CoUninitialize();
 }

[PATCH v2 06/10] windowscodecs/tests: Add some tests for the query reader container format.

[Nikolay Sivov]

From: Nikolay Sivov

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

diff --git a/dlls/windowscodecs/tests/metadata.c b/dlls/windowscodecs/tests/metadata.c
index b8c095b04f3..ec236928d74 100644
--- a/dlls/windowscodecs/tests/metadata.c
+++ b/dlls/windowscodecs/tests/metadata.c
@@ -5024,6 +5024,9 @@ static void test_metadata_App1(void)
     hr = IWICMetadataQueryReader_GetLocation(query_reader, ARRAY_SIZE(path), path, &length);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ok(!lstrcmpW(path, L"/"), "Unexpected path %s.\n", wine_dbgstr_w(path));
+    hr = IWICMetadataQueryReader_GetContainerFormat(query_reader, &format);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(IsEqualGUID(&format, &GUID_ContainerFormatJpeg), "Unexpected format %s.\n", wine_dbgstr_guid(&format));

PropVariantInit(&value);
     hr = IWICMetadataQueryReader_GetMetadataByName(query_reader, L"/app1", &value);
@@ -5037,6 +5040,10 @@ static void test_metadata_App1(void)
     hr = IWICMetadataQueryReader_GetLocation(query_reader2, ARRAY_SIZE(path), path, &length);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ok(!lstrcmpW(path, L"/app1"), "Unexpected path %s.\n", wine_dbgstr_w(path));
+    hr = IWICMetadataQueryReader_GetContainerFormat(query_reader2, &format);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    todo_wine
+    ok(IsEqualGUID(&format, &GUID_MetadataFormatApp1), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
     IWICMetadataQueryReader_Release(query_reader2);
     PropVariantClear(&value);

@@ -5049,6 +5056,10 @@ static void test_metadata_App1(void)
     hr = IWICMetadataQueryReader_GetLocation(query_reader2, ARRAY_SIZE(path), path, &length);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ok(!lstrcmpW(path, L"/app1/ifd"), "Unexpected path %s.\n", wine_dbgstr_w(path));
+    hr = IWICMetadataQueryReader_GetContainerFormat(query_reader2, &format);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    todo_wine
+    ok(IsEqualGUID(&format, &GUID_MetadataFormatIfd), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
     IWICMetadataQueryReader_Release(query_reader2);
     PropVariantClear(&value);

@@ -5061,6 +5072,10 @@ static void test_metadata_App1(void)
     hr = IWICMetadataQueryReader_GetLocation(query_reader2, ARRAY_SIZE(path), path, &length);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ok(!lstrcmpW(path, L"/app1/ifd/gps"), "Unexpected path %s.\n", wine_dbgstr_w(path));
+    hr = IWICMetadataQueryReader_GetContainerFormat(query_reader2, &format);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    todo_wine
+    ok(IsEqualGUID(&format, &GUID_MetadataFormatGps), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
     IWICMetadataQueryReader_Release(query_reader2);
     PropVariantClear(&value);

@@ -5073,6 +5088,10 @@ static void test_metadata_App1(void)
     hr = IWICMetadataQueryReader_GetLocation(query_reader2, ARRAY_SIZE(path), path, &length);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ok(!lstrcmpW(path, L"/app1/ifd/exif"), "Unexpected path %s.\n", wine_dbgstr_w(path));
+    hr = IWICMetadataQueryReader_GetContainerFormat(query_reader2, &format);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    todo_wine
+    ok(IsEqualGUID(&format, &GUID_MetadataFormatExif), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
     IWICMetadataQueryReader_Release(query_reader2);
     PropVariantClear(&value);

@@ -5149,6 +5168,9 @@ static void test_metadata_App1(void)
         hr = IWICMetadataQueryWriter_GetLocation(query_writer2, ARRAY_SIZE(path), path, &length);
         ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
         ok(!lstrcmpW(path, L"/app1/ifd"), "Unexpected path %s.\n", wine_dbgstr_w(path));
+        hr = IWICMetadataQueryWriter_GetContainerFormat(query_writer2, &format);
+        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+        ok(IsEqualGUID(&format, &GUID_MetadataFormatIfd), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
         IWICMetadataQueryWriter_Release(query_writer2);
     }
     PropVariantClear(&value);
@@ -5165,6 +5187,9 @@ static void test_metadata_App1(void)
         hr = IWICMetadataQueryWriter_GetLocation(query_writer2, ARRAY_SIZE(path), path, &length);
         ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
         ok(!lstrcmpW(path, L"/app1/ifd/gps"), "Unexpected path %s.\n", wine_dbgstr_w(path));
+        hr = IWICMetadataQueryWriter_GetContainerFormat(query_writer2, &format);
+        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+        ok(IsEqualGUID(&format, &GUID_MetadataFormatGps), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
         IWICMetadataQueryWriter_Release(query_writer2);
     }
     PropVariantClear(&value);
@@ -5181,6 +5206,9 @@ static void test_metadata_App1(void)
         hr = IWICMetadataQueryWriter_GetLocation(query_writer2, ARRAY_SIZE(path), path, &length);
         ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
         ok(!lstrcmpW(path, L"/app1/ifd/exif"), "Unexpected path %s.\n", wine_dbgstr_w(path));
+        hr = IWICMetadataQueryWriter_GetContainerFormat(query_writer2, &format);
+        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+        ok(IsEqualGUID(&format, &GUID_MetadataFormatExif), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
         IWICMetadataQueryWriter_Release(query_writer2);
     }
     PropVariantClear(&value);

[PATCH v2 07/10] windowscodecs/metadata: Split up query parsing into helpers according to the query grammar.

[Nikolay Sivov]

From: Nikolay Sivov

Signed-off-by: Nikolay Sivov 
---
 dlls/windowscodecs/metadataquery.c  | 686 ++++++++++++++++------------
 dlls/windowscodecs/tests/metadata.c |  78 ++--
 2 files changed, 417 insertions(+), 347 deletions(-)

diff --git a/dlls/windowscodecs/metadataquery.c b/dlls/windowscodecs/metadataquery.c
index b1235dbda6f..25c7e553e62 100644
--- a/dlls/windowscodecs/metadataquery.c
+++ b/dlls/windowscodecs/metadataquery.c
@@ -194,421 +194,509 @@ static VARTYPE map_type(struct string_t *str)
     return VT_ILLEGAL;
 }

-static HRESULT get_token(struct string_t *elem, PROPVARIANT *id, PROPVARIANT *schema, int *idx)
+struct query_component
 {
-    const WCHAR *start, *end, *p;
-    WCHAR *bstr;
-    struct string_t next_elem;
+    unsigned int index;
+    PROPVARIANT schema;
+    PROPVARIANT id;
+    IWICMetadataReader *handler;
+};
+
+struct query_parser
+{
+    const WCHAR *ptr;
+
+    WCHAR *scratch;
+    WCHAR *query;
+
+    struct query_component *components;
+    size_t count;
+    size_t capacity;
+
+    struct query_component *last;
+    struct query_component *prev;
+
     HRESULT hr;
+};

-    TRACE("%s, len %d\n", wine_dbgstr_wn(elem->str, elem->len), elem->len);
+static bool wincodecs_array_reserve(void **elements, size_t *capacity, size_t count, size_t size)
+{
+    size_t new_capacity, max_capacity;
+    void *new_elements;

-    PropVariantInit(id);
-    PropVariantInit(schema);
+    if (count <= *capacity)
+        return true;

-    if (!elem->len) return S_OK;
+    max_capacity = ~(SIZE_T)0 / size;
+    if (count > max_capacity)
+        return false;

-    start = elem->str;
+    new_capacity = max(4, *capacity);
+    while (new_capacity < count && new_capacity <= max_capacity / 2)
+        new_capacity *= 2;
+    if (new_capacity < count)
+        new_capacity = max_capacity;

-    if (*start == '[')
-    {
-        WCHAR *idx_end;
+    new_elements = realloc(*elements, new_capacity * size);
+    if (!new_elements)
+        return false;

-        if (start[1] < '0' || start[1] > '9') return DISP_E_TYPEMISMATCH;
+    *elements = new_elements;
+    *capacity = new_capacity;

-        *idx = wcstol(start + 1, &idx_end, 10);
-        if (idx_end > elem->str + elem->len) return WINCODEC_ERR_INVALIDQUERYREQUEST;
-        if (*idx_end != ']') return WINCODEC_ERR_INVALIDQUERYREQUEST;
-        if (*idx < 0) return WINCODEC_ERR_INVALIDQUERYREQUEST;
-        end = idx_end + 1;
+    return true;
+}

-        next_elem.str = end;
-        next_elem.len = elem->len - (end - start);
-        hr = get_token(&next_elem, id, schema, idx);
-        if (hr != S_OK)
-        {
-            TRACE("get_token error %#lx\n", hr);
-            return hr;
-        }
-        elem->len = (end - start) + next_elem.len;
+static bool parser_skip_char(struct query_parser *parser, WCHAR ch)
+{
+    if (FAILED(parser->hr)) return true;

-        TRACE("indexed %s [%d]\n", wine_dbgstr_wn(elem->str, elem->len), *idx);
-        return S_OK;
-    }
-    else if (*start == '{')
+    if (*parser->ptr != ch)
     {
-        VARTYPE vt;
-        PROPVARIANT next_token;
+        parser->hr = WINCODEC_ERR_INVALIDQUERYREQUEST;
+        return true;
+    }
+    parser->ptr++;
+    return false;
+}

-        end = wmemchr(start + 1, '=', elem->len - 1);
-        if (!end) return WINCODEC_ERR_INVALIDQUERYREQUEST;
-        if (end > elem->str + elem->len) return WINCODEC_ERR_INVALIDQUERYREQUEST;
+static bool parser_unescape(struct query_parser *parser)
+{
+    if (*parser->ptr == '\')
+    {
+        parser->ptr++;
+        if (!*parser->ptr) return true;
+    }

-        next_elem.str = start + 1;
-        next_elem.len = end - start - 1;
-        vt = map_type(&next_elem);
-        TRACE("type %s => %d\n", wine_dbgstr_wn(next_elem.str, next_elem.len), vt);
-        if (vt == VT_ILLEGAL) return WINCODEC_ERR_WRONGSTATE;
+    return false;
+}

-        next_token.vt = VT_BSTR;
-        next_token.bstrVal = SysAllocStringLen(NULL, elem->len - (end - start) + 1);
-        if (!next_token.bstrVal) return E_OUTOFMEMORY;
+static void parse_query_index(struct query_parser *parser, unsigned int *ret)
+{
+    unsigned int index = 0, d;

-        bstr = next_token.bstrVal;
+    if (parser_skip_char(parser, '[')) return;

-        end++;
-        while (*end && *end != '}' && end - start < elem->len)
-        {
-            if (*end == '\') end++;
-            *bstr++ = *end++;
-        }
-        if (*end != '}')
-        {
-            PropVariantClear(&next_token);
-            return WINCODEC_ERR_INVALIDQUERYREQUEST;
-        }
-        *bstr = 0;
-        TRACE("schema/id %s\n", wine_dbgstr_w(next_token.bstrVal));
+    if (*parser->ptr == '*' && *(parser->ptr + 1) == ']')
+    {
+        FIXME("[*] index value is not supported.\n");
+        parser->ptr += 2;
+        parser->hr = E_UNEXPECTED;
+        return;
+    }

-        if (vt == VT_CLSID)
-        {
-            id->vt = VT_CLSID;
-            id->puuid = CoTaskMemAlloc(sizeof(GUID));
-            if (!id->puuid)
-            {
-                PropVariantClear(&next_token);
-                return E_OUTOFMEMORY;
-            }
+    /* Sign prefix is not allowed */

-            hr = UuidFromStringW(next_token.bstrVal, id->puuid);
-        }
+    while (*parser->ptr)
+    {
+        if (*parser->ptr >= '0' && *parser->ptr <= '9')
+            d = *parser->ptr - '0';
+        else if (*parser->ptr == ']')
+            break;
         else
-            hr = PropVariantChangeType(id, &next_token, 0, vt);
-        PropVariantClear(&next_token);
-        if (hr != S_OK)
         {
-            PropVariantClear(id);
-            PropVariantClear(schema);
-            return hr;
+            parser->hr = WINCODEC_ERR_INVALIDQUERYCHARACTER;
+            return;
         }

-        end++;
-        if (*end == ':')
-        {
-            PROPVARIANT next_id, next_schema;
-            int next_idx = 0;
+        index = index * 10 + d;
+        parser->ptr++;
+    }

-            next_elem.str = end + 1;
-            next_elem.len = elem->len - (end - start + 1);
-            hr = get_token(&next_elem, &next_id, &next_schema, &next_idx);
-            if (hr != S_OK)
-            {
-                TRACE("get_token error %#lx\n", hr);
-                return hr;
-            }
-            elem->len = (end - start + 1) + next_elem.len;
+    if (*parser->ptr != ']')
+    {
+        parser->hr = WINCODEC_ERR_INVALIDQUERYCHARACTER;
+        return;
+    }
+    parser_skip_char(parser, ']');

-            TRACE("id %s [%d]\n", wine_dbgstr_wn(elem->str, elem->len), *idx);
+    *ret = index;
+}

-            if (next_schema.vt != VT_EMPTY)
-            {
-                PropVariantClear(&next_id);
-                PropVariantClear(&next_schema);
-                return WINCODEC_ERR_WRONGSTATE;
-            }
+static void parse_query_data_item(struct query_parser *parser, PROPVARIANT *item)
+{
+    struct string_t span;
+    PROPVARIANT v;
+    GUID guid;
+    VARTYPE vt;

-            *schema = *id;
-            *id = next_id;
+    if (parser_skip_char(parser, '{')) return;

-            return S_OK;
+    /* Type */
+    span.str = parser->ptr;
+    span.len = 0;
+    while (*parser->ptr && *parser->ptr != '=')
+    {
+        span.len++;
+        parser->ptr++;
+    }
+
+    if (parser_skip_char(parser, '=')) return;
+
+    vt = map_type(&span);
+    if (vt == VT_ILLEGAL)
+    {
+        parser->hr = WINCODEC_ERR_WRONGSTATE;
+        return;
+    }
+
+    /* Value */
+    span.str = parser->ptr;
+    span.len = 0;
+    while (*parser->ptr && *parser->ptr != '}')
+    {
+        if (parser_unescape(parser)) break;
+        parser->scratch[span.len++] = *parser->ptr;
+        parser->ptr++;
+    }
+
+    if (parser_skip_char(parser, '}')) return;
+
+    parser->scratch[span.len] = 0;
+
+    if (vt == VT_CLSID)
+    {
+        if (UuidFromStringW(parser->scratch, &guid))
+        {
+            parser->hr = WINCODEC_ERR_INVALIDQUERYREQUEST;
+            return;
         }

-        elem->len = end - start;
-        return S_OK;
+        parser->hr = InitPropVariantFromCLSID(&guid, item);
     }
+    else
+    {
+        v.vt = VT_LPWSTR;
+        v.pwszVal = parser->scratch;
+        parser->hr = PropVariantChangeType(item, &v, 0, vt);
+    }
+}

-    end = wmemchr(start, '/', elem->len);
-    if (!end) end = start + elem->len;
+static HRESULT init_propvar_from_string(const WCHAR *str, PROPVARIANT *var)
+{
+    size_t size = (wcslen(str) + 1) * sizeof(*str);
+    WCHAR *s;

-    p = wmemchr(start, ':', end - start);
-    if (p)
+    if (!(s = CoTaskMemAlloc(size)))
+        return E_OUTOFMEMORY;
+    memcpy(s, str, size);
+
+    var->pwszVal = s;
+    var->vt = VT_LPWSTR;
+    return S_OK;
+}
+
+static void parse_query_name(struct query_parser *parser, PROPVARIANT *item)
+{
+    size_t len = 0;
+
+    while (*parser->ptr && (*parser->ptr != '/' && *parser->ptr != ':'))
     {
-        next_elem.str = p + 1;
-        next_elem.len = end - p - 1;
+        if (parser_unescape(parser)) break;
+        parser->scratch[len++] = *parser->ptr;
+        parser->ptr++;
+    }

-        elem->len = p - start;
+    if (!len)
+    {
+        parser->hr = WINCODEC_ERR_INVALIDQUERYREQUEST;
+        return;
     }
+
+    parser->scratch[len] = 0;
+
+    parser->hr = init_propvar_from_string(parser->scratch, item);
+}
+
+static void parse_query_item(struct query_parser *parser, PROPVARIANT *item)
+{
+    if (FAILED(parser->hr))
+        return;
+
+    if (*parser->ptr == '{')
+        parse_query_data_item(parser, item);
     else
-        elem->len = end - start;
+        parse_query_name(parser, item);
+}

-    id->vt = VT_BSTR;
-    id->bstrVal = SysAllocStringLen(NULL, elem->len + 1);
-    if (!id->bstrVal) return E_OUTOFMEMORY;
+static void parse_query_component(struct query_parser *parser)
+{
+    struct query_component comp = { 0 };
+    GUID guid;

-    bstr = id->bstrVal;
-    p = elem->str;
-    while (p - elem->str < elem->len)
+    if (*parser->ptr != '/')
     {
-        if (*p == '\') p++;
-        *bstr++ = *p++;
+        parser->hr = WINCODEC_ERR_PROPERTYNOTSUPPORTED;
+        return;
     }
-    *bstr = 0;
-    TRACE("%s [%d]\n", wine_dbgstr_variant((VARIANT *)id), *idx);
+    parser->ptr++;
+
+    /* Optional index */
+    if (*parser->ptr == '[')
+        parse_query_index(parser, &comp.index);

-    if (*p == ':')
+    parse_query_item(parser, &comp.id);
+    if (*parser->ptr == ':')
     {
-        PROPVARIANT next_id, next_schema;
-        int next_idx = 0;
+         parser_skip_char(parser, ':');

-        hr = get_token(&next_elem, &next_id, &next_schema, &next_idx);
-        if (hr != S_OK)
+         comp.schema = comp.id;
+         PropVariantInit(&comp.id);
+         parse_query_item(parser, &comp.id);
+    }
+
+    /* Resolve known names. */
+    if (comp.id.vt == VT_LPWSTR)
+    {
+        if (SUCCEEDED(WICMapShortNameToGuid(comp.id.pwszVal, &guid)))
         {
-            TRACE("get_token error %#lx\n", hr);
-            PropVariantClear(id);
-            PropVariantClear(schema);
-            return hr;
+            PropVariantClear(&comp.id);
+            parser->hr = InitPropVariantFromCLSID(&guid, &comp.id);
         }
-        elem->len += next_elem.len + 1;
-
-        TRACE("id %s [%d]\n", wine_dbgstr_wn(elem->str, elem->len), *idx);
+    }

-        if (next_schema.vt != VT_EMPTY)
+    if (SUCCEEDED(parser->hr))
+    {
+        if (comp.id.vt == VT_CLSID)
         {
-            PropVariantClear(&next_id);
-            PropVariantClear(&next_schema);
-            PropVariantClear(id);
-            PropVariantClear(schema);
-            return WINCODEC_ERR_WRONGSTATE;
+            PropVariantClear(&comp.schema);
+            if (comp.index)
+            {
+                comp.schema.vt = VT_UI2;
+                comp.schema.uiVal = comp.index;
+            }
         }

-        *schema = *id;
-        *id = next_id;
+        if (!wincodecs_array_reserve((void **)&parser->components, &parser->capacity,
+                parser->count + 1, sizeof(*parser->components)))
+        {
+            parser->hr = E_OUTOFMEMORY;
+            PropVariantClear(&comp.schema);
+            PropVariantClear(&comp.id);
+        }
+        else
+        {
+            parser->components[parser->count++] = comp;
+        }
     }
-
-    return S_OK;
 }

-static HRESULT find_reader_from_block(IWICMetadataBlockReader *block_reader, UINT index,
-                                      GUID *guid, IWICMetadataReader **reader)
+static HRESULT parser_set_top_level_metadata_handler(struct query_handler *query_handler,
+        struct query_parser *parser)
 {
+    struct query_component *comp;
+    IWICMetadataReader *handler;
     HRESULT hr;
     GUID format;
-    IWICMetadataReader *new_reader;
     UINT count, i, matched_index;

-    *reader = NULL;
+    comp = &parser->components[0];
+    if (comp->id.vt != VT_CLSID)
+        return E_UNEXPECTED;

-    hr = IWICMetadataBlockReader_GetCount(block_reader, &count);
+    hr = IWICMetadataBlockReader_GetCount(query_handler->object.block_reader, &count);
     if (hr != S_OK) return hr;

matched_index = 0;

-    for (i = 0; i < count; i++)
+    for (i = 0; i < count; ++i)
     {
-        hr = IWICMetadataBlockReader_GetReaderByIndex(block_reader, i, &new_reader);
-        if (hr != S_OK) return hr;
+        if (is_writer_handler(query_handler))
+            hr = IWICMetadataBlockWriter_GetWriterByIndex(query_handler->object.block_writer, i, (IWICMetadataWriter **)&handler);
+        else
+            hr = IWICMetadataBlockReader_GetReaderByIndex(query_handler->object.block_reader, i, &handler);
+
+        if (FAILED(hr))
+            break;

-        hr = IWICMetadataReader_GetMetadataFormat(new_reader, &format);
+        hr = IWICMetadataReader_GetMetadataFormat(handler, &format);
         if (hr == S_OK)
         {
-            if (IsEqualGUID(&format, guid))
+            if (IsEqualGUID(&format, comp->id.puuid))
             {
-                if (matched_index == index)
-                {
-                    *reader = new_reader;
-                    return S_OK;
-                }
+                if (matched_index == comp->index)
+                    break;

matched_index++;
             }
         }

-        IWICMetadataReader_Release(new_reader);
-        if (hr != S_OK) return hr;
+        IWICMetadataReader_Release(handler);
+        handler = NULL;
+
+        if (hr != S_OK) break;
     }

-    return WINCODEC_ERR_PROPERTYNOTFOUND;
+    if (FAILED(hr)) return hr;
+
+    comp->handler = handler;
+    return comp->handler ? S_OK : WINCODEC_ERR_PROPERTYNOTFOUND;
 }

-static HRESULT get_next_reader(IWICMetadataReader *reader, UINT index,
-                               GUID *guid, IWICMetadataReader **new_reader)
+static void parser_resolve_component_handlers(struct query_handler *query_handler, struct query_parser *parser)
 {
-    HRESULT hr;
-    PROPVARIANT schema, id, value;
+    PROPVARIANT value;
+    const WCHAR *url;
+    GUID guid;
+    size_t i;

-    *new_reader = NULL;
+    if (FAILED(parser->hr = parser_set_top_level_metadata_handler(query_handler, parser)))
+        return;

-    PropVariantInit(&schema);
-    PropVariantInit(&id);
-    PropVariantInit(&value);
-
-    if (index)
+    /* First component is handled via block reader/writer. */
+    for (i = 1; i < parser->count; ++i)
     {
-        schema.vt = VT_UI2;
-        schema.uiVal = index;
-    }
+        struct query_component *prev_comp = &parser->components[i - 1];
+        struct query_component *comp = &parser->components[i];

-    id.vt = VT_CLSID;
-    id.puuid = guid;
-    hr = IWICMetadataReader_GetValue(reader, &schema, &id, &value);
-    if (hr != S_OK) return hr;
+        if (!prev_comp->handler)
+            continue;

-    if (value.vt == VT_UNKNOWN)
-        hr = IUnknown_QueryInterface(value.punkVal, &IID_IWICMetadataReader, (void **)new_reader);
-    else
-        hr = WINCODEC_ERR_UNEXPECTEDMETADATATYPE;
+        /* Expand schema urls for "known" formats. */
+        if (comp->schema.vt == VT_LPWSTR)
+        {
+            if (SUCCEEDED(IWICMetadataReader_GetMetadataFormat(prev_comp->handler, &guid)))
+            {
+                url = map_shortname_to_schema(&guid, comp->schema.pwszVal);
+                if (url)
+                {
+                    PropVariantClear(&comp->schema);
+                    init_propvar_from_string(url, &comp->schema);
+                }
+            }
+        }

-    PropVariantClear(&value);
-    return hr;
+        PropVariantInit(&value);
+        if (FAILED(parser->hr = IWICMetadataReader_GetValue(prev_comp->handler, &comp->schema, &comp->id, &value)))
+            break;
+
+        if (value.vt == VT_UNKNOWN)
+        {
+            parser->hr = IUnknown_QueryInterface(value.punkVal, is_writer_handler(query_handler) ?
+                    &IID_IWICMetadataWriter : &IID_IWICMetadataReader, (void **)&comp->handler);
+        }
+        PropVariantClear(&value);
+
+        if (FAILED(parser->hr))
+            break;
+    }
 }

-static HRESULT WINAPI query_handler_GetMetadataByName(IWICMetadataQueryWriter *iface, LPCWSTR query, PROPVARIANT *value)
+static HRESULT parse_query(struct query_handler *query_handler, const WCHAR *user_query,
+        struct query_parser *parser)
 {
-    struct query_handler *This = impl_from_IWICMetadataQueryWriter(iface);
-    struct string_t elem;
-    WCHAR *full_query;
-    const WCHAR *p;
-    int index, len;
-    PROPVARIANT tk_id, tk_schema, new_value;
-    GUID guid;
-    IWICMetadataReader *reader;
-    HRESULT hr = S_OK;
+    WCHAR *query;
+    size_t len;

-    TRACE("(%p,%s,%p)\n", This, wine_dbgstr_w(query), value);
+    memset(parser, 0, sizeof(*parser));

-    len = lstrlenW(query) + 1;
-    if (This->root) len += lstrlenW(This->root);
-    full_query = malloc(len * sizeof(WCHAR));
-    full_query[0] = 0;
-    if (This->root)
-        lstrcpyW(full_query, This->root);
-    lstrcatW(full_query, query);
+    len = wcslen(user_query) + 1;
+    if (query_handler->root) len += wcslen(query_handler->root);

-    PropVariantInit(&tk_id);
-    PropVariantInit(&tk_schema);
-    PropVariantInit(&new_value);
+    if (!(query = malloc(len * sizeof(WCHAR))))
+        return parser->hr = E_OUTOFMEMORY;

-    reader = NULL;
-    p = full_query;
+    query[0] = 0;
+    if (query_handler->root)
+        wcscpy(query, query_handler->root);
+    wcscat(query, user_query);

-    while (*p)
+    if (!(parser->scratch = malloc(len * sizeof(WCHAR))))
     {
-        if (*p != '/')
-        {
-            WARN("query should start with '/'\n");
-            hr = WINCODEC_ERR_PROPERTYNOTSUPPORTED;
-            break;
-        }
+        parser->hr = E_OUTOFMEMORY;
+        free(query);
+        return parser->hr;
+    }

-        p++;
+    parser->query = query;
+    parser->ptr = query;

-        index = 0;
-        elem.str = p;
-        elem.len = lstrlenW(p);
-        hr = get_token(&elem, &tk_id, &tk_schema, &index);
-        if (hr != S_OK)
-        {
-            WARN("get_token error %#lx\n", hr);
-            break;
-        }
-        TRACE("parsed %d characters: %s, index %d\n", elem.len, wine_dbgstr_wn(elem.str, elem.len), index);
-        TRACE("id %s, schema %s\n", wine_dbgstr_variant((VARIANT *)&tk_id), wine_dbgstr_variant((VARIANT *)&tk_schema));
+    while (*parser->ptr && parser->hr == S_OK)
+        parse_query_component(parser);

-        if (!elem.len) break;
+    if (FAILED(parser->hr)) return parser->hr;

-        if (tk_id.vt == VT_CLSID || (tk_id.vt == VT_BSTR && WICMapShortNameToGuid(tk_id.bstrVal, &guid) == S_OK))
-        {
-            WCHAR *root;
+    if (!parser->count)
+        return parser->hr = WINCODEC_ERR_INVALIDQUERYREQUEST;

-            if (tk_schema.vt != VT_EMPTY)
-            {
-                FIXME("unsupported schema vt %u\n", tk_schema.vt);
-                PropVariantClear(&tk_schema);
-            }
+    parser_resolve_component_handlers(query_handler, parser);

-            if (tk_id.vt == VT_CLSID) guid = *tk_id.puuid;
+    /* Validate that query is usable - it should produce an object or
+       an object followed by a value id. */
+    if (SUCCEEDED(parser->hr))
+    {
+        parser->last = &parser->components[parser->count - 1];
+        if (parser->last->handler)
+            return S_OK;

-            if (reader)
-            {
-                IWICMetadataReader *new_reader;
+        if (parser->count < 2)
+            return parser->hr = WINCODEC_ERR_INVALIDQUERYREQUEST;

-                hr = get_next_reader(reader, index, &guid, &new_reader);
-                IWICMetadataReader_Release(reader);
-                reader = new_reader;
-            }
-            else
-                hr = find_reader_from_block(This->object.block_reader, index, &guid, &reader);
+        parser->prev = &parser->components[parser->count - 2];
+        if (!parser->prev->handler)
+            return parser->hr = WINCODEC_ERR_INVALIDQUERYREQUEST;
+    }

-            if (hr != S_OK) break;
+    return parser->hr;
+}

-            root = SysAllocStringLen(NULL, elem.str + elem.len - full_query + 2);
-            if (!root)
-            {
-                hr = E_OUTOFMEMORY;
-                break;
-            }
-            lstrcpynW(root, full_query, p - full_query + elem.len + 1);
+static void parser_cleanup(struct query_parser *parser)
+{
+    size_t i;

-            PropVariantClear(&new_value);
-            new_value.vt = VT_UNKNOWN;
-            hr = MetadataQueryReader_CreateInstance(This->object.block_reader, root, (IWICMetadataQueryReader **)&new_value.punkVal);
-            SysFreeString(root);
-            if (hr != S_OK) break;
-        }
-        else
-        {
-            PROPVARIANT schema, id;
+    for (i = 0; i < parser->count; ++i)
+    {
+        if (parser->components[i].handler)
+            IWICMetadataReader_Release(parser->components[i].handler);
+        PropVariantClear(&parser->components[i].schema);
+        PropVariantClear(&parser->components[i].id);
+    }
+    free(parser->components);
+    free(parser->scratch);
+    free(parser->query);
+}

-            if (!reader)
-            {
-                hr = WINCODEC_ERR_INVALIDQUERYREQUEST;
-                break;
-            }
+static HRESULT WINAPI query_handler_GetMetadataByName(IWICMetadataQueryWriter *iface, LPCWSTR query, PROPVARIANT *ret)
+{
+    struct query_handler *handler = impl_from_IWICMetadataQueryWriter(iface);
+    struct query_component *comp;
+    struct query_parser parser;
+    PROPVARIANT value;
+    HRESULT hr;

-            if (tk_schema.vt == VT_BSTR)
-            {
-                hr = IWICMetadataReader_GetMetadataFormat(reader, &guid);
-                if (hr != S_OK) break;
+    TRACE("(%p,%s,%p)\n", iface, wine_dbgstr_w(query), ret);

-                schema.vt = VT_LPWSTR;
-                schema.pwszVal = (LPWSTR)map_shortname_to_schema(&guid, tk_schema.bstrVal);
-                if (!schema.pwszVal)
-                    schema.pwszVal = tk_schema.bstrVal;
-            }
-            else
-                schema = tk_schema;
+    PropVariantInit(&value);
+    if (SUCCEEDED(hr = parse_query(handler, query, &parser)))
+    {
+        comp = parser.last;

-            if (tk_id.vt == VT_BSTR)
+        if (comp->handler)
+        {
+            value.vt = VT_UNKNOWN;
+            if (is_writer_handler(handler))
             {
-                id.vt = VT_LPWSTR;
-                id.pwszVal = tk_id.bstrVal;
+                hr = MetadataQueryWriter_CreateInstance(handler->object.block_writer, parser.query,
+                        (IWICMetadataQueryWriter **)&value.punkVal);
             }
             else
-                id = tk_id;
-
-            PropVariantClear(&new_value);
-            hr = IWICMetadataReader_GetValue(reader, &schema, &id, &new_value);
-            if (hr != S_OK) break;
+            {
+                hr = MetadataQueryReader_CreateInstance(handler->object.block_reader, parser.query,
+                        (IWICMetadataQueryReader **)&value.punkVal);
+            }
+        }
+        else
+        {
+            hr = IWICMetadataReader_GetValue(parser.prev->handler, &comp->schema, &comp->id, &value);
         }
-
-        p += elem.len;
-
-        PropVariantClear(&tk_id);
-        PropVariantClear(&tk_schema);
     }

-    if (reader)
-        IWICMetadataReader_Release(reader);
-
-    PropVariantClear(&tk_id);
-    PropVariantClear(&tk_schema);
-
-    if (hr == S_OK && value)
-        *value = new_value;
+    if (ret)
+        *ret = value;
     else
-        PropVariantClear(&new_value);
+        PropVariantClear(&value);

-    free(full_query);
+    parser_cleanup(&parser);

return hr;
 }
diff --git a/dlls/windowscodecs/tests/metadata.c b/dlls/windowscodecs/tests/metadata.c
index ec236928d74..b680448f25c 100644
--- a/dlls/windowscodecs/tests/metadata.c
+++ b/dlls/windowscodecs/tests/metadata.c
@@ -4338,8 +4338,8 @@ static void test_queryreader(void)
         { FALSE, &data3, L"/[1]ifd/{str=R\}ATING}", S_OK, VT_LPSTR, 0, the_best },

{ FALSE, &data1, L"[0]/ifd/Rating", WINCODEC_ERR_PROPERTYNOTSUPPORTED },
-        { TRUE, &data1, L"/[+1]ifd/Rating", WINCODEC_ERR_INVALIDQUERYCHARACTER },
-        { TRUE, &data1, L"/[-1]ifd/Rating", WINCODEC_ERR_INVALIDQUERYCHARACTER },
+        { FALSE, &data1, L"/[+1]ifd/Rating", WINCODEC_ERR_INVALIDQUERYCHARACTER },
+        { FALSE, &data1, L"/[-1]ifd/Rating", WINCODEC_ERR_INVALIDQUERYCHARACTER },
         { FALSE, &data1, L"/ifd/{\str=Rating}", WINCODEC_ERR_WRONGSTATE },
         { FALSE, &data1, L"/ifd/{badtype=0}", WINCODEC_ERR_WRONGSTATE },
         { TRUE, &data1, L"/ifd/{uint=0x1234}", DISP_E_TYPEMISMATCH },
@@ -5117,7 +5117,6 @@ static void test_metadata_App1(void)
     hr = IWICMetadataQueryReader_GetMetadataByName(query_reader, L"/app1/ifd/{ushort=34665}", &value);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ok(value.vt == VT_UNKNOWN, "Unexpected value type: %u.\n", value.vt);
-    todo_wine
     check_interface(value.punkVal, &IID_IWICMetadataQueryReader, TRUE);
     PropVariantClear(&value);

@@ -5125,7 +5124,6 @@ static void test_metadata_App1(void)
     hr = IWICMetadataQueryReader_GetMetadataByName(query_reader, L"/app1/ifd/{ushort=34853}", &value);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ok(value.vt == VT_UNKNOWN, "Unexpected value type: %u.\n", value.vt);
-    todo_wine
     check_interface(value.punkVal, &IID_IWICMetadataQueryReader, TRUE);
     PropVariantClear(&value);

@@ -5141,19 +5139,14 @@ static void test_metadata_App1(void)
     ok(value.vt == VT_UNKNOWN, "Unexpected value type: %u.\n", value.vt);

check_interface(value.punkVal, &IID_IWICMetadataQueryReader, TRUE);
-    todo_wine
     check_interface(value.punkVal, &IID_IWICMetadataQueryWriter, TRUE);

hr = IUnknown_QueryInterface(value.punkVal, &IID_IWICMetadataQueryWriter, (void **)&query_writer2);
-    todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-    if (hr == S_OK)
-    {
-        hr = IWICMetadataQueryWriter_GetLocation(query_writer2, ARRAY_SIZE(path), path, &length);
-        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-        ok(!lstrcmpW(path, L"/app1"), "Unexpected path %s.\n", wine_dbgstr_w(path));
-        IWICMetadataQueryWriter_Release(query_writer2);
-    }
+    hr = IWICMetadataQueryWriter_GetLocation(query_writer2, ARRAY_SIZE(path), path, &length);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(!lstrcmpW(path, L"/app1"), "Unexpected path %s.\n", wine_dbgstr_w(path));
+    IWICMetadataQueryWriter_Release(query_writer2);
     PropVariantClear(&value);

PropVariantInit(&value);
@@ -5161,18 +5154,15 @@ static void test_metadata_App1(void)
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ok(value.vt == VT_UNKNOWN, "Unexpected value type: %u.\n", value.vt);
     hr = IUnknown_QueryInterface(value.punkVal, &IID_IWICMetadataQueryWriter, (void **)&query_writer2);
-    todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-    if (hr == S_OK)
-    {
-        hr = IWICMetadataQueryWriter_GetLocation(query_writer2, ARRAY_SIZE(path), path, &length);
-        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-        ok(!lstrcmpW(path, L"/app1/ifd"), "Unexpected path %s.\n", wine_dbgstr_w(path));
-        hr = IWICMetadataQueryWriter_GetContainerFormat(query_writer2, &format);
-        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-        ok(IsEqualGUID(&format, &GUID_MetadataFormatIfd), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
-        IWICMetadataQueryWriter_Release(query_writer2);
-    }
+    hr = IWICMetadataQueryWriter_GetLocation(query_writer2, ARRAY_SIZE(path), path, &length);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(!lstrcmpW(path, L"/app1/ifd"), "Unexpected path %s.\n", wine_dbgstr_w(path));
+    hr = IWICMetadataQueryWriter_GetContainerFormat(query_writer2, &format);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    todo_wine
+    ok(IsEqualGUID(&format, &GUID_MetadataFormatIfd), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
+    IWICMetadataQueryWriter_Release(query_writer2);
     PropVariantClear(&value);

PropVariantInit(&value);
@@ -5180,18 +5170,15 @@ static void test_metadata_App1(void)
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ok(value.vt == VT_UNKNOWN, "Unexpected value type: %u.\n", value.vt);
     hr = IUnknown_QueryInterface(value.punkVal, &IID_IWICMetadataQueryWriter, (void **)&query_writer2);
-    todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-    if (hr == S_OK)
-    {
-        hr = IWICMetadataQueryWriter_GetLocation(query_writer2, ARRAY_SIZE(path), path, &length);
-        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-        ok(!lstrcmpW(path, L"/app1/ifd/gps"), "Unexpected path %s.\n", wine_dbgstr_w(path));
-        hr = IWICMetadataQueryWriter_GetContainerFormat(query_writer2, &format);
-        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-        ok(IsEqualGUID(&format, &GUID_MetadataFormatGps), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
-        IWICMetadataQueryWriter_Release(query_writer2);
-    }
+    hr = IWICMetadataQueryWriter_GetLocation(query_writer2, ARRAY_SIZE(path), path, &length);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(!lstrcmpW(path, L"/app1/ifd/gps"), "Unexpected path %s.\n", wine_dbgstr_w(path));
+    hr = IWICMetadataQueryWriter_GetContainerFormat(query_writer2, &format);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    todo_wine
+    ok(IsEqualGUID(&format, &GUID_MetadataFormatGps), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
+    IWICMetadataQueryWriter_Release(query_writer2);
     PropVariantClear(&value);

PropVariantInit(&value);
@@ -5199,18 +5186,15 @@ static void test_metadata_App1(void)
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ok(value.vt == VT_UNKNOWN, "Unexpected value type: %u.\n", value.vt);
     hr = IUnknown_QueryInterface(value.punkVal, &IID_IWICMetadataQueryWriter, (void **)&query_writer2);
-    todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-    if (hr == S_OK)
-    {
-        hr = IWICMetadataQueryWriter_GetLocation(query_writer2, ARRAY_SIZE(path), path, &length);
-        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-        ok(!lstrcmpW(path, L"/app1/ifd/exif"), "Unexpected path %s.\n", wine_dbgstr_w(path));
-        hr = IWICMetadataQueryWriter_GetContainerFormat(query_writer2, &format);
-        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-        ok(IsEqualGUID(&format, &GUID_MetadataFormatExif), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
-        IWICMetadataQueryWriter_Release(query_writer2);
-    }
+    hr = IWICMetadataQueryWriter_GetLocation(query_writer2, ARRAY_SIZE(path), path, &length);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    ok(!lstrcmpW(path, L"/app1/ifd/exif"), "Unexpected path %s.\n", wine_dbgstr_w(path));
+    hr = IWICMetadataQueryWriter_GetContainerFormat(query_writer2, &format);
+    ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+    todo_wine
+    ok(IsEqualGUID(&format, &GUID_MetadataFormatExif), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
+    IWICMetadataQueryWriter_Release(query_writer2);
     PropVariantClear(&value);

PropVariantInit(&value);
@@ -5235,9 +5219,7 @@ static void test_metadata_App1(void)
     hr = IWICMetadataQueryWriter_GetMetadataByName(query_writer, L"/app1/ifd/{ushort=34665}", &value);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     ok(value.vt == VT_UNKNOWN, "Unexpected value type: %u.\n", value.vt);
-    todo_wine
     check_interface(value.punkVal, &IID_IWICMetadataQueryReader, TRUE);
-    todo_wine
     check_interface(value.punkVal, &IID_IWICMetadataQueryWriter, TRUE);
     PropVariantClear(&value);

[PATCH v2 08/10] windowscodecs/metadata: Handle empty items in queries.

[Nikolay Sivov]

From: Nikolay Sivov

Signed-off-by: Nikolay Sivov 
---
 dlls/windowscodecs/metadataquery.c  |  8 ++++++++
 dlls/windowscodecs/tests/metadata.c | 16 ++++------------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/dlls/windowscodecs/metadataquery.c b/dlls/windowscodecs/metadataquery.c
index 25c7e553e62..39b4677bdfc 100644
--- a/dlls/windowscodecs/metadataquery.c
+++ b/dlls/windowscodecs/metadataquery.c
@@ -322,6 +322,14 @@ static void parse_query_data_item(struct query_parser *parser, PROPVARIANT *item

if (parser_skip_char(parser, '{')) return;

+    /* Empty "{}" item represents VT_EMPTY. */
+    if (*parser->ptr == '}')
+    {
+        item->vt = VT_EMPTY;
+        parser_skip_char(parser, '}');
+        return;
+    }
+
     /* Type */
     span.str = parser->ptr;
     span.len = 0;
diff --git a/dlls/windowscodecs/tests/metadata.c b/dlls/windowscodecs/tests/metadata.c
index b680448f25c..6b322d24c88 100644
--- a/dlls/windowscodecs/tests/metadata.c
+++ b/dlls/windowscodecs/tests/metadata.c
@@ -781,23 +781,15 @@ static void test_metadata_unknown(void)

PropVariantInit(&value);
     hr = IWICMetadataQueryReader_GetMetadataByName(query_reader2, L"/{}", &value);
-    todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-    if (hr == S_OK)
-    {
-        compare_blob(&value, metadata_unknown, sizeof(metadata_unknown));
-        PropVariantClear(&value);
-    }
+    compare_blob(&value, metadata_unknown, sizeof(metadata_unknown));
+    PropVariantClear(&value);

PropVariantInit(&value);
     hr = IWICMetadataQueryReader_GetMetadataByName(query_reader, L"/unknown/{}", &value);
-    todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-    if (hr == S_OK)
-    {
-        compare_blob(&value, metadata_unknown, sizeof(metadata_unknown));
-        PropVariantClear(&value);
-    }
+    compare_blob(&value, metadata_unknown, sizeof(metadata_unknown));
+    PropVariantClear(&value);

IWICMetadataQueryReader_Release(query_reader2);
     IWICMetadataQueryReader_Release(query_reader);

[PATCH v2 09/10] windowscodecs/metadata: Support creating query handlers over bare metadata handlers.

[Nikolay Sivov]

From: Nikolay Sivov

Signed-off-by: Nikolay Sivov 
---
 dlls/windowscodecs/encoder.c           |   2 +-
 dlls/windowscodecs/gifformat.c         |   6 +-
 dlls/windowscodecs/imgfactory.c        |   4 +-
 dlls/windowscodecs/metadataquery.c     | 168 ++++++++++++++-----------
 dlls/windowscodecs/tests/metadata.c    |   7 --
 dlls/windowscodecs/wincodecs_private.h |   4 +-
 6 files changed, 105 insertions(+), 86 deletions(-)

diff --git a/dlls/windowscodecs/encoder.c b/dlls/windowscodecs/encoder.c
index f40fd2fa005..de1dadc70c2 100644
--- a/dlls/windowscodecs/encoder.c
+++ b/dlls/windowscodecs/encoder.c
@@ -479,7 +479,7 @@ static HRESULT WINAPI CommonEncoderFrame_GetMetadataQueryWriter(IWICBitmapFrameE
     if (!(encoder->parent->encoder_info.flags & ENCODER_FLAGS_SUPPORTS_METADATA))
         return WINCODEC_ERR_UNSUPPORTEDOPERATION;

-    return MetadataQueryWriter_CreateInstance(&encoder->IWICMetadataBlockWriter_iface, NULL, ppIMetadataQueryWriter);
+    return MetadataQueryWriter_CreateInstance(&encoder->IWICMetadataBlockWriter_iface, ppIMetadataQueryWriter);
 }

static const IWICBitmapFrameEncodeVtbl CommonEncoderFrame_Vtbl = {
diff --git a/dlls/windowscodecs/gifformat.c b/dlls/windowscodecs/gifformat.c
index 0b32f0ddfa7..a347ee3bc50 100644
--- a/dlls/windowscodecs/gifformat.c
+++ b/dlls/windowscodecs/gifformat.c
@@ -837,7 +837,7 @@ static HRESULT WINAPI GifFrameDecode_GetMetadataQueryReader(IWICBitmapFrameDecod
     if (!ppIMetadataQueryReader)
         return E_INVALIDARG;

-    return MetadataQueryReader_CreateInstance(&This->IWICMetadataBlockReader_iface, NULL, ppIMetadataQueryReader);
+    return MetadataQueryReader_CreateInstance(&This->IWICMetadataBlockReader_iface, ppIMetadataQueryReader);
 }

static HRESULT WINAPI GifFrameDecode_GetColorContexts(IWICBitmapFrameDecode *iface,
@@ -1216,7 +1216,7 @@ static HRESULT WINAPI GifDecoder_GetMetadataQueryReader(IWICBitmapDecoder *iface

if (!ppIMetadataQueryReader) return E_INVALIDARG;

-    return MetadataQueryReader_CreateInstance(&This->IWICMetadataBlockReader_iface, NULL, ppIMetadataQueryReader);
+    return MetadataQueryReader_CreateInstance(&This->IWICMetadataBlockReader_iface, ppIMetadataQueryReader);
 }

static HRESULT WINAPI GifDecoder_GetPreview(IWICBitmapDecoder *iface,
@@ -2098,7 +2098,7 @@ static HRESULT WINAPI GifFrameEncode_GetMetadataQueryWriter(IWICBitmapFrameEncod
     if (!encode->initialized)
         return WINCODEC_ERR_NOTINITIALIZED;

-    return MetadataQueryWriter_CreateInstance(&encode->IWICMetadataBlockWriter_iface, NULL, writer);
+    return MetadataQueryWriter_CreateInstance(&encode->IWICMetadataBlockWriter_iface, writer);
 }

static const IWICBitmapFrameEncodeVtbl GifFrameEncode_Vtbl =
diff --git a/dlls/windowscodecs/imgfactory.c b/dlls/windowscodecs/imgfactory.c
index 0608be76c08..5af765e3852 100644
--- a/dlls/windowscodecs/imgfactory.c
+++ b/dlls/windowscodecs/imgfactory.c
@@ -1769,7 +1769,7 @@ static HRESULT WINAPI ComponentFactory_CreateQueryReaderFromBlockReader(IWICComp
     if (!block_reader || !query_reader)
         return E_INVALIDARG;

-    return MetadataQueryReader_CreateInstance(block_reader, NULL, query_reader);
+    return MetadataQueryReader_CreateInstance(block_reader, query_reader);
 }

static HRESULT WINAPI ComponentFactory_CreateQueryWriterFromBlockWriter(IWICComponentFactory *iface,
@@ -1780,7 +1780,7 @@ static HRESULT WINAPI ComponentFactory_CreateQueryWriterFromBlockWriter(IWICComp
     if (!block_writer || !query_writer)
         return E_INVALIDARG;

-    return MetadataQueryWriter_CreateInstance(block_writer, NULL, query_writer);
+    return MetadataQueryWriter_CreateInstance(block_writer, query_writer);
 }

static HRESULT WINAPI ComponentFactory_CreateEncoderPropertyBag(IWICComponentFactory *iface,
diff --git a/dlls/windowscodecs/metadataquery.c b/dlls/windowscodecs/metadataquery.c
index 39b4677bdfc..b0f454d9fe3 100644
--- a/dlls/windowscodecs/metadataquery.c
+++ b/dlls/windowscodecs/metadataquery.c
@@ -38,6 +38,8 @@ enum metadata_object_type
 {
     BLOCK_READER,
     BLOCK_WRITER,
+    READER,
+    WRITER,
 };

struct query_handler
@@ -49,6 +51,8 @@ struct query_handler
         IUnknown *handler;
         IWICMetadataBlockReader *block_reader;
         IWICMetadataBlockWriter *block_writer;
+        IWICMetadataReader *reader;
+        IWICMetadataWriter *writer;
     } object;
     enum metadata_object_type object_type;
     WCHAR *root;
@@ -56,7 +60,14 @@ struct query_handler

static bool is_writer_handler(const struct query_handler *handler)
 {
-    return handler->object_type == BLOCK_WRITER;
+    return handler->object_type == BLOCK_WRITER
+            || handler->object_type == WRITER;
+}
+
+static bool is_block_handler(const struct query_handler *handler)
+{
+    return handler->object_type == BLOCK_READER
+            || handler->object_type == BLOCK_WRITER;
 }

static inline struct query_handler *impl_from_IWICMetadataQueryWriter(IWICMetadataQueryWriter *iface)
@@ -115,28 +126,27 @@ static HRESULT WINAPI query_handler_GetContainerFormat(IWICMetadataQueryWriter *

TRACE("(%p,%p)\n", iface, format);

-    return IWICMetadataBlockReader_GetContainerFormat(handler->object.block_reader, format);
+    return is_block_handler(handler) ? IWICMetadataBlockReader_GetContainerFormat(handler->object.block_reader, format):
+            IWICMetadataReader_GetMetadataFormat(handler->object.reader, format);
 }

static HRESULT WINAPI query_handler_GetLocation(IWICMetadataQueryWriter *iface, UINT len, WCHAR *location, UINT *ret_len)
 {
     struct query_handler *handler = impl_from_IWICMetadataQueryWriter(iface);
-    const WCHAR *root;
     UINT actual_len;

TRACE("(%p,%u,%p,%p)\n", iface, len, location, ret_len);

if (!ret_len) return E_INVALIDARG;

-    root = handler->root ? handler->root : L"/";
-    actual_len = lstrlenW(root) + 1;
+    actual_len = lstrlenW(handler->root) + 1;

if (location)
     {
         if (len < actual_len)
             return WINCODEC_ERR_INSUFFICIENTBUFFER;

-        memcpy(location, root, actual_len * sizeof(WCHAR));
+        memcpy(location, handler->root, actual_len * sizeof(WCHAR));
     }

*ret_len = actual_len;
@@ -199,15 +209,20 @@ struct query_component
     unsigned int index;
     PROPVARIANT schema;
     PROPVARIANT id;
-    IWICMetadataReader *handler;
+    union
+    {
+        IUnknown *handler;
+        IWICMetadataReader *reader;
+        IWICMetadataWriter *writer;
+    };
 };

struct query_parser
 {
     const WCHAR *ptr;
+    const WCHAR *query;

WCHAR *scratch;
-    WCHAR *query;

struct query_component *components;
     size_t count;
@@ -427,6 +442,18 @@ static void parse_query_item(struct query_parser *parser, PROPVARIANT *item)
         parse_query_name(parser, item);
 }

+static void parse_add_component(struct query_parser *parser, struct query_component *comp)
+{
+    if (!wincodecs_array_reserve((void **)&parser->components, &parser->capacity,
+            parser->count + 1, sizeof(*parser->components)))
+    {
+        parser->hr = E_OUTOFMEMORY;
+        return;
+    }
+
+    parser->components[parser->count++] = *comp;
+}
+
 static void parse_query_component(struct query_parser *parser)
 {
     struct query_component comp = { 0 };
@@ -475,30 +502,36 @@ static void parse_query_component(struct query_parser *parser)
             }
         }

-        if (!wincodecs_array_reserve((void **)&parser->components, &parser->capacity,
-                parser->count + 1, sizeof(*parser->components)))
+        parse_add_component(parser, &comp);
+
+        if (FAILED(parser->hr))
         {
-            parser->hr = E_OUTOFMEMORY;
             PropVariantClear(&comp.schema);
             PropVariantClear(&comp.id);
         }
-        else
-        {
-            parser->components[parser->count++] = comp;
-        }
     }
 }

static HRESULT parser_set_top_level_metadata_handler(struct query_handler *query_handler,
         struct query_parser *parser)
 {
+    UINT count, i, matched_index;
     struct query_component *comp;
-    IWICMetadataReader *handler;
-    HRESULT hr;
+    IWICMetadataReader *reader;
     GUID format;
-    UINT count, i, matched_index;
+    HRESULT hr;
+
+    /* Nested handlers are created on IWICMetadataReader/IWICMetadataWriter instances
+       directly. Same applies to the query writers created with CreateQueryWriter()/CreateQueryWriterFromReader().
+
+       However decoders and encoders will be using block handlers. */
+
+    if (!is_block_handler(query_handler))
+        return S_OK;

comp = &parser->components[0];
+
+    /* 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;

@@ -510,14 +543,14 @@ static HRESULT parser_set_top_level_metadata_handler(struct query_handler *query
     for (i = 0; i < count; ++i)
     {
         if (is_writer_handler(query_handler))
-            hr = IWICMetadataBlockWriter_GetWriterByIndex(query_handler->object.block_writer, i, (IWICMetadataWriter **)&handler);
+            hr = IWICMetadataBlockWriter_GetWriterByIndex(query_handler->object.block_writer, i, (IWICMetadataWriter **)&reader);
         else
-            hr = IWICMetadataBlockReader_GetReaderByIndex(query_handler->object.block_reader, i, &handler);
+            hr = IWICMetadataBlockReader_GetReaderByIndex(query_handler->object.block_reader, i, &reader);

if (FAILED(hr))
             break;

-        hr = IWICMetadataReader_GetMetadataFormat(handler, &format);
+        hr = IWICMetadataReader_GetMetadataFormat(reader, &format);
         if (hr == S_OK)
         {
             if (IsEqualGUID(&format, comp->id.puuid))
@@ -529,16 +562,16 @@ static HRESULT parser_set_top_level_metadata_handler(struct query_handler *query
             }
         }

-        IWICMetadataReader_Release(handler);
-        handler = NULL;
+        IWICMetadataReader_Release(reader);
+        reader = NULL;

if (hr != S_OK) break;
     }

if (FAILED(hr)) return hr;

-    comp->handler = handler;
-    return comp->handler ? S_OK : WINCODEC_ERR_PROPERTYNOTFOUND;
+    comp->reader = reader;
+    return comp->reader ? S_OK : WINCODEC_ERR_PROPERTYNOTFOUND;
 }

static void parser_resolve_component_handlers(struct query_handler *query_handler, struct query_parser *parser)
@@ -551,7 +584,8 @@ static void parser_resolve_component_handlers(struct query_handler *query_handle
     if (FAILED(parser->hr = parser_set_top_level_metadata_handler(query_handler, parser)))
         return;

-    /* First component is handled via block reader/writer. */
+    /* The first component contains the root handler for this query. It's provided either
+       through block reader or specified explicitly on query creation. */
     for (i = 1; i < parser->count; ++i)
     {
         struct query_component *prev_comp = &parser->components[i - 1];
@@ -563,7 +597,7 @@ static void parser_resolve_component_handlers(struct query_handler *query_handle
         /* Expand schema urls for "known" formats. */
         if (comp->schema.vt == VT_LPWSTR)
         {
-            if (SUCCEEDED(IWICMetadataReader_GetMetadataFormat(prev_comp->handler, &guid)))
+            if (SUCCEEDED(IWICMetadataReader_GetMetadataFormat(prev_comp->reader, &guid)))
             {
                 url = map_shortname_to_schema(&guid, comp->schema.pwszVal);
                 if (url)
@@ -575,7 +609,7 @@ static void parser_resolve_component_handlers(struct query_handler *query_handle
         }

PropVariantInit(&value);
-        if (FAILED(parser->hr = IWICMetadataReader_GetValue(prev_comp->handler, &comp->schema, &comp->id, &value)))
+        if (FAILED(parser->hr = IWICMetadataReader_GetValue(prev_comp->reader, &comp->schema, &comp->id, &value)))
             break;

if (value.vt == VT_UNKNOWN)
@@ -590,35 +624,36 @@ static void parser_resolve_component_handlers(struct query_handler *query_handle
     }
 }

-static HRESULT parse_query(struct query_handler *query_handler, const WCHAR *user_query,
+static HRESULT parse_query(struct query_handler *query_handler, const WCHAR *query,
         struct query_parser *parser)
 {
-    WCHAR *query;
+    struct query_component comp = { 0 };
     size_t len;

memset(parser, 0, sizeof(*parser));

-    len = wcslen(user_query) + 1;
-    if (query_handler->root) len += wcslen(query_handler->root);
-
-    if (!(query = malloc(len * sizeof(WCHAR))))
-        return parser->hr = E_OUTOFMEMORY;
-
-    query[0] = 0;
-    if (query_handler->root)
-        wcscpy(query, query_handler->root);
-    wcscat(query, user_query);
+    /* Unspecified item is only allowed at root level. Replace it with an empty item notation,
+       so that it can work properly for the readers and fail, as it should, for the block readers. */
+    if (!wcscmp(query, L"/"))
+        query = L"/{}";

+    len = wcslen(query) + 1;
     if (!(parser->scratch = malloc(len * sizeof(WCHAR))))
     {
         parser->hr = E_OUTOFMEMORY;
-        free(query);
         return parser->hr;
     }

parser->query = query;
     parser->ptr = query;

+    if (!is_block_handler(query_handler))
+    {
+        comp.handler = query_handler->object.handler;
+        IUnknown_AddRef(comp.handler);
+        parse_add_component(parser, &comp);
+    }
+
     while (*parser->ptr && parser->hr == S_OK)
         parse_query_component(parser);

@@ -655,55 +690,46 @@ static void parser_cleanup(struct query_parser *parser)
     for (i = 0; i < parser->count; ++i)
     {
         if (parser->components[i].handler)
-            IWICMetadataReader_Release(parser->components[i].handler);
+            IUnknown_Release(parser->components[i].handler);
         PropVariantClear(&parser->components[i].schema);
         PropVariantClear(&parser->components[i].id);
     }
     free(parser->components);
     free(parser->scratch);
-    free(parser->query);
 }

-static HRESULT WINAPI query_handler_GetMetadataByName(IWICMetadataQueryWriter *iface, LPCWSTR query, PROPVARIANT *ret)
+static HRESULT create_query_handler(IUnknown *block_handler, enum metadata_object_type object_type,
+        const WCHAR *location, IWICMetadataQueryWriter **ret);
+
+static HRESULT WINAPI query_handler_GetMetadataByName(IWICMetadataQueryWriter *iface, LPCWSTR query, PROPVARIANT *value)
 {
     struct query_handler *handler = impl_from_IWICMetadataQueryWriter(iface);
-    struct query_component *comp;
+    struct query_component *last, *prev;
     struct query_parser parser;
-    PROPVARIANT value;
     HRESULT hr;

-    TRACE("(%p,%s,%p)\n", iface, wine_dbgstr_w(query), ret);
+    TRACE("(%p,%s,%p)\n", iface, wine_dbgstr_w(query), value);

-    PropVariantInit(&value);
     if (SUCCEEDED(hr = parse_query(handler, query, &parser)))
     {
-        comp = parser.last;
+        last = parser.last;
+        prev = parser.prev;

-        if (comp->handler)
+        if (last->handler)
         {
-            value.vt = VT_UNKNOWN;
-            if (is_writer_handler(handler))
+            if (value)
             {
-                hr = MetadataQueryWriter_CreateInstance(handler->object.block_writer, parser.query,
-                        (IWICMetadataQueryWriter **)&value.punkVal);
-            }
-            else
-            {
-                hr = MetadataQueryReader_CreateInstance(handler->object.block_reader, parser.query,
-                        (IWICMetadataQueryReader **)&value.punkVal);
+                value->vt = VT_UNKNOWN;
+                hr = create_query_handler(last->handler, is_writer_handler(handler) ? WRITER : READER,
+                        parser.query, (IWICMetadataQueryWriter **)&value->punkVal);
             }
         }
         else
         {
-            hr = IWICMetadataReader_GetValue(parser.prev->handler, &comp->schema, &comp->id, &value);
+            hr = IWICMetadataReader_GetValue(prev->reader, &last->schema, &last->id, value);
         }
     }

-    if (ret)
-        *ret = value;
-    else
-        PropVariantClear(&value);
-
     parser_cleanup(&parser);

return hr;
@@ -870,6 +896,8 @@ static HRESULT create_query_handler(IUnknown *block_handler, enum metadata_objec
     IUnknown_AddRef(block_handler);
     obj->object.handler = block_handler;
     obj->object_type = object_type;
+    if (!root)
+        root = L"/";
     obj->root = wcsdup(root);

*ret = &obj->IWICMetadataQueryWriter_iface;
@@ -877,16 +905,14 @@ static HRESULT create_query_handler(IUnknown *block_handler, enum metadata_objec
     return S_OK;
 }

-HRESULT MetadataQueryReader_CreateInstance(IWICMetadataBlockReader *block_reader, const WCHAR *root,
-        IWICMetadataQueryReader **out)
+HRESULT MetadataQueryReader_CreateInstance(IWICMetadataBlockReader *block_reader, IWICMetadataQueryReader **out)
 {
-    return create_query_handler((IUnknown *)block_reader, BLOCK_READER, root, (IWICMetadataQueryWriter **)out);
+    return create_query_handler((IUnknown *)block_reader, BLOCK_READER, NULL, (IWICMetadataQueryWriter **)out);
 }

-HRESULT MetadataQueryWriter_CreateInstance(IWICMetadataBlockWriter *block_writer, const WCHAR *root,
-        IWICMetadataQueryWriter **out)
+HRESULT MetadataQueryWriter_CreateInstance(IWICMetadataBlockWriter *block_writer, IWICMetadataQueryWriter **out)
 {
-    return create_query_handler((IUnknown *)block_writer, BLOCK_WRITER, root, out);
+    return create_query_handler((IUnknown *)block_writer, BLOCK_WRITER, NULL, out);
 }

static const struct
diff --git a/dlls/windowscodecs/tests/metadata.c b/dlls/windowscodecs/tests/metadata.c
index 6b322d24c88..f0322da21f7 100644
--- a/dlls/windowscodecs/tests/metadata.c
+++ b/dlls/windowscodecs/tests/metadata.c
@@ -5034,7 +5034,6 @@ static void test_metadata_App1(void)
     ok(!lstrcmpW(path, L"/app1"), "Unexpected path %s.\n", wine_dbgstr_w(path));
     hr = IWICMetadataQueryReader_GetContainerFormat(query_reader2, &format);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-    todo_wine
     ok(IsEqualGUID(&format, &GUID_MetadataFormatApp1), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
     IWICMetadataQueryReader_Release(query_reader2);
     PropVariantClear(&value);
@@ -5050,7 +5049,6 @@ static void test_metadata_App1(void)
     ok(!lstrcmpW(path, L"/app1/ifd"), "Unexpected path %s.\n", wine_dbgstr_w(path));
     hr = IWICMetadataQueryReader_GetContainerFormat(query_reader2, &format);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-    todo_wine
     ok(IsEqualGUID(&format, &GUID_MetadataFormatIfd), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
     IWICMetadataQueryReader_Release(query_reader2);
     PropVariantClear(&value);
@@ -5066,7 +5064,6 @@ static void test_metadata_App1(void)
     ok(!lstrcmpW(path, L"/app1/ifd/gps"), "Unexpected path %s.\n", wine_dbgstr_w(path));
     hr = IWICMetadataQueryReader_GetContainerFormat(query_reader2, &format);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-    todo_wine
     ok(IsEqualGUID(&format, &GUID_MetadataFormatGps), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
     IWICMetadataQueryReader_Release(query_reader2);
     PropVariantClear(&value);
@@ -5082,7 +5079,6 @@ static void test_metadata_App1(void)
     ok(!lstrcmpW(path, L"/app1/ifd/exif"), "Unexpected path %s.\n", wine_dbgstr_w(path));
     hr = IWICMetadataQueryReader_GetContainerFormat(query_reader2, &format);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-    todo_wine
     ok(IsEqualGUID(&format, &GUID_MetadataFormatExif), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
     IWICMetadataQueryReader_Release(query_reader2);
     PropVariantClear(&value);
@@ -5152,7 +5148,6 @@ static void test_metadata_App1(void)
     ok(!lstrcmpW(path, L"/app1/ifd"), "Unexpected path %s.\n", wine_dbgstr_w(path));
     hr = IWICMetadataQueryWriter_GetContainerFormat(query_writer2, &format);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-    todo_wine
     ok(IsEqualGUID(&format, &GUID_MetadataFormatIfd), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
     IWICMetadataQueryWriter_Release(query_writer2);
     PropVariantClear(&value);
@@ -5168,7 +5163,6 @@ static void test_metadata_App1(void)
     ok(!lstrcmpW(path, L"/app1/ifd/gps"), "Unexpected path %s.\n", wine_dbgstr_w(path));
     hr = IWICMetadataQueryWriter_GetContainerFormat(query_writer2, &format);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-    todo_wine
     ok(IsEqualGUID(&format, &GUID_MetadataFormatGps), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
     IWICMetadataQueryWriter_Release(query_writer2);
     PropVariantClear(&value);
@@ -5184,7 +5178,6 @@ static void test_metadata_App1(void)
     ok(!lstrcmpW(path, L"/app1/ifd/exif"), "Unexpected path %s.\n", wine_dbgstr_w(path));
     hr = IWICMetadataQueryWriter_GetContainerFormat(query_writer2, &format);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-    todo_wine
     ok(IsEqualGUID(&format, &GUID_MetadataFormatExif), "Unexpected format %s.\n", wine_dbgstr_guid(&format));
     IWICMetadataQueryWriter_Release(query_writer2);
     PropVariantClear(&value);
diff --git a/dlls/windowscodecs/wincodecs_private.h b/dlls/windowscodecs/wincodecs_private.h
index 9858a1fdebb..733c1e8c438 100644
--- a/dlls/windowscodecs/wincodecs_private.h
+++ b/dlls/windowscodecs/wincodecs_private.h
@@ -239,8 +239,8 @@ extern HRESULT IMDReader_CreateInstance(REFIID iid, void **ppv);
 extern HRESULT GCEReader_CreateInstance(REFIID iid, void **ppv);
 extern HRESULT APEReader_CreateInstance(REFIID iid, void **ppv);
 extern HRESULT GifCommentReader_CreateInstance(REFIID iid, void **ppv);
-extern HRESULT MetadataQueryReader_CreateInstance(IWICMetadataBlockReader *, const WCHAR *, IWICMetadataQueryReader **);
-extern HRESULT MetadataQueryWriter_CreateInstance(IWICMetadataBlockWriter *, const WCHAR *, IWICMetadataQueryWriter **);
+extern HRESULT MetadataQueryReader_CreateInstance(IWICMetadataBlockReader *, IWICMetadataQueryReader **);
+extern HRESULT MetadataQueryWriter_CreateInstance(IWICMetadataBlockWriter *, IWICMetadataQueryWriter **);
 extern HRESULT stream_initialize_from_filehandle(IWICStream *iface, HANDLE hfile);

static inline const char *debug_wic_rect(const WICRect *rect)

[PATCH v2 10/10] windowscodecs: Implement CreateQueryWriter().

[Nikolay Sivov]

From: Nikolay Sivov

Signed-off-by: Nikolay Sivov 
---
 dlls/windowscodecs/encoder.c           |  2 +-
 dlls/windowscodecs/gifformat.c         |  6 ++--
 dlls/windowscodecs/imgfactory.c        | 25 ++++++++++----
 dlls/windowscodecs/metadataquery.c     | 48 ++++++++++++++++++++++++--
 dlls/windowscodecs/tests/metadata.c    | 27 ++++++++-------
 dlls/windowscodecs/wincodecs_private.h |  6 ++--
 6 files changed, 85 insertions(+), 29 deletions(-)

diff --git a/dlls/windowscodecs/encoder.c b/dlls/windowscodecs/encoder.c
index de1dadc70c2..e9b9ae181ab 100644
--- a/dlls/windowscodecs/encoder.c
+++ b/dlls/windowscodecs/encoder.c
@@ -479,7 +479,7 @@ static HRESULT WINAPI CommonEncoderFrame_GetMetadataQueryWriter(IWICBitmapFrameE
     if (!(encoder->parent->encoder_info.flags & ENCODER_FLAGS_SUPPORTS_METADATA))
         return WINCODEC_ERR_UNSUPPORTEDOPERATION;

-    return MetadataQueryWriter_CreateInstance(&encoder->IWICMetadataBlockWriter_iface, ppIMetadataQueryWriter);
+    return MetadataQueryWriter_CreateInstanceFromBlockWriter(&encoder->IWICMetadataBlockWriter_iface, ppIMetadataQueryWriter);
 }

static const IWICBitmapFrameEncodeVtbl CommonEncoderFrame_Vtbl = {
diff --git a/dlls/windowscodecs/gifformat.c b/dlls/windowscodecs/gifformat.c
index a347ee3bc50..554d4cf14fd 100644
--- a/dlls/windowscodecs/gifformat.c
+++ b/dlls/windowscodecs/gifformat.c
@@ -837,7 +837,7 @@ static HRESULT WINAPI GifFrameDecode_GetMetadataQueryReader(IWICBitmapFrameDecod
     if (!ppIMetadataQueryReader)
         return E_INVALIDARG;

-    return MetadataQueryReader_CreateInstance(&This->IWICMetadataBlockReader_iface, ppIMetadataQueryReader);
+    return MetadataQueryReader_CreateInstanceFromBlockReader(&This->IWICMetadataBlockReader_iface, ppIMetadataQueryReader);
 }

static HRESULT WINAPI GifFrameDecode_GetColorContexts(IWICBitmapFrameDecode *iface,
@@ -1216,7 +1216,7 @@ static HRESULT WINAPI GifDecoder_GetMetadataQueryReader(IWICBitmapDecoder *iface

if (!ppIMetadataQueryReader) return E_INVALIDARG;

-    return MetadataQueryReader_CreateInstance(&This->IWICMetadataBlockReader_iface, ppIMetadataQueryReader);
+    return MetadataQueryReader_CreateInstanceFromBlockReader(&This->IWICMetadataBlockReader_iface, ppIMetadataQueryReader);
 }

static HRESULT WINAPI GifDecoder_GetPreview(IWICBitmapDecoder *iface,
@@ -2098,7 +2098,7 @@ static HRESULT WINAPI GifFrameEncode_GetMetadataQueryWriter(IWICBitmapFrameEncod
     if (!encode->initialized)
         return WINCODEC_ERR_NOTINITIALIZED;

-    return MetadataQueryWriter_CreateInstance(&encode->IWICMetadataBlockWriter_iface, writer);
+    return MetadataQueryWriter_CreateInstanceFromBlockWriter(&encode->IWICMetadataBlockWriter_iface, writer);
 }

static const IWICBitmapFrameEncodeVtbl GifFrameEncode_Vtbl =
diff --git a/dlls/windowscodecs/imgfactory.c b/dlls/windowscodecs/imgfactory.c
index 5af765e3852..8de1d370a38 100644
--- a/dlls/windowscodecs/imgfactory.c
+++ b/dlls/windowscodecs/imgfactory.c
@@ -1000,12 +1000,23 @@ static HRESULT WINAPI ImagingFactory_CreateFastMetadataEncoderFromFrameDecode(
 }

static HRESULT WINAPI ImagingFactory_CreateQueryWriter(IWICImagingFactory2 *iface,
-    REFGUID guidMetadataFormat, const GUID *pguidVendor,
-    IWICMetadataQueryWriter **ppIQueryWriter)
+        REFGUID format, const GUID *vendor, IWICMetadataQueryWriter **query_writer)
 {
-    FIXME("(%p,%s,%s,%p): stub\n", iface, debugstr_guid(guidMetadataFormat),
-        debugstr_guid(pguidVendor), ppIQueryWriter);
-    return E_NOTIMPL;
+    IWICMetadataWriter *writer;
+    HRESULT hr;
+
+    TRACE("(%p,%s,%s,%p)\n", iface, debugstr_guid(format), debugstr_guid(vendor), query_writer);
+
+    *query_writer = NULL;
+
+    hr = create_metadata_writer(format, vendor, 0, &writer);
+    if (SUCCEEDED(hr))
+    {
+        hr = MetadataQueryWriter_CreateInstance(writer, query_writer);
+        IWICMetadataWriter_Release(writer);
+    }
+
+    return hr;
 }

static HRESULT WINAPI ImagingFactory_CreateQueryWriterFromReader(IWICImagingFactory2 *iface,
@@ -1769,7 +1780,7 @@ static HRESULT WINAPI ComponentFactory_CreateQueryReaderFromBlockReader(IWICComp
     if (!block_reader || !query_reader)
         return E_INVALIDARG;

-    return MetadataQueryReader_CreateInstance(block_reader, query_reader);
+    return MetadataQueryReader_CreateInstanceFromBlockReader(block_reader, query_reader);
 }

static HRESULT WINAPI ComponentFactory_CreateQueryWriterFromBlockWriter(IWICComponentFactory *iface,
@@ -1780,7 +1791,7 @@ static HRESULT WINAPI ComponentFactory_CreateQueryWriterFromBlockWriter(IWICComp
     if (!block_writer || !query_writer)
         return E_INVALIDARG;

-    return MetadataQueryWriter_CreateInstance(block_writer, query_writer);
+    return MetadataQueryWriter_CreateInstanceFromBlockWriter(block_writer, query_writer);
 }

static HRESULT WINAPI ComponentFactory_CreateEncoderPropertyBag(IWICComponentFactory *iface,
diff --git a/dlls/windowscodecs/metadataquery.c b/dlls/windowscodecs/metadataquery.c
index b0f454d9fe3..d8289c44201 100644
--- a/dlls/windowscodecs/metadataquery.c
+++ b/dlls/windowscodecs/metadataquery.c
@@ -735,6 +735,17 @@ static HRESULT WINAPI query_handler_GetMetadataByName(IWICMetadataQueryWriter *i
     return hr;
 }

+static WCHAR *query_get_guid_item_string(WCHAR *str, unsigned int len, const GUID *guid)
+{
+    if (SUCCEEDED(WICMapGuidToShortName(guid, len, str, NULL)))
+        return str;
+
+    swprintf(str, len, L"{guid=%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x}",
+            guid->Data1, guid->Data2, guid->Data3, guid->Data4[0], guid->Data4[1], guid->Data4[2],
+            guid->Data4[3], guid->Data4[4], guid->Data4[5], guid->Data4[6], guid->Data4[7]);
+    return str;
+}
+
 struct string_enumerator
 {
     IEnumString IEnumString_iface;
@@ -886,6 +897,9 @@ static HRESULT create_query_handler(IUnknown *block_handler, enum metadata_objec
         const WCHAR *root, IWICMetadataQueryWriter **ret)
 {
     struct query_handler *obj;
+    WCHAR buff[64];
+    HRESULT hr;
+    GUID guid;

obj = calloc(1, sizeof(*obj));
     if (!obj)
@@ -897,7 +911,25 @@ static HRESULT create_query_handler(IUnknown *block_handler, enum metadata_objec
     obj->object.handler = block_handler;
     obj->object_type = object_type;
     if (!root)
-        root = L"/";
+    {
+        if (is_block_handler(obj))
+        {
+            root = L"/";
+        }
+        else
+        {
+            if (FAILED(hr = IWICMetadataReader_GetMetadataFormat(obj->object.reader, &guid)))
+            {
+                IWICMetadataQueryWriter_Release(&obj->IWICMetadataQueryWriter_iface);
+                return hr;
+            }
+
+            buff[0] = '/';
+            query_get_guid_item_string(buff + 1, ARRAY_SIZE(buff) - 1, &guid);
+            root = buff;
+        }
+    }
+
     obj->root = wcsdup(root);

*ret = &obj->IWICMetadataQueryWriter_iface;
@@ -905,16 +937,26 @@ static HRESULT create_query_handler(IUnknown *block_handler, enum metadata_objec
     return S_OK;
 }

-HRESULT MetadataQueryReader_CreateInstance(IWICMetadataBlockReader *block_reader, IWICMetadataQueryReader **out)
+HRESULT MetadataQueryReader_CreateInstanceFromBlockReader(IWICMetadataBlockReader *block_reader, IWICMetadataQueryReader **out)
 {
     return create_query_handler((IUnknown *)block_reader, BLOCK_READER, NULL, (IWICMetadataQueryWriter **)out);
 }

-HRESULT MetadataQueryWriter_CreateInstance(IWICMetadataBlockWriter *block_writer, IWICMetadataQueryWriter **out)
+HRESULT MetadataQueryWriter_CreateInstanceFromBlockWriter(IWICMetadataBlockWriter *block_writer, IWICMetadataQueryWriter **out)
 {
     return create_query_handler((IUnknown *)block_writer, BLOCK_WRITER, NULL, out);
 }

+HRESULT MetadataQueryReader_CreateInstance(IWICMetadataReader *reader, IWICMetadataQueryReader **out)
+{
+    return create_query_handler((IUnknown *)reader, READER, NULL, (IWICMetadataQueryWriter **)out);
+}
+
+HRESULT MetadataQueryWriter_CreateInstance(IWICMetadataWriter *writer, IWICMetadataQueryWriter **out)
+{
+    return create_query_handler((IUnknown *)writer, WRITER, NULL, out);
+}
+
 static const struct
 {
     const GUID *guid;
diff --git a/dlls/windowscodecs/tests/metadata.c b/dlls/windowscodecs/tests/metadata.c
index f0322da21f7..8e22e111796 100644
--- a/dlls/windowscodecs/tests/metadata.c
+++ b/dlls/windowscodecs/tests/metadata.c
@@ -5666,13 +5666,7 @@ static void test_CreateQueryWriter(void)

/* "Unknown" format. */
     hr = IWICImagingFactory_CreateQueryWriter(factory, &GUID_MetadataFormatUnknown, NULL, &query_writer);
-    todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-    if (FAILED(hr))
-    {
-        IWICImagingFactory_Release(factory);
-        return;
-    }

hr = IWICMetadataQueryWriter_GetLocation(query_writer, ARRAY_SIZE(buff), buff, &len);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
@@ -5684,18 +5678,26 @@ static void test_CreateQueryWriter(void)

PropVariantInit(&value);
     hr = IWICMetadataQueryWriter_GetMetadataByName(query_writer, L"/", &value);
+    todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-    ok(value.vt == VT_BLOB, "Unexpected value type %u.\n", value.vt);
-    ok(!value.blob.cbSize, "Unexpected size %lu.\n", value.blob.cbSize);
-    ok(!value.blob.pBlobData, "Unexpected data pointer %p.\n", value.blob.pBlobData);
-    PropVariantClear(&value);
+    if (hr == S_OK)
+    {
+        ok(value.vt == VT_BLOB, "Unexpected value type %u.\n", value.vt);
+        ok(!value.blob.cbSize, "Unexpected size %lu.\n", value.blob.cbSize);
+        ok(!value.blob.pBlobData, "Unexpected data pointer %p.\n", value.blob.pBlobData);
+        PropVariantClear(&value);
+    }

hr = IWICMetadataQueryWriter_GetEnumerator(query_writer, &enum_string);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
     hr = IEnumString_Next(enum_string, 1, &str, &fetched);
+    todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
-    ok(!wcscmp(str, L"/{}"), "Unexpected string %s.\n", wine_dbgstr_w(str));
-    CoTaskMemFree(str);
+    if (hr == S_OK)
+    {
+        ok(!wcscmp(str, L"/{}"), "Unexpected string %s.\n", wine_dbgstr_w(str));
+        CoTaskMemFree(str);
+    }
     hr = IEnumString_Next(enum_string, 1, &str, &fetched);
     ok(hr == S_FALSE, "Unexpected hr %#lx.\n", hr);
     IEnumString_Release(enum_string);
@@ -5810,7 +5812,6 @@ if (hr == S_OK)
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);

hr = IWICComponentFactory_CreateQueryWriter(factory, &GUID_MetadataFormatChunktIME, NULL, &query_writer);
-    todo_wine
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);

hr = IWICComponentFactory_CreateQueryWriterFromReader(factory, (IWICMetadataQueryReader *)query_writer,
diff --git a/dlls/windowscodecs/wincodecs_private.h b/dlls/windowscodecs/wincodecs_private.h
index 733c1e8c438..5b193bb1b26 100644
--- a/dlls/windowscodecs/wincodecs_private.h
+++ b/dlls/windowscodecs/wincodecs_private.h
@@ -239,8 +239,10 @@ extern HRESULT IMDReader_CreateInstance(REFIID iid, void **ppv);
 extern HRESULT GCEReader_CreateInstance(REFIID iid, void **ppv);
 extern HRESULT APEReader_CreateInstance(REFIID iid, void **ppv);
 extern HRESULT GifCommentReader_CreateInstance(REFIID iid, void **ppv);
-extern HRESULT MetadataQueryReader_CreateInstance(IWICMetadataBlockReader *, IWICMetadataQueryReader **);
-extern HRESULT MetadataQueryWriter_CreateInstance(IWICMetadataBlockWriter *, IWICMetadataQueryWriter **);
+extern HRESULT MetadataQueryReader_CreateInstanceFromBlockReader(IWICMetadataBlockReader *, IWICMetadataQueryReader **);
+extern HRESULT MetadataQueryWriter_CreateInstanceFromBlockWriter(IWICMetadataBlockWriter *, IWICMetadataQueryWriter **);
+extern HRESULT MetadataQueryReader_CreateInstance(IWICMetadataReader *, IWICMetadataQueryReader **);
+extern HRESULT MetadataQueryWriter_CreateInstance(IWICMetadataWriter *, IWICMetadataQueryWriter **);
 extern HRESULT stream_initialize_from_filehandle(IWICStream *iface, HANDLE hfile);

static inline const char *debug_wic_rect(const WICRect *rect)

[Nikolay Sivov (＠nsivov)]

On Fri Feb 14 01:48:04 2025 +0000, Nikolay Sivov wrote:
> changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/7329/diffs?diff_id=157519&start_sha=1eef8b4edd8bfb23bb5e19d7443f11366b06009c#58988b8c3e6898b4fb4a6e0c70701153665ecaa7_414_414)

Right, this makes no sense. Changed now.

[Nikolay Sivov (＠nsivov)]

On Fri Feb 14 00:57:53 2025 +0000, Esme Povirk wrote:
> I don't think `reader` is ever released.

Fixed.

[Nikolay Sivov (＠nsivov)]

On Fri Feb 14 00:59:54 2025 +0000, Esme Povirk wrote:
> I don't think this is released before `query_reader` is assigned NULL below.

Fixed as well.

[Esme Povirk (＠madewokherd)]

The amount of complexity here is making review difficult. This may take a while.

[Esme Povirk (＠madewokherd)]

Esme Povirk (@madewokherd) commented about dlls/windowscodecs/tests/metadata.c:
> +if (hr == S_OK)
> +{
> - hr = IWICMetadataQueryWriter_GetLocation(query_writer, ARRAY_SIZE(buff), buff, &len);
> - ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
> - ok(!wcscmp(buff, L"/ifd"), "Unexpected location %s.\n", wine_dbgstr_w(buff));
> - IWICMetadataQueryWriter_Release(query_writer);
>
> +}
> -
> - IWICMetadataQueryReader_Release(query_reader);
> - IWICMetadataQueryReader_Release(query_reader2);
> -
> - PropVariantClear(&value);
> -
> - /* IWICMetadataReader -> IWICMetadataWriter -> IWICMetadataQueryWriter -> CreateQueryWriterFromReader(). */
> - hr = create_query_writer_from_metadata_reader(factory, reader, &GUID_ContainerFormatJpeg, &query_writer);
> - ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);

The `query_writer` object created here is overwritten a few lines later.

[Esme Povirk (＠madewokherd)]

Esme Povirk (@madewokherd) commented about dlls/windowscodecs/tests/metadata.c:
> - ok(!wcscmp(buff, L"/ifd"), "Unexpected location %s.\n", wine_dbgstr_w(buff));
> - IWICMetadataQueryWriter_Release(query_writer);
>
> +}
> -
> - IWICMetadataQueryReader_Release(query_reader);
> - IWICMetadataQueryReader_Release(query_reader2);
> -
> - PropVariantClear(&value);
> -
> - /* IWICMetadataReader -> IWICMetadataWriter -> IWICMetadataQueryWriter -> CreateQueryWriterFromReader(). */
> - hr = create_query_writer_from_metadata_reader(factory, reader, &GUID_ContainerFormatJpeg, &query_writer);
> - ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
> -
> - hr = IWICComponentFactory_CreateQueryWriter(factory, &GUID_MetadataFormatChunktIME, NULL, &query_writer);
> - todo_wine
> - ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);

This one is overwritten as well.

[Esme Povirk (＠madewokherd)]

Esme Povirk (@madewokherd) commented about dlls/windowscodecs/tests/metadata.c:
> - IWICMetadataQueryReader_Release(query_reader2);
> -
> - PropVariantClear(&value);
> -
> - /* IWICMetadataReader -> IWICMetadataWriter -> IWICMetadataQueryWriter -> CreateQueryWriterFromReader(). */
> - hr = create_query_writer_from_metadata_reader(factory, reader, &GUID_ContainerFormatJpeg, &query_writer);
> - ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
> -
> - hr = IWICComponentFactory_CreateQueryWriter(factory, &GUID_MetadataFormatChunktIME, NULL, &query_writer);
> - todo_wine
> - ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
> -
> - hr = IWICComponentFactory_CreateQueryWriterFromReader(factory, (IWICMetadataQueryReader *)query_writer,
> -
>        NULL, &query_writer2);
> - todo_wine
> - ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);

`query_writer2` is never freed.

[Esme Povirk (＠madewokherd)]

Esme Povirk (@madewokherd) commented about dlls/windowscodecs/tests/metadata.c:
> -
> - hr = IWICComponentFactory_CreateQueryWriterFromReader(factory, (IWICMetadataQueryReader *)query_writer,
> -
>        NULL, &query_writer2);
> - todo_wine
> - ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
> -
> - hr = create_query_writer(factory, &data1, &query_writer);
> - ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
> -
> - if (hr == S_OK)
> -
>    IWICMetadataQueryWriter_Release(query_writer);
> -
> - /* Using decoder frame. */
> - hr = CoCreateInstance(&CLSID_WICPngDecoder, NULL, CLSCTX_INPROC_SERVER,
> -
>        &IID_IWICBitmapDecoder, (void **)&decoder);
> - ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);

`decoder` is never freed.

[Esme Povirk (＠madewokherd)]

Esme Povirk (@madewokherd) commented about dlls/windowscodecs/tests/metadata.c:
> -
>    IWICMetadataQueryWriter_Release(query_writer);
> -
> - /* Using decoder frame. */
> - hr = CoCreateInstance(&CLSID_WICPngDecoder, NULL, CLSCTX_INPROC_SERVER,
> -
>        &IID_IWICBitmapDecoder, (void **)&decoder);
> - ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
> -
> - stream = create_stream(pngimage, sizeof(pngimage));
> - hr = IWICBitmapDecoder_Initialize(decoder, stream, WICDecodeMetadataCacheOnLoad);
> - ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
> - IStream_Release(stream);
> -
> - hr = IWICBitmapDecoder_GetFrame(decoder, 0, &decoder_frame);
> - ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
> - hr = IWICBitmapFrameDecode_QueryInterface(decoder_frame, &IID_IWICMetadataBlockReader, (void **)&block_reader);
> - ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);

`block_reader` is never freed.

[Esme Povirk (＠madewokherd)]

Can you split up the patch "windowscodecs/metadata: Split up query parsing into helpers according to the query grammar"? I feel that a lot of the rework could be done while keeping the `get_token` interface intact, even if it doesn't survive to the end.

[Nikolay Sivov (＠nsivov)]

On Fri Feb 14 15:25:22 2025 +0000, Esme Povirk wrote:
> Can you split up the patch "windowscodecs/metadata: Split up query
> parsing into helpers according to the query grammar"? I feel that a lot
> of the rework could be done while keeping the `get_token` interface
> intact, even if it doesn't survive to the end.

Ok, I’ll see how difficult it would be. It’s probably could be used in place of add_component() during such transition. What I definitely want to get rid of is recursive calls to get_token() at the end, and other simplifications.