winearts patch

[Jeremy Shaw]

At Wed, 07 Jan 2004 21:22:33 +0100,
Eric Pouech wrote:
> 
> 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...

I am submitting a patch to do a HeapRealloc() instead of
HeapFree()/HeapAlloc(). It also assigns pointers to NULL after a
HeapFree().

> > (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)
> 

Ok, in WriteMaxFrags() I changed:

	*bytes -= written;

to 

    if (written < toWrite)
	*bytes = 0;
    else
	*bytes -= written;


> > (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.

Yeah, I looked at it closer after I read the email, and decided to
remove the line altogether. There is already a call to
wodUpdatePlayedTotal() that should take care of updating
wwo->dwPlayedTotal.

> > (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.

I think arts is fine without it...

Jeremy Shaw.