[PATCH 0/3] MR5190: dmime: MIDI tempotrack

[Yuxuan Shui (＠yshui)]

Parse MIDI meta set tempo events, and create a tempotrack for it.

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

[PATCH 1/3] dmime: Use linked list for tempotrack.

[Yuxuan Shui]

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

Preparation for supporting setting TempoParam on tempotracks.
---
 dlls/dmime/tempotrack.c | 74 +++++++++++++++++++++++++++--------------
 1 file changed, 49 insertions(+), 25 deletions(-)

diff --git a/dlls/dmime/tempotrack.c b/dlls/dmime/tempotrack.c
index 028ebc8879c..1f338b8f40d 100644
--- a/dlls/dmime/tempotrack.c
+++ b/dlls/dmime/tempotrack.c
@@ -27,12 +27,16 @@ WINE_DECLARE_DEBUG_CHANNEL(dmfile);
  * IDirectMusicTempoTrack implementation
  */
 
+struct tempo_entry {
+    struct list entry;
+    DMUS_IO_TEMPO_ITEM item;
+};
+
 typedef struct IDirectMusicTempoTrack {
     IDirectMusicTrack8 IDirectMusicTrack8_iface;
     struct dmobject dmobj;  /* IPersistStream only */
     LONG ref;
-    DMUS_IO_TEMPO_ITEM *items;
-    unsigned int count;
+    struct list items;
 } IDirectMusicTempoTrack;
 
 /* IDirectMusicTempoTrack IDirectMusicTrack8 part: */
@@ -82,7 +86,11 @@ static ULONG WINAPI tempo_track_Release(IDirectMusicTrack8 *iface)
     TRACE("(%p) ref=%ld\n", This, ref);
 
     if (!ref) {
-        free(This->items);
+        struct tempo_entry *item, *next_item;
+        LIST_FOR_EACH_ENTRY_SAFE(item, next_item, &This->items, struct tempo_entry, entry) {
+            list_remove(&item->entry);
+            free(item);
+        }
         free(This);
     }
 
@@ -141,31 +149,34 @@ static HRESULT WINAPI tempo_track_Play(IDirectMusicTrack8 *iface, void *pStateDa
 }
 
 static HRESULT WINAPI tempo_track_GetParam(IDirectMusicTrack8 *iface, REFGUID type, MUSIC_TIME music_time,
-        MUSIC_TIME *next_time, void *param)
+        MUSIC_TIME *out_next_time, void *param)
 {
     IDirectMusicTempoTrack *This = impl_from_IDirectMusicTrack8(iface);
-    DMUS_IO_TEMPO_ITEM *item = This->items, *end = item + This->count;
+    struct tempo_entry *item, *next_item;
     DMUS_TEMPO_PARAM *tempo = param;
+    MUSIC_TIME next_time = 0;
 
-    TRACE("(%p, %s, %ld, %p, %p)\n", This, debugstr_dmguid(type), music_time, next_time, param);
+    TRACE("(%p, %s, %ld, %p, %p)\n", This, debugstr_dmguid(type), music_time, out_next_time, param);
 
     if (!param) return E_POINTER;
     if (!IsEqualGUID(type, &GUID_TempoParam)) return DMUS_E_GET_UNSUPPORTED;
-    if (item == end) return DMUS_E_NOT_FOUND;
-
-    tempo->mtTime = item->lTime - music_time;
-    tempo->dblTempo = item->dblTempo;
-
-    for (; item < end; item++)
+    if (list_empty(&This->items)) return DMUS_E_NOT_FOUND;
+    
+    /* if music_time is before the first item, we return the first item. */
+    LIST_FOR_EACH_ENTRY_SAFE(item, next_item, &This->items, struct tempo_entry, entry)
     {
-        MUSIC_TIME time = item->lTime - music_time;
-        if (next_time) *next_time = item->lTime - music_time;
-        if (time > 0) break;
-        tempo->mtTime = time;
-        tempo->dblTempo = item->dblTempo;
+        tempo->mtTime = item->item.lTime - music_time;
+        tempo->dblTempo = item->item.dblTempo;
+
+        next_time = tempo->mtTime;
+        if (next_time > 0) break;
+        next_time = 0;
+        if (item->entry.next == &This->items) break;
+        next_time = next_item->item.lTime - music_time;
+        if (next_time > 0) break;
     }
 
-    if (next_time && item == end) *next_time = 0;
+    if (out_next_time) *out_next_time = next_time;
     return S_OK;
 }
 
@@ -315,8 +326,10 @@ static HRESULT WINAPI tempo_IPersistStream_Load(IPersistStream *iface, IStream *
 {
     IDirectMusicTempoTrack *This = impl_from_IPersistStream(iface);
     struct chunk_entry chunk = {0};
-    unsigned int i;
+    DMUS_IO_TEMPO_ITEM *items;
+    ULONG i = 0;
     HRESULT hr;
+    UINT count;
 
     TRACE("%p, %p\n", This, stream);
 
@@ -328,18 +341,28 @@ static HRESULT WINAPI tempo_IPersistStream_Load(IPersistStream *iface, IStream *
     if (chunk.id != DMUS_FOURCC_TEMPO_TRACK)
         return DMUS_E_UNSUPPORTED_STREAM;
 
-    hr = stream_chunk_get_array(stream, &chunk, (void **)&This->items, &This->count,
+    hr = stream_chunk_get_array(stream, &chunk, (void **)&items, &count,
             sizeof(DMUS_IO_TEMPO_ITEM));
     if (FAILED(hr))
         return hr;
 
-    for (i = 0; i < This->count; i++) {
-        TRACE_(dmfile)("DMUS_IO_TEMPO_ITEM #%u\n", i);
-        TRACE_(dmfile)(" - lTime = %lu\n", This->items[i].lTime);
-        TRACE_(dmfile)(" - dblTempo = %g\n", This->items[i].dblTempo);
+    for (i = 0; i < count; i++)
+    {
+        struct tempo_entry *entry = calloc(1, sizeof(*entry));
+        if (!entry) hr = E_OUTOFMEMORY;
+        else
+        {
+            entry->item = items[i];
+            list_add_tail(&This->items, &entry->entry);
+        }
+
+        TRACE_(dmfile)("DMUS_IO_TEMPO_ITEM #%lu\n", i);
+        TRACE_(dmfile)(" - lTime = %lu\n", items[i].lTime);
+        TRACE_(dmfile)(" - dblTempo = %g\n", items[i].dblTempo);
     }
 
-    return S_OK;
+    free(items);
+    return hr;
 }
 
 static const IPersistStreamVtbl persiststream_vtbl = {
@@ -366,6 +389,7 @@ HRESULT create_dmtempotrack(REFIID lpcGUID, void **ppobj)
     dmobject_init(&track->dmobj, &CLSID_DirectMusicTempoTrack,
                   (IUnknown *)&track->IDirectMusicTrack8_iface);
     track->dmobj.IPersistStream_iface.lpVtbl = &persiststream_vtbl;
+    list_init(&track->items);
 
     hr = IDirectMusicTrack8_QueryInterface(&track->IDirectMusicTrack8_iface, lpcGUID, ppobj);
     IDirectMusicTrack8_Release(&track->IDirectMusicTrack8_iface);
-- 
GitLab


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

[PATCH 2/3] dmime: Implement setting TempoParam for tempotracks.

[Yuxuan Shui]

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

And add some tests.
---
 dlls/dmime/tempotrack.c  | 19 +++++++++++++++-
 dlls/dmime/tests/dmime.c | 49 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/dlls/dmime/tempotrack.c b/dlls/dmime/tempotrack.c
index 1f338b8f40d..d44b5f4b3e6 100644
--- a/dlls/dmime/tempotrack.c
+++ b/dlls/dmime/tempotrack.c
@@ -200,9 +200,26 @@ static HRESULT WINAPI tempo_track_SetParam(IDirectMusicTrack8 *iface, REFGUID ty
         return S_OK;
     }
     if (IsEqualGUID(type, &GUID_TempoParam)) {
+        struct tempo_entry *item, *next_item;
+        DMUS_TEMPO_PARAM *tempo_param = param;
+        struct tempo_entry *entry;
         if (!param)
             return E_POINTER;
-        FIXME("GUID_TempoParam not handled yet\n");
+        if (!(entry = calloc(1, sizeof(*entry))))
+            return E_OUTOFMEMORY;
+        entry->item.lTime = time;
+        entry->item.dblTempo = tempo_param->dblTempo;
+        if (list_empty(&This->items))
+            list_add_tail(&This->items, &entry->entry);
+        else
+        {
+            LIST_FOR_EACH_ENTRY_SAFE(item, next_item, &This->items, struct tempo_entry, entry)
+                if (item->entry.next == &This->items || next_item->item.lTime > time)
+                {
+                    list_add_after(&item->entry, &entry->entry);
+                    break;
+                }
+        }
         return S_OK;
     }
 
diff --git a/dlls/dmime/tests/dmime.c b/dlls/dmime/tests/dmime.c
index 97a6088936e..69d8cf09605 100644
--- a/dlls/dmime/tests/dmime.c
+++ b/dlls/dmime/tests/dmime.c
@@ -4456,6 +4456,54 @@ static void check_dmus_tempo_pmsg_(int line, DMUS_TEMPO_PMSG *msg, MUSIC_TIME ti
     ok_(__FILE__, line)(msg->dblTempo == tempo, "got tempo %f\n", msg->dblTempo);
 }
 
+static void test_tempo_track(void)
+{
+    HRESULT hr;
+    IDirectMusicTrack *track;
+    DMUS_TEMPO_PARAM param;
+    MUSIC_TIME next;
+    hr = CoCreateInstance(&CLSID_DirectMusicTempoTrack, NULL, CLSCTX_INPROC_SERVER,
+            &IID_IDirectMusicTrack, (void **)&track);
+    ok(hr == S_OK, "got %#lx\n", hr);
+
+    hr = IDirectMusicTrack_GetParam(track, &GUID_TempoParam, 0, &next, ¶m);
+    ok(hr == DMUS_E_NOT_FOUND, "got %#lx\n", hr);
+
+    param.dblTempo = 150;
+    param.mtTime = 10;
+    hr = IDirectMusicTrack_SetParam(track, &GUID_TempoParam, 10, ¶m);
+    ok(hr == S_OK, "got %#lx\n", hr);
+
+    hr = IDirectMusicTrack_GetParam(track, &GUID_TempoParam, 0, &next, ¶m);
+    ok(hr == S_OK, "got %#lx\n", hr);
+    ok(param.dblTempo == 150, "got %f, expected 150\n", param.dblTempo);
+    ok(param.mtTime == 10, "got %lu, expected 10\n", param.mtTime);
+    ok(next == 10, "got %lu, expected 10\n", next);
+
+    hr = IDirectMusicTrack_GetParam(track, &GUID_TempoParam, 10, &next, ¶m);
+    ok(hr == S_OK, "got %#lx\n", hr);
+    ok(param.dblTempo == 150, "got %f, expected 150\n", param.dblTempo);
+    ok(param.mtTime == 0, "got %lu, expected 0\n", param.mtTime);
+    ok(next == 0, "got %lu, expected 0\n", next);
+
+    hr = IDirectMusicTrack_GetParam(track, &GUID_TempoParam, 11, &next, ¶m);
+    ok(hr == S_OK, "got %#lx\n", hr);
+    ok(param.dblTempo == 150, "got %f, expected 150\n", param.dblTempo);
+    ok(param.mtTime == -1, "got %lu, expected 0\n", param.mtTime);
+    ok(next == 0, "got %lu, expected 0\n", next);
+
+    param.dblTempo = 180;
+    param.mtTime = 20;
+    hr = IDirectMusicTrack_SetParam(track, &GUID_TempoParam, 20, ¶m);
+    ok(hr == S_OK, "got %#lx\n", hr);
+
+    hr = IDirectMusicTrack_GetParam(track, &GUID_TempoParam, 11, &next, ¶m);
+    ok(hr == S_OK, "got %#lx\n", hr);
+    ok(param.dblTempo == 150, "got %f, expected 150\n", param.dblTempo);
+    ok(param.mtTime == -1, "got %ld, expected -1\n", param.mtTime);
+    ok(next == 9, "got %lu, expected 9\n", next);
+}
+
 static void test_tempo_track_play(void)
 {
     static const DWORD message_types[] =
@@ -5045,6 +5093,7 @@ START_TEST(dmime)
     test_wave_pmsg(0);
     test_wave_pmsg(10);
     test_sequence_track();
+    test_tempo_track();
     test_band_track_play();
     test_tempo_track_play();
     test_connect_to_collection();
-- 
GitLab


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

[PATCH 3/3] dmime: Parse MIDI Set Tempo meta events and generate a tempotrack.

[Yuxuan Shui]

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

---
 dlls/dmime/midi.c        | 80 ++++++++++++++++++++++++++++++++++++----
 dlls/dmime/tests/dmime.c | 18 ++++++++-
 2 files changed, 88 insertions(+), 10 deletions(-)

diff --git a/dlls/dmime/midi.c b/dlls/dmime/midi.c
index 647ef1d123c..b509394927e 100644
--- a/dlls/dmime/midi.c
+++ b/dlls/dmime/midi.c
@@ -35,18 +35,49 @@ struct midi_event
 {
     MUSIC_TIME delta_time;
     BYTE status;
-    BYTE data[2];
+    union
+    {
+        struct
+        {
+            BYTE data[2];
+        };
+        struct
+        {
+            BYTE meta_type;
+            ULONG tempo;
+        };
+    };
 };
 
 struct midi_parser
 {
     IDirectMusicTrack *chordtrack;
     IDirectMusicTrack *bandtrack;
+    IDirectMusicTrack *tempotrack;
     MUSIC_TIME time;
     IStream *stream;
     DWORD division;
 };
 
+enum meta_event_type
+{
+    MIDI_META_SEQUENCE_NUMBER = 0x00,
+    MIDI_META_TEXT_EVENT = 0x01,
+    MIDI_META_COPYRIGHT_NOTICE = 0x02,
+    MIDI_META_TRACK_NAME = 0x03,
+    MIDI_META_INSTRUMENT_NAME = 0x04,
+    MIDI_META_LYRIC = 0x05,
+    MIDI_META_MARKER = 0x06,
+    MIDI_META_CUE_POINT = 0x07,
+    MIDI_META_CHANNEL_PREFIX_ASSIGNMENT = 0x20,
+    MIDI_META_END_OF_TRACK = 0x2f,
+    MIDI_META_SET_TEMPO = 0x51,
+    MIDI_META_SMPTE_OFFSET = 0x54,
+    MIDI_META_TIME_SIGNATURE = 0x58,
+    MIDI_META_KEY_SIGNATURE = 0x59,
+    MIDI_META_SEQUENCER_SPECIFIC = 0x7f,
+};
+
 static HRESULT stream_read_at_most(IStream *stream, void *buffer, ULONG size, ULONG *bytes_left)
 {
     HRESULT hr;
@@ -76,8 +107,9 @@ static HRESULT read_variable_length_number(IStream *stream, DWORD *out, ULONG *b
 
 static HRESULT read_midi_event(IStream *stream, struct midi_event *event, BYTE *last_status, ULONG *bytes_left)
 {
-    BYTE byte, status_type, meta_type;
+    BYTE byte, status_type;
     DWORD length;
+    BYTE data[3];
     LARGE_INTEGER offset;
     HRESULT hr = S_OK;
     DWORD delta_time;
@@ -96,21 +128,31 @@ static HRESULT read_midi_event(IStream *stream, struct midi_event *event, BYTE *
 
     if (event->status == MIDI_META)
     {
-        meta_type = byte;
+        event->meta_type = byte;
 
         if ((hr = read_variable_length_number(stream, &length, bytes_left)) != S_OK) return hr;
 
-        switch (meta_type)
+        switch (event->meta_type)
         {
+        case MIDI_META_SET_TEMPO:
+            if (length != 3)
+            {
+                ERR("Invalid MIDI meta event length %lu for set tempo event.\n", length);
+                return E_FAIL;
+            }
+            if (FAILED(hr = stream_read_at_most(stream, data, 3, bytes_left))) return hr;
+            event->tempo = (data[0] << 16) | (data[1] << 8) | data[2];
+            break;
         default:
             if (*bytes_left < length) return S_FALSE;
             offset.QuadPart = length;
             if (FAILED(hr = IStream_Seek(stream, offset, STREAM_SEEK_CUR, NULL))) return hr;
-            FIXME("MIDI meta event type %#02x, length %lu, time +%lu. not supported\n", meta_type,
-                    length, delta_time);
+            FIXME("MIDI meta event type %#02x, length %lu, time +%lu. not supported\n",
+                    event->meta_type, length, delta_time);
             *bytes_left -= length;
+            event->tempo = 0;
         }
-        TRACE("MIDI meta event type %#02x, length %lu, time +%lu\n", meta_type, length, delta_time);
+        TRACE("MIDI meta event type %#02x, length %lu, time +%lu\n", event->meta_type, length, delta_time);
         return S_OK;
     }
     else if (event->status == MIDI_SYSEX1 || event->status == MIDI_SYSEX2)
@@ -148,6 +190,23 @@ static HRESULT read_midi_event(IStream *stream, struct midi_event *event, BYTE *
     return S_OK;
 }
 
+static HRESULT midi_parser_handle_set_tempo(struct midi_parser *parser, struct midi_event *event)
+{
+    DMUS_TEMPO_PARAM tempo;
+    MUSIC_TIME dmusic_time = (ULONGLONG)parser->time * DMUS_PPQ / parser->division;
+    HRESULT hr;
+    if (!parser->tempotrack)
+    {
+        if (FAILED(hr = CoCreateInstance(&CLSID_DirectMusicTempoTrack, NULL, CLSCTX_INPROC_SERVER,
+                           &IID_IDirectMusicTrack, (void **)&parser->tempotrack)))
+            return hr;
+    }
+    tempo.mtTime = dmusic_time;
+    tempo.dblTempo = 60 * 1000000.0 / event->tempo;
+    TRACE("Adding tempo at time %lu, tempo %f\n", dmusic_time, tempo.dblTempo);
+    return IDirectMusicTrack_SetParam(parser->tempotrack, &GUID_TempoParam, dmusic_time, &tempo);
+}
+
 static HRESULT midi_parser_handle_program_change(struct midi_parser *parser, struct midi_event *event)
 {
     HRESULT hr;
@@ -206,7 +265,9 @@ static HRESULT midi_parser_parse(struct midi_parser *parser, IDirectMusicSegment
         while ((hr = read_midi_event(parser->stream, &event, &last_status, &length)) == S_OK)
         {
             parser->time += event.delta_time;
-            if ((event.status & 0xf0) == MIDI_PROGRAM_CHANGE)
+            if (event.status == 0xff && event.meta_type == MIDI_META_SET_TEMPO)
+                hr = midi_parser_handle_set_tempo(parser, &event);
+            else if ((event.status & 0xf0) == MIDI_PROGRAM_CHANGE)
                 hr = midi_parser_handle_program_change(parser, &event);
             if (FAILED(hr)) break;
         }
@@ -223,6 +284,8 @@ static HRESULT midi_parser_parse(struct midi_parser *parser, IDirectMusicSegment
     if (SUCCEEDED(hr)) hr = IDirectMusicSegment8_SetLength(segment, music_length);
     if (SUCCEEDED(hr)) hr = IDirectMusicSegment8_InsertTrack(segment, parser->bandtrack, 0xffff);
     if (SUCCEEDED(hr)) hr = IDirectMusicSegment8_InsertTrack(segment, parser->chordtrack, 0xffff);
+    if (SUCCEEDED(hr) && parser->tempotrack)
+        hr = IDirectMusicSegment8_InsertTrack(segment, parser->tempotrack, 0xffff);
 
     return hr;
 }
@@ -232,6 +295,7 @@ static void midi_parser_destroy(struct midi_parser *parser)
     IStream_Release(parser->stream);
     if (parser->bandtrack) IDirectMusicTrack_Release(parser->bandtrack);
     if (parser->chordtrack) IDirectMusicTrack_Release(parser->chordtrack);
+    if (parser->tempotrack) IDirectMusicTrack_Release(parser->tempotrack);
     free(parser);
 }
 
diff --git a/dlls/dmime/tests/dmime.c b/dlls/dmime/tests/dmime.c
index 69d8cf09605..87f89b04e4b 100644
--- a/dlls/dmime/tests/dmime.c
+++ b/dlls/dmime/tests/dmime.c
@@ -1646,8 +1646,10 @@ static void test_midi(void)
     WCHAR test_mid[MAX_PATH], bogus_mid[MAX_PATH];
     HRESULT hr;
     ULONG ret;
+    MUSIC_TIME next;
     DMUS_PMSG *msg;
     DMUS_PATCH_PMSG *patch;
+    DMUS_TEMPO_PARAM tempo_param;
 #include <pshpack1.h>
     struct
     {
@@ -1682,7 +1684,7 @@ static void test_midi(void)
 
     expect_track(segment, BandTrack, -1, 0);
     expect_track(segment, ChordTrack, -1, 1);
-    todo_wine expect_track(segment, TempoTrack, -1, 2);
+    expect_track(segment, TempoTrack, -1, 2);
     todo_wine expect_track(segment, TimeSigTrack, -1, 3);
     todo_wine expect_track(segment, SeqTrack, -1, 4);
     /* no more tracks */
@@ -1760,8 +1762,20 @@ static void test_midi(void)
     IStream_Release(stream);
     expect_track(segment, BandTrack, -1, 0);
     expect_track(segment, ChordTrack, -1, 1);
-    todo_wine expect_track(segment, TempoTrack, -1, 2);
+    expect_track(segment, TempoTrack, -1, 2);
     todo_wine expect_track(segment, SeqTrack, -1, 3);
+
+    hr = IDirectMusicSegment_GetParam(segment, &GUID_TempoParam, -1, DMUS_SEG_ALLTRACKS, 0, &next, &tempo_param);
+    ok(hr == S_OK, "got %#lx\n", hr);
+    ok(next == 24, "got %ld, expected 24\n", next);
+    ok(tempo_param.mtTime == 24, "got %ld, expected 24\n", tempo_param.mtTime);
+    ok(tempo_param.dblTempo == 300.0, "got %f, expected 300.0\n", tempo_param.dblTempo);
+
+    hr = IDirectMusicSegment_GetParam(segment, &GUID_TempoParam, -1, DMUS_SEG_ALLTRACKS, 26, &next, &tempo_param);
+    ok(hr == S_OK, "got %#lx\n", hr);
+    ok(next == 0, "got %ld, expected 24\n", next);
+    ok(tempo_param.mtTime == -2, "got %ld, expected -6\n", tempo_param.mtTime);
+    ok(tempo_param.dblTempo == 300.0, "got %f, expected 300.0\n", tempo_param.dblTempo);
     IDirectMusicSegment_Release(segment);
 
     /* parse MIDI file with a track with 0 length, but has an event. */
-- 
GitLab

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

[PATCH 3/3] dmime: Parse MIDI Set Tempo meta events and generate a tempotrack.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=143542

Your paranoid android.


=== build (build log) ===

error: patch failed: dlls/dmime/midi.c:35
error: patch failed: dlls/dmime/tests/dmime.c:1646
Task: Patch failed to apply

=== debian11 (build log) ===

error: patch failed: dlls/dmime/midi.c:35
error: patch failed: dlls/dmime/tests/dmime.c:1646
Task: Patch failed to apply

=== debian11b (build log) ===

error: patch failed: dlls/dmime/midi.c:35
error: patch failed: dlls/dmime/tests/dmime.c:1646
Task: Patch failed to apply

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/dmime/tempotrack.c:
> TRACE("(%p) ref=%ld\n", This, ref);
>
> if (!ref) {
> -        free(This->items);
> +        struct tempo_entry *item, *next_item;
> +        LIST_FOR_EACH_ENTRY_SAFE(item, next_item, &This->items, struct tempo_entry, entry) {

```suggestion:-3+0

    if (!ref)
    {
        struct tempo_entry *item, *next_item;

        LIST_FOR_EACH_ENTRY_SAFE(item, next_item, &This->items, struct tempo_entry, entry)
        {
```

Lets fix those braces instead.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5190#note_63103

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/dmime/tempotrack.c:
> * IDirectMusicTempoTrack implementation
>   */
>
> +struct tempo_entry {
struct tempo_entry
{

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5190#note_63102

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/dmime/midi.c:
> +static HRESULT midi_parser_handle_set_tempo(struct midi_parser *parser, struct midi_event *event)
> +{
> +    DMUS_TEMPO_PARAM tempo;
> +    MUSIC_TIME dmusic_time = (ULONGLONG)parser->time * DMUS_PPQ / parser->division;
> +    HRESULT hr;
> +    if (!parser->tempotrack)
> +    {
> +        if (FAILED(hr = CoCreateInstance(&CLSID_DirectMusicTempoTrack, NULL, CLSCTX_INPROC_SERVER,
> +                           &IID_IDirectMusicTrack, (void **)&parser->tempotrack)))
> +            return hr;
> +    }
> +    tempo.mtTime = dmusic_time;
> +    tempo.dblTempo = 60 * 1000000.0 / event->tempo;
> +    TRACE("Adding tempo at time %lu, tempo %f\n", dmusic_time, tempo.dblTempo);
> +    return IDirectMusicTrack_SetParam(parser->tempotrack, &GUID_TempoParam, dmusic_time, &tempo);

I would say the same here for instance, adding blank lines here and there helps making things more readable.

```suggestion:-12+0
    DMUS_TEMPO_PARAM tempo;
    MUSIC_TIME dmusic_time = (ULONGLONG)parser->time * DMUS_PPQ / parser->division;
    HRESULT hr;

    if (!parser->tempotrack)
    {
        if (FAILED(hr = CoCreateInstance(&CLSID_DirectMusicTempoTrack, NULL, CLSCTX_INPROC_SERVER,
                           &IID_IDirectMusicTrack, (void **)&parser->tempotrack)))
            return hr;
    }

    tempo.mtTime = dmusic_time;
    tempo.dblTempo = 60 * 1000000.0 / event->tempo;
    TRACE("Adding tempo at time %lu, tempo %f\n", dmusic_time, tempo.dblTempo);
    return IDirectMusicTrack_SetParam(parser->tempotrack, &GUID_TempoParam, dmusic_time, &tempo);
```

IMO you could also combine the two ifs.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5190#note_63104

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/dmime/tempotrack.c:
> return DMUS_E_UNSUPPORTED_STREAM;
>
> -    hr = stream_chunk_get_array(stream, &chunk, (void **)&This->items, &This->count,
> +    hr = stream_chunk_get_array(stream, &chunk, (void **)&items, &count,
>              sizeof(DMUS_IO_TEMPO_ITEM));
>      if (FAILED(hr))
>          return hr;
>
> -    for (i = 0; i < This->count; i++) {
> -        TRACE_(dmfile)("DMUS_IO_TEMPO_ITEM #%u\n", i);
> -        TRACE_(dmfile)(" - lTime = %lu\n", This->items[i].lTime);
> -        TRACE_(dmfile)(" - dblTempo = %g\n", This->items[i].dblTempo);
> +    for (i = 0; i < count; i++)
> +    {
> +        struct tempo_entry *entry = calloc(1, sizeof(*entry));
> +        if (!entry) hr = E_OUTOFMEMORY;

You should break probably here, or add SUCCEEDED(hr) to the loop conditions.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5190#note_63105

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/dmime/tests/dmime.c:
> +    ok(hr == S_OK, "got %#lx\n", hr);
> +    ok(param.dblTempo == 150, "got %f, expected 150\n", param.dblTempo);
> +    ok(param.mtTime == -1, "got %lu, expected 0\n", param.mtTime);
> +    ok(next == 0, "got %lu, expected 0\n", next);
> +
> +    param.dblTempo = 180;
> +    param.mtTime = 20;
> +    hr = IDirectMusicTrack_SetParam(track, &GUID_TempoParam, 20, ¶m);
> +    ok(hr == S_OK, "got %#lx\n", hr);
> +
> +    hr = IDirectMusicTrack_GetParam(track, &GUID_TempoParam, 11, &next, ¶m);
> +    ok(hr == S_OK, "got %#lx\n", hr);
> +    ok(param.dblTempo == 150, "got %f, expected 150\n", param.dblTempo);
> +    ok(param.mtTime == -1, "got %ld, expected -1\n", param.mtTime);
> +    ok(next == 9, "got %lu, expected 9\n", next);
> +}
> You're leaking the track here.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5190#note_63106