[PATCH v9 0/4] MR4852: Add support from AudioPathConfig
-- v9: dmime: Semi-support creating an audio path from config. dmime: IDirectMusicPerformance::CreateAudioPath should fail when config is NULL. dmime: Parse AudioPathConfig https://gitlab.winehq.org/wine/wine/-/merge_requests/4852
From: Yuxuan Shui <yshui(a)codeweavers.com> AudioPaths are created from IDirectMusicPerformance::CreateAudioPath and CreateStandardAudioPath, and don't have an IDirectMusicObject or an IPersistStream interface. On the other hand AudioPathConfigs are loaded from files, and do have IDirectMusicObject and IPersistStream. They were somehow confused with each other and implemented in the same struct, this commit fixes that. --- dlls/dmime/audiopath.c | 101 +++++++++++++++++++++++++++++++------ dlls/dmime/dmime_main.c | 4 +- dlls/dmime/dmime_private.h | 1 + dlls/dmime/tests/dmime.c | 12 ++--- 4 files changed, 93 insertions(+), 25 deletions(-) diff --git a/dlls/dmime/audiopath.c b/dlls/dmime/audiopath.c index 65691f4705f..02b89dc549b 100644 --- a/dlls/dmime/audiopath.c +++ b/dlls/dmime/audiopath.c @@ -23,7 +23,6 @@ WINE_DEFAULT_DEBUG_CHANNEL(dmime); struct IDirectMusicAudioPathImpl { IDirectMusicAudioPath IDirectMusicAudioPath_iface; - struct dmobject dmobj; LONG ref; IDirectMusicPerformance8* pPerf; IDirectMusicGraph* pToolGraph; @@ -33,14 +32,21 @@ struct IDirectMusicAudioPathImpl { BOOL fActive; }; +struct audio_path_config +{ + IUnknown IUnknown_iface; + struct dmobject dmobj; + LONG ref; +}; + static inline struct IDirectMusicAudioPathImpl *impl_from_IDirectMusicAudioPath(IDirectMusicAudioPath *iface) { return CONTAINING_RECORD(iface, struct IDirectMusicAudioPathImpl, IDirectMusicAudioPath_iface); } -static inline struct IDirectMusicAudioPathImpl *impl_from_IPersistStream(IPersistStream *iface) +static inline struct audio_path_config *impl_from_IPersistStream(IPersistStream *iface) { - return CONTAINING_RECORD(iface, struct IDirectMusicAudioPathImpl, dmobj.IPersistStream_iface); + return CONTAINING_RECORD(iface, struct audio_path_config, dmobj.IPersistStream_iface); } void set_audiopath_perf_pointer(IDirectMusicAudioPath *iface, IDirectMusicPerformance8 *performance) @@ -74,10 +80,6 @@ static HRESULT WINAPI IDirectMusicAudioPathImpl_QueryInterface (IDirectMusicAudi if (IsEqualIID (riid, &IID_IDirectMusicAudioPath) || IsEqualIID (riid, &IID_IUnknown)) *ppobj = &This->IDirectMusicAudioPath_iface; - else if (IsEqualIID (riid, &IID_IDirectMusicObject)) - *ppobj = &This->dmobj.IDirectMusicObject_iface; - else if (IsEqualIID (riid, &IID_IPersistStream)) - *ppobj = &This->dmobj.IPersistStream_iface; if (*ppobj) { IUnknown_AddRef((IUnknown*)*ppobj); @@ -255,7 +257,7 @@ static const IDirectMusicAudioPathVtbl DirectMusicAudioPathVtbl = { }; /* IDirectMusicObject */ -static HRESULT WINAPI path_IDirectMusicObject_ParseDescriptor(IDirectMusicObject *iface, +static HRESULT WINAPI path_config_IDirectMusicObject_ParseDescriptor(IDirectMusicObject *iface, IStream *stream, DMUS_OBJECTDESC *desc) { struct chunk_entry riff = {0}; @@ -294,13 +296,13 @@ static const IDirectMusicObjectVtbl dmobject_vtbl = { dmobj_IDirectMusicObject_Release, dmobj_IDirectMusicObject_GetDescriptor, dmobj_IDirectMusicObject_SetDescriptor, - path_IDirectMusicObject_ParseDescriptor + path_config_IDirectMusicObject_ParseDescriptor }; /* IPersistStream */ -static HRESULT WINAPI path_IPersistStream_Load(IPersistStream *iface, IStream *stream) +static HRESULT WINAPI path_config_IPersistStream_Load(IPersistStream *iface, IStream *stream) { - struct IDirectMusicAudioPathImpl *This = impl_from_IPersistStream(iface); + struct audio_path_config *This = impl_from_IPersistStream(iface); FIXME("(%p, %p): Loading not implemented yet\n", This, stream); @@ -314,11 +316,64 @@ static const IPersistStreamVtbl persiststream_vtbl = { dmobj_IPersistStream_Release, dmobj_IPersistStream_GetClassID, unimpl_IPersistStream_IsDirty, - path_IPersistStream_Load, + path_config_IPersistStream_Load, unimpl_IPersistStream_Save, unimpl_IPersistStream_GetSizeMax }; +static inline struct audio_path_config *impl_from_IUnknown(IUnknown *iface) +{ + return CONTAINING_RECORD(iface, struct audio_path_config, IUnknown_iface); +} + +static HRESULT WINAPI path_config_IUnknown_QueryInterface(IUnknown *iface, REFIID riid, void **ppobj) +{ + struct audio_path_config *This = impl_from_IUnknown(iface); + + TRACE("(%p, %s, %p)\n", This, debugstr_dmguid(riid), ppobj); + + *ppobj = NULL; + + if (IsEqualIID (riid, &IID_IUnknown)) + *ppobj = &This->IUnknown_iface; + else if (IsEqualIID (riid, &IID_IDirectMusicObject)) + *ppobj = &This->dmobj.IDirectMusicObject_iface; + else if (IsEqualIID (riid, &IID_IPersistStream)) + *ppobj = &This->dmobj.IPersistStream_iface; + + if (*ppobj) + { + IUnknown_AddRef((IUnknown*)*ppobj); + return S_OK; + } + + WARN("(%p, %s, %p): not found\n", This, debugstr_dmguid(riid), ppobj); + return E_NOINTERFACE; +} + +static ULONG WINAPI path_config_IUnknown_AddRef(IUnknown *unk) +{ + struct audio_path_config *This = impl_from_IUnknown(unk); + return InterlockedIncrement(&This->ref); +} + +static ULONG WINAPI path_config_IUnknown_Release(IUnknown *unk) +{ + struct audio_path_config *This = impl_from_IUnknown(unk); + ULONG ref = InterlockedDecrement(&This->ref); + + if (!ref) + free(This); + return ref; +} + +static const IUnknownVtbl path_config_unk_vtbl = +{ + path_config_IUnknown_QueryInterface, + path_config_IUnknown_AddRef, + path_config_IUnknown_Release, +}; + /* for ClassFactory */ HRESULT create_dmaudiopath(REFIID riid, void **ppobj) { @@ -329,12 +384,26 @@ HRESULT create_dmaudiopath(REFIID riid, void **ppobj) if (!(obj = calloc(1, sizeof(*obj)))) return E_OUTOFMEMORY; obj->IDirectMusicAudioPath_iface.lpVtbl = &DirectMusicAudioPathVtbl; obj->ref = 1; - dmobject_init(&obj->dmobj, &CLSID_DirectMusicAudioPathConfig, - (IUnknown *)&obj->IDirectMusicAudioPath_iface); - obj->dmobj.IDirectMusicObject_iface.lpVtbl = &dmobject_vtbl; - obj->dmobj.IPersistStream_iface.lpVtbl = &persiststream_vtbl; hr = IDirectMusicAudioPath_QueryInterface(&obj->IDirectMusicAudioPath_iface, riid, ppobj); IDirectMusicAudioPath_Release(&obj->IDirectMusicAudioPath_iface); return hr; } + +HRESULT create_dmaudiopath_config(REFIID riid, void **ppobj) +{ + struct audio_path_config *obj; + HRESULT hr; + + *ppobj = NULL; + if (!(obj = calloc(1, sizeof(*obj)))) return E_OUTOFMEMORY; + dmobject_init(&obj->dmobj, &CLSID_DirectMusicAudioPathConfig, &obj->IUnknown_iface); + obj->IUnknown_iface.lpVtbl = &path_config_unk_vtbl; + obj->dmobj.IDirectMusicObject_iface.lpVtbl = &dmobject_vtbl; + obj->dmobj.IPersistStream_iface.lpVtbl = &persiststream_vtbl; + obj->ref = 1; + + hr = IUnknown_QueryInterface(&obj->IUnknown_iface, riid, ppobj); + IUnknown_Release(&obj->IUnknown_iface); + return hr; +} diff --git a/dlls/dmime/dmime_main.c b/dlls/dmime/dmime_main.c index 89e70bcd1bd..05d094fc2a2 100644 --- a/dlls/dmime/dmime_main.c +++ b/dlls/dmime/dmime_main.c @@ -120,7 +120,7 @@ static IClassFactoryImpl ParamControlTrack_CF = {{&classfactory_vtbl}, create_dm static IClassFactoryImpl MarkerTrack_CF = {{&classfactory_vtbl}, create_dmmarkertrack}; static IClassFactoryImpl LyricsTrack_CF = {{&classfactory_vtbl}, create_dmlyricstrack}; static IClassFactoryImpl SegTriggerTrack_CF = {{&classfactory_vtbl}, create_dmsegtriggertrack}; -static IClassFactoryImpl AudioPath_CF = {{&classfactory_vtbl}, create_dmaudiopath}; +static IClassFactoryImpl AudioPathConfig_CF = {{&classfactory_vtbl}, create_dmaudiopath_config}; static IClassFactoryImpl WaveTrack_CF = {{&classfactory_vtbl}, create_dmwavetrack}; @@ -182,7 +182,7 @@ HRESULT WINAPI DllGetClassObject(REFCLSID rclsid, REFIID riid, LPVOID *ppv) return S_OK; } else if (IsEqualCLSID (rclsid, &CLSID_DirectMusicAudioPathConfig) && IsEqualIID (riid, &IID_IClassFactory)) { - *ppv = &AudioPath_CF; + *ppv = &AudioPathConfig_CF; IClassFactory_AddRef((IClassFactory*)*ppv); return S_OK; } else if (IsEqualCLSID (rclsid, &CLSID_DirectMusicWaveTrack) && IsEqualIID (riid, &IID_IClassFactory)) { diff --git a/dlls/dmime/dmime_private.h b/dlls/dmime/dmime_private.h index ed1589ece1a..7b744419c38 100644 --- a/dlls/dmime/dmime_private.h +++ b/dlls/dmime/dmime_private.h @@ -57,6 +57,7 @@ extern HRESULT create_dmsegment(REFIID riid, void **ret_iface); extern HRESULT create_dmsegmentstate(REFIID riid, void **ret_iface); extern HRESULT create_dmgraph(REFIID riid, void **ret_iface); extern HRESULT create_dmaudiopath(REFIID riid, void **ret_iface); +extern HRESULT create_dmaudiopath_config(REFIID riid, void **ret_iface); extern HRESULT create_dmlyricstrack(REFIID riid, void **ret_iface); extern HRESULT create_dmmarkertrack(REFIID riid, void **ret_iface); diff --git a/dlls/dmime/tests/dmime.c b/dlls/dmime/tests/dmime.c index f5ae845cf97..2b7c744acdb 100644 --- a/dlls/dmime/tests/dmime.c +++ b/dlls/dmime/tests/dmime.c @@ -908,12 +908,10 @@ static void test_COM_audiopath(void) /* IDirectMusicObject and IPersistStream are not supported */ hr = IDirectMusicAudioPath_QueryInterface(dmap, &IID_IDirectMusicObject, (void**)&unk); - todo_wine ok(FAILED(hr) && !unk, "Unexpected IDirectMusicObject interface: hr=%#lx, iface=%p\n", - hr, unk); + ok(FAILED(hr) && !unk, "Unexpected IDirectMusicObject interface: hr=%#lx, iface=%p\n", hr, unk); if (unk) IUnknown_Release(unk); hr = IDirectMusicAudioPath_QueryInterface(dmap, &IID_IPersistStream, (void**)&unk); - todo_wine ok(FAILED(hr) && !unk, "Unexpected IPersistStream interface: hr=%#lx, iface=%p\n", - hr, unk); + ok(FAILED(hr) && !unk, "Unexpected IPersistStream interface: hr=%#lx, iface=%p\n", hr, unk); if (unk) IUnknown_Release(unk); /* Same refcount for all DirectMusicAudioPath interfaces */ @@ -988,7 +986,7 @@ static void test_COM_audiopathconfig(void) /* IDirectMusicAudioPath not supported */ hr = CoCreateInstance(&CLSID_DirectMusicAudioPathConfig, NULL, CLSCTX_INPROC_SERVER, &IID_IDirectMusicAudioPath, (void**)&dmap); - todo_wine ok(FAILED(hr) && !dmap, + ok(FAILED(hr) && !dmap, "Unexpected IDirectMusicAudioPath interface: hr=%#lx, iface=%p\n", hr, dmap); /* IDirectMusicObject and IPersistStream supported */ @@ -1018,8 +1016,8 @@ static void test_COM_audiopathconfig(void) /* IDirectMusicAudioPath still not supported */ hr = IDirectMusicObject_QueryInterface(dmo, &IID_IDirectMusicAudioPath, (void**)&dmap); - todo_wine ok(FAILED(hr) && !dmap, - "Unexpected IDirectMusicAudioPath interface: hr=%#lx, iface=%p\n", hr, dmap); + ok(FAILED(hr) && !dmap, + "Unexpected IDirectMusicAudioPath interface: hr=%#lx, iface=%p\n", hr, dmap); while (IDirectMusicObject_Release(dmo)); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4852
From: Yuxuan Shui <yshui(a)codeweavers.com> --- dlls/dmime/audiopath.c | 285 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 281 insertions(+), 4 deletions(-) diff --git a/dlls/dmime/audiopath.c b/dlls/dmime/audiopath.c index 02b89dc549b..21385729c21 100644 --- a/dlls/dmime/audiopath.c +++ b/dlls/dmime/audiopath.c @@ -32,11 +32,31 @@ struct IDirectMusicAudioPathImpl { BOOL fActive; }; +struct audio_path_pchannel_to_buffer +{ + struct list entry; + DMUS_IO_PCHANNELTOBUFFER_HEADER header; + + ULONG num_of_guids; + GUID guids[]; +}; + +struct audio_path_port_config +{ + struct list entry; + DMUS_IO_PORTCONFIG_HEADER header; + DMUS_PORTPARAMS8 params; + + struct list pchannel_to_buffer_entries; +}; + struct audio_path_config { IUnknown IUnknown_iface; struct dmobject dmobj; LONG ref; + + struct list port_config_entries; }; static inline struct IDirectMusicAudioPathImpl *impl_from_IDirectMusicAudioPath(IDirectMusicAudioPath *iface) @@ -299,15 +319,256 @@ static const IDirectMusicObjectVtbl dmobject_vtbl = { path_config_IDirectMusicObject_ParseDescriptor }; +static HRESULT parse_pchannel_to_buffers_list(struct audio_path_port_config *This, IStream *stream, const struct chunk_entry *pchl) +{ + struct chunk_entry chunk = { .parent = pchl }; + struct audio_path_pchannel_to_buffer *pchannel_to_buffer = NULL; + DMUS_IO_PCHANNELTOBUFFER_HEADER header; + ULONG bytes, i; + HRESULT hr; + + while ((hr = stream_next_chunk(stream, &chunk)) == S_OK) + { + switch (chunk.id) + { + case DMUS_FOURCC_PCHANNELS_ITEM: + pchannel_to_buffer = calloc(1, sizeof(*pchannel_to_buffer)); + if (!pchannel_to_buffer) + return E_OUTOFMEMORY; + + hr = stream_read(stream, &header, sizeof(header)); + if (FAILED(hr)) + break; + + TRACE("Got PChannelToBuffer header:\n"); + TRACE("\tdwPChannelCount: %lu\n", header.dwPChannelCount); + TRACE("\tdwPChannelBase: %lu\n", header.dwPChannelBase); + TRACE("\tdwBufferCount: %lu\n", header.dwBufferCount); + TRACE("\tdwFlags: %lx\n", header.dwFlags); + + bytes = chunk.size - sizeof(header); + + if (bytes % sizeof(GUID)) + { + WARN("Invalid size of GUIDs array\n"); + hr = E_FAIL; + break; + } + + pchannel_to_buffer = calloc(1, sizeof(*pchannel_to_buffer) + bytes); + if (!pchannel_to_buffer) + { + hr = E_OUTOFMEMORY; + break; + } + + pchannel_to_buffer->num_of_guids = bytes / sizeof(GUID); + pchannel_to_buffer->header = header; + TRACE("number of GUIDs: %lu\n", pchannel_to_buffer->num_of_guids); + + hr = stream_read(stream, pchannel_to_buffer->guids, bytes); + if (FAILED(hr)) + break; + + for (i = 0; i < pchannel_to_buffer->num_of_guids; i++) + TRACE("\tguids[%lu]: %s\n", i, debugstr_dmguid(&pchannel_to_buffer->guids[i])); + + list_add_tail(&This->pchannel_to_buffer_entries, &pchannel_to_buffer->entry); + pchannel_to_buffer = NULL; + break; + case FOURCC_LIST: + if (chunk.type == DMUS_FOURCC_UNFO_LIST) + TRACE("Unfo list in PChannelToBuffer is ignored\n"); + else + WARN("Unknown %s\n", debugstr_chunk(&chunk)); + break; + case MAKEFOURCC('p','b','g','d'): + FIXME("Undocumented %s\n", debugstr_chunk(&chunk)); + break; + default: + WARN("Unknown %s\n", debugstr_chunk(&chunk)); + break; + } + + if (FAILED(hr)) + break; + } + + if (FAILED(hr) && pchannel_to_buffer) + free(pchannel_to_buffer); + return hr; +} + +static void port_config_destroy(struct audio_path_port_config *port_config) +{ + struct audio_path_pchannel_to_buffer *pchannel_to_buffer, *next_pchannel_to_buffer; + LIST_FOR_EACH_ENTRY_SAFE(pchannel_to_buffer, next_pchannel_to_buffer, &port_config->pchannel_to_buffer_entries, + struct audio_path_pchannel_to_buffer, entry) + { + list_remove(&pchannel_to_buffer->entry); + free(pchannel_to_buffer); + } + free(port_config); +} + +static HRESULT parse_port_config_list(struct audio_path_config *This, IStream *stream, const struct chunk_entry *pcfl) +{ + struct chunk_entry chunk = { .parent = pcfl }; + struct audio_path_port_config *port_config = calloc(1, sizeof(*port_config)); + HRESULT hr; + + if (!port_config) + return E_OUTOFMEMORY; + + list_init(&port_config->pchannel_to_buffer_entries); + + while ((hr = stream_next_chunk(stream, &chunk)) == S_OK) + { + switch (chunk.id) + { + case DMUS_FOURCC_PORTCONFIG_ITEM: + hr = stream_chunk_get_data(stream, &chunk, &port_config->header, sizeof(port_config->header)); + if (FAILED(hr)) + break; + TRACE("Got PortConfig header:\n"); + TRACE("\tdwPChannelBase: %lu\n", port_config->header.dwPChannelBase); + TRACE("\tdwPChannelCount: %lu\n", port_config->header.dwPChannelCount); + TRACE("\tdwFlags: %lx\n", port_config->header.dwFlags); + TRACE("\tguidPort: %s\n", debugstr_dmguid(&port_config->header.guidPort)); + break; + case DMUS_FOURCC_PORTPARAMS_ITEM: + hr = stream_chunk_get_data(stream, &chunk, &port_config->params, sizeof(port_config->params)); + if (FAILED(hr)) + break; + TRACE("Got PortConfig params:\n"); + TRACE("\tdwSize: %lu\n", port_config->params.dwSize); + TRACE("\tdwValidParams: %lx\n", port_config->params.dwValidParams); + if (port_config->params.dwValidParams & DMUS_PORTPARAMS_VOICES) + TRACE("\tvoices: %lu\n", port_config->params.dwVoices); + if (port_config->params.dwValidParams & DMUS_PORTPARAMS_CHANNELGROUPS) + TRACE("\tchannel groups: %lu\n", port_config->params.dwChannelGroups); + if (port_config->params.dwValidParams & DMUS_PORTPARAMS_AUDIOCHANNELS) + TRACE("\taudio channels: %lu\n", port_config->params.dwAudioChannels); + if (port_config->params.dwValidParams & DMUS_PORTPARAMS_SAMPLERATE) + TRACE("\tsample rate: %lu\n", port_config->params.dwSampleRate); + if (port_config->params.dwValidParams & DMUS_PORTPARAMS_EFFECTS) + TRACE("\teffects: %lx\n", port_config->params.dwEffectFlags); + if (port_config->params.dwValidParams & DMUS_PORTPARAMS_SHARE) + TRACE("\tshare: %d\n", port_config->params.fShare); + if (port_config->params.dwValidParams & DMUS_PORTPARAMS_FEATURES) + TRACE("\tfeatures: %lx\n", port_config->params.dwFeatures); + break; + case FOURCC_LIST: + if (chunk.type == DMUS_FOURCC_DSBUFFER_LIST) + FIXME("DirectSound buffer descriptors is not supported\n"); + else if (chunk.type == DMUS_FOURCC_PCHANNELS_LIST) + hr = parse_pchannel_to_buffers_list(port_config, stream, &chunk); + else if (chunk.type == DMUS_FOURCC_UNFO_LIST) + TRACE("Unfo list in PortConfig is ignored\n"); + else + WARN("Unknown %s\n", debugstr_chunk(&chunk)); + break; + default: + WARN("Unknown %s\n", debugstr_chunk(&chunk)); + } + + if (FAILED(hr)) + break; + } + + if (FAILED(hr)) + port_config_destroy(port_config); + else + list_add_tail(&This->port_config_entries, &port_config->entry); + return hr; +} + +static HRESULT parse_port_configs_list(struct audio_path_config *This, IStream *stream, const struct chunk_entry *pcsl) +{ + struct chunk_entry chunk = { .parent = pcsl }; + HRESULT hr; + + while ((hr = stream_next_chunk(stream, &chunk)) == S_OK) + { + switch (MAKE_IDTYPE(chunk.id, chunk.type)) + { + case MAKE_IDTYPE(FOURCC_LIST, DMUS_FOURCC_PORTCONFIG_LIST): + hr = parse_port_config_list(This, stream, &chunk); + if (FAILED(hr)) + return hr; + break; + default: + WARN("Unknown %s\n", debugstr_chunk(&chunk)); + break; + } + } + return SUCCEEDED(hr) ? S_OK : hr; +} + /* IPersistStream */ static HRESULT WINAPI path_config_IPersistStream_Load(IPersistStream *iface, IStream *stream) { struct audio_path_config *This = impl_from_IPersistStream(iface); + struct chunk_entry riff = {0}, chunk = {0}; + HRESULT hr = S_OK; + + FIXME("(%p, %p): Loading\n", This, stream); + + if (!stream) + return E_POINTER; + + hr = stream_get_chunk(stream, &riff); + if (FAILED(hr)) + { + WARN("Failed to get chunk %#lx.\n", hr); + return hr; + } + + if (MAKE_IDTYPE(riff.id, riff.type) != MAKE_IDTYPE(FOURCC_RIFF, DMUS_FOURCC_AUDIOPATH_FORM)) + { + WARN("loading failed: unexpected %s\n", debugstr_chunk(&riff)); + return E_UNEXPECTED; + } - FIXME("(%p, %p): Loading not implemented yet\n", This, stream); + if (FAILED(hr = dmobj_parsedescriptor(stream, &riff, &This->dmobj.desc, + DMUS_OBJ_OBJECT | DMUS_OBJ_VERSION | DMUS_OBJ_NAME | DMUS_OBJ_CATEGORY)) + || FAILED(hr = stream_reset_chunk_data(stream, &riff))) + { + WARN("Failed to parse descriptor %#lx.\n", hr); + return hr; + } + This->dmobj.desc.guidClass = CLSID_DirectMusicAudioPathConfig; + This->dmobj.desc.dwValidData |= DMUS_OBJ_CLASS; - return IDirectMusicObject_ParseDescriptor(&This->dmobj.IDirectMusicObject_iface, stream, - &This->dmobj.desc); + chunk.parent = &riff; + while ((hr = stream_next_chunk(stream, &chunk)) == S_OK) + { + switch (MAKE_IDTYPE(chunk.id, chunk.type)) + { + case MAKE_IDTYPE(FOURCC_LIST, DMUS_FOURCC_PORTCONFIGS_LIST): + hr = parse_port_configs_list(This, stream, &chunk); + if (FAILED(hr)) + return hr; + case MAKE_IDTYPE(FOURCC_LIST, DMUS_FOURCC_DSBUFFER_LIST): + FIXME("buffer attributes are not supported\n"); + break; + case MAKE_IDTYPE(FOURCC_LIST, MAKEFOURCC('p','a','p','d')): + FIXME("Undocumented %s\n", debugstr_chunk(&chunk)); + break; + case MAKE_IDTYPE(FOURCC_LIST, DMUS_FOURCC_UNFO_LIST): + WARN("Unknown %s\n", debugstr_chunk(&chunk)); + break; + case DMUS_FOURCC_GUID_CHUNK: + case DMUS_FOURCC_VERSION_CHUNK: + /* Already parsed in dmobj_parsedescriptor. */ + break; + default: + WARN("Unknown %s\n", debugstr_chunk(&chunk)); + break; + } + } + TRACE("Finished parsing %#lx\n", hr); + return SUCCEEDED(hr) ? S_OK : hr; } static const IPersistStreamVtbl persiststream_vtbl = { @@ -354,7 +615,11 @@ static HRESULT WINAPI path_config_IUnknown_QueryInterface(IUnknown *iface, REFII static ULONG WINAPI path_config_IUnknown_AddRef(IUnknown *unk) { struct audio_path_config *This = impl_from_IUnknown(unk); - return InterlockedIncrement(&This->ref); + ULONG ref = InterlockedIncrement(&This->ref); + + TRACE("(%p): ref=%ld\n", This, ref); + + return ref; } static ULONG WINAPI path_config_IUnknown_Release(IUnknown *unk) @@ -362,8 +627,18 @@ static ULONG WINAPI path_config_IUnknown_Release(IUnknown *unk) struct audio_path_config *This = impl_from_IUnknown(unk); ULONG ref = InterlockedDecrement(&This->ref); + TRACE("(%p): ref=%ld\n", This, ref); + if (!ref) + { + struct audio_path_port_config *config, *next_config; + LIST_FOR_EACH_ENTRY_SAFE(config, next_config, &This->port_config_entries, struct audio_path_port_config, entry) + { + list_remove(&config->entry); + port_config_destroy(config); + } free(This); + } return ref; } @@ -403,6 +678,8 @@ HRESULT create_dmaudiopath_config(REFIID riid, void **ppobj) obj->dmobj.IPersistStream_iface.lpVtbl = &persiststream_vtbl; obj->ref = 1; + list_init(&obj->port_config_entries); + hr = IUnknown_QueryInterface(&obj->IUnknown_iface, riid, ppobj); IUnknown_Release(&obj->IUnknown_iface); return hr; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4852
From: Yuxuan Shui <yshui(a)codeweavers.com> --- dlls/dmime/performance.c | 2 +- dlls/dmime/tests/dmime.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/dlls/dmime/performance.c b/dlls/dmime/performance.c index bd596750f34..bce5f96352b 100644 --- a/dlls/dmime/performance.c +++ b/dlls/dmime/performance.c @@ -1654,7 +1654,7 @@ static HRESULT WINAPI performance_CreateAudioPath(IDirectMusicPerformance8 *ifac FIXME("(%p, %p, %d, %p): stub\n", This, pSourceConfig, fActivate, ret_iface); - if (!ret_iface) return E_POINTER; + if (!ret_iface || !pSourceConfig) return E_POINTER; if (!This->audio_paths_enabled) return DMUS_E_AUDIOPATH_INACTIVE; create_dmaudiopath(&IID_IDirectMusicAudioPath, (void **)&pPath); diff --git a/dlls/dmime/tests/dmime.c b/dlls/dmime/tests/dmime.c index 2b7c744acdb..e9036511e9d 100644 --- a/dlls/dmime/tests/dmime.c +++ b/dlls/dmime/tests/dmime.c @@ -1256,6 +1256,7 @@ static void test_COM_performance(void) IDirectMusicPerformance *dmp = (IDirectMusicPerformance*)0xdeadbeef; IDirectMusicPerformance *dmp2; IDirectMusicPerformance8 *dmp8; + IDirectMusicAudioPath *dmap = NULL; ULONG refcount; HRESULT hr; @@ -1284,6 +1285,9 @@ static void test_COM_performance(void) ok (refcount == 3, "refcount == %lu, expected 3\n", refcount); hr = IDirectMusicPerformance_QueryInterface(dmp, &IID_IDirectMusicPerformance8, (void**)&dmp8); ok(hr == S_OK, "QueryInterface for IID_IDirectMusicPerformance8 failed: %#lx\n", hr); + hr = IDirectMusicPerformance8_CreateAudioPath(dmp8, NULL, TRUE, &dmap); + ok(hr == E_POINTER, "Unexpected result from CreateAudioPath: %#lx\n", hr); + ok(dmap == NULL, "Unexpected dmap pointer\n"); refcount = IDirectMusicPerformance_Release(dmp); ok (refcount == 3, "refcount == %lu, expected 3\n", refcount); refcount = IDirectMusicPerformance8_Release(dmp8); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4852
From: Yuxuan Shui <yshui(a)codeweavers.com> --- dlls/dmime/audiopath.c | 83 ++++++++++++++++++++++++++++++++++++++ dlls/dmime/dmime_private.h | 2 + dlls/dmime/performance.c | 52 +++++++++++++++++++++++- 3 files changed, 135 insertions(+), 2 deletions(-) diff --git a/dlls/dmime/audiopath.c b/dlls/dmime/audiopath.c index 21385729c21..9a138fc751b 100644 --- a/dlls/dmime/audiopath.c +++ b/dlls/dmime/audiopath.c @@ -649,6 +649,89 @@ static const IUnknownVtbl path_config_unk_vtbl = path_config_IUnknown_Release, }; +HRESULT path_config_get_audio_path_params(IUnknown *iface, WAVEFORMATEX *format, DSBUFFERDESC *desc, DMUS_PORTPARAMS *params) +{ + struct audio_path_config *This = impl_from_IUnknown(iface); + struct list *first_port_config, *first_pchannel_to_buffer; + struct audio_path_port_config *port_config; + struct audio_path_pchannel_to_buffer *pchannel_to_buffer; + GUID *guids; + + first_port_config = list_head(&This->port_config_entries); + if (list_next(&This->port_config_entries, first_port_config)) + FIXME("Only one port config supported. %p -> %p\n", first_port_config, list_next(&This->port_config_entries, first_port_config)); + port_config = LIST_ENTRY(first_port_config, struct audio_path_port_config, entry); + first_pchannel_to_buffer = list_head(&port_config->pchannel_to_buffer_entries); + if (list_next(&port_config->pchannel_to_buffer_entries, first_pchannel_to_buffer)) + FIXME("Only one pchannel to buffer entry supported.\n"); + pchannel_to_buffer = LIST_ENTRY(first_pchannel_to_buffer, struct audio_path_pchannel_to_buffer, entry); + + /* Secondary buffer description */ + memset(format, 0, sizeof(*format)); + format->wFormatTag = WAVE_FORMAT_PCM; + format->nChannels = 1; + format->nSamplesPerSec = 44000; + format->nAvgBytesPerSec = 44000*2; + format->nBlockAlign = 2; + format->wBitsPerSample = 16; + format->cbSize = 0; + + memset(desc, 0, sizeof(*desc)); + desc->dwSize = sizeof(*desc); + desc->dwFlags = DSBCAPS_CTRLFX | DSBCAPS_CTRLVOLUME | DSBCAPS_GLOBALFOCUS; + desc->dwBufferBytes = DSBSIZE_MIN; + desc->dwReserved = 0; + desc->lpwfxFormat = format; + desc->guid3DAlgorithm = GUID_NULL; + + guids = pchannel_to_buffer->guids; + if (pchannel_to_buffer->num_of_guids == 2) + { + if ((!IsEqualGUID(&guids[0], &GUID_Buffer_Reverb) && !IsEqualGUID(&guids[0], &GUID_Buffer_Stereo)) || + (!IsEqualGUID(&guids[1], &GUID_Buffer_Reverb) && !IsEqualGUID(&guids[1], &GUID_Buffer_Stereo)) || + IsEqualGUID(&guids[0], &guids[1])) + FIXME("Only a stereo plus reverb buffer is supported\n"); + else + { + desc->dwFlags |= DSBCAPS_CTRLPAN | DSBCAPS_CTRLFREQUENCY; + format->nChannels = 2; + format->nBlockAlign *= 2; + format->nAvgBytesPerSec *=2; + } + } + else if (pchannel_to_buffer->num_of_guids == 1) + { + if (IsEqualGUID(guids, &GUID_Buffer_Stereo)) + { + desc->dwFlags |= DSBCAPS_CTRLPAN | DSBCAPS_CTRLFREQUENCY; + format->nChannels = 2; + format->nBlockAlign *= 2; + format->nAvgBytesPerSec *=2; + } + else if (IsEqualGUID(guids, &GUID_Buffer_3D_Dry)) + desc->dwFlags |= DSBCAPS_CTRL3D | DSBCAPS_CTRLFREQUENCY | DSBCAPS_MUTE3DATMAXDISTANCE; + else if (IsEqualGUID(guids, &GUID_Buffer_Mono)) + desc->dwFlags |= DSBCAPS_CTRLPAN | DSBCAPS_CTRLFREQUENCY; + else + FIXME("Unsupported buffer guid %s\n", debugstr_dmguid(guids)); + } + else + FIXME("Multiple buffers not supported\n"); + + *params = port_config->params; + if (!(params->dwValidParams & DMUS_PORTPARAMS_CHANNELGROUPS)) + { + params->dwValidParams |= DMUS_PORTPARAMS_CHANNELGROUPS; + params->dwChannelGroups = (port_config->header.dwPChannelCount + 15) / 16; + } + if (!(params->dwValidParams & DMUS_PORTPARAMS_AUDIOCHANNELS)) + { + params->dwValidParams |= DMUS_PORTPARAMS_AUDIOCHANNELS; + params->dwAudioChannels = format->nChannels; + } + return S_OK; +} + /* for ClassFactory */ HRESULT create_dmaudiopath(REFIID riid, void **ppobj) { diff --git a/dlls/dmime/dmime_private.h b/dlls/dmime/dmime_private.h index 7b744419c38..96ccd5daf8b 100644 --- a/dlls/dmime/dmime_private.h +++ b/dlls/dmime/dmime_private.h @@ -93,6 +93,8 @@ extern HRESULT performance_send_segment_tick(IDirectMusicPerformance8 *iface, MU extern HRESULT performance_send_segment_end(IDirectMusicPerformance8 *iface, MUSIC_TIME music_time, IDirectMusicSegmentState *state, BOOL abort); +HRESULT path_config_get_audio_path_params(IUnknown *iface, WAVEFORMATEX *format, DSBUFFERDESC *desc, DMUS_PORTPARAMS *params); + /***************************************************************************** * Auxiliary definitions */ diff --git a/dlls/dmime/performance.c b/dlls/dmime/performance.c index bce5f96352b..ab2b2df167e 100644 --- a/dlls/dmime/performance.c +++ b/dlls/dmime/performance.c @@ -1651,16 +1651,64 @@ static HRESULT WINAPI performance_CreateAudioPath(IDirectMusicPerformance8 *ifac { struct performance *This = impl_from_IDirectMusicPerformance8(iface); IDirectMusicAudioPath *pPath; + IDirectMusicObject *dmo; + IDirectSoundBuffer *buffer, *primary_buffer; + DMUS_OBJECTDESC objDesc; + DMUS_PORTPARAMS8 port_params; + HRESULT hr; + WAVEFORMATEX format; + DSBUFFERDESC desc; - FIXME("(%p, %p, %d, %p): stub\n", This, pSourceConfig, fActivate, ret_iface); + FIXME("(%p, %p, %d, %p): semi-stub\n", This, pSourceConfig, fActivate, ret_iface); if (!ret_iface || !pSourceConfig) return E_POINTER; if (!This->audio_paths_enabled) return DMUS_E_AUDIOPATH_INACTIVE; + hr = IUnknown_QueryInterface(pSourceConfig, &IID_IDirectMusicObject, (void **)&dmo); + if (FAILED(hr)) + return hr; + + hr = IDirectMusicObject_GetDescriptor(dmo, &objDesc); + IDirectMusicObject_Release(dmo); + if (FAILED(hr)) + return hr; + + if (!IsEqualCLSID(&objDesc.guidClass, &CLSID_DirectMusicAudioPathConfig)) + { + ERR("Unexpected object class %s for source config.\n", debugstr_dmguid(&objDesc.guidClass)); + return E_INVALIDARG; + } + + hr = path_config_get_audio_path_params(pSourceConfig, &format, &desc, &port_params); + if (FAILED(hr)) + return hr; + + hr = perf_dmport_create(This, &port_params); + if (FAILED(hr)) + return hr; + + hr = IDirectSound_CreateSoundBuffer(This->dsound, &desc, &buffer, NULL); + if (FAILED(hr)) + return DSERR_BUFFERLOST; + + /* Update description for creating primary buffer */ + desc.dwFlags |= DSBCAPS_PRIMARYBUFFER; + desc.dwFlags &= ~DSBCAPS_CTRLFX; + desc.dwBufferBytes = 0; + desc.lpwfxFormat = NULL; + + hr = IDirectSound_CreateSoundBuffer(This->dsound, &desc, &primary_buffer, NULL); + if (FAILED(hr)) { + IDirectSoundBuffer_Release(buffer); + return DSERR_BUFFERLOST; + } + create_dmaudiopath(&IID_IDirectMusicAudioPath, (void **)&pPath); set_audiopath_perf_pointer(pPath, iface); + set_audiopath_dsound_buffer(pPath, buffer); + set_audiopath_primary_dsound_buffer(pPath, primary_buffer); + TRACE(" returning IDirectMusicAudioPath interface at %p.\n", *ret_iface); - /** TODO */ *ret_iface = pPath; return IDirectMusicAudioPath_Activate(*ret_iface, fActivate); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4852
`calloc(1, offsetof(struct bla, array[count]))`
That's not how the C standard shows how you should allocate struct with flexible member. This is the example: ```c int m = /* some value */; struct s *p = malloc(sizeof(struct s) + sizeof(double [m])); ``` * * * I think worst case here is we would allocate a few more bytes? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4852#note_57925
On Thu Jan 18 13:22:54 2024 +0000, Yuxuan Shui wrote:
`calloc(1, offsetof(struct bla, array[count]))` That's not how the C standard shows how ~you should~ one would typically allocate struct with flexible member. This is the example:
int m = /* some value */; struct s *p = malloc(sizeof(struct s) + sizeof(double [m]));* * * I think worst case here is we would allocate a few more bytes? Which is the point of asserting the struct size: it will be impossible to mis-allocate if it doesn't have implicit padding, and make undefined behavior impossible to happen, and that regardless how you do the allocation.
Then we do such allocations very consistently using offsetof, which makes sure that any type change of the array will be handled properly. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4852#note_57926
On Thu Jan 18 13:19:18 2024 +0000, Rémi Bernon wrote:
Which is the point of asserting the struct size: it will be impossible to mis-allocate if it doesn't have implicit padding, and make undefined behavior impossible to happen, and that regardless how you do the allocation. Then we do such allocations very consistently using offsetof, which makes sure that any type change of the array will be handled properly. I would argue using `offsetof` to allocate is the bad practice here. Because it needs the assert there to be correct (what if you forget?), whereas `sizeof + sizeof` is always undefined behavior free. And the assert there is bad for portability.
And there are places where there are missing asserts: https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/dmsynth/synth.c?ref_t... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4852#note_57927
On Thu Jan 18 13:28:39 2024 +0000, Yuxuan Shui wrote:
I would argue using `offsetof` to allocate is the bad practice here. Because it needs the assert there to be correct (what if you forget?), whereas `sizeof + sizeof` is always undefined behavior free. And the assert there is bad for portability. And there are places where there are missing asserts: https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/dmsynth/synth.c?ref_t... Missing asserts should definitely be added. Yes you can forget, as you can make a typo in the sizeof. But forgetting the assert might not be as bad as a sizeof typo.
Once they are there things are statically checked and therefore better than any other way which requires manual review to check that the types are truly the same, and to do it again every time it is changed. I don't see a portability issue here, C_ASSERT is portable and uses the standard C _Static_assert whenever available. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4852#note_57928
I don't see a portability issue here
What I meant is on a new platform the assert can fail because the alignment changed (e.g. 32 vs 64 bit), and now you need to rearrange the struct to make all platforms happy. Whereas always using `sizeof + sizeof` doesn't have this problem. Anyway, I will add the asserts, just feel this isn't the optimal way of using flexible members. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4852#note_57929
participants (3)
-
Rémi Bernon -
Yuxuan Shui
-
Yuxuan Shui (＠yshui)