[pulseaudio-discuss] [PATCH 09/10] Implement some functions for win32

Colin Guthrie gmane at colin.guthr.ie
Thu Jan 13 02:52:45 PST 2011


'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/]




More information about the pulseaudio-discuss mailing list