[PATCH v2 0/4] MR40: mfplat: Consolidate stream and file IMFByteStream methods
-- v2: mfplat/tests: Add tests for IMFByteStream position. mfplat: Consolidate stream and file IMFByteStream Read/Write callback vtables. mfplat: Simplify IMFByteStream async Read/Write methods. mfplat: Consolidate stream and file IMFByteStream vtables. https://gitlab.winehq.org/wine/wine/-/merge_requests/40
From: Andrew Eikum <aeikum(a)codeweavers.com> Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> --- dlls/mfplat/main.c | 99 +++++++++++++++++++--------------------------- 1 file changed, 40 insertions(+), 59 deletions(-) diff --git a/dlls/mfplat/main.c b/dlls/mfplat/main.c index 24d39d81b62..9e6ff26b2f7 100644 --- a/dlls/mfplat/main.c +++ b/dlls/mfplat/main.c @@ -3954,7 +3954,7 @@ static HRESULT WINAPI bytestream_stream_GetCapabilities(IMFByteStream *iface, DW return S_OK; } -static HRESULT WINAPI bytestream_GetCapabilities(IMFByteStream *iface, DWORD *capabilities) +static HRESULT WINAPI bytestream_file_GetCapabilities(IMFByteStream *iface, DWORD *capabilities) { struct bytestream *stream = impl_from_IMFByteStream(iface); @@ -3965,14 +3965,14 @@ static HRESULT WINAPI bytestream_GetCapabilities(IMFByteStream *iface, DWORD *ca return S_OK; } -static HRESULT WINAPI bytestream_SetLength(IMFByteStream *iface, QWORD length) +static HRESULT WINAPI bytestream_file_SetLength(IMFByteStream *iface, QWORD length) { FIXME("%p, %s\n", iface, wine_dbgstr_longlong(length)); return E_NOTIMPL; } -static HRESULT WINAPI bytestream_file_GetCurrentPosition(IMFByteStream *iface, QWORD *position) +static HRESULT WINAPI bytestream_GetCurrentPosition(IMFByteStream *iface, QWORD *position) { struct bytestream *stream = impl_from_IMFByteStream(iface); @@ -4070,7 +4070,7 @@ static HRESULT WINAPI bytestream_EndRead(IMFByteStream *iface, IMFAsyncResult *r return bytestream_complete_io_request(stream, ASYNC_STREAM_OP_READ, result, byte_read); } -static HRESULT WINAPI bytestream_Write(IMFByteStream *iface, const BYTE *data, ULONG count, ULONG *written) +static HRESULT WINAPI bytestream_file_Write(IMFByteStream *iface, const BYTE *data, ULONG count, ULONG *written) { FIXME("%p, %p, %lu, %p\n", iface, data, count, written); @@ -4096,22 +4096,44 @@ static HRESULT WINAPI bytestream_EndWrite(IMFByteStream *iface, IMFAsyncResult * return bytestream_complete_io_request(stream, ASYNC_STREAM_OP_WRITE, result, written); } -static HRESULT WINAPI bytestream_Seek(IMFByteStream *iface, MFBYTESTREAM_SEEK_ORIGIN seek, LONGLONG offset, - DWORD flags, QWORD *current) +static HRESULT WINAPI bytestream_Seek(IMFByteStream *iface, MFBYTESTREAM_SEEK_ORIGIN origin, LONGLONG offset, + DWORD flags, QWORD *current) { - FIXME("%p, %u, %s, 0x%08lx, %p\n", iface, seek, wine_dbgstr_longlong(offset), flags, current); + struct bytestream *stream = impl_from_IMFByteStream(iface); + HRESULT hr = S_OK; - return E_NOTIMPL; + TRACE("%p, %u, %s, %#lx, %p.\n", iface, origin, wine_dbgstr_longlong(offset), flags, current); + + EnterCriticalSection(&stream->cs); + + switch (origin) + { + case msoBegin: + stream->position = offset; + break; + case msoCurrent: + stream->position += offset; + break; + default: + WARN("Unknown origin mode %d.\n", origin); + hr = E_INVALIDARG; + } + + *current = stream->position; + + LeaveCriticalSection(&stream->cs); + + return hr; } -static HRESULT WINAPI bytestream_Flush(IMFByteStream *iface) +static HRESULT WINAPI bytestream_file_Flush(IMFByteStream *iface) { FIXME("%p\n", iface); return E_NOTIMPL; } -static HRESULT WINAPI bytestream_Close(IMFByteStream *iface) +static HRESULT WINAPI bytestream_file_Close(IMFByteStream *iface) { FIXME("%p\n", iface); @@ -4136,21 +4158,21 @@ static const IMFByteStreamVtbl bytestream_file_vtbl = bytestream_QueryInterface, bytestream_AddRef, bytestream_Release, - bytestream_GetCapabilities, + bytestream_file_GetCapabilities, bytestream_file_GetLength, - bytestream_SetLength, - bytestream_file_GetCurrentPosition, + bytestream_file_SetLength, + bytestream_GetCurrentPosition, bytestream_SetCurrentPosition, bytestream_file_IsEndOfStream, bytestream_file_Read, bytestream_BeginRead, bytestream_EndRead, - bytestream_Write, + bytestream_file_Write, bytestream_BeginWrite, bytestream_EndWrite, bytestream_Seek, - bytestream_Flush, - bytestream_Close + bytestream_file_Flush, + bytestream_file_Close }; static HRESULT WINAPI bytestream_stream_GetLength(IMFByteStream *iface, QWORD *length) @@ -4187,17 +4209,6 @@ static HRESULT WINAPI bytestream_stream_SetLength(IMFByteStream *iface, QWORD le return hr; } -static HRESULT WINAPI bytestream_stream_GetCurrentPosition(IMFByteStream *iface, QWORD *position) -{ - struct bytestream *stream = impl_from_IMFByteStream(iface); - - TRACE("%p, %p.\n", iface, position); - - *position = stream->position; - - return S_OK; -} - static HRESULT WINAPI bytestream_stream_IsEndOfStream(IMFByteStream *iface, BOOL *ret) { struct bytestream *stream = impl_from_IMFByteStream(iface); @@ -4260,36 +4271,6 @@ static HRESULT WINAPI bytestream_stream_Write(IMFByteStream *iface, const BYTE * return hr; } -static HRESULT WINAPI bytestream_stream_Seek(IMFByteStream *iface, MFBYTESTREAM_SEEK_ORIGIN origin, LONGLONG offset, - DWORD flags, QWORD *current) -{ - struct bytestream *stream = impl_from_IMFByteStream(iface); - HRESULT hr = S_OK; - - TRACE("%p, %u, %s, %#lx, %p.\n", iface, origin, wine_dbgstr_longlong(offset), flags, current); - - EnterCriticalSection(&stream->cs); - - switch (origin) - { - case msoBegin: - stream->position = offset; - break; - case msoCurrent: - stream->position += offset; - break; - default: - WARN("Unknown origin mode %d.\n", origin); - hr = E_INVALIDARG; - } - - *current = stream->position; - - LeaveCriticalSection(&stream->cs); - - return hr; -} - static HRESULT WINAPI bytestream_stream_Flush(IMFByteStream *iface) { struct bytestream *stream = impl_from_IMFByteStream(iface); @@ -4314,7 +4295,7 @@ static const IMFByteStreamVtbl bytestream_stream_vtbl = bytestream_stream_GetCapabilities, bytestream_stream_GetLength, bytestream_stream_SetLength, - bytestream_stream_GetCurrentPosition, + bytestream_GetCurrentPosition, bytestream_SetCurrentPosition, bytestream_stream_IsEndOfStream, bytestream_stream_Read, @@ -4323,7 +4304,7 @@ static const IMFByteStreamVtbl bytestream_stream_vtbl = bytestream_stream_Write, bytestream_BeginWrite, bytestream_EndWrite, - bytestream_stream_Seek, + bytestream_Seek, bytestream_stream_Flush, bytestream_stream_Close, }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/40
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=115194 Your paranoid android. === debian11 (32 bit report) === mfplat: Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x70baff74). Report validation errors: mfplat:mfplat prints too much data (41263 bytes) === debian11 (32 bit WoW report) === Report validation errors: mfplat:mfplat prints too much data (42073 bytes)
This merge request was approved by Alexandre Julliard. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/40
From: Andrew Eikum <aeikum(a)codeweavers.com> Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> --- dlls/mfplat/main.c | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/dlls/mfplat/main.c b/dlls/mfplat/main.c index 9e6ff26b2f7..8cdf0d1c71f 100644 --- a/dlls/mfplat/main.c +++ b/dlls/mfplat/main.c @@ -4373,7 +4373,6 @@ static HRESULT WINAPI bytestream_stream_read_callback_Invoke(IRtwqAsyncCallback { struct bytestream *stream = impl_from_read_callback_IRtwqAsyncCallback(iface); struct async_stream_op *op; - LARGE_INTEGER position; IUnknown *object; HRESULT hr; @@ -4384,13 +4383,8 @@ static HRESULT WINAPI bytestream_stream_read_callback_Invoke(IRtwqAsyncCallback EnterCriticalSection(&stream->cs); - position.QuadPart = op->position; - if (SUCCEEDED(hr = IStream_Seek(stream->stream, position, STREAM_SEEK_SET, NULL))) - { - if (SUCCEEDED(hr = IStream_Read(stream->stream, op->u.dest, op->requested_length, &op->actual_length))) - stream->position += op->actual_length; - } - + hr = IMFByteStream_Read(&stream->IMFByteStream_iface, op->u.dest, op->requested_length, &op->actual_length); + if(FAILED(hr)) TRACE("Read failed: %#lx\n", hr); IMFAsyncResult_SetStatus(op->caller, hr); list_add_tail(&stream->pending, &op->entry); @@ -4405,7 +4399,6 @@ static HRESULT WINAPI bytestream_stream_write_callback_Invoke(IRtwqAsyncCallback { struct bytestream *stream = impl_from_read_callback_IRtwqAsyncCallback(iface); struct async_stream_op *op; - LARGE_INTEGER position; IUnknown *object; HRESULT hr; @@ -4416,13 +4409,8 @@ static HRESULT WINAPI bytestream_stream_write_callback_Invoke(IRtwqAsyncCallback EnterCriticalSection(&stream->cs); - position.QuadPart = op->position; - if (SUCCEEDED(hr = IStream_Seek(stream->stream, position, STREAM_SEEK_SET, NULL))) - { - if (SUCCEEDED(hr = IStream_Write(stream->stream, op->u.src, op->requested_length, &op->actual_length))) - stream->position += op->actual_length; - } - + hr = IMFByteStream_Write(&stream->IMFByteStream_iface, op->u.src, op->requested_length, &op->actual_length); + if(FAILED(hr)) TRACE("Write failed: %#lx\n", hr); IMFAsyncResult_SetStatus(op->caller, hr); list_add_tail(&stream->pending, &op->entry); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/40
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=115195 Your paranoid android. === debian11 (32 bit report) === mfplat: Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x70baff74). Report validation errors: mfplat:mfplat prints too much data (41263 bytes) === debian11 (32 bit WoW report) === mfplat: Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x70baff74). Report validation errors: mfplat:mfplat prints too much data (41251 bytes)
From: Andrew Eikum <aeikum(a)codeweavers.com> Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> --- dlls/mfplat/main.c | 52 +++++++++------------------------------------- 1 file changed, 10 insertions(+), 42 deletions(-) diff --git a/dlls/mfplat/main.c b/dlls/mfplat/main.c index 8cdf0d1c71f..1e4e026d1d7 100644 --- a/dlls/mfplat/main.c +++ b/dlls/mfplat/main.c @@ -4369,7 +4369,7 @@ static const IMFAttributesVtbl bytestream_attributes_vtbl = mfattributes_CopyAllItems }; -static HRESULT WINAPI bytestream_stream_read_callback_Invoke(IRtwqAsyncCallback *iface, IRtwqAsyncResult *result) +static HRESULT WINAPI bytestream_read_callback_Invoke(IRtwqAsyncCallback *iface, IRtwqAsyncResult *result) { struct bytestream *stream = impl_from_read_callback_IRtwqAsyncCallback(iface); struct async_stream_op *op; @@ -4395,7 +4395,7 @@ static HRESULT WINAPI bytestream_stream_read_callback_Invoke(IRtwqAsyncCallback return S_OK; } -static HRESULT WINAPI bytestream_stream_write_callback_Invoke(IRtwqAsyncCallback *iface, IRtwqAsyncResult *result) +static HRESULT WINAPI bytestream_write_callback_Invoke(IRtwqAsyncCallback *iface, IRtwqAsyncResult *result) { struct bytestream *stream = impl_from_read_callback_IRtwqAsyncCallback(iface); struct async_stream_op *op; @@ -4421,22 +4421,22 @@ static HRESULT WINAPI bytestream_stream_write_callback_Invoke(IRtwqAsyncCallback return S_OK; } -static const IRtwqAsyncCallbackVtbl bytestream_stream_read_callback_vtbl = +static const IRtwqAsyncCallbackVtbl bytestream_read_callback_vtbl = { bytestream_callback_QueryInterface, bytestream_read_callback_AddRef, bytestream_read_callback_Release, bytestream_callback_GetParameters, - bytestream_stream_read_callback_Invoke, + bytestream_read_callback_Invoke, }; -static const IRtwqAsyncCallbackVtbl bytestream_stream_write_callback_vtbl = +static const IRtwqAsyncCallbackVtbl bytestream_write_callback_vtbl = { bytestream_callback_QueryInterface, bytestream_write_callback_AddRef, bytestream_write_callback_Release, bytestream_callback_GetParameters, - bytestream_stream_write_callback_Invoke, + bytestream_write_callback_Invoke, }; /*********************************************************************** @@ -4462,8 +4462,8 @@ HRESULT WINAPI MFCreateMFByteStreamOnStream(IStream *stream, IMFByteStream **byt object->IMFByteStream_iface.lpVtbl = &bytestream_stream_vtbl; object->attributes.IMFAttributes_iface.lpVtbl = &bytestream_attributes_vtbl; - object->read_callback.lpVtbl = &bytestream_stream_read_callback_vtbl; - object->write_callback.lpVtbl = &bytestream_stream_write_callback_vtbl; + object->read_callback.lpVtbl = &bytestream_read_callback_vtbl; + object->write_callback.lpVtbl = &bytestream_write_callback_vtbl; InitializeCriticalSection(&object->cs); list_init(&object->pending); @@ -4487,38 +4487,6 @@ HRESULT WINAPI MFCreateMFByteStreamOnStream(IStream *stream, IMFByteStream **byt return S_OK; } -static HRESULT WINAPI bytestream_file_read_callback_Invoke(IRtwqAsyncCallback *iface, IRtwqAsyncResult *result) -{ - FIXME("%p, %p.\n", iface, result); - - return E_NOTIMPL; -} - -static HRESULT WINAPI bytestream_file_write_callback_Invoke(IRtwqAsyncCallback *iface, IRtwqAsyncResult *result) -{ - FIXME("%p, %p.\n", iface, result); - - return E_NOTIMPL; -} - -static const IRtwqAsyncCallbackVtbl bytestream_file_read_callback_vtbl = -{ - bytestream_callback_QueryInterface, - bytestream_read_callback_AddRef, - bytestream_read_callback_Release, - bytestream_callback_GetParameters, - bytestream_file_read_callback_Invoke, -}; - -static const IRtwqAsyncCallbackVtbl bytestream_file_write_callback_vtbl = -{ - bytestream_callback_QueryInterface, - bytestream_write_callback_AddRef, - bytestream_write_callback_Release, - bytestream_callback_GetParameters, - bytestream_file_write_callback_Invoke, -}; - static HRESULT WINAPI bytestream_file_getservice_QueryInterface(IMFGetService *iface, REFIID riid, void **obj) { struct bytestream *stream = impl_bytestream_from_IMFGetService(iface); @@ -4625,8 +4593,8 @@ static HRESULT create_file_bytestream(MF_FILE_ACCESSMODE accessmode, MF_FILE_OPE object->IMFByteStream_iface.lpVtbl = &bytestream_file_vtbl; object->attributes.IMFAttributes_iface.lpVtbl = &bytestream_attributes_vtbl; object->IMFGetService_iface.lpVtbl = &bytestream_file_getservice_vtbl; - object->read_callback.lpVtbl = &bytestream_file_read_callback_vtbl; - object->write_callback.lpVtbl = &bytestream_file_write_callback_vtbl; + object->read_callback.lpVtbl = &bytestream_read_callback_vtbl; + object->write_callback.lpVtbl = &bytestream_write_callback_vtbl; InitializeCriticalSection(&object->cs); list_init(&object->pending); object->capabilities = capabilities; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/40
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=115196 Your paranoid android. === debian11 (32 bit report) === Report validation errors: mfplat:mfplat prints too much data (42065 bytes) === debian11 (32 bit WoW report) === Report validation errors: mfplat:mfplat prints too much data (42093 bytes)
From: Andrew Eikum <aeikum(a)codeweavers.com> Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> Signed-off-by: Alexandre Julliard <julliard(a)winehq.org> --- dlls/mfplat/tests/mfplat.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/dlls/mfplat/tests/mfplat.c b/dlls/mfplat/tests/mfplat.c index 3f7ab161b59..28b89de4b1f 100644 --- a/dlls/mfplat/tests/mfplat.c +++ b/dlls/mfplat/tests/mfplat.c @@ -1972,6 +1972,7 @@ static void test_MFCreateMFByteStreamOnStream(void) ULONG ref, size; HRESULT hr; UINT count; + QWORD to; if(!pMFCreateMFByteStreamOnStream) { @@ -2055,11 +2056,45 @@ static void test_MFCreateMFByteStreamOnStream(void) ok(hr == S_OK, "Failed to get stream capabilities, hr %#lx.\n", hr); ok(caps == (MFBYTESTREAM_IS_READABLE | MFBYTESTREAM_IS_SEEKABLE), "Unexpected caps %#lx.\n", caps); + /* IMFByteStream maintains position separately from IStream */ + caps = 0; + hr = IStream_Read(stream, &caps, sizeof(caps), &size); + ok(hr == S_OK, "Failed to read from raw stream, hr %#lx.\n", hr); + ok(size == 4, "Unexpected size.\n"); + ok(caps == 0xffff0000, "Unexpected content.\n"); + caps = 0; hr = IMFByteStream_Read(bytestream, (BYTE *)&caps, sizeof(caps), &size); ok(hr == S_OK, "Failed to read from stream, hr %#lx.\n", hr); + ok(size == 4, "Unexpected size.\n"); ok(caps == 0xffff0000, "Unexpected content.\n"); + caps = 0; + hr = IStream_Read(stream, &caps, sizeof(caps), &size); + ok(hr == S_OK, "Failed to read from raw stream, hr %#lx.\n", hr); + ok(size == 0, "Unexpected size.\n"); + ok(caps == 0, "Unexpected content.\n"); + + hr = IMFByteStream_Seek(bytestream, msoBegin, 0, 0, &to); + ok(hr == S_OK, "Failed to read from stream, hr %#lx.\n", hr); + + hr = IStream_Read(stream, &caps, sizeof(caps), &size); + ok(hr == S_OK, "Failed to read from raw stream, hr %#lx.\n", hr); + ok(size == 0, "Unexpected size.\n"); + ok(caps == 0, "Unexpected content.\n"); + + caps = 0; + hr = IMFByteStream_Read(bytestream, (BYTE *)&caps, sizeof(caps), &size); + ok(hr == S_OK, "Failed to read from stream, hr %#lx.\n", hr); + ok(size == 4, "Unexpected size.\n"); + ok(caps == 0xffff0000, "Unexpected content.\n"); + + caps = 0; + hr = IMFByteStream_Read(bytestream, (BYTE *)&caps, sizeof(caps), &size); + ok(hr == S_OK, "Failed to read from stream, hr %#lx.\n", hr); + ok(size == 0, "Unexpected size.\n"); + ok(caps == 0, "Unexpected content.\n"); + IMFAttributes_Release(attributes); IMFByteStream_Release(bytestream); IStream_Release(stream); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/40
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=115197 Your paranoid android. === w8 (32 bit report) === mfplat: mfplat.c:680: Test failed: timeout mfplat.c:676: Test failed: Unexpected hr 0xc00d36da. mfplat.c:680: Test failed: timeout === debian11 (32 bit report) === mfplat: Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x70baff74). Report validation errors: mfplat:mfplat prints too much data (41263 bytes) === debian11 (32 bit Arabic:Morocco report) === mfplat: Unhandled exception: assertion failed in 32-bit code (0xf7f58559). === debian11 (32 bit WoW report) === Report validation errors: mfplat:mfplat prints too much data (42351 bytes)
participants (4)
-
Alexandre Julliard (＠julliard) -
Andrew Eikum
-
Andrew Eikum (＠aeikum)
-
Marvin