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