[PATCH 2/5] winmm: Preserve fractions in microseconds to compensate rounding errors.

[Akihiro Sagawa]

Otherwise, a song is played faster than expected in some cases.

For instance, we assume that 40 pan control change (CC#10) messages are
expressed in 12.5ms each,
 * Prior art omits fractions, i.e. 12ms + 12ms + ... = 480ms.
 * Proposal technique saves fractions, i.e. 12ms + 13ms + ... = 500ms.
Ideal duration is 500ms. So, the proposed method is better than prior one.

Signed-off-by: Akihiro Sagawa <sagawa.aki(a)gmail.com>
---
 dlls/winmm/winmm.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

[Andrew Eikum]

Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com>

On Tue, Jun 26, 2018 at 10:10:47PM +0900, Akihiro Sagawa wrote:
> Otherwise, a song is played faster than expected in some cases.
>
> For instance, we assume that 40 pan control change (CC#10) messages are
> expressed in 12.5ms each,
>  * Prior art omits fractions, i.e. 12ms + 12ms + ... = 480ms.
>  * Proposal technique saves fractions, i.e. 12ms + 13ms + ... = 500ms.
> Ideal duration is 500ms. So, the proposed method is better than prior one.
>
> Signed-off-by: Akihiro Sagawa <sagawa.aki(a)gmail.com>
> ---
>  dlls/winmm/winmm.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)

> diff --git a/dlls/winmm/winmm.c b/dlls/winmm/winmm.c
> index 4bba1a7..0328721 100644
> --- a/dlls/winmm/winmm.c
> +++ b/dlls/winmm/winmm.c
> @@ -921,6 +921,7 @@ typedef struct WINE_MIDIStream {
>      DWORD			dwElapsedMS;
>      WORD			wFlags;
>      WORD			status;
> +    WORD			remainder;
>      HANDLE			hEvent;
>      LPMIDIHDR			lpMidiHdr;
>  } WINE_MIDIStream;
> @@ -965,13 +966,13 @@ static	DWORD	MMSYSTEM_MidiStream_Convert(WINE_MIDIStream* lpMidiStrm, DWORD puls
>      } else if (lpMidiStrm->dwTimeDiv > 0x8000) { /* SMPTE, unchecked FIXME? */
>   int	nf = 256 - HIBYTE(lpMidiStrm->dwTimeDiv);	/* number of frames     */
>   int	nsf = LOBYTE(lpMidiStrm->dwTimeDiv);		/* number of sub-frames */
> -	ret = (pulse * 1000) / (nf * nsf);
> +	ret = (pulse * 1000000) / (nf * nsf);
>      } else {
> -	ret = (DWORD)((double)pulse * ((double)lpMidiStrm->dwTempo / 1000) /
> +	ret = (DWORD)((double)pulse * (double)lpMidiStrm->dwTempo /
>             (double)lpMidiStrm->dwTimeDiv);
>      }
>
> -    return ret;
> +    return ret; /* in microseconds */
>  }
>
> static DWORD midistream_get_playing_position(WINE_MIDIStream* lpMidiStrm)
> @@ -1007,6 +1008,8 @@ static	BOOL	MMSYSTEM_MidiStream_MessageHandler(WINE_MIDIStream* lpMidiStrm, LPWI
>              lpMidiStrm->status = MSM_STATUS_STOPPED;
>              lpMidiStrm->dwPulses = 0;
>              lpMidiStrm->dwElapsedMS = 0;
> +            lpMidiStrm->dwPositionMS = 0;
> +            lpMidiStrm->remainder = 0;
>              LeaveCriticalSection(&lpMidiStrm->lock);
>              /* this is not quite what MS doc says... */
>              midiOutReset(lpMidiStrm->hDevice);
> @@ -1178,8 +1181,11 @@ start_header:
>
> /* do we have to wait ? */
>   if (me->dwDeltaTime) {
> +	    DWORD delta;
>       EnterCriticalSection(&lpMidiStrm->lock);
> -	    lpMidiStrm->dwPositionMS += MMSYSTEM_MidiStream_Convert(lpMidiStrm, me->dwDeltaTime);
> +	    delta = lpMidiStrm->remainder + MMSYSTEM_MidiStream_Convert(lpMidiStrm, me->dwDeltaTime);
> +	    lpMidiStrm->dwPositionMS += delta / 1000;
> +	    lpMidiStrm->remainder = delta % 1000;
>       lpMidiStrm->dwPulses += me->dwDeltaTime;
>       LeaveCriticalSection(&lpMidiStrm->lock);
>
> @@ -1331,6 +1337,7 @@ MMRESULT WINAPI midiStreamOpen(HMIDISTRM* lphMidiStrm, LPUINT lpuDeviceID,
>      lpMidiStrm->dwTempo = 500000;  /* micro seconds per quarter note, i.e. 120 BPM */
>      lpMidiStrm->dwTimeDiv = 24;    /* ticks per quarter note */
>      lpMidiStrm->dwPositionMS = 0;
> +    lpMidiStrm->remainder = 0;
>      lpMidiStrm->status = MSM_STATUS_PAUSED;
>      lpMidiStrm->dwElapsedMS = 0;

>

[Andrew Eikum]

Sorry, didn't mean to sign-off on this, I pushed the wrong button in
my mail client...

On Mon, Jul 09, 2018 at 09:39:56AM -0500, Andrew Eikum wrote:
> > @@ -1178,8 +1181,11 @@ start_header:
> >
> > /* do we have to wait ? */
> >   if (me->dwDeltaTime) {
> > +	    DWORD delta;
> >       EnterCriticalSection(&lpMidiStrm->lock);
> > -	    lpMidiStrm->dwPositionMS += MMSYSTEM_MidiStream_Convert(lpMidiStrm, me->dwDeltaTime);
> > +	    delta = lpMidiStrm->remainder + MMSYSTEM_MidiStream_Convert(lpMidiStrm, me->dwDeltaTime);
> > +	    lpMidiStrm->dwPositionMS += delta / 1000;
> > +	    lpMidiStrm->remainder = delta % 1000;
> >       lpMidiStrm->dwPulses += me->dwDeltaTime;
> >       LeaveCriticalSection(&lpMidiStrm->lock);

Instead of storing the remainder in a separate variable, does it make
sense to change the units of dwPositionMS to microseconds?

Andrew