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

[Eduard Permyakov]

On 2021-09-13 6:49 p.m., Andrew Eikum wrote:
> 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

Fixed in the newest version of the patch. I did not consider a case 
where an existing
buffer's frequency (and consequently writelead) could be changed.

>> 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?

If the committed_mixpos is greater than 0 and less than dsb->writelead, 
this means
that a previously committed chunk has not had time to be played 
completely. This
essentially means that the app is overwriting a single buffer region in 
rapid succession.
I think this is an extremely exotic case. Since there is only a fixed 
amount of space for
the committed chunk, we would need to discard either part of the "old" 
committed
chunk, or part of the "new" committed chunk. For simplicity of the code, 
I have kept
it such that any part of the "old" committed chunk is discarded in such 
a case.

>>> @@ -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
>>