[PATCH 0/3] MR2247: mf/tests: Test ProcessInput and ProcessOutput for WMV decoder DMO.
SetInputType tests will randomly failed after PATCH 3 (It was OK in PATCH 2). I spend many time on figuring out why. And I found a weird thing: If I comment out the compare_nv12() in check_dmo_output_data_buffer_(), the test failures will be gone. I can't think out the reason. I don't understand why compare_nv12() in check_dmo_output_data_buffer_() affects SetInputType tests. And I don't want to waste too much time on the test stuff. Implementing the decoder is more important, so I marked the failed tests as flaky. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2247
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/mf/tests/transform.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/dlls/mf/tests/transform.c b/dlls/mf/tests/transform.c index f806485df78..0c9f7548d42 100644 --- a/dlls/mf/tests/transform.c +++ b/dlls/mf/tests/transform.c @@ -1323,11 +1323,11 @@ static void check_dmo_set_input_type(IMediaObject *media_object, const GUID *sub { flag = flags[i]; hr = IMediaObject_SetInputType(media_object, 1, NULL, flag); - ok(hr == DMO_E_INVALIDSTREAMINDEX, "SetInputType returned %#lx for flag %#lx.", hr, flag); + ok(hr == DMO_E_INVALIDSTREAMINDEX, "SetInputType returned %#lx for flag %#lx.\n", hr, flag); hr = IMediaObject_SetInputType(media_object, 1, &bad_media_type, flag); - ok(hr == DMO_E_INVALIDSTREAMINDEX, "SetInputType returned %#lx for flag %#lx.", hr, flag); + ok(hr == DMO_E_INVALIDSTREAMINDEX, "SetInputType returned %#lx for flag %#lx.\n", hr, flag); hr = IMediaObject_SetInputType(media_object, 1, good_media_type, flag); - ok(hr == DMO_E_INVALIDSTREAMINDEX, "SetInputType returned %#lx for flag %#lx.", hr, flag); + ok(hr == DMO_E_INVALIDSTREAMINDEX, "SetInputType returned %#lx for flag %#lx.\n", hr, flag); } /* Test unaccepted type. */ @@ -1337,10 +1337,10 @@ static void check_dmo_set_input_type(IMediaObject *media_object, const GUID *sub if (!(flag & DMO_SET_TYPEF_CLEAR)) { hr = IMediaObject_SetInputType(media_object, 0, NULL, flag); - ok(hr == DMO_E_TYPE_NOT_ACCEPTED, "SetInputType returned %#lx for flag %#lx.", hr, flag); + ok(hr == DMO_E_TYPE_NOT_ACCEPTED, "SetInputType returned %#lx for flag %#lx.\n", hr, flag); } hr = IMediaObject_SetInputType(media_object, 0, &bad_media_type, flag); - ok(hr == DMO_E_TYPE_NOT_ACCEPTED, "SetInputType returned %#lx for flag %#lx.", hr, flag); + ok(hr == DMO_E_TYPE_NOT_ACCEPTED, "SetInputType returned %#lx for flag %#lx.\n", hr, flag); } /* Test clearing the type. */ @@ -1348,7 +1348,7 @@ static void check_dmo_set_input_type(IMediaObject *media_object, const GUID *sub { flag = DMO_SET_TYPEF_CLEAR | flags[i]; hr = IMediaObject_SetInputType(media_object, 0, NULL, flag); - ok(hr == S_OK, "SetInputType returned %#lx for flag %#lx.", hr, flag); + ok(hr == S_OK, "SetInputType returned %#lx for flag %#lx.\n", hr, flag); } /* Test accepted type. */ @@ -1356,7 +1356,7 @@ static void check_dmo_set_input_type(IMediaObject *media_object, const GUID *sub { flag = flags[i]; hr = IMediaObject_SetInputType(media_object, 0, good_media_type, flag); - ok(hr == S_OK, "SetInputType returned %#lx for flag %#lx.", hr, flag); + ok(hr == S_OK, "SetInputType returned %#lx for flag %#lx.\n", hr, flag); } /* Test unconsidered header member.*/ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2247
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/mf/tests/transform.c | 145 +++++++++++++++++++++++++++++++++++++- 1 file changed, 143 insertions(+), 2 deletions(-) diff --git a/dlls/mf/tests/transform.c b/dlls/mf/tests/transform.c index 0c9f7548d42..1d026870a80 100644 --- a/dlls/mf/tests/transform.c +++ b/dlls/mf/tests/transform.c @@ -55,6 +55,105 @@ DEFINE_GUID(MFVideoFormat_WMV_Unknown,0x7ce12ca9,0xbfbf,0x43d9,0x9d,0x00,0x82,0x DEFINE_GUID(mft_output_sample_incomplete,0xffffff,0xffff,0xffff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff); +struct buffer_impl +{ + IMediaBuffer IMediaBuffer_iface; + LONG refcount; + BYTE *data; + DWORD length; + DWORD max_length; +}; + +static inline struct buffer_impl *impl_from_IMediaBuffer(IMediaBuffer *iface) +{ + return CONTAINING_RECORD(iface, struct buffer_impl, IMediaBuffer_iface); +} + +static HRESULT WINAPI buffer_impl_QueryInterface(IMediaBuffer *iface, REFIID iid, void **obj) +{ + if (IsEqualIID(iid, &IID_IMediaBuffer) + || IsEqualIID(iid, &IID_IUnknown)) + { + *obj = iface; + return S_OK; + } + return E_NOINTERFACE; +} + +static ULONG WINAPI buffer_impl_AddRef(IMediaBuffer *iface) +{ + struct buffer_impl *buffer = impl_from_IMediaBuffer(iface); + return InterlockedIncrement(&buffer->refcount); +} + +static ULONG WINAPI buffer_impl_Release(IMediaBuffer *iface) +{ + struct buffer_impl *buffer = impl_from_IMediaBuffer(iface); + ULONG ref = InterlockedDecrement(&buffer->refcount); + if (!ref) + { + free(buffer->data); + free(buffer); + } + return ref; +} + +static HRESULT WINAPI buffer_impl_SetLength(IMediaBuffer *iface, DWORD length) +{ + struct buffer_impl *buffer = impl_from_IMediaBuffer(iface); + if (length > buffer->max_length) + return E_INVALIDARG; + buffer->length = length; + return S_OK; +} + +static HRESULT WINAPI buffer_impl_GetMaxLength(IMediaBuffer *iface, DWORD *max_length) +{ + struct buffer_impl *buffer = impl_from_IMediaBuffer(iface); + if (!max_length) + return E_POINTER; + *max_length = buffer->max_length; + return S_OK; +} + +static HRESULT WINAPI buffer_impl_GetBufferAndLength(IMediaBuffer *iface, BYTE **data, DWORD *length) +{ + struct buffer_impl *buffer = impl_from_IMediaBuffer(iface); + if (!data || ! length) + return E_POINTER; + *data = buffer->data; + *length = buffer->length; + return S_OK; +} + +static IMediaBufferVtbl buffer_impl_vtbl = { + buffer_impl_QueryInterface, + buffer_impl_AddRef, + buffer_impl_Release, + buffer_impl_SetLength, + buffer_impl_GetMaxLength, + buffer_impl_GetBufferAndLength, +}; + +HRESULT buffer_impl_create(DWORD max_length, struct buffer_impl **ret) +{ + struct buffer_impl *buffer; + + if (!(buffer = calloc(1, sizeof(*buffer)))) + return E_OUTOFMEMORY; + if (!(buffer->data = calloc(max_length, sizeof(*buffer->data)))) + { + free(buffer); + return E_OUTOFMEMORY; + } + buffer->IMediaBuffer_iface.lpVtbl = &buffer_impl_vtbl; + buffer->refcount = 1; + buffer->length = 0; + buffer->max_length = max_length; + *ret = buffer; + return S_OK; +} + static BOOL is_compressed_subtype(const GUID *subtype) { if (IsEqualGUID(subtype, &MEDIASUBTYPE_WMV1) @@ -332,6 +431,7 @@ static void init_dmo_media_type_video(DMO_MEDIA_TYPE *media_type, header->bmiHeader.biSize = sizeof(header->bmiHeader); header->bmiHeader.biWidth = width; header->bmiHeader.biHeight = height; + header->bmiHeader.biPlanes = 1; header->bmiHeader.biBitCount = subtype_to_bpp(subtype); header->bmiHeader.biCompression = subtype_to_compression(subtype); @@ -1523,7 +1623,6 @@ static void check_dmo_set_output_type(IMediaObject *media_object, const GUID *su } } - static HRESULT WINAPI test_unk_QueryInterface(IUnknown *iface, REFIID riid, void **obj) { if (IsEqualIID(riid, &IID_IUnknown)) @@ -5075,10 +5174,14 @@ static void test_wmv_decoder_media_object(void) {MFMediaType_Video, MEDIASUBTYPE_RGB555, TRUE, FALSE, 512, FORMAT_VideoInfo, NULL, 88, (BYTE *)&expected_output_info[11]}, {MFMediaType_Video, MEDIASUBTYPE_RGB8, TRUE, FALSE, 256, FORMAT_VideoInfo, NULL, 1112, (BYTE *)&expected_output_info[12]}, }; + const DWORD data_width = 96, data_height = 96; const POINT test_size[] = {{16, 16}, {96, 96}, {320, 240}}; - DWORD in_count, out_count, size, expected_size, alignment; + DWORD in_count, out_count, size, expected_size, alignment, wmv_data_length, status; + struct buffer_impl *input_media_buffer, *output_media_buffer; + DMO_OUTPUT_DATA_BUFFER output_data_buffer; DMO_MEDIA_TYPE media_type, *type; IMediaObject *media_object; + const BYTE *wmv_data; char buffer[1024]; ULONG ret, i, j; HRESULT hr; @@ -5202,6 +5305,44 @@ static void test_wmv_decoder_media_object(void) winetest_pop_context(); } + /* Test ProcessInput. */ + load_resource(L"wmvencdata.bin", &wmv_data, &wmv_data_length); + wmv_data_length = *((DWORD *)wmv_data); + wmv_data += sizeof(DWORD); + hr = buffer_impl_create(wmv_data_length, &input_media_buffer); + ok(hr == S_OK, "Failed to create input media buffer.\n"); + memcpy(input_media_buffer->data, wmv_data, wmv_data_length); + input_media_buffer->length = wmv_data_length; + + init_dmo_media_type_video(type, &MEDIASUBTYPE_WMV1, data_width, data_height); + hr = IMediaObject_SetInputType(media_object, 0, type, 0); + ok(hr == S_OK, "SetInputType returned %#lx.\n", hr); + init_dmo_media_type_video(type, &MEDIASUBTYPE_NV12, data_width, data_height); + hr = IMediaObject_SetOutputType(media_object, 0, type, 0); + ok(hr == S_OK, "SetOutputType returned %#lx.\n", hr); + + hr = IMediaObject_ProcessInput(media_object, 0, &input_media_buffer->IMediaBuffer_iface, 0, 0, 0); + todo_wine + ok(hr == S_OK, "ProcessInput returned %#lx.\n", hr); + + /* Test ProcessOutput. */ + hr = IMediaObject_GetOutputSizeInfo(media_object, 0, &size, &alignment); + ok(hr == S_OK, "GetOutputSizeInfo returned %#lx.\n", hr); + hr = buffer_impl_create(size, &output_media_buffer); + ok(hr == S_OK, "Failed to create output media buffer.\n"); + output_data_buffer.pBuffer = &output_media_buffer->IMediaBuffer_iface; + output_data_buffer.dwStatus = 0xdeadbeef; + output_data_buffer.rtTimestamp = 0xdeadbeef; + output_data_buffer.rtTimelength = 0xdeadbeef; + hr = IMediaObject_ProcessOutput(media_object, 0, 1, &output_data_buffer, &status); + todo_wine + ok(hr == S_OK, "ProcessOutput returned %#lx.\n", hr); + + ret = IMediaBuffer_Release(&output_media_buffer->IMediaBuffer_iface); + ok(ret == 0, "Release returned %lu\n", ret); + ret = IMediaBuffer_Release(&input_media_buffer->IMediaBuffer_iface); + ok(ret == 0, "Release returned %lu\n", ret); + ret = IMediaObject_Release(media_object); ok(ret == 0, "Release returned %lu\n", ret); CoUninitialize(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2247
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/mf/tests/transform.c | 74 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 72 insertions(+), 2 deletions(-) diff --git a/dlls/mf/tests/transform.c b/dlls/mf/tests/transform.c index 1d026870a80..fe02921ae73 100644 --- a/dlls/mf/tests/transform.c +++ b/dlls/mf/tests/transform.c @@ -55,6 +55,12 @@ DEFINE_GUID(MFVideoFormat_WMV_Unknown,0x7ce12ca9,0xbfbf,0x43d9,0x9d,0x00,0x82,0x DEFINE_GUID(mft_output_sample_incomplete,0xffffff,0xffff,0xffff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff); +/* Shorter aliases for flags. */ +#define O_SYNCPOINT DMO_OUTPUT_DATA_BUFFERF_SYNCPOINT +#define O_TIME DMO_OUTPUT_DATA_BUFFERF_TIME +#define O_TIMELENGTH DMO_OUTPUT_DATA_BUFFERF_TIMELENGTH +#define O_INCOMPLETE DMO_OUTPUT_DATA_BUFFERF_INCOMPLETE + struct buffer_impl { IMediaBuffer IMediaBuffer_iface; @@ -1456,6 +1462,7 @@ static void check_dmo_set_input_type(IMediaObject *media_object, const GUID *sub { flag = flags[i]; hr = IMediaObject_SetInputType(media_object, 0, good_media_type, flag); + flaky ok(hr == S_OK, "SetInputType returned %#lx for flag %#lx.\n", hr, flag); } @@ -1470,6 +1477,7 @@ static void check_dmo_set_input_type(IMediaObject *media_object, const GUID *sub header->bmiHeader.biXPelsPerMeter = 0xdead; header->bmiHeader.biYPelsPerMeter = 0xdead; hr = IMediaObject_SetInputType(media_object, 0, good_media_type, 0); + flaky ok(hr == S_OK, "SetInputType returned %#lx.\n", hr); /* Test invalid major type. */ @@ -1623,6 +1631,50 @@ static void check_dmo_set_output_type(IMediaObject *media_object, const GUID *su } } +#define check_dmo_output_data_buffer(a, b, c) check_dmo_output_data_buffer_(__LINE__, a, b, c) +static DWORD check_dmo_output_data_buffer_(int line, DMO_OUTPUT_DATA_BUFFER *output_data_buffer, + const struct sample_desc *sample_desc, const WCHAR *expect_data_filename) +{ + const struct buffer_desc *buffer_desc = &sample_desc->buffers[0]; + DWORD diff, data_length, buffer_length, expect_length; + BYTE *data, *buffer; + HRESULT hr; + + if (output_data_buffer->dwStatus & DMO_OUTPUT_DATA_BUFFERF_TIME) + ok_(__FILE__, line)(output_data_buffer->rtTimestamp == sample_desc->sample_time, + "Unexpected time %I64d, expected %I64d.\n", + output_data_buffer->rtTimestamp, sample_desc->sample_time); + if (output_data_buffer->dwStatus & DMO_OUTPUT_DATA_BUFFERF_TIMELENGTH) + ok_(__FILE__, line)(output_data_buffer->rtTimelength == sample_desc->sample_duration, + "Unexpected duration %I64d, expected %I64d.\n", + output_data_buffer->rtTimelength, sample_desc->sample_duration); + + load_resource(expect_data_filename, (const BYTE **)&data, &data_length); + + expect_length = buffer_desc->length; + if (expect_length == -1) + { + expect_length = *(DWORD *)data; + data += sizeof(DWORD); + data_length = data_length - sizeof(DWORD); + } + + hr = IMediaBuffer_GetBufferAndLength(output_data_buffer->pBuffer, &buffer, &buffer_length); + ok(hr == S_OK, "GetBufferAndLength returned %#lx.\n", hr); + ok_(__FILE__, line)(buffer_length == expect_length, "Unexpected length %#lx, expected %#lx\n", buffer_length, expect_length); + + diff = 0; + if (data_length < buffer_length) + ok_(__FILE__, line)(0, "Missing %#lx bytes\n", buffer_length - data_length); + else if (!buffer_desc->compare) + diff = compare_bytes(buffer, &buffer_length, NULL, data); + else + diff = buffer_desc->compare(buffer, &buffer_length, &buffer_desc->rect, data); + + return diff; +} + + static HRESULT WINAPI test_unk_QueryInterface(IUnknown *iface, REFIID riid, void **obj) { if (IsEqualIID(riid, &IID_IUnknown)) @@ -5176,7 +5228,17 @@ static void test_wmv_decoder_media_object(void) }; const DWORD data_width = 96, data_height = 96; const POINT test_size[] = {{16, 16}, {96, 96}, {320, 240}}; - DWORD in_count, out_count, size, expected_size, alignment, wmv_data_length, status; + const struct buffer_desc output_buffer_desc_nv12 = + { + .length = data_width * data_height * 3 / 2, + .compare = compare_nv12, .dump = dump_nv12, .rect = {.right = 82, .bottom = 84}, + }; + const struct sample_desc output_sample_desc_nv12 = + { + .sample_time = 0, .sample_duration = 333333, + .buffer_count = 1, .buffers = &output_buffer_desc_nv12, + }; + DWORD in_count, out_count, size, expected_size, alignment, wmv_data_length, status, diff; struct buffer_impl *input_media_buffer, *output_media_buffer; DMO_OUTPUT_DATA_BUFFER output_data_buffer; DMO_MEDIA_TYPE media_type, *type; @@ -5321,7 +5383,7 @@ static void test_wmv_decoder_media_object(void) hr = IMediaObject_SetOutputType(media_object, 0, type, 0); ok(hr == S_OK, "SetOutputType returned %#lx.\n", hr); - hr = IMediaObject_ProcessInput(media_object, 0, &input_media_buffer->IMediaBuffer_iface, 0, 0, 0); + hr = IMediaObject_ProcessInput(media_object, 0, &input_media_buffer->IMediaBuffer_iface, 0, 0, 333333); todo_wine ok(hr == S_OK, "ProcessInput returned %#lx.\n", hr); @@ -5337,6 +5399,14 @@ static void test_wmv_decoder_media_object(void) hr = IMediaObject_ProcessOutput(media_object, 0, 1, &output_data_buffer, &status); todo_wine ok(hr == S_OK, "ProcessOutput returned %#lx.\n", hr); + todo_wine + ok(output_data_buffer.dwStatus == (O_SYNCPOINT | O_TIME | O_TIMELENGTH), + "Got unexpected dwStatus %#lx.\n", output_data_buffer.dwStatus); + if (hr == S_OK) + { + diff = check_dmo_output_data_buffer(&output_data_buffer, &output_sample_desc_nv12, L"nv12frame.bmp"); + ok(diff == 0, "Got %lu%% diff.\n", diff); + } ret = IMediaBuffer_Release(&output_media_buffer->IMediaBuffer_iface); ok(ret == 0, "Release returned %lu\n", ret); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2247
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=129791 Your paranoid android. === w8 (32 bit report) === mf: transform.c:3978: Test failed: h264dec: invalid h264 length transform.c:3978: Test failed: h264dec: invalid h264 buffer === w1064v1507 (64 bit report) === mf: transform.c:3978: Test failed: h264dec: invalid h264 length transform.c:3978: Test failed: h264dec: invalid h264 buffer
SetInputType tests will randomly failed after PATCH 3 (It was OK in PATCH 2). I spend many time on figuring out why. And I found a weird thing: If I comment out the compare_nv12() in check_dmo_output_data_buffer\_(), the test failures will be gone.
I can't think out the reason. I don't understand why compare_nv12() in check_dmo_output_data_buffer\_() affects SetInputType tests. And I don't want to waste too much time on the test stuff. Implementing the decoder is more important, so I marked the failed tests as flaky.
The problem is likely some uninitialized data on the stack that reflects into the media type. Initializing `buffer` in `check_dmo_set_input_type` to 0 seems to fix the issue, but maybe it'd be better to figure which field is left uninitialized by `init_dmo_media_type_video` instead. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2247#note_24980
Rémi Bernon (@rbernon) commented about dlls/mf/tests/transform.c:
DEFINE_GUID(mft_output_sample_incomplete,0xffffff,0xffff,0xffff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff);
+struct buffer_impl +{ + IMediaBuffer IMediaBuffer_iface; + LONG refcount; + BYTE *data; + DWORD length; + DWORD max_length; +};
```suggestion:-7+0 struct media_buffer { IMediaBuffer IMediaBuffer_iface; LONG refcount; DWORD length; DWORD max_length; BYTE data[]; }; ``` With struct name and method prefixes changed everywhere else, and you can then use a single alloc / free (with allocation size being `offsetof(struct media_buffer, data[size])`). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2247#note_24982
Rémi Bernon (@rbernon) commented about dlls/mf/tests/transform.c:
+ struct buffer_impl *buffer = impl_from_IMediaBuffer(iface); + if (!data || ! length) + return E_POINTER; + *data = buffer->data; + *length = buffer->length; + return S_OK; +} + +static IMediaBufferVtbl buffer_impl_vtbl = { + buffer_impl_QueryInterface, + buffer_impl_AddRef, + buffer_impl_Release, + buffer_impl_SetLength, + buffer_impl_GetMaxLength, + buffer_impl_GetBufferAndLength, +};
static IMediaBufferVtbl buffer_impl_vtbl =
{
buffer_impl_QueryInterface,
buffer_impl_AddRef,
buffer_impl_Release,
buffer_impl_SetLength,
buffer_impl_GetMaxLength,
buffer_impl_GetBufferAndLength,
};
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2247#note_24983
Rémi Bernon (@rbernon) commented about dlls/mf/tests/transform.c:
DEFINE_GUID(mft_output_sample_incomplete,0xffffff,0xffff,0xffff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff);
+/* Shorter aliases for flags. */ +#define O_SYNCPOINT DMO_OUTPUT_DATA_BUFFERF_SYNCPOINT +#define O_TIME DMO_OUTPUT_DATA_BUFFERF_TIME +#define O_TIMELENGTH DMO_OUTPUT_DATA_BUFFERF_TIMELENGTH +#define O_INCOMPLETE DMO_OUTPUT_DATA_BUFFERF_INCOMPLETE
I'd prefer not to add aliases. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2247#note_24984
Rémi Bernon (@rbernon) commented about dlls/mf/tests/transform.c:
hr = IMediaObject_ProcessOutput(media_object, 0, 1, &output_data_buffer, &status); todo_wine ok(hr == S_OK, "ProcessOutput returned %#lx.\n", hr); + todo_wine + ok(output_data_buffer.dwStatus == (O_SYNCPOINT | O_TIME | O_TIMELENGTH), + "Got unexpected dwStatus %#lx.\n", output_data_buffer.dwStatus);
If this feels too long with the full constants I think you can use a local `expect_status` variable set before the `ok` call, and compare it to `output_data_buffer.dwStatus`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2247#note_24985
participants (4)
-
Marvin -
Rémi Bernon -
Ziqing Hui
-
Ziqing Hui (＠zhui)