October 2001 - Wine-Devel - list.winehq.org

Re: audio.c converted from WaitForSingleObject to poll

by Simon Britnell

Please forgive the length of this missive, I have not the time to write a short one. There are a couple of (serious) bugs in my patch to do with notifications (more latere). It has, however, uncovered a couple of race conditions that applications may trigger to hang the wodPlayer with or without this patch. -- BUSY LOOP -- There is no busy loop because: retval=poll( pollfd.all, wwo->state==WINE_WS_PLAYING), -1 ); only polls the dsp when in the playing state and: if( !wodPlayer_WriteFragments(wwo) ) { /* stop playing if there's nothing to play */ wwo->state = WINE_WS_STOPPED; } Causes the state to become stopped if we run out of things to write. -- BUGS -- Some wave completions will not be notified until something else is played (Doh!). I'm still thinking about this, but the (wrong) easy solution would be to notify on completion of the write instead of completion of the playback. wodGetPosition doesn't keep pace with how much has actually been played. This causes sound to fail in HalfLife. I'd like to remedy this by keeping track of the total written rather than the total played and to return that value less the amount ioctl says is still in the buffer to be played. -- RACE CONDITION -- wodPlayer_Notify (which, as you may guess from the bugs above, is my arch nemesis) triggers a callback into the application. If the application makes any wod calls which wait on the wodPlayer thread to set an event (wodReset,wodRestart or wodPause), the wodPlayer thread hangs waiting for wodPlayer_Notify to return, which is waiting for wodPlayer to SetEvent(wwo->hEvent). Any other threads attempting to wodReset, wodRestart or wodPause will then pile up behind the stalled wodPlayer, potentially hanging the whole program. Looking at the code, it appears at first blush that we may simply remove the SetEvent() and the WaitForSingleObject() calls to remove this race condition. It seems unlikely that this will cause any problems. -- WHERE TO FROM HERE -- I shall put my poll patch to one side and provide two patches against the encumbent audio.c The first patch will remove the SetEvent -> WaitForSingleObject on wodPause,wodReset and wodRestart. The second patch will update wodGetPosition per my above suggestion. I will then re-examine the poll patch/notification timeliness issue to see if I can come up with something reasonable. Questions? Objections? Flames ? :) __________________________________________________ Do You Yahoo!? Make a great connection at Yahoo! Personals. http://personals.yahoo.com

Oct. 31, 2001

Re: audio.c converted from WaitForSingleObject to poll

by Ove Kaaven

On Wed, 31 Oct 2001, eric pouech wrote: > > > where's the busy wait ? the code blocks on poll for two types on events > > > - /dev/dsp becomes writable (some new fragments can be written) > > > > So, what do you think happens when /dev/dsp is *always* writable (due to > > insufficient data from the app)? Then it'll never block. > you just need to change the condition in poll for the number of fd:s to > be looked upon and use 2 only when playing and some wavehdr have been > queued but not sent yet to OSS Yes... but the patch should not be applied without doing so, which was my point... but I have concerns that notification accuracy will suffer, too.

Oct. 31, 2001

Re: audio.c converted from WaitForSingleObject to poll

by eric pouech

> > where's the busy wait ? the code blocks on poll for two types on events > > - /dev/dsp becomes writable (some new fragments can be written) > > So, what do you think happens when /dev/dsp is *always* writable (due to > insufficient data from the app)? Then it'll never block. you just need to change the condition in poll for the number of fd:s to be looked upon and use 2 only when playing and some wavehdr have been queued but not sent yet to OSS A+ -- --------------- Eric Pouech (http://perso.wanadoo.fr/eric.pouech/) "The future will be better tomorrow", Vice President Dan Quayle

Oct. 31, 2001

Re: audio.c converted from WaitForSingleObject to poll

by Ove Kaaven

On Wed, 31 Oct 2001, eric pouech wrote: > where's the busy wait ? the code blocks on poll for two types on events > - /dev/dsp becomes writable (some new fragments can be written) So, what do you think happens when /dev/dsp is *always* writable (due to insufficient data from the app)? Then it'll never block.

Oct. 31, 2001

Re: audio.c converted from WaitForSingleObject to poll

by eric pouech

> The Mux takes all inputs and multiplexes them into the > circular buffer. It blocks when the buffer is full. In > the case of wine, I would guess the Mux isn't really > needed, just some function that writes into the > circular buffer, blocking if its full. no the Win32 API doesn't behave like this. queueing a buffer for playback is supposed to be non blocking so we shouldn't limit the size of the buffer > Also the buffer size is a multiple of the fragment > size of the output device. For OSS the output buffer > is split into fragments, for SBLive 16 fragments of 4K > each. again, the Win32 API doesn't put any constraints on the size of input buffers, so we need to do the split in order to fill the OSS fragments A+ -- --------------- Eric Pouech (http://perso.wanadoo.fr/eric.pouech/) "The future will be better tomorrow", Vice President Dan Quayle

Oct. 31, 2001

Re: audio.c converted from WaitForSingleObject to poll

by eric pouech

Ove Kaaven wrote: > > On Wed, 31 Oct 2001, Simon Britnell wrote: > > > 2) Removes the need to calculate a sleep interval in > > the wodPlayer main loop, as it loops only when dsp is > > ready to be written or a command is ready to be > > received. > > Well, it seems like a bad idea to make the player thread enter a busy loop > when there isn't enough data to fill the device buffers, don't you think? > (Well, though I'd like buffer complete notifications to happen more > frequently than they do in Wine's current implementation, making the > player thread use 100% CPU isn't the way...) where's the busy wait ? the code blocks on poll for two types on events - /dev/dsp becomes writable (some new fragments can be written) - a new message has been sent so the playback thread is blocked otherwise (unless I underlooked something) A+ -- --------------- Eric Pouech (http://perso.wanadoo.fr/eric.pouech/) "The future will be better tomorrow", Vice President Dan Quayle

Oct. 31, 2001

Re: LockFile - any words of warning?

by Rein Klazes

On Wed, 31 Oct 2001 10:37:11 -0800, you wrote: > > > > Have a look at Corel's wine tree, where it is implemented. > > Please excuse my ignorance; how do I get at the Corel tree? Hmm, the instructions seem to have been moved off Corel's site and not reappeared at Xandra's yet. The cvs server is still working OK: export CVSROOT=:pserver:cvs(a)opensource.corel.com:/home/cvs/r cvs login (pass is "cvs") cvs co corelwine Rein. -- Rein Klazes rklazes(a)xs4all.nl

Oct. 31, 2001

RE: LockFile - any words of warning?

by Medland, Bill

Thanks, but > -----Original Message----- > From: Rein Klazes [mailto:rklazes(a)xs4all.invalid] > Sent: Wednesday, October 31, 2001 9:32 AM > To: Bill Medland > Cc: wine-devel(a)winehq.com > Subject: Re: LockFile - any words of warning? > > > On Wed, 31 Oct 2001 08:31:11 -0800, you wrote: > > > I am going to have to implement the LockFile/UnlockFile > functions. Does > > anyone have any words of warning? (Like it ain't as easy > as calling fcntl) > > Have a look at Corel's wine tree, where it is implemented. Please excuse my ignorance; how do I get at the Corel tree?

Oct. 31, 2001

LockFile - any words of warning?

by Bill Medland

I am going to have to implement the LockFile/UnlockFile functions. Does anyone have any words of warning? (Like it ain't as easy as calling fcntl) Bill

Oct. 31, 2001

Re: audio.c converted from WaitForSingleObject to poll

by Paul Lorenz

Hi, I'm writing a multi-track recorder, so this is a problem I've encountered. My solution went like this. Mux in own thread, Player in own thread with circular buffer in the middle. The Mux takes all inputs and multiplexes them into the circular buffer. It blocks when the buffer is full. In the case of wine, I would guess the Mux isn't really needed, just some function that writes into the circular buffer, blocking if its full. The player reads from the buffer, blocks on empty, writes to /dev/dsp otherwise. Also the buffer size is a multiple of the fragment size of the output device. For OSS the output buffer is split into fragments, for SBLive 16 fragments of 4K each. So the player also blocks on the buffer, when there are less than fragment_size bytes available. In this model, there are no timed waits or yields, just blocks and notifies, so happily no spinlock type situations. I have all this written, unfortunately its in Java, but the concept should be translatable to C. I hope this is relevant/useful. Paul Lorenz --- Ove Kaaven <ovehk(a)ping.uio.no> wrote: > > On Wed, 31 Oct 2001, Simon Britnell wrote: > > > 2) Removes the need to calculate a sleep interval > in > > the wodPlayer main loop, as it loops only when dsp > is > > ready to be written or a command is ready to be > > received. > > Well, it seems like a bad idea to make the player > thread enter a busy loop > when there isn't enough data to fill the device > buffers, don't you think? > (Well, though I'd like buffer complete notifications > to happen more > frequently than they do in Wine's current > implementation, making the > player thread use 100% CPU isn't the way...) > > > __________________________________________________ Do You Yahoo!? Make a great connection at Yahoo! Personals. http://personals.yahoo.com

Oct. 31, 2001