[PATCH v9 0/1] MR10339: quartz/tests: Add tests for IMediaSeeking time formats in avisplit.
Added comprehensive tests for IMediaSeeking_ConvertTimeFormat. Replaced magic numbers with descriptive constants. Verified on Windows 10. Signed-off-by: José Luis Jiménez López josejl1987@gmail.com -- v9: quartz/tests/avisplit: Add tests for IMediaSeeking time formats. https://gitlab.winehq.org/wine/wine/-/merge_requests/10339
From: Jose JL <josejl1987@gmail.com> Verified on Windows 10. Signed-off-by: José Luis Jiménez López <josejl1987@gmail.com> --- dlls/quartz/tests/avisplit.c | 132 ++++++++++++++++++++++++++++------- 1 file changed, 107 insertions(+), 25 deletions(-) diff --git a/dlls/quartz/tests/avisplit.c b/dlls/quartz/tests/avisplit.c index cb756a07c5d..ab7c4dae7fb 100644 --- a/dlls/quartz/tests/avisplit.c +++ b/dlls/quartz/tests/avisplit.c @@ -1477,6 +1477,12 @@ static void test_seeking(void) { LONGLONG time, current, stop, prev_stop, earliest, latest, duration; const WCHAR *filename = load_resource(L"test.avi"); + const LONGLONG invalid_time = 0xdeadbeefLL; + const LONGLONG one_second = 10000000; + const LONGLONG one_msec = 10000; + const LONGLONG test_duration = 5 * one_second; + const LONGLONG seek_start = 1500 * one_msec; + const LONGLONG seek_stop = 3500 * one_msec; IBaseFilter *filter = create_avi_splitter(); IFilterGraph2 *graph = connect_input(filter, filename); IMediaSeeking *seeking; @@ -1493,17 +1499,61 @@ static void test_seeking(void) { const GUID *guid; HRESULT hr; + HRESULT hr_set_time; } format_tests[] = { - {&TIME_FORMAT_MEDIA_TIME, S_OK}, - {&TIME_FORMAT_FRAME, S_OK}, - - {&TIME_FORMAT_BYTE, S_FALSE}, - {&TIME_FORMAT_NONE, S_FALSE}, - {&TIME_FORMAT_SAMPLE, S_FALSE}, - {&TIME_FORMAT_FIELD, S_FALSE}, - {&testguid, S_FALSE}, + {&TIME_FORMAT_MEDIA_TIME, S_OK, S_OK}, + {&TIME_FORMAT_FRAME, S_OK, S_OK}, + {&TIME_FORMAT_BYTE, S_FALSE, E_INVALIDARG}, + {&TIME_FORMAT_NONE, S_FALSE, E_INVALIDARG}, + {&TIME_FORMAT_SAMPLE, S_FALSE, E_INVALIDARG}, + {&TIME_FORMAT_FIELD, S_FALSE, E_INVALIDARG}, + {&testguid, S_FALSE, E_INVALIDARG}, + }; + + const struct + { + const GUID *source_guid; + LONGLONG source_time; + const GUID *target_guid; + LONGLONG target_time; + HRESULT hr; + BOOL wine_todo; + } time_format_tests[] = + { + {&TIME_FORMAT_MEDIA_TIME, one_second, &TIME_FORMAT_MEDIA_TIME, one_second, S_OK, FALSE}, + {&TIME_FORMAT_MEDIA_TIME, one_second, &TIME_FORMAT_FRAME, 1, S_OK, TRUE}, + {&TIME_FORMAT_MEDIA_TIME, one_second, &TIME_FORMAT_FIELD, invalid_time, E_INVALIDARG, FALSE}, + {&TIME_FORMAT_MEDIA_TIME, one_second, &TIME_FORMAT_SAMPLE, 1, S_OK, TRUE}, + {&TIME_FORMAT_MEDIA_TIME, one_second, &TIME_FORMAT_BYTE, invalid_time, E_INVALIDARG, FALSE}, + + {&TIME_FORMAT_FRAME, 30, &TIME_FORMAT_MEDIA_TIME, 30 * one_second, S_OK, TRUE}, + {&TIME_FORMAT_FRAME, 30, &TIME_FORMAT_FRAME, 30, S_OK, TRUE}, + {&TIME_FORMAT_FRAME, 30, &TIME_FORMAT_FIELD, invalid_time, E_INVALIDARG, FALSE}, + {&TIME_FORMAT_FRAME, 30, &TIME_FORMAT_SAMPLE, invalid_time, E_INVALIDARG, FALSE}, + {&TIME_FORMAT_FRAME, 30, &TIME_FORMAT_BYTE, invalid_time, E_INVALIDARG, FALSE}, + + {&TIME_FORMAT_FIELD, 60, &TIME_FORMAT_MEDIA_TIME, invalid_time, E_INVALIDARG, FALSE}, + {&TIME_FORMAT_FIELD, 60, &TIME_FORMAT_FRAME, invalid_time, E_INVALIDARG, FALSE}, + {&TIME_FORMAT_FIELD, 60, &TIME_FORMAT_FIELD, 60, S_OK, TRUE}, + {&TIME_FORMAT_FIELD, 60, &TIME_FORMAT_SAMPLE, invalid_time, E_INVALIDARG, FALSE}, + {&TIME_FORMAT_FIELD, 60, &TIME_FORMAT_BYTE, invalid_time, E_INVALIDARG, FALSE}, + + {&TIME_FORMAT_SAMPLE, 48000, &TIME_FORMAT_MEDIA_TIME, 48000 * one_second, S_OK, TRUE}, + {&TIME_FORMAT_SAMPLE, 48000, &TIME_FORMAT_FRAME, invalid_time, E_INVALIDARG, FALSE}, + {&TIME_FORMAT_SAMPLE, 48000, &TIME_FORMAT_FIELD, invalid_time, E_INVALIDARG, FALSE}, + {&TIME_FORMAT_SAMPLE, 48000, &TIME_FORMAT_SAMPLE, 48000, S_OK, TRUE}, + {&TIME_FORMAT_SAMPLE, 48000, &TIME_FORMAT_BYTE, invalid_time, E_INVALIDARG, FALSE}, + + {&TIME_FORMAT_BYTE, 192000, &TIME_FORMAT_MEDIA_TIME, invalid_time, E_INVALIDARG, FALSE}, + {&TIME_FORMAT_BYTE, 192000, &TIME_FORMAT_FRAME, invalid_time, E_INVALIDARG, FALSE}, + {&TIME_FORMAT_BYTE, 192000, &TIME_FORMAT_FIELD, invalid_time, E_INVALIDARG, FALSE}, + {&TIME_FORMAT_BYTE, 192000, &TIME_FORMAT_SAMPLE, invalid_time, E_INVALIDARG, FALSE}, + {&TIME_FORMAT_BYTE, 192000, &TIME_FORMAT_BYTE, 192000, S_OK, TRUE}, + + {&TIME_FORMAT_NONE, 1, &TIME_FORMAT_MEDIA_TIME, invalid_time, E_INVALIDARG, FALSE}, + {&TIME_FORMAT_MEDIA_TIME, one_second, &TIME_FORMAT_NONE, invalid_time, E_INVALIDARG, FALSE}, }; IBaseFilter_FindPin(filter, L"Stream 00", &pin); @@ -1538,10 +1588,14 @@ static void test_seeking(void) for (i = 0; i < ARRAY_SIZE(format_tests); ++i) { hr = IMediaSeeking_IsFormatSupported(seeking, format_tests[i].guid); - todo_wine_if(i == 1) ok(hr == format_tests[i].hr, "Got hr %#lx for format %s.\n", + todo_wine_if(i == 1) + ok(hr == format_tests[i].hr, "Got hr %#lx for format %s.\n", hr, wine_dbgstr_guid(format_tests[i].guid)); } + hr = IMediaSeeking_SetTimeFormat(seeking, &TIME_FORMAT_MEDIA_TIME); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IMediaSeeking_QueryPreferredFormat(seeking, &format); ok(hr == S_OK, "Got hr %#lx.\n", hr); ok(IsEqualGUID(&format, &TIME_FORMAT_MEDIA_TIME), "Got format %s.\n", wine_dbgstr_guid(&format)); @@ -1555,6 +1609,34 @@ static void test_seeking(void) hr = IMediaSeeking_IsUsingTimeFormat(seeking, &TIME_FORMAT_FRAME); ok(hr == S_FALSE, "Got hr %#lx.\n", hr); + for (i = 0; i < ARRAY_SIZE(format_tests); ++i) + { + hr = IMediaSeeking_SetTimeFormat(seeking, format_tests[i].guid); + todo_wine_if(i == 1) + ok(hr == format_tests[i].hr_set_time, "Got hr %#lx for format %s.\n", + hr, wine_dbgstr_guid(format_tests[i].guid)); + } + + hr = IMediaSeeking_SetTimeFormat(seeking, &TIME_FORMAT_MEDIA_TIME); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + + for (i = 0; i < ARRAY_SIZE(time_format_tests); ++i) + { + LONGLONG result = invalid_time; + + hr = IMediaSeeking_ConvertTimeFormat( + seeking, + &result, + time_format_tests[i].target_guid, + time_format_tests[i].source_time, + time_format_tests[i].source_guid); + + todo_wine_if(time_format_tests[i].wine_todo) + ok(hr == time_format_tests[i].hr, "test %u hr %#lx.\n", i, hr); + todo_wine_if(time_format_tests[i].wine_todo) + ok(result == time_format_tests[i].target_time, "wrong value %I64x at test %u.\n", result, i); + } + hr = IMediaSeeking_SetTimeFormat(seeking, &TIME_FORMAT_SAMPLE); ok(hr == E_INVALIDARG, "Got hr %#lx.\n", hr); hr = IMediaSeeking_SetTimeFormat(seeking, &TIME_FORMAT_FRAME); @@ -1575,9 +1657,9 @@ static void test_seeking(void) duration = 0; hr = IMediaSeeking_GetDuration(seeking, &duration); ok(hr == S_OK, "Got hr %#lx.\n", hr); - ok(duration == 50000000, "Got duration %I64d.\n", duration); + ok(duration == test_duration, "Got duration %I64d.\n", duration); - stop = current = 0xdeadbeef; + stop = current = invalid_time; hr = IMediaSeeking_GetStopPosition(seeking, &stop); ok(hr == S_OK, "Got hr %#lx.\n", hr); ok(stop == duration, "Expected time %s, got %s.\n", @@ -1585,27 +1667,27 @@ static void test_seeking(void) hr = IMediaSeeking_GetCurrentPosition(seeking, ¤t); ok(hr == S_OK, "Got hr %#lx.\n", hr); ok(!current, "Got time %s.\n", wine_dbgstr_longlong(current)); - stop = current = 0xdeadbeef; + stop = current = invalid_time; hr = IMediaSeeking_GetPositions(seeking, ¤t, &stop); ok(hr == S_OK, "Got hr %#lx.\n", hr); ok(!current, "Got time %s.\n", wine_dbgstr_longlong(current)); ok(stop == duration, "Expected time %s, got %s.\n", wine_dbgstr_longlong(duration), wine_dbgstr_longlong(stop)); - time = 0xdeadbeef; + time = invalid_time; hr = IMediaSeeking_ConvertTimeFormat(seeking, &time, &TIME_FORMAT_MEDIA_TIME, 0x123456789a, NULL); ok(hr == S_OK, "Got hr %#lx.\n", hr); ok(time == 0x123456789a, "Got time %s.\n", wine_dbgstr_longlong(time)); - time = 0xdeadbeef; + time = invalid_time; hr = IMediaSeeking_ConvertTimeFormat(seeking, &time, NULL, 0x123456789a, &TIME_FORMAT_MEDIA_TIME); ok(hr == S_OK, "Got hr %#lx.\n", hr); ok(time == 0x123456789a, "Got time %s.\n", wine_dbgstr_longlong(time)); - time = 0xdeadbeef; + time = invalid_time; hr = IMediaSeeking_ConvertTimeFormat(seeking, &time, NULL, 123, &TIME_FORMAT_FRAME); todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); - todo_wine ok(time == 123 * 10000000, "Got time %s.\n", wine_dbgstr_longlong(time)); + todo_wine ok(time == 123 * one_second, "Got time %s.\n", wine_dbgstr_longlong(time)); - earliest = latest = 0xdeadbeef; + earliest = latest = invalid_time; hr = IMediaSeeking_GetAvailable(seeking, &earliest, &latest); ok(hr == S_OK, "Got hr %#lx.\n", hr); ok(!earliest, "Got time %s.\n", wine_dbgstr_longlong(earliest)); @@ -1630,23 +1712,23 @@ static void test_seeking(void) hr = IMediaSeeking_GetPreroll(seeking, &time); todo_wine ok(hr == E_NOTIMPL, "Got hr %#lx.\n", hr); - current = 1500 * 10000; - stop = 3500 * 10000; + current = seek_start; + stop = seek_stop; hr = IMediaSeeking_SetPositions(seeking, ¤t, AM_SEEKING_AbsolutePositioning, &stop, AM_SEEKING_AbsolutePositioning); ok(hr == S_OK, "Got hr %#lx.\n", hr); - ok(current == 1500 * 10000, "Got time %s.\n", wine_dbgstr_longlong(current)); - ok(stop == 3500 * 10000, "Got time %s.\n", wine_dbgstr_longlong(stop)); + ok(current == seek_start, "Got time %s.\n", wine_dbgstr_longlong(current)); + ok(stop == seek_stop, "Got time %s.\n", wine_dbgstr_longlong(stop)); - stop = current = 0xdeadbeef; + stop = current = invalid_time; hr = IMediaSeeking_GetPositions(seeking, ¤t, &stop); ok(hr == S_OK, "Got hr %#lx.\n", hr); /* Native snaps to the nearest frame. */ ok(current > 0 && current < duration, "Got time %s.\n", wine_dbgstr_longlong(current)); ok(stop > 0 && stop < duration && stop > current, "Got time %s.\n", wine_dbgstr_longlong(stop)); - current = 1500 * 10000; - stop = 3500 * 10000; + current = seek_start; + stop = seek_stop; hr = IMediaSeeking_SetPositions(seeking, ¤t, AM_SEEKING_AbsolutePositioning | AM_SEEKING_ReturnTime, &stop, AM_SEEKING_AbsolutePositioning | AM_SEEKING_ReturnTime); ok(hr == S_OK, "Got hr %#lx.\n", hr); @@ -1661,7 +1743,7 @@ static void test_seeking(void) NULL, AM_SEEKING_NoPositioning); ok(hr == S_OK, "Got hr %#lx.\n", hr); - stop = current = 0xdeadbeef; + stop = current = invalid_time; hr = IMediaSeeking_GetPositions(seeking, ¤t, &stop); ok(hr == S_OK, "Got hr %#lx.\n", hr); ok(!current, "Got time %s.\n", wine_dbgstr_longlong(current)); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10339
Thanks for the patch. I'm not sure what the repeated pushes are for, but it triggers CI and uses resources every time, so in the future please try to avoid it if possible. I don't think the replacing of constants makes this easier to read. They aren't magic; they're meaningful values. 0xdeadbeef isn't "invalid" per se but rather a sentinel to make sure that the value really is written. Replacing 10000 / 10000000 with symbolic constants is probably an improvement though. ``` + {&TIME_FORMAT_MEDIA_TIME, one_second, &TIME_FORMAT_MEDIA_TIME, one_second, S_OK, FALSE}, ``` I'd omit the FALSE at the end. It may be worth testing conversions that would require rounding. Instead of putting "invalid_time" in the table I would say something like ``` if (hr == S_OK) ok(time == tests[i].target_time); else ok(time == 0xdeadbeef); ``` .... ``` + hr = IMediaSeeking_ConvertTimeFormat( + seeking, + &result, + time_format_tests[i].target_guid, + time_format_tests[i].source_time, + time_format_tests[i].source_guid); ``` I'd prefer to not put one argument per line. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10339#note_132393
participants (3)
-
Elizabeth Figura (＠zfigura) -
Jose JL
-
José Luis Jiménez López