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