Re: [PATCH 1/6] mfplat: Implement MFCreateDXGIDeviceManager().

[Nikolay Sivov]

On 8/28/19 4:37 PM, Jactry Zeng wrote:
> +struct mfdxgi_dev_mgr
> +{
> - IMFDXGIDeviceManager IMFDXGIDeviceManager_iface;
> - LONG ref;
> - UINT token;
>
> +};

To be consistent with the rest of mf* modules, please use 
'dxgi_device_manager' and 'refcount' for names.
> +static HRESULT WINAPI mfdxgi_dev_mgr_CloseDeviceHandle(IMFDXGIDeviceManager *iface, HANDLE device)
> +{
> - struct mfdxgi_dev_mgr *This = impl_from_IMFDXGIDeviceManager(iface);
> -
> - FIXME("(%p)->(%p): stub.\n", This, device);
> -
> - return E_NOTIMPL;
>
> +}

'This' -> 'manager', iface pointer in traces instead of This.
> - dev_mgr->token = MFGetSystemTime();

This truncates result which is not pretty. Maybe it's enough to have 
global counter for this?
> - struct mfdxgi_dev_mgr *dev_mgr;

'dev_mgr' -> 'object'.
> - Sleep(50);
> - token2 = 0;
> - hr = pMFCreateDXGIDeviceManager(&token2, &manager2);
> - ok(hr == S_OK, "MFCreateDXGIDeviceManager failed: %#x.\n", hr);
> - EXPECT_REF(manager2, 1);
> - ok(token2 && token2 != token, "got wrong token: %u, %u.\n", token2, token);
> - ok(manager != manager2, "got wrong pointer: %p.\n", manager2);
> - EXPECT_REF(manager, 1);
> -
> - IMFDXGIDeviceManager_Release(manager);
> - IMFDXGIDeviceManager_Release(manager2);
> -
> - memset(&ret1, 0, sizeof(ret1));
> - thread1 = CreateThread(NULL, 0, create_dxgi_dev_mgr, &ret1, 0, NULL);
> - ok(WaitForSingleObject(thread1, 1000) == WAIT_OBJECT_0, "wait failed.\n");
> - Sleep(50);
> - memset(&ret2, 0, sizeof(ret2));
> - thread2 = CreateThread(NULL, 0, create_dxgi_dev_mgr, &ret2, 0, NULL);
> - ok(WaitForSingleObject(thread2, 1000) == WAIT_OBJECT_0, "wait failed.\n");
> - ok(ret1.dev_mgr != ret2.dev_mgr, "got wrong pointer: %p, %p.\n", ret1.dev_mgr, ret2.dev_mgr);
> - ok(ret1.token != ret2.token, "got wrong token: %u, %u.\n", ret1.token, ret2.token);
> -
> - IMFDXGIDeviceManager_Release(ret1.dev_mgr);
> - IMFDXGIDeviceManager_Release(ret2.dev_mgr);

Do we need this? It only shows that token is different for newly created 
manager instance which is also true for same thread case.
> - test_create_dxgi_device_manager();

'test_dxgi_device_manager' would be better, because we're going to test 
more than just creating it.