[PATCH v5] dsound: Commit next audio chunk between play cursor and write cursor to playing.

[Andrew Eikum]

On Mon, Sep 13, 2021 at 10:33:38AM -0500, Andrew Eikum wrote:
> This is looking pretty good. I'm going to run it through my set of
> dsound test applications
> 

Bad news, it causes a crash in the Darwinia 2 demo for me :(

    $ md5sum darwinia-demo2.exe
    3f5d3fd40db15dd1a7e51891f385c57f  darwinia-demo2.exe

    https://www.moddb.com/games/darwinia/downloads/darwinia-windows-demo

Andrew

> On Fri, Sep 10, 2021 at 06:36:23PM +0300, Eduard Permyakov wrote:
> > diff --git a/dlls/dsound/buffer.c b/dlls/dsound/buffer.c
> > index dc3b54906ce..63468732e1d 100644
> > --- a/dlls/dsound/buffer.c
> > +++ b/dlls/dsound/buffer.c
> > @@ -101,6 +101,24 @@ static int __cdecl notify_compar(const void *l, const void *r)
> >      return 1;
> >  }
> >  
> > +static void commit_next_chunk(IDirectSoundBufferImpl *dsb)
> > +{
> > +    void *dstbuff = dsb->committedbuff, *srcbuff = dsb->buffer->memory;
> > +    DWORD srcoff = dsb->sec_mixpos, srcsize = dsb->buflen, cpysize = dsb->writelead;
> > +
> > +    if(cpysize > srcsize - srcoff) {
> > +        DWORD overflow = cpysize - (srcsize - srcoff);
> > +        memcpy(dstbuff, (BYTE*)srcbuff + srcoff, srcsize - srcoff);
> > +        memcpy((BYTE*)dstbuff + (srcsize - srcoff), srcbuff, overflow);
> > +    }else{
> > +        memcpy(dstbuff, (BYTE*)srcbuff + srcoff, cpysize);
> > +    }
> > +
> > +    dsb->use_committed = TRUE;
> > +    dsb->committed_mixpos = 0;
> 
> Is it right to always reset this to zero? Is it possible an app could
> write to the committed chunk while committed_mixpos is non-zero, and
> so reset committed_mixpos when it shouldn't be?
> 
> > @@ -153,7 +153,12 @@ struct IDirectSoundBufferImpl
> >      LONG64                      freqAccNum;
> >      /* used for mixing */
> >      DWORD                       sec_mixpos;
> > -
> > +    /* Holds a copy of the next 'writelead' bytes, to be used for mixing. This makes it
> > +     * so that these bytes get played once even if this region of the buffer gets overwritten,
> > +     * which is more in-line with native DirectSound behavior. */
> > +    BOOL                        use_committed;
> > +    LPVOID                      committedbuff;
> > +    DWORD                       committed_mixpos;
> 
> I think use_committed (and committed_mixpos?) should be initialized in
> Duplicate. What about SetCurrentPosition?
> 
> Andrew
>