| Message ID | 20100107222343.GA90709@triton8.kn-bremen.de |
|---|---|
| State | New |
| Headers | show |
On Thu, 7 Jan 2010, Juergen Lock wrote: > In this case it was missing on FreeBSD <= 6.x (Which also doesn't have > SNDCTL_DSP_POLICY yet so the version doesn't get used anyway.) I've commited slightly different fix for the issue, thanks.
On Fri, Jan 08, 2010 at 11:27:13AM +0300, malc wrote: > On Thu, 7 Jan 2010, Juergen Lock wrote: > > > In this case it was missing on FreeBSD <= 6.x (Which also doesn't have > > SNDCTL_DSP_POLICY yet so the version doesn't get used anyway.) > > I've commited slightly different fix for the issue, thanks. > Hmm looking at the last hunk of the commit, >[...] >@@ -289,9 +292,17 @@ static int oss_open (int in, struct oss_params *req, > if (conf.debug) { > dolog ("OSS version = %#x\n", version); > } >+#endif > > #ifdef SNDCTL_DSP_POLICY >- if (conf.policy >= 0 && version >= 0x040000) { >+ if (conf.policy >= 0 >+#ifdef OSS_GETVERSION >+ && version >= 0x040000 >+#else >+ 0 ...these last two lines (#else and 0) probably should go, I dont think the compiler likes whitespace between digits. :) >+#endif >+ ) >+ { > int policy = conf.policy; > if (ioctl (fd, SNDCTL_DSP_POLICY, &policy)) { > oss_logerr2 (errno, typ, "Failed to set timing policy to %d\n", >-- >1.6.6 And also I forgot to say this is stable-0.12 material too. And thanx for committing! Juergen
On Fri, 8 Jan 2010, Juergen Lock wrote: > On Fri, Jan 08, 2010 at 11:27:13AM +0300, malc wrote: > > On Thu, 7 Jan 2010, Juergen Lock wrote: > > > > > In this case it was missing on FreeBSD <= 6.x (Which also doesn't have > > > SNDCTL_DSP_POLICY yet so the version doesn't get used anyway.) > > > > I've commited slightly different fix for the issue, thanks. > > > Hmm looking at the last hunk of the commit, > > >[...] > >@@ -289,9 +292,17 @@ static int oss_open (int in, struct oss_params *req, > > if (conf.debug) { > > dolog ("OSS version = %#x\n", version); > > } > >+#endif > > > > #ifdef SNDCTL_DSP_POLICY > >- if (conf.policy >= 0 && version >= 0x040000) { > >+ if (conf.policy >= 0 > >+#ifdef OSS_GETVERSION > >+ && version >= 0x040000 > >+#else > >+ 0 > > ...these last two lines (#else and 0) probably should go, I dont think > the compiler likes whitespace between digits. :) Uh, yeah, my bad, sorry, hopefuly fixed now.. > > >+#endif > >+ ) > >+ { > > int policy = conf.policy; > > if (ioctl (fd, SNDCTL_DSP_POLICY, &policy)) { > > oss_logerr2 (errno, typ, "Failed to set timing policy to %d\n", > >-- > >1.6.6 > > And also I forgot to say this is stable-0.12 material too. You really should talk to the people who know what that means :) > And thanx for committing! > Juergen >
On Sat, Jan 09, 2010 at 12:33:44AM +0300, malc wrote: > On Fri, 8 Jan 2010, Juergen Lock wrote: > > > On Fri, Jan 08, 2010 at 11:27:13AM +0300, malc wrote: > > > On Thu, 7 Jan 2010, Juergen Lock wrote: > > > > > > > In this case it was missing on FreeBSD <= 6.x (Which also doesn't have > > > > SNDCTL_DSP_POLICY yet so the version doesn't get used anyway.) > > > > > > I've commited slightly different fix for the issue, thanks. > > > > > Hmm looking at the last hunk of the commit, > > > > >[...] > > >@@ -289,9 +292,17 @@ static int oss_open (int in, struct oss_params *req, > > > if (conf.debug) { > > > dolog ("OSS version = %#x\n", version); > > > } > > >+#endif > > > > > > #ifdef SNDCTL_DSP_POLICY > > >- if (conf.policy >= 0 && version >= 0x040000) { > > >+ if (conf.policy >= 0 > > >+#ifdef OSS_GETVERSION > > >+ && version >= 0x040000 > > >+#else > > >+ 0 > > > > ...these last two lines (#else and 0) probably should go, I dont think > > the compiler likes whitespace between digits. :) > > Uh, yeah, my bad, sorry, hopefuly fixed now.. > Yeah looking better now... Thanx! > > > > >+#endif > > >+ ) > > >+ { > > > int policy = conf.policy; > > > if (ioctl (fd, SNDCTL_DSP_POLICY, &policy)) { > > > oss_logerr2 (errno, typ, "Failed to set timing policy to %d\n", > > >-- > > >1.6.6 > > > > And also I forgot to say this is stable-0.12 material too. > > You really should talk to the people who know what that means :) Oh sorry I should have guessed not all committers do merges to stable branches... Anthony? :) Greets, Juergen
Am 09.01.2010 um 14:45 schrieb Juergen Lock: > On Sat, Jan 09, 2010 at 12:33:44AM +0300, malc wrote: >> On Fri, 8 Jan 2010, Juergen Lock wrote: >> >>> And also I forgot to say this is stable-0.12 material too. >> >> You really should talk to the people who know what that means :) > > Oh sorry I should have guessed not all committers do merges to stable > branches... Anthony? :) In this case you should probably resubmit one accumulated top-level patch marked [PATCH][For stable-0.12] or similar (instead of having two commits cherry-picked). Andreas
On Sat, Jan 09, 2010 at 03:03:43PM +0100, Andreas Färber wrote: > > Am 09.01.2010 um 14:45 schrieb Juergen Lock: > > > On Sat, Jan 09, 2010 at 12:33:44AM +0300, malc wrote: > >> On Fri, 8 Jan 2010, Juergen Lock wrote: > >> > >>> And also I forgot to say this is stable-0.12 material too. > >> > >> You really should talk to the people who know what that means :) > > > > Oh sorry I should have guessed not all committers do merges to stable > > branches... Anthony? :) > > In this case you should probably resubmit one accumulated top-level > patch marked [PATCH][For stable-0.12] or similar (instead of having > two commits cherry-picked). Ok I can do that, but first I have another patch to the same code, turns out the ioctl doesn't actually work yet on FreeBSD even if it is defined... Cheers, Juergen
--- a/audio/ossaudio.c +++ b/audio/ossaudio.c @@ -240,7 +240,7 @@ static int oss_open (int in, struct oss_ struct oss_params *obt, int *pfd) { int fd; - int version; + int version = 0; int oflags = conf.exclusive ? O_EXCL : 0; audio_buf_info abinfo; int fmt, freq, nchannels; @@ -281,10 +281,12 @@ static int oss_open (int in, struct oss_ goto err; } +#ifdef OSS_GETVERSION if (ioctl (fd, OSS_GETVERSION, &version)) { oss_logerr2 (errno, typ, "Failed to get OSS version\n"); version = 0; } +#endif if (conf.debug) { dolog ("OSS version = %#x\n", version);
In this case it was missing on FreeBSD <= 6.x (Which also doesn't have SNDCTL_DSP_POLICY yet so the version doesn't get used anyway.) Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de>