Jeremy Shaw a écrit :
Hello,
This message is largely for Chris Morgan's review, but I thought I should open it to any interested parties.
I have added wave-in support for arts. I also fixed some bugs in the waveout code, and made some "improvements". This code seems to work really well for me. If no one sees any problems I will submit the patch to the patches mailing list.
Here is a brief description of the changes I made:
(1) In ARTS_CloseDevice() (now named ARTS_CloseWaveOutDevice()) and wodPlayer_NotifyWait()
I added 'wwo->sound_buffer = 0' after the HeapFree(). Without it, HeapFree() was being called twice on the same buffer, and would crash the second time.
Well, I think the bug isn't here but rather in wodPlayer_WriteMaxFrags. If the buffer is too small: - we shouldn't destroy the current buffer (and hopefully create a new one) but realloc the buffer... - moreover, destroying the buffer without setting the sound_buffer field to NULL is calling for dangling pointers...
(2) ARTS_AddRingMessage()
Added the same fix I submitted earlier for wineoss in regards to ring buffer resizing.
(3) wodPlayer_WriteMaxFrags()
DWORD *bytes, points to the number of bytes the winearts driver thinks it should be able to write. However, arts sometimes lies, and says it has (for example) 512 bytes available, but when you call arts_write() it returns zero bytes written.
In that case, I update *bytes to point to 0, otherwise wodPlayer_WriteMaxFrags() will keep being called in vain.
correct. It should even be done as soon as we write less bytes than expected (content of *byte at upon entry of wodPlayer_WriteMaxFrags)
(4a) wodPlayer_FeedDSP()
The old code had the following checks:
/* input queue empty and output buffer with no space */if (!wwo->lpPlayPtr && availInQ) { TRACE("Run out of wavehdr:s... flushing\n"); wwo->dwPlayedTotal = wwo->dwWrittenTotal; return INFINITE; }
if(!availInQ) { TRACE("no more room, no need to try to feed\n"); return wodPlayer_DSPWait(wwo); }
I could not understand the purpose of the '&& availInQ' in the first check. It seems that if we are out of wavehdrs, then it does not matter if there is space in the availInQ or not, because we don't have anything to write. So I removed the && availInQ part.
The purpose of the test is to try to always keep the input queue full: - therefore if we no longer have wave headers to feed the queue with, and
Looking at it now, I realize that maybe I need to add 'if (!availInQ)' before 'wwo->dwPlayedTotal = wwo->dwWrittenTotal'?
IMO, the wwo->dwPlayedTotal = wwo->dwWrittenTotal is wrong in all cases. If we have no data to write, just way for some new headers to arrive.
(4b) wodPlayer_FeedDSP()
In the while loop, I could not figure out the purpose of 'availInQ > SPACE_THRESHOLD', so I changed it to 'availInQ'. In reality, the SPACE_THRESHOLD check could probably be left in, I am not sure what the purpose was in the first place.
Dunno either. Reading Chris response, we need this on OSS (because sounds is processed by fragments of fixed size). I don't know how arts behaves here... Thrashing it altogether could also work.
A+