Re: [PATCH v9 0/10] MR4982: Load and play MIDI

[Rémi Bernon (＠rbernon)]

Rémi Bernon (@rbernon) commented about dlls/dmime/midi.c:
> +{
> - struct midi_sequence_track *This = impl_from_IDirectMusicTrack8(iface);
> - FIXME("(%p, %p): stub\n", This, pStateData);
> - return S_OK;
>
> +}
> -
>
> +static HRESULT WINAPI midi_sequence_track_Play(IDirectMusicTrack8 *iface, void *state_data,
> -
>    MUSIC_TIME start_time, MUSIC_TIME end_time, MUSIC_TIME time_offset, DWORD segment_flags,
> -
>    IDirectMusicPerformance *performance, IDirectMusicSegmentState *segment_state, DWORD track_id)
>
> +{
> - struct midi_sequence_track *This = impl_from_IDirectMusicTrack8(iface);
> -
> - TRACE("(%p, %p, %ld, %ld, %ld, %#lx, %p, %p, %ld)\n", This, state_data, start_time, end_time,
> -
>        time_offset, segment_flags, performance, segment_state, track_id);
> -
> - return E_NOTIMPL;

Unless they are tested and supposed to return E_NOTIMPL, functions which are unimplemented and return E_NOTIMPL should generally have a FIXME message instead of a TRACE. Same thing below.

For functions which are called too often, a `static int once` flag can be used to only print the message once.

Same comment for several functions below here.

(And I see where this comes from, but dmusic has a lot of old code, and doesn't follow recent Wine code best practices.)