'Twas brillig, and Maarten Bosmans at 13/01/11 10:17 did gyre and gimble: > 2011/1/11 Colin Guthrie <gmane at colin.guthr.ie>: >> 'Twas brillig, and Maarten Bosmans at 11/01/11 10:51 did gyre and gimble: >>> I'll prepare a new patch series with all your points adressed. >> >> Sweet. Feel free to supply a remote from which I can pull if it's easier >> (doesn't matter much to me). > > I've opened a GitHub account. Let's see if that works for you. At > https://github.com/mkbosmans/pulseaudio/compare/mingw32-build > you'll find basically the same set of patches I sent to the list, with > your comments incorporated and some other changes, which I'll discuss > below. Cool. I'll add it to my remotes! >> I'm not 100% sure of setenv vs putenv but both appear to be as thread >> safe as the other (the only reason I can think of to favour one over the >> other). > > I've changed it in a separate one-line commit (the first one), to make > it (and the rationale in the commit message) stand out between all the > win32 related changes, as this touches Linux (though no change in > behaviour is expected) > https://github.com/mkbosmans/pulseaudio/commit/912a20925475f370f9f9926a7dccc930cbecfb05 Sounds good. I'll do extra testing before merging just to make sure it doesn't introduce any nasties (tho' I sure it wont). >> It dates back a while (045c1d60: Glitch free merge to trunk - in svn >> days), but putenv() was used prior to that and introduced in >> 9c87a65ce91c38b60c19ae108a51a2e8ce46a85c. Doesn't look like it was used >> over setenv() for any real reason, so I guess that should be fine. > > Hmm, yeah, that was the time of Lennart's huge commits touching almost > all of the codebase at once. That certainly has diminished the value > of git history digging a bit. I came across this often when trying to > find when API was changed in order to change the win32 code too. Yeah :( Boo for SVN merging. It would have perhaps been possible to make the conversion to git a bit nicer if this branch was somehow factored in to the history, but such is life. > I am a bit further now. The daemon starts successfully with null-sink > and protocol-native-tcp. And pactl list works on the local server. So > no audio yet, but getting there. That's still good process. Lots has changed since 0.9.10 or whenever the last win32 build dates from so it's great you've gotten so far already :) > The change I would like to see reviewed, before merging are the read, > write => pa_read, pa_write changes in fdsem.c > https://github.com/mkbosmans/pulseaudio/commit/a02aad4471a07d862cdafc11a06b46c8b54aff55#diff-2 > This is necessary, because on windows the code in pipe.c makes a pipe > out of two sockets instead of file descriptors. That means that a bare > read/write won't work, because it expects an fd. pa_read/pa_write > provides a nice wrapper that uses recv/send in case of Windows > sockets. But as pa_read/pa_write also does a loop to cover up EINTR, > I'm not sure whether the code is still correct on Linux. (Limited > testing of this code on Linux reveals no problems though) OK, I'm not really an expert here either, but I'll take a look. If I'm in doubt I wont include that one just now and try and get someone more qualified to give their opinion. > The last commit in the mingw32-build branch is an attempt to fix > module-waveout, but it is very much work in progress. So I suggest you > pull from the commit before that. > https://github.com/mkbosmans/pulseaudio/commit/415ec823dd24a7c69bda26679df7648402c6a9a9 Cool, no worries. I'll see what I can do. It might not be until the weekend tho'. Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited [http://www.tribalogic.net/] Open Source: Mageia Contributor [http://www.mageia.org/] PulseAudio Hacker [http://www.pulseaudio.org/] Trac Hacker [http://trac.edgewall.org/]
