Re: [PATCH] dmime: Rewrite IDirectMusicSegment8 GetParam

[Michael Stefaniuc]

Hello Alistair,

this doesn't look right. Any Group (group = 0xFFFFFFFF) shouldn't need
any special treatment, maybe except to check that it is combined with
index DMUS_SEG_ANYTRACK.

bye
    michael

On 11/17/19 5:43 AM, Alistair Leslie-Hughes wrote:
> Removed IPersistStream CLSID checks as this was just wrong.
> Added support for Any Group (group = 0xFFFFFFFF)
> Rename Parameters and variables.
> Remove initial values of variables as they aren't required.
> Signed-off-by: Alistair Leslie-Hughes
>
> dlls/dmime/segment.c | 87 ++++++++++++++++++++++----------------------
>  1 file changed, 44 insertions(+), 43 deletions(-)
>
> diff --git a/dlls/dmime/segment.c b/dlls/dmime/segment.c
> index d47c2f2c230..850b710ee82 100644
> --- a/dlls/dmime/segment.c
> +++ b/dlls/dmime/segment.c
> @@ -366,58 +366,59 @@ static HRESULT WINAPI IDirectMusicSegment8Impl_RemoveNotificationType(IDirectMus
>  }
>
> static HRESULT WINAPI IDirectMusicSegment8Impl_GetParam(IDirectMusicSegment8 *iface,
> -
>    REFGUID rguidType, DWORD dwGroupBits, DWORD dwIndex, MUSIC_TIME mtTime, MUSIC_TIME *pmtNext,
> -
>    void *pParam)
> -
>    REFGUID type, DWORD group, DWORD index, MUSIC_TIME time, MUSIC_TIME *next,
> -
>    void *param)
>
> {
> - IDirectMusicSegment8Impl *This = impl_from_IDirectMusicSegment8(iface);
> - CLSID pIt_clsid;
> - struct list* pEntry = NULL;
> - IDirectMusicTrack* pTrack = NULL;
> - IPersistStream* pCLSIDStream = NULL;
> - LPDMUS_PRIVATE_SEGMENT_TRACK pIt = NULL;
> - HRESULT hr = S_OK;
> - IDirectMusicSegment8Impl *This = impl_from_IDirectMusicSegment8(iface);
> - struct list *item;
> - IDirectMusicTrack *track;
> - DMUS_PRIVATE_SEGMENT_TRACK *segment;
> - HRESULT hr;
> - FIXME("(%p, %s, 0x%x, %d, %d, %p, %p)\n", This, debugstr_dmguid(rguidType), dwGroupBits, dwIndex, mtTime, pmtNext, pParam);
> -
> - if (DMUS_SEG_ANYTRACK == dwIndex) {
> -
> - LIST_FOR_EACH (pEntry, &This->Tracks) {
> -
>  pIt = LIST_ENTRY(pEntry, DMUS_PRIVATE_SEGMENT_TRACK, entry);
> - FIXME("(%p, %s, 0x%x, %d, %d, %p, %p) Semi-stub\n", This, debugstr_dmguid(type), group,
> -
>            index, time, next, param);
>
> -
>  hr = IDirectMusicTrack_QueryInterface(pIt->pTrack, &IID_IPersistStream, (void**) &pCLSIDStream);
> -
>  if (FAILED(hr)) {
> - ERR("(%p): object %p don't implement IPersistStream Interface. Expect a crash (critical problem)\n", This, pIt->pTrack);
> - continue ;
> -
>  }
> - if (index == DMUS_SEG_ANYTRACK) {
> -
>    LIST_FOR_EACH (item, &This->Tracks) {
> -
>        segment = LIST_ENTRY(item, DMUS_PRIVATE_SEGMENT_TRACK, entry);
>
> -
>  TRACE(" - %p -> 0x%x,%p\n", pIt, pIt->dwGroupBits, pIt->pTrack);
> -
>        TRACE(" - %p -> 0x%x,%p\n", segment, segment->dwGroupBits, segment->pTrack);
>
> -
>  if (0xFFFFFFFF != dwGroupBits && 0 == (pIt->dwGroupBits & dwGroupBits)) continue ;
> -
>  hr = IPersistStream_GetClassID(pCLSIDStream, &pIt_clsid);
> -
>  IPersistStream_Release(pCLSIDStream); pCLSIDStream = NULL;
> -
>  if (FAILED(hr)) {
> - ERR("(%p): non-implemented GetClassID for object %p\n", This, pIt->pTrack);
> - continue ;
> -
>  }
> -
>  if (FALSE == IsEqualGUID(&pIt_clsid, rguidType)) continue ;
> -
>  if (FAILED(IDirectMusicTrack_IsParamSupported(pIt->pTrack, rguidType))) continue ;
> -
>  hr = IDirectMusicTrack_GetParam(pIt->pTrack, rguidType, mtTime, pmtNext, pParam);
> -
>  if (SUCCEEDED(hr)) return hr;
> -
>        if (0xFFFFFFFF != group && 0 == (segment->dwGroupBits & group)) continue ;
> -
> -
>        if (FAILED(IDirectMusicTrack_IsParamSupported(segment->pTrack, type))) continue ;
> -
>        hr = IDirectMusicTrack_GetParam(segment->pTrack, type, time, next, param);
> -
>        if (SUCCEEDED(hr)) return hr;
> -
>    }
> -
> -
>    WARN("(%p): not found\n", This);
> -
>    return DMUS_E_TRACK_NOT_FOUND;
> }
> -
>
> ERR("(%p): not found\n", This);
> -
>
> return DMUS_E_TRACK_NOT_FOUND;
> -
>
> }
> -
>
> hr = IDirectMusicSegment8Impl_GetTrack(iface, &GUID_NULL, dwGroupBits, dwIndex, &pTrack);
> -
>
> if (FAILED(hr)) {
> -
>
> ERR("(%p): not found\n", This);
> -
>
> return DMUS_E_TRACK_NOT_FOUND;
> -
>
> }
> - if (group == 0xFFFFFFFF) {
> -
>    if (index != 0)
> -
>        FIXME("Currently only first index is supported.\n");
>
> - hr = IDirectMusicTrack_GetParam(pTrack, rguidType, mtTime, pmtNext, pParam);
> - IDirectMusicTrack_Release(pTrack); pTrack = NULL;
> -
>    LIST_FOR_EACH (item, &This->Tracks) {
> -
>        segment = LIST_ENTRY(item, DMUS_PRIVATE_SEGMENT_TRACK, entry);
> -
>        if (SUCCEEDED(IDirectMusicTrack_IsParamSupported(segment->pTrack, type)))
> -
>            return IDirectMusicTrack_GetParam(segment->pTrack, type, time, next, param);
> -
>    }
>
> - return hr;
> -
>    WARN("(%p): not found\n", This);
> -
>    return DMUS_E_TRACK_NOT_FOUND;
> - }
> -
> - hr = IDirectMusicSegment8Impl_GetTrack(iface, &GUID_NULL, group, index, &track);
> - if (FAILED(hr)) {
> -
>    ERR("(%p): not found\n", This);
> -
>    return DMUS_E_TRACK_NOT_FOUND;
> - }
> -
> - hr = IDirectMusicTrack_GetParam(track, type, time, next, param);
> - IDirectMusicTrack_Release(track); track = NULL;
> -
> - return hr;
>
> }
>
> static HRESULT WINAPI IDirectMusicSegment8Impl_SetParam(IDirectMusicSegment8 *iface,