[pulseaudio-discuss] NetBSD patches

[Jared D. McNeill]

On Thu, Jan 22, 2009 at 4:55 PM, Lennart Poettering
<lennart at poettering.net> wrote:
>
> On Thu, 22.01.09 15:53, Jared D. McNeill (jmcneill at NetBSD.org) wrote:
>
>> > patch-ad (seems very NetBSD specific to me, although the $LIBOSSAUDIO
>> >          part appears interesting, though I am not entirely sure what
>> >          it does. If it it does what I think it does there should be
>> >          a proper autoconf check for it. I'd then be willing to merge
>> >          that part)
>>
>> Yeah, the LIBOSSAUDIO change would be good (if what you think it does
>> is use -lossaudio where available, then it does what you think it
>> does). The rest of this patch is specific to how pkgsrc handles
>> configuration files and should not be merged.
>
> Yes, this is mostly what I thought it would do. Although I am not sure
> I understand how it does what it does. Does it redirect the ioctl()
> call?

Yes it emulates OSS on top of the native audio APIs.

>> > patch-ac (The #if 0 is a bit too radical for my taste ;-). For n_cpus
>> >          there is now a proper function pa_ncpus() elsewhere)
>>
>> The first revision of that section of the patch patch was #ifdef
>> __linux__, then it was later changed to #if 0 by another pkgsrc
>> developer with the following message:
>>
>> ``The rtstutter test is afunctional for all operating systems but one.  Make it
>> go one operating system further.
>
> Uh? I don't follow. What's the point of breaking rtstutter on Linux
> like this?

As I said, I didn't write the patch, nor is that comment from me.

How about a --disable-tests flag to configure so we can skip building
it altogether?

>> Old patch-ac disabled the guts of tests/rtstutter.c for all but Linux.  But it
>> isn't that portable on Linux either as it stands, and errors out (on EeePC) at
>> link time with:
>>
>> rtstutter.c:(.text+0xbd): undefined reference to `pthread_setaffinity_np'
>>
>> Just bring Linux in line with other platforms by changing the #ifdef __linux__
>> to #if 0.''
>
> pthread_setaffinity_np seems to be a feature of glibc >= 2.3.4. What
> version of glibc does your EeePC distro ship?
>
> The proper fix is to add an autoconf check for this function of
> course, and use it only if the function is available. I have now added
> such a test.

As I said above, that is not my comment.

>> > patch-aa (The definition of SNDCTL_DSP_GETODELAY needs to be made
>> >         specific for NetBSD. The looking for /dev/audio and
>> >         /dev/sound is suspicous at best. Despite from that this
>> >         should probably be merged.)
>>
>> The SNDCTL_DSP_GETODELAY section is a workaround for NetBSD 4.0 and
>> below, where the ioctl wasn't implemented. Changing it to something
>> like this would be fine;
>>
>>   #if defined(__NetBSD__) && !defined(SNDCTL_DSP_GETODELAY)
>>   ..
>
> Yes, such a patch I would merge.
>
>> As far as /dev/sound and /dev/audio, NetBSD's native audio API is not
>> OSS. OSS compatibility is provided by libossaudio, which emulates OSS
>> ioctls. All native OSS applications on NetBSD use either /dev/sound*
>> or /dev/audio* (pretty much the same device, just a slight difference
>> in behaviour on open).
>
> On Linux /dev/sound is unknown and /dev/audio is some weird
> not-really-supported, not-really-standardized, mostly obsolete
> interface. If emulating these is useful on NetBSD an #ifdef check is
> probably appropriate here, too.
>
> (Also given that this check becomes quite complex now and it is done
> multiple times it should be moved to an explicit function)

Since they are the only entry points to the audio subsystem on NetBSD,
it is necessary.

Ideally, you would want something like:

#if defined(__NetBSD__)
#include <paths.h>
[check path against _PATH_SOUND and _PATH_AUDIO]
#endif


>> > BTW, I'd prefer if you guys would CC each patch to our ML (you don't
>> > even need to subscribe, I'll whitelist you). I just noticed yet
>> > another patch appeared in CVS. I think it is always of interest to
>> > merge patches as much as possible upstream, especially in the NetBSD
>> > case because a lot of changes are also relevant on the other BSDs. But
>> > if you don't tell me about those patches I cannot merge them. And I am
>> > not really interested in going through all those downstream repos all
>> > the time and picking the patches I like.
>>
>> I will do that from now on, I can't promise that the other developers
>> will do the same :-)
>
> Thanks!
>
>> > Also, git-format-patch formatted patches are highly preferred so that I
>> > don't have to spend precious time on finding out who I have to
>> > attribute patches to.
>>
>> Not quite sure what that is, but I will find out. Please keep in mind
>> that the patches maintained in pkgsrc are against release versions of
>> software only.
>
> You NetBSD folks are not infected by the git virus yet, are you? Still
> stuck with CVS?

Stuck in the most literal sense. Either the replacements don't scale
to our massive repository or don't have the features we need.

Cheers,
Jared