[PATCH 0/3] MR7653: windowscodecs/metadata: Always initialize handler pointer.
In case of empty block reader we currently return uninitialized pointer that is accessed later. Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7653
From: Nikolay Sivov <nsivov(a)codeweavers.com> In case of empty block reader we currently return uninitialized pointer that is accessed later. Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/windowscodecs/metadataquery.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/windowscodecs/metadataquery.c b/dlls/windowscodecs/metadataquery.c index 609ecb7ec42..8dc71906b50 100644 --- a/dlls/windowscodecs/metadataquery.c +++ b/dlls/windowscodecs/metadataquery.c @@ -528,8 +528,8 @@ static void parse_query_component(struct query_parser *parser) static HRESULT parser_set_top_level_metadata_handler(struct query_handler *query_handler, struct query_parser *parser) { + IWICMetadataReader *handler = NULL; struct query_component *comp; - IWICMetadataReader *handler; HRESULT hr; GUID format; UINT count, i, matched_index; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7653
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/windowscodecs/libjpeg.c | 93 +++++++++++++++++++++++++- dlls/windowscodecs/metadataquery.c | 2 +- dlls/windowscodecs/wincodecs_private.h | 2 + 3 files changed, 94 insertions(+), 3 deletions(-) diff --git a/dlls/windowscodecs/libjpeg.c b/dlls/windowscodecs/libjpeg.c index ad6f58e3f02..806cdfb0a99 100644 --- a/dlls/windowscodecs/libjpeg.c +++ b/dlls/windowscodecs/libjpeg.c @@ -37,6 +37,13 @@ WINE_DEFAULT_DEBUG_CHANNEL(wincodecs); WINE_DECLARE_DEBUG_CHANNEL(jpeg); +enum jpeg_markers +{ + APP1 = 0xffe1, + SOS = 0xffda, + EOI = 0xffd9, +}; + static void error_exit_fn(j_common_ptr cinfo) { char message[JMSG_LENGTH_MAX]; @@ -323,10 +330,92 @@ static HRESULT CDECL jpeg_decoder_copy_pixels(struct decoder* iface, UINT frame, static HRESULT CDECL jpeg_decoder_get_metadata_blocks(struct decoder* iface, UINT frame, UINT *count, struct decoder_block **blocks) { - FIXME("stub\n"); + struct jpeg_decoder *This = impl_from_decoder(iface); + struct + { + struct decoder_block *blocks; + size_t capacity, count; + } results; + struct decoder_block block; + USHORT marker, length; + ULONGLONG offset; + BYTE header[4]; + bool add_block; + HRESULT hr; + *count = 0; *blocks = NULL; - return S_OK; + + /* Skip SOI marker. */ + hr = stream_seek(This->stream, 2, STREAM_SEEK_SET, NULL); + if (FAILED(hr)) + return hr; + + /* TODO: support for App0, Xmp, and chrominance/luminance blocks is missing. */ + + marker = 0; + offset = 2; + memset(&results, 0, sizeof(results)); + for (;;) + { + if (stream_read(This->stream, header, 4, NULL) != S_OK) + break; + offset += 4; + + marker = header[0] << 8 | header[1]; + length = header[2] * 256 + header[3] - 2; + + add_block = false; + if (marker == APP1) + { + /* APP1 marker might appear multiple times, it's reused for different metadata blocks. */ + if (stream_read(This->stream, header, 4, NULL) != S_OK) + break; + stream_seek(This->stream, -4, STREAM_SEEK_CUR, NULL); + + if (!memcmp(header, "Exif", 4)) + { + block.offset = offset; + block.length = length; + block.options = DECODER_BLOCK_READER_CLSID; + block.reader_clsid = CLSID_WICApp1MetadataReader; + + add_block = true; + } + } + else if (marker == EOI || marker == SOS) + { + break; + } + + if (add_block) + { + if (!wincodecs_array_reserve((void **)&results.blocks, &results.capacity, + results.count + 1, sizeof(*results.blocks))) + { + hr = E_OUTOFMEMORY; + break; + } + + results.blocks[results.count++] = block; + } + + if (FAILED(stream_seek(This->stream, length, STREAM_SEEK_CUR, NULL))) + break; + offset += length; + } + + if (hr == S_OK) + { + *count = results.count; + *blocks = results.blocks; + } + else + { + free(results.blocks); + } + + return hr; } static HRESULT CDECL jpeg_decoder_get_color_context(struct decoder* This, UINT frame, UINT num, diff --git a/dlls/windowscodecs/metadataquery.c b/dlls/windowscodecs/metadataquery.c index 8dc71906b50..9794a35214b 100644 --- a/dlls/windowscodecs/metadataquery.c +++ b/dlls/windowscodecs/metadataquery.c @@ -247,7 +247,7 @@ struct query_parser HRESULT hr; }; -static bool wincodecs_array_reserve(void **elements, size_t *capacity, size_t count, size_t size) +bool wincodecs_array_reserve(void **elements, size_t *capacity, size_t count, size_t size) { size_t new_capacity, max_capacity; void *new_elements; diff --git a/dlls/windowscodecs/wincodecs_private.h b/dlls/windowscodecs/wincodecs_private.h index 9cf842721f7..48de3971e15 100644 --- a/dlls/windowscodecs/wincodecs_private.h +++ b/dlls/windowscodecs/wincodecs_private.h @@ -245,6 +245,8 @@ extern HRESULT MetadataQueryReader_CreateInstance(IWICMetadataReader *, IWICMeta extern HRESULT MetadataQueryWriter_CreateInstance(IWICMetadataWriter *, IWICMetadataQueryWriter **); extern HRESULT stream_initialize_from_filehandle(IWICStream *iface, HANDLE hfile); +extern bool wincodecs_array_reserve(void **elements, size_t *capacity, size_t count, size_t size); + static inline const char *debug_wic_rect(const WICRect *rect) { if (!rect) return "(null)"; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7653
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/windowscodecs/tests/metadata.c | 121 +++++++++++++++++++++++++++- 1 file changed, 120 insertions(+), 1 deletion(-) diff --git a/dlls/windowscodecs/tests/metadata.c b/dlls/windowscodecs/tests/metadata.c index 9fd42805e04..68c4c063d02 100644 --- a/dlls/windowscodecs/tests/metadata.c +++ b/dlls/windowscodecs/tests/metadata.c @@ -402,6 +402,25 @@ static const char animatedgif[] = { 0x21,0x01,0x0C,'p','l','a','i','n','t','e','x','t',' ','#','2',0x00,0x3B }; +/* convert -size 4x8 canvas:white white.jpeg */ +static const char jpeg[] = +{ + 0xff, 0xd8, 0xff, 0xe0, 0x00, 0x10, 0x4a, 0x46, 0x49, 0x46, 0x00, 0x01, + 0x01, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0xff, 0xdb, 0x00, 0x43, + 0x00, 0x03, 0x02, 0x02, 0x02, 0x02, 0x02, 0x03, 0x02, 0x02, 0x02, 0x03, + 0x03, 0x03, 0x03, 0x04, 0x06, 0x04, 0x04, 0x04, 0x04, 0x04, 0x08, 0x06, + 0x06, 0x05, 0x06, 0x09, 0x08, 0x0a, 0x0a, 0x09, 0x08, 0x09, 0x09, 0x0a, + 0x0c, 0x0f, 0x0c, 0x0a, 0x0b, 0x0e, 0x0b, 0x09, 0x09, 0x0d, 0x11, 0x0d, + 0x0e, 0x0f, 0x10, 0x10, 0x11, 0x10, 0x0a, 0x0c, 0x12, 0x13, 0x12, 0x10, + 0x13, 0x0f, 0x10, 0x10, 0x10, 0xff, 0xc0, 0x00, 0x0b, 0x08, 0x00, 0x08, + 0x00, 0x04, 0x01, 0x01, 0x11, 0x00, 0xff, 0xc4, 0x00, 0x14, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x09, 0xff, 0xc4, 0x00, 0x14, 0x10, 0x01, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0xff, 0xda, 0x00, 0x08, 0x01, 0x01, 0x00, 0x00, 0x3f, 0x00, + 0x54, 0xdf, 0xff, 0xd9 +}; + static ULONG get_refcount(void *iface) { IUnknown *unknown = iface; @@ -620,8 +639,12 @@ static void compare_metadata(IWICMetadataReader *reader, const struct test_data ok(td[i].count == value.blob.cbSize, "Expected count %d, got %ld\n", td[i].count, value.blob.cbSize); ok(!memcmp(td[i].string, value.blob.pBlobData, td[i].count), "Expected %s, got %s\n", td[i].string, value.blob.pBlobData); } + else if (value.vt == VT_UI1) + { + ok(value.bVal == td[i].value[0], "Expected value %#x got %#x.\n", (BYTE)td[i].value[0], value.bVal); + } else - ok(value.uhVal.QuadPart == td[i].value[0], "Eexpected value %#I64x got %#lx/%#lx\n", + ok(value.uhVal.QuadPart == td[i].value[0], "Expected value %#I64x got %#lx/%#lx\n", td[i].value[0], value.uhVal.u.LowPart, value.uhVal.u.HighPart); PropVariantClear(&schema); @@ -5578,6 +5601,101 @@ static void test_metadata_App1(void) IWICComponentFactory_Release(factory); } +static void test_metadata_App0(void) +{ + static const struct test_data default_data[] = + { + { VT_UI2, 0, 0, { 0 }, NULL, L"Version" }, + { VT_UI1, 1, 0, { 0 }, NULL, L"Units" }, + { VT_UI2, 2, 0, { 0 }, NULL, L"DpiX" }, + { VT_UI2, 3, 0, { 0 }, NULL, L"DpiY" }, + { VT_UI1, 4, 0, { 0 }, NULL, L"Xthumbnail" }, + { VT_UI1, 5, 0, { 0 }, NULL, L"Ythumbnail" }, + { VT_BLOB, 6, 0, { 0 }, NULL, L"ThumbnailData" }, + }; + + static const struct test_data td[] = + { + { VT_UI2, 0, 0, { 0x101 }, NULL, L"Version" }, + { VT_UI1, 1, 0, { 0 }, NULL, L"Units" }, + { VT_UI2, 2, 0, { 0x1 }, NULL, L"DpiX" }, + { VT_UI2, 3, 0, { 0x1 }, NULL, L"DpiY" }, + { VT_UI1, 4, 0, { 0 }, NULL, L"Xthumbnail" }, + { VT_UI1, 5, 0, { 0 }, NULL, L"Ythumbnail" }, + { VT_BLOB, 6, 0, { 0 }, NULL, L"ThumbnailData" }, + }; + + IWICMetadataReader *reader; + IWICMetadataWriter *writer; + PROPVARIANT id; + UINT count, i; + GUID format; + HRESULT hr; + + hr = CoCreateInstance(&CLSID_WICApp0MetadataReader, NULL, CLSCTX_INPROC_SERVER, + &IID_IWICMetadataReader, (void **)&reader); + todo_wine + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + if (FAILED(hr)) return; + + check_interface(reader, &IID_IWICMetadataReader, TRUE); + check_interface(reader, &IID_IPersist, TRUE); + check_interface(reader, &IID_IPersistStream, TRUE); + check_interface(reader, &IID_IWICPersistStream, TRUE); + check_interface(reader, &IID_IWICStreamProvider, TRUE); + check_interface(reader, &IID_IWICMetadataBlockReader, FALSE); + check_persist_classid(reader, &CLSID_WICApp0MetadataReader); + + hr = IWICMetadataReader_GetCount(reader, NULL); + ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); + + count = 0; + hr = IWICMetadataReader_GetCount(reader, &count); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(count == 7, "Unexpected count %u.\n", count); + compare_metadata(reader, default_data, count); + + for (i = 0; i < count; ++i) + { + id.vt = VT_EMPTY; + hr = IWICMetadataReader_GetValueByIndex(reader, i, NULL, &id, NULL); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(id.vt == VT_UI2, "Unexpected id type %d.\n", id.vt); + } + + hr = IWICMetadataReader_GetMetadataFormat(reader, &format); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(IsEqualGUID(&format, &GUID_MetadataFormatApp0), "Unexpected format %s.\n", wine_dbgstr_guid(&format)); + + test_reader_container_format(reader, &GUID_ContainerFormatJpeg); + + load_stream(reader, (const char *)jpeg + 6, 16, 0); + check_persist_options(reader, 0); + + hr = IWICMetadataReader_GetCount(reader, &count); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(count == 7, "Unexpected count %u.\n", count); + compare_metadata(reader, td, count); + + hr = CoCreateInstance(&CLSID_WICApp0MetadataWriter, NULL, CLSCTX_INPROC_SERVER, + &IID_IWICMetadataWriter, (void **)&writer); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + check_interface(writer, &IID_IWICMetadataWriter, TRUE); + check_interface(writer, &IID_IWICMetadataReader, TRUE); + check_interface(writer, &IID_IPersist, TRUE); + check_interface(writer, &IID_IPersistStream, TRUE); + check_interface(writer, &IID_IWICPersistStream, TRUE); + check_interface(writer, &IID_IWICStreamProvider, TRUE); + check_persist_classid(writer, &CLSID_WICApp0MetadataWriter); + + hr = IWICMetadataWriter_GetCount(writer, &count); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(count == 7, "Unexpected count %u.\n", count); + + IWICMetadataWriter_Release(writer); +} + static void test_CreateMetadataWriterFromReader(void) { IStream *stream, *stream2, *ifd_stream, *writer_stream; @@ -6380,6 +6498,7 @@ START_TEST(metadata) test_metadata_GIF_comment(); test_metadata_query_writer(); test_metadata_App1(); + test_metadata_App0(); test_CreateMetadataWriterFromReader(); test_CreateMetadataWriter(); test_metadata_writer(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7653
Esme Povirk (@madewokherd) commented about dlls/windowscodecs/tests/metadata.c:
+ { VT_UI2, 2, 0, { 0x1 }, NULL, L"DpiX" }, + { VT_UI2, 3, 0, { 0x1 }, NULL, L"DpiY" }, + { VT_UI1, 4, 0, { 0 }, NULL, L"Xthumbnail" }, + { VT_UI1, 5, 0, { 0 }, NULL, L"Ythumbnail" }, + { VT_BLOB, 6, 0, { 0 }, NULL, L"ThumbnailData" }, + }; + + IWICMetadataReader *reader; + IWICMetadataWriter *writer; + PROPVARIANT id; + UINT count, i; + GUID format; + HRESULT hr; + + hr = CoCreateInstance(&CLSID_WICApp0MetadataReader, NULL, CLSCTX_INPROC_SERVER, + &IID_IWICMetadataReader, (void **)&reader); `reader` is never released.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7653#note_98983
participants (3)
-
Esme Povirk (＠madewokherd) -
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)