Hello,

On Tue, Jul 25, 2017 at 5:48 PM, Alistair Leslie-Hughes <leslie_alistair@hotmail.com> wrote:
Hi Jefferson,
>
> +typedef struct {
> +������ ������ IWMReaderCallback IWMReaderCallback_iface;
> +������ ������ LONG ref;
> +������ ������ HRESULT (*onStatusCallback)(WMT_STATUS, HRESULT, WMT_ATTR_DATATYPE, BYTE*, void*, void*);
> +������ ������ HRESULT (*onSampleCallback)(DWORD, QWORD, QWORD, DWORD, INSSBuffer*, void*, void*);
> +������ ������ void *pvCallbackContext;
> +} TestWMReaderCallback;

I'm still a little confused as to why you need this structure when the
IWMReaderCallbackVtbl with a few globals would work just as well.

Well, I it could be useful to only implement QueryInterface, AddRef, and Release once, and use dependency injection to supply the other methods to the vtable.������ I wasn't sure whether I was over-engineering that, however.
������

> +static HRESULT onStatus_test_wmreader_play_wma(
> +������ ������ WMT_STATUS status,
> +������ ������ HRESULT hr,
> +������ ������ WMT_ATTR_DATATYPE dwType,
> +������ ������ BYTE *pValue,
> +������ ������ void *pvContext,
> +������ ������ void *pvCallbackContext)
> +{
> +������ ������ struct callback_context_test_wmreader_play_wma *pTestContext =
> +������ ������ ������ ������ (struct callback_context_test_wmreader_play_wma*)pvCallbackContext;
> +������ ������ DWORD *dwordValue;
> +
> +������ ������ CHECK_EXPECT(onStatus);
> +������ ������ ok(status == onStatus_expected_statuses_test_wmreader_play_wma[pTestContext->onStatus_call_count],
> +������ ������ ������ ������"onStatus called with unexpected status: %d\n", status);
> +������ ������ if (status == WMT_OPENED) {
> +������ ������ ������ ������ ok(pvContext == pTestContext->pvContextOpen,
> +������ ������ ������ ������ ������ ������"onStatus was not passed pvContext: expected %p, actual %p\n", pTestContext->pvContextOpen, pvContext);
> +������ ������ }
> +������ ������ else /* status == WMT_STARTED, or later */ {
> +������ ������ ������ ������ ok(pvContext == pTestContext->pvContextStart,
> +������ ������ ������ ������ ������ ������"onStatus was not passed pvContext: expected %p, actual %p\n", pTestContext->pvContextStart, pvContext);
> +������ ������ }
> +������ ������ switch (status) {
> +������ ������ case WMT_STARTED:
> +������ ������ ������ ������ // MSDN says dwType is a QWORD, but it is actually a DWORD
> +������ ������ ������ ������ ok(dwType == WMT_TYPE_DWORD, "onStatus pValue not WMT_TYPE_QWORD (%d), but %d\n", (int)WMT_TYPE_DWORD, dwType);
WMT_TYPE_QWORD typo?

> +������ ������ ������ ������ dwordValue = (DWORD*)pValue;
> +������ ������ ������ ������ ok(*dwordValue == 0, "onStatus call for WMT_STARTED not passed correct starting timestamp: %u\n", *dwordValue);
> +������ ������ ������ ������ SET_EXPECT(onSample);
> +������ ������ ������ ������ break;
> +������ ������ default:
> +������ ������ ������ ������ break;
> +������ ������ }
The switch and if could be merged

Very well.



> +������ ������ /* Get format count. */
> +������ ������ formatCount = 0;
> +������ ������ hr = IWMReader_GetOutputFormatCount(reader, 0, &formatCount);
> +������ ������ todo_wine ok(hr == S_OK, "Failed to get output format count: 0x%08x\n", hr);
formatCount should be checked


Okay - I'll have the test check that formatCount is equal to 37.������ That's the format count that I get on my windows machine.������ The number 37 seems oddly specific; I'm not sure how much wiggle room wmvcore implementations have to provide varying sets of formats.������ However it would be best if wine's were identical to the Windows behavior, obviously.


Best Regards
������Alistair Leslie-Hughes