On Mon, Feb 01, 2021 at 04:56:38PM +0200, Arkadiusz Hiler wrote:
From: Andrew Eikum <aeikum(a)codeweavers.com>
I'm fine with how you've done it, but if you'd like Git authorship for the tests, you could send that patch separately. In spatialaudio.c:
+static HRESULT activate_stream(SpatialAudioStreamImpl *stream) +{ + WAVEFORMATEXTENSIBLE *object_fmtex = (WAVEFORMATEXTENSIBLE *)stream->params.ObjectFormat; + HRESULT hr; + REFERENCE_TIME period; + + if(!(object_fmtex->Format.wFormatTag == WAVE_FORMAT_IEEE_FLOAT || + (object_fmtex->Format.wFormatTag == WAVE_FORMAT_EXTENSIBLE && + IsEqualGUID(&object_fmtex->SubFormat, &KSDATAFORMAT_SUBTYPE_IEEE_FLOAT)))){ + FIXME("Only float formats are supported for now\n"); + return E_INVALIDARG; + } + + hr = IMMDevice_Activate(stream->sa_client->mmdev, &IID_IAudioClient, + CLSCTX_INPROC_SERVER, NULL, (void**)&stream->client); + if(FAILED(hr)){ + WARN("Activate failed: %08x\n", hr); + return hr; + } + + hr = IAudioClient_GetDevicePeriod(stream->client, &period, NULL); + if(FAILED(hr)){ + WARN("GetDevicePeriod failed: %08x\n", hr); + IAudioClient_Release(stream->client); + return hr; + } + + stream->stream_fmtex.Format.wFormatTag = WAVE_FORMAT_EXTENSIBLE; + static_mask_to_channels(stream->params.StaticObjectTypeMask, + &stream->stream_fmtex.Format.nChannels, &stream->stream_fmtex.dwChannelMask, + stream->static_object_map); + stream->stream_fmtex.Format.nSamplesPerSec = stream->params.ObjectFormat->nSamplesPerSec; + stream->stream_fmtex.Format.wBitsPerSample = stream->params.ObjectFormat->wBitsPerSample; + stream->stream_fmtex.Format.nBlockAlign = (stream->stream_fmtex.Format.nChannels * stream->stream_fmtex.Format.wBitsPerSample) / 8; + stream->stream_fmtex.Format.nAvgBytesPerSec = stream->stream_fmtex.Format.nSamplesPerSec * stream->stream_fmtex.Format.nBlockAlign; + stream->stream_fmtex.Format.cbSize = sizeof(WAVEFORMATEXTENSIBLE) - sizeof(WAVEFORMATEX); + stream->stream_fmtex.Samples.wValidBitsPerSample = stream->stream_fmtex.Format.wBitsPerSample; + stream->stream_fmtex.SubFormat = KSDATAFORMAT_SUBTYPE_IEEE_FLOAT; + + hr = IAudioClient_Initialize(stream->client, AUDCLNT_SHAREMODE_SHARED, + AUDCLNT_STREAMFLAGS_EVENTCALLBACK | AUDCLNT_STREAMFLAGS_NOPERSIST, + period * MAX_PERIODS, 0, &stream->stream_fmtex.Format, NULL); + if(FAILED(hr)){ + WARN("Initialize failed: %08x\n", hr); + IAudioClient_Release(stream->client); + return hr; + } + + /* XXX: OK that this is the user's handle? */
We can remove this comment, we are cloning the handle below.
+static HRESULT WINAPI SAC_ActivateSpatialAudioStream(ISpatialAudioClient *iface, + const PROPVARIANT *prop, REFIID riid, void **stream) +{ + SpatialAudioImpl *This = impl_from_ISpatialAudioClient(iface); + SpatialAudioObjectRenderStreamActivationParams *params; + HRESULT hr; + + TRACE("(%p)->(%s, %p)\n", This, debugstr_guid(riid), stream); + + if(IsEqualIID(riid, &IID_ISpatialAudioObjectRenderStream)){ + SpatialAudioStreamImpl *obj; + + if(prop && + (prop->vt != VT_BLOB || + prop->u.blob.cbSize != sizeof(SpatialAudioObjectRenderStreamActivationParams))){ + WARN("Got invalid params\n"); + *stream = NULL; + return E_INVALIDARG; + } + + params = (SpatialAudioObjectRenderStreamActivationParams*) prop->u.blob.pBlobData; + + if(params->StaticObjectTypeMask & AudioObjectType_Dynamic){ + *stream = NULL; + return E_INVALIDARG; + } + + if(!params->ObjectFormat || memcmp(params->ObjectFormat, &This->object_fmtex.Format, sizeof(*params->ObjectFormat) + params->ObjectFormat->cbSize)) { + *stream = NULL; + return AUDCLNT_E_UNSUPPORTED_FORMAT; + } + + obj = heap_alloc_zero(sizeof(SpatialAudioStreamImpl)); + + obj->ISpatialAudioObjectRenderStream_iface.lpVtbl = &ISpatialAudioObjectRenderStream_vtbl; + obj->ref = 1; + memcpy(&obj->params, params, sizeof(obj->params)); + + obj->update_frames = ~0; + + InitializeCriticalSection(&obj->lock); + list_init(&obj->objects); + + obj->sa_client = This; + SAC_AddRef(&This->ISpatialAudioClient_iface); + + obj->params.ObjectFormat = clone_fmtex(obj->params.ObjectFormat); + + if(obj->params.EventHandle != INVALID_HANDLE_VALUE && + obj->params.EventHandle != 0)
The tests show you can't give a NULL or invalid handle, so I think you could check for this and bail out earlier. In tests/spatialaudio.c:
+static void fill_activation_params(SpatialAudioObjectRenderStreamActivationParams *activation_params) +{ + activation_params->StaticObjectTypeMask = \ + AudioObjectType_FrontLeft | + AudioObjectType_FrontRight | + AudioObjectType_FrontCenter | + AudioObjectType_LowFrequency | + AudioObjectType_SideLeft | + AudioObjectType_SideRight | + AudioObjectType_BackLeft | + AudioObjectType_BackRight | + AudioObjectType_TopFrontLeft | + AudioObjectType_TopFrontRight | + AudioObjectType_TopBackLeft | + AudioObjectType_TopBackRight; + + activation_params->MinDynamicObjectCount = 0; + activation_params->MaxDynamicObjectCount = 0; + activation_params->Category = AudioCategory_GameEffects; + activation_params->EventHandle = event; + activation_params->NotifyObject = NULL; + + activation_params->ObjectFormat = &format;
Mixed tabs/spaces (and all over this file).
+static void test_audio_object_buffers(void) +{ + HRESULT hr; + ISpatialAudioObjectRenderStream *sas = NULL; + ISpatialAudioObject *sao[4]; + UINT32 dyn_object_count, frame_count, buffer_length; + BYTE *buffer; + INT i; + + SpatialAudioObjectRenderStreamActivationParams activation_params; + PROPVARIANT activation_params_prop; + + PropVariantInit(&activation_params_prop); + activation_params_prop.vt = VT_BLOB; + activation_params_prop.blob.cbSize = sizeof(activation_params); + activation_params_prop.blob.pBlobData = (BYTE*) &activation_params; + + fill_activation_params(&activation_params); + hr = ISpatialAudioClient_ActivateSpatialAudioStream(sac, &activation_params_prop, &IID_ISpatialAudioObjectRenderStream, (void**)&sas); + ok(hr == S_OK, "Failed to activate spatial audio stream: 0x%08x\n", hr); + + hr = ISpatialAudioObjectRenderStream_ActivateSpatialAudioObject(sas, AudioObjectType_FrontLeft, &sao[0]); + ok(hr == S_OK, "Failed to activate spatial audio object: 0x%08x\n", hr); + + hr = ISpatialAudioObjectRenderStream_Start(sas); + ok(hr == S_OK, "Failed to activate spatial audio render stream: 0x%08x\n", hr); + + hr = ISpatialAudioObjectRenderStream_ActivateSpatialAudioObject(sas, AudioObjectType_FrontRight, &sao[1]); + ok(hr == S_OK, "Failed to activate spatial audio object: 0x%08x\n", hr); + + hr = WaitForSingleObject(event, 200); + ok(hr == WAIT_OBJECT_0, "Expected event to be flagged: 0x%08x\n", hr); + + hr = ISpatialAudioObjectRenderStream_ActivateSpatialAudioObject(sas, AudioObjectType_SideLeft, &sao[2]); + ok(hr == S_OK, "Failed to activate spatial audio object: 0x%08x\n", hr); + + hr = ISpatialAudioObjectRenderStream_BeginUpdatingAudioObjects(sas, &dyn_object_count, &frame_count); + ok(hr == S_OK, "Failed to beging updating audio objects: 0x%08x\n", hr); + ok(dyn_object_count == 0, "Unexpected dynamic objects\n"); + ok(frame_count > 0, "Expected to get non-zero frames to update\n"); + + hr = ISpatialAudioObjectRenderStream_ActivateSpatialAudioObject(sas, AudioObjectType_SideRight, &sao[3]); + ok(hr == S_OK, "Failed to activate spatial audio object: 0x%08x\n", hr); + + for (i = 0; i < ARRAYSIZE(sao); i++) + { + hr = ISpatialAudioObject_GetBuffer(sao[i], &buffer, &buffer_length); + ok(hr == S_OK, "Expected to be able to get buffers for audio object: 0x%08x\n", hr); + ok(buffer != NULL, "Expected to get a non-NULL buffer\n"); + ok(buffer_length == frame_count * format.wBitsPerSample / 8, "Expected buffer length to be sample_size * frame_count = %hu but got %u\n", + frame_count * format.wBitsPerSample / 8, buffer_length); + ok(is_buffer_zeroed(buffer, buffer_length), "Expected audio object's buffer to be zeroed\n"); + } + + hr = ISpatialAudioObjectRenderStream_EndUpdatingAudioObjects(sas); + ok(hr == S_OK, "Failed to end updating audio objects: 0x%08x\n", hr); + + hr = WaitForSingleObject(event, 200); + ok(hr == WAIT_OBJECT_0, "Expected event to be flagged: 0x%08x\n", hr); + + hr = ISpatialAudioObjectRenderStream_BeginUpdatingAudioObjects(sas, &dyn_object_count, &frame_count); + ok(hr == S_OK, "Failed to beging updating audio objects: 0x%08x\n", hr); + ok(dyn_object_count == 0, "Unexpected dynamic objects\n"); + ok(frame_count > 0, "Expected to get non-zero frames to update\n"); + + /* one more iteration but not with every object */ + for (i = 0; i < ARRAYSIZE(sao) - 1; i++) + { + hr = ISpatialAudioObject_GetBuffer(sao[i], &buffer, &buffer_length); + ok(hr == S_OK, "Expected to be able to get buffers for audio object: 0x%08x\n", hr); + ok(buffer != NULL, "Expected to get a non-NULL buffer\n"); + ok(buffer_length == frame_count * format.wBitsPerSample / 8, "Expected buffer length to be sample_size * frame_count = %hu but got %u\n", + frame_count * format.wBitsPerSample / 8, buffer_length); + ok(is_buffer_zeroed(buffer, buffer_length), "Expected audio object's buffer to be zeroed\n"); + } + + hr = ISpatialAudioObjectRenderStream_EndUpdatingAudioObjects(sas); + ok(hr == S_OK, "Failed to end updating audio objects: 0x%08x\n", hr); + + /* ending the stream */ + hr = ISpatialAudioObject_SetEndOfStream(sao[0], 0); + todo_wine ok(hr == SPTLAUDCLNT_E_OUT_OF_ORDER, "Expected that ending the stream at this point won't be allowed: 0x%08x\n", hr); + + hr = WaitForSingleObject(event, 10000);
This wait timeout can be decreased.
+START_TEST(spatialaudio) +{ + HRESULT hr; + + event = CreateEventA(NULL, FALSE, FALSE, "spatial-audio-test-prog-event"); + ok (event != NULL, "Failed to create event, last error: 0x%08x\n", GetLastError());
Extra space here. Andrew