[PATCH v2 0/1] MR5040: dmband: Implement getting/setting GUID_BandParam on band tracks.

[Yuxuan Shui (＠yshui)]

split from !4982 

that MR doesn't use this anymore but might still be good to have.

--
  v2: dmband: Implement getting/setting GUID_BandParam on band tracks.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5040

[PATCH v2 1/1] dmband: Implement getting/setting GUID_BandParam on band tracks.

[Yuxuan Shui]

From: Yuxuan Shui <yshui(a)codeweavers.com>

---
 dlls/dmband/bandtrack.c    | 43 +++++++++++++++++++--
 dlls/dmband/tests/dmband.c | 78 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 117 insertions(+), 4 deletions(-)

diff --git a/dlls/dmband/bandtrack.c b/dlls/dmband/bandtrack.c
index 4756ed194ef..8d147c095c0 100644
--- a/dlls/dmband/bandtrack.c
+++ b/dlls/dmband/bandtrack.c
@@ -198,19 +198,35 @@ static HRESULT WINAPI band_track_Play(IDirectMusicTrack8 *iface, void *state_dat
 }
 
 static HRESULT WINAPI band_track_GetParam(IDirectMusicTrack8 *iface, REFGUID type, MUSIC_TIME time,
-        MUSIC_TIME *next, void *param)
+        MUSIC_TIME *out_next, void *param)
 {
     struct band_track *This = impl_from_IDirectMusicTrack8(iface);
+    struct band_entry *band, *next_band;
+    DMUS_BAND_PARAM *bandparam;
 
-    TRACE("(%p, %s, %ld, %p, %p)\n", This, debugstr_dmguid(type), time, next, param);
+    TRACE("(%p, %s, %ld, %p, %p)\n", This, debugstr_dmguid(type), time, out_next, param);
 
     if (!type)
         return E_POINTER;
     if (!IsEqualGUID(type, &GUID_BandParam))
         return DMUS_E_GET_UNSUPPORTED;
+    if (list_empty(&This->bands))
+        return DMUS_E_NOT_FOUND;
 
-    FIXME("GUID_BandParam not handled yet\n");
+    bandparam = param;
+    if (out_next) *out_next = 0;
 
+    LIST_FOR_EACH_ENTRY_SAFE(band, next_band, &This->bands, struct band_entry, entry)
+    {
+        /* we want to return the first band even when there's nothing with lBandTime <= time*/
+        bandparam->pBand = band->band;
+        bandparam->mtTimePhysical = band->head.lBandTimePhysical;
+        if (band->entry.next == &This->bands) break;
+        if (out_next) *out_next = next_band->head.lBandTimeLogical;
+        if (next_band->head.lBandTimeLogical > time) break;
+    }
+
+    IDirectMusicBand_AddRef(bandparam->pBand);
     return S_OK;
 }
 
@@ -227,7 +243,26 @@ static HRESULT WINAPI band_track_SetParam(IDirectMusicTrack8 *iface, REFGUID typ
         return DMUS_E_TYPE_UNSUPPORTED;
 
     if (IsEqualGUID(type, &GUID_BandParam))
-        FIXME("GUID_BandParam not handled yet\n");
+    {
+        struct band_entry *new_entry = NULL, *entry, *next_entry;
+        DMUS_BAND_PARAM *band_param = param;
+        if (!band_param || !band_param->pBand)
+            return E_POINTER;
+        if (!(new_entry = calloc(1, sizeof(*new_entry))))
+            return E_OUTOFMEMORY;
+        new_entry->band = band_param->pBand;
+        new_entry->head.lBandTimeLogical = time;
+        new_entry->head.lBandTimePhysical = band_param->mtTimePhysical;
+        IDirectMusicBand_AddRef(new_entry->band);
+        if (list_empty(&This->bands))
+                list_add_tail(&This->bands, &new_entry->entry);
+        else
+        {
+            LIST_FOR_EACH_ENTRY_SAFE(entry, next_entry, &This->bands, struct band_entry, entry)
+                if (entry->entry.next == &This->bands || next_entry->head.lBandTimeLogical > time)
+                    list_add_after(&entry->entry, &new_entry->entry);
+        }
+    }
     else if (IsEqualGUID(type, &GUID_Clear_All_Bands))
         FIXME("GUID_Clear_All_Bands not handled yet\n");
     else if (IsEqualGUID(type, &GUID_ConnectToDLSCollection))
diff --git a/dlls/dmband/tests/dmband.c b/dlls/dmband/tests/dmband.c
index d88606976e4..82081f70047 100644
--- a/dlls/dmband/tests/dmband.c
+++ b/dlls/dmband/tests/dmband.c
@@ -176,9 +176,12 @@ static void test_dmband(void)
 
 static void test_bandtrack(void)
 {
+    DMUS_BAND_PARAM bandparam = { 0 };
     IDirectMusicTrack8 *dmt8;
+    IDirectMusicBand *dmb[3];
     IPersistStream *ps;
     CLSID class;
+    MUSIC_TIME mt;
     ULARGE_INTEGER size;
     char buf[64] = { 0 };
     HRESULT hr;
@@ -266,6 +269,81 @@ static void test_bandtrack(void)
         }
     }
 
+    for (i = 0; i < 3; i++)
+    {
+        hr = CoCreateInstance(&CLSID_DirectMusicBand, NULL, CLSCTX_INPROC_SERVER,
+                &IID_IDirectMusicBand, (void **)&dmb[i]);
+        ok(hr == S_OK, "DirectMusicBand create failed: %#lx, expected S_OK\n", hr);
+    }
+
+    hr = IDirectMusicTrack8_GetParam(dmt8, &GUID_BandParam, 0, &mt, &bandparam);
+    ok(hr == DMUS_E_NOT_FOUND, "hr %#lx, expected not found\n", hr);
+    ok(bandparam.pBand == NULL, "Got band %p, expected NULL\n", bandparam.pBand);
+
+    bandparam.mtTimePhysical = 10;
+    bandparam.pBand = dmb[0];
+    hr = IDirectMusicTrack8_SetParam(dmt8, &GUID_BandParam, 10, &bandparam);
+    ok(hr == S_OK, "SetParam failed: %#lx, expected S_OK\n", hr);
+
+    hr = IDirectMusicTrack8_GetParam(dmt8, &GUID_BandParam, 10, &mt, &bandparam);
+    ok(hr == S_OK, "GetParam failed: %#lx, expected S_OK\n", hr);
+    ok(mt == 0, "Got time %lu, expected 0\n", mt);
+    ok(bandparam.mtTimePhysical == 10, "Got physical time %lu, expected 10\n", bandparam.mtTimePhysical);
+    IDirectMusicBand_Release(bandparam.pBand);
+    bandparam.pBand = NULL;
+    bandparam.mtTimePhysical = 0;
+
+    /* get with time before the first band. */
+    hr = IDirectMusicTrack8_GetParam(dmt8, &GUID_BandParam, 1, &mt, &bandparam);
+    ok(hr == S_OK, "GetParam failed: %#lx, expected S_OK\n", hr);
+    ok(mt == 0, "Got time %lu, expected 0\n", mt);
+    ok(bandparam.mtTimePhysical == 10, "Got physical time %lu, expected 10\n", bandparam.mtTimePhysical);
+    IDirectMusicBand_Release(bandparam.pBand);
+    bandparam.pBand = NULL;
+    bandparam.mtTimePhysical = 0;
+
+    bandparam.mtTimePhysical = 100;
+    bandparam.pBand = dmb[1];
+    hr = IDirectMusicTrack8_SetParam(dmt8, &GUID_BandParam, 100, &bandparam);
+    ok(hr == S_OK, "SetParam failed: %#lx, expected S_OK\n", hr);
+
+    hr = IDirectMusicTrack8_GetParam(dmt8, &GUID_BandParam, 10, &mt, &bandparam);
+    ok(hr == S_OK, "GetParam failed: %#lx, expected S_OK\n", hr);
+    ok(mt == 100, "Got time %lu, expected 100\n", mt);
+    ok(bandparam.mtTimePhysical == 10, "Got physical time %lu, expected 10\n", bandparam.mtTimePhysical);
+    IDirectMusicBand_Release(bandparam.pBand);
+    bandparam.pBand = NULL;
+    bandparam.mtTimePhysical = 0;
+
+    hr = IDirectMusicTrack8_GetParam(dmt8, &GUID_BandParam, 20, &mt, &bandparam);
+    ok(hr == S_OK, "GetParam failed: %#lx, expected S_OK\n", hr);
+    ok(mt == 100, "Got time %lu, expected 100\n", mt);
+    ok(bandparam.mtTimePhysical == 10, "Got physical time %lu, expected 10\n", bandparam.mtTimePhysical);
+    IDirectMusicBand_Release(bandparam.pBand);
+    bandparam.pBand = NULL;
+    bandparam.mtTimePhysical = 0;
+
+    /* time return by GetParam is actually not a relative time. */
+    bandparam.mtTimePhysical = 250;
+    bandparam.pBand = dmb[2];
+    hr = IDirectMusicTrack8_SetParam(dmt8, &GUID_BandParam, 250, &bandparam);
+    ok(hr == S_OK, "SetParam failed: %#lx, expected S_OK\n", hr);
+
+    hr = IDirectMusicTrack8_GetParam(dmt8, &GUID_BandParam, 120, &mt, &bandparam);
+    ok(hr == S_OK, "GetParam failed: %#lx, expected S_OK\n", hr);
+    ok(mt == 250, "Got time %lu, expected 250\n", mt);
+    ok(bandparam.mtTimePhysical == 100, "Got physical time %lu, expected 0\n", bandparam.mtTimePhysical);
+    IDirectMusicBand_Release(bandparam.pBand);
+    bandparam.pBand = NULL;
+    bandparam.mtTimePhysical = 0;
+
+    for (i = 0; i < 3; i++)
+        IDirectMusicBand_Release(dmb[i]);
+
+    bandparam.pBand = NULL;
+    hr = IDirectMusicTrack8_SetParam(dmt8, &GUID_BandParam, 0, &bandparam);
+    ok(hr == E_POINTER, "hr %#lx, expected E_POINTER\n", hr);
+
     hr = IDirectMusicTrack8_AddNotificationType(dmt8, NULL);
     ok(hr == E_NOTIMPL, "IDirectMusicTrack8_AddNotificationType failed: %#lx\n", hr);
     hr = IDirectMusicTrack8_RemoveNotificationType(dmt8, NULL);
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/5040

[Yuxuan Shui (＠yshui)]

hopefully this is enough testing? @rbernon

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5040#note_60707

[PATCH v2 0/1] MR5040: dmband: Implement getting/setting GUID_BandParam on band tracks. - approved

[Rémi Bernon]

This merge request was approved by Rémi Bernon.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5040

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/dmband/bandtrack.c:
> +    TRACE("(%p, %s, %ld, %p, %p)\n", This, debugstr_dmguid(type), time, out_next, param);
>
> if (!type)
>          return E_POINTER;
>      if (!IsEqualGUID(type, &GUID_BandParam))
>          return DMUS_E_GET_UNSUPPORTED;
> +    if (list_empty(&This->bands))
> +        return DMUS_E_NOT_FOUND;
>
> -    FIXME("GUID_BandParam not handled yet\n");
> +    bandparam = param;
> +    if (out_next) *out_next = 0;
>
> +    LIST_FOR_EACH_ENTRY_SAFE(band, next_band, &This->bands, struct band_entry, entry)
> +    {
> +        /* we want to return the first band even when there's nothing with lBandTime <= time*/
        /* we want to return the first band even when there's nothing with lBandTime &lt;= time */

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5040#note_60787

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/dmband/bandtrack.c:
> if (IsEqualGUID(type, &GUID_BandParam))
> -        FIXME("GUID_BandParam not handled yet\n");
> +    {
> +        struct band_entry *new_entry = NULL, *entry, *next_entry;
> +        DMUS_BAND_PARAM *band_param = param;
> +        if (!band_param || !band_param->pBand)
> +            return E_POINTER;
> +        if (!(new_entry = calloc(1, sizeof(*new_entry))))
> +            return E_OUTOFMEMORY;
> +        new_entry->band = band_param->pBand;
> +        new_entry->head.lBandTimeLogical = time;
> +        new_entry->head.lBandTimePhysical = band_param->mtTimePhysical;
> +        IDirectMusicBand_AddRef(new_entry->band);
> +        if (list_empty(&This->bands))
> +                list_add_tail(&This->bands, &new_entry->entry);
> +        else

```suggestion:-2+0
        if (list_empty(&This->bands))
            list_add_tail(&This->bands, &new_entry->entry);
        else
```

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5040#note_60788

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/dmband/bandtrack.c:
> +        DMUS_BAND_PARAM *band_param = param;
> +        if (!band_param || !band_param->pBand)
> +            return E_POINTER;
> +        if (!(new_entry = calloc(1, sizeof(*new_entry))))
> +            return E_OUTOFMEMORY;
> +        new_entry->band = band_param->pBand;
> +        new_entry->head.lBandTimeLogical = time;
> +        new_entry->head.lBandTimePhysical = band_param->mtTimePhysical;
> +        IDirectMusicBand_AddRef(new_entry->band);
> +        if (list_empty(&This->bands))
> +                list_add_tail(&This->bands, &new_entry->entry);
> +        else
> +        {
> +            LIST_FOR_EACH_ENTRY_SAFE(entry, next_entry, &This->bands, struct band_entry, entry)
> +                if (entry->entry.next == &This->bands || next_entry->head.lBandTimeLogical > time)
> +                    list_add_after(&entry->entry, &new_entry->entry);
> I think you'll need to break out of the loop here?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5040#note_60789

[Rémi Bernon]

Actually approved too quickly, it looks good and thanks for the tests, then there's a couple of nit and probably some missing break?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5040#note_60790

[Yuxuan Shui (＠yshui)]

On Sat Feb 10 10:24:37 2024 +0000, Rémi Bernon wrote:
> I think you'll need to break out of the loop here?
> i am so sorry :face_palm:

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5040#note_60795