[SoX-devel] better sndio support in SoX

Post by Jan Stary
Hi Eric,
recently, Alex Ratchov (of OpenBSD's sndio) has added a diff
to src/sndio.c that enables a finer setting of the desired
parameters of the underlying audio (namely, bit width).
Currently, the diff (attached) exists as a patch to the OpenBSD
port of SoX (which I maintain). Would you please consider
incorporating this change into SoX directly?

Hi Jan,

Seems reasonable; but I have little experience with using sndio
with Måns sox, much less the API. Perhaps Måns has an opinion,
too.

Fwiw, it would be helpful if example files could be
provided/generated showing where the improvement is.

Thanks. Sorry for the late response; I've been mostly
unplugged lately :)

Post by Jan Stary
Thank you
Jan
$OpenBSD$
--- src/sndio.c.orig Mon Jan 30 04:01:44 2012
+++ src/sndio.c Tue Feb 9 23:23:00 2016
@@ -113,8 +113,6 @@ static int startany(sox_format_t *ft, unsigned mode)
else
reqpar.rchan = ft->signal.channels;
}
- if (ft->signal.precision > 0)
- reqpar.bits = ft->signal.precision;
switch (ft->encoding.encoding) {
reqpar.sig = 1;
@@ -127,6 +125,12 @@ static int startany(sox_format_t *ft, unsigned mode)
}
if (ft->encoding.bits_per_sample > 0)
reqpar.bits = ft->encoding.bits_per_sample;
+ else if (ft->signal.precision > 0)
+ reqpar.bits = ft->signal.precision;
+ else
+ reqpar.bits = SOX_DEFAULT_PRECISION;
+ reqpar.bps = (reqpar.bits + 7) / 8;
+ reqpar.msb = 1;
if (ft->encoding.reverse_bytes != sox_option_default) {
reqpar.le = SIO_LE_NATIVE;
if (ft->encoding.reverse_bytes)

------------------------------------------------------------------------------

Måns Rullgård

2016-09-20 20:06:15 UTC

Hi Jan,
Seems reasonable; but I have little experience with using sndio
with Måns sox, much less the API. Perhaps Måns has an opinion,
too.

None of my changes should have any impact on this.

Post by Eric Wong
Fwiw, it would be helpful if example files could be
provided/generated showing where the improvement is.

I'm not familiar with sndio, but it looks like it changes how 24-bit
samples are passed, packed rather than padded to 32 bits.

The patch looks sane, and if OpenBSD is anyway building with this, it's
probably good to pick it up.

Post by Jan Stary
$OpenBSD$
--- src/sndio.c.orig Mon Jan 30 04:01:44 2012
+++ src/sndio.c Tue Feb 9 23:23:00 2016
@@ -113,8 +113,6 @@ static int startany(sox_format_t *ft, unsigned mode)
else
reqpar.rchan = ft->signal.channels;
}
- if (ft->signal.precision > 0)
- reqpar.bits = ft->signal.precision;
switch (ft->encoding.encoding) {
reqpar.sig = 1;
@@ -127,6 +125,12 @@ static int startany(sox_format_t *ft, unsigned mode)
}
if (ft->encoding.bits_per_sample > 0)
reqpar.bits = ft->encoding.bits_per_sample;
+ else if (ft->signal.precision > 0)
+ reqpar.bits = ft->signal.precision;
+ else
+ reqpar.bits = SOX_DEFAULT_PRECISION;
+ reqpar.bps = (reqpar.bits + 7) / 8;
+ reqpar.msb = 1;
if (ft->encoding.reverse_bytes != sox_option_default) {
reqpar.le = SIO_LE_NATIVE;
if (ft->encoding.reverse_bytes)

--
Måns Rullgård

------------------------------------------------------------------------------

Jan Stary

2016-09-20 21:36:28 UTC

Post by Eric Wong
Seems reasonable; but I have little experience with using sndio
with Måns sox, much less the API. Perhaps Måns has an opinion,
too.
Fwiw, it would be helpful if example files could be
provided/generated showing where the improvement is.

Any 24bit PCM file will exemplify this.

Post by Eric Wong
Thanks. Sorry for the late response; I've been mostly
unplugged lately :)

No problem :-)

Post by Eric Wong
I'm not familiar with sndio, but it looks like it changes how 24-bit
samples are passed, packed rather than padded to 32 bits.

Yes, that's the point. With this patch,
SoX asks the underlying audio subsystem
to actually use the paramaters of the audio format.

(Whether the audio subsytem supports those parameters
is another question of course; the sndio sytem, as many other
sound daemons, will convert to and from almost anything.)

Post by Eric Wong
The patch looks sane, and if OpenBSD is anyway building with this, it's
probably good to pick it up.

Yes, the OpenBSD port of SoX is using this.
(I am not sure whether Alex actually commited yet.)
http://marc.info/?l=openbsd-ports&m=147395657332262&w=2

Changes to sndio.c should not affect anything else in SoX I believe.

Jan

------------------------------------------------------------------------------

Eric Wong

2016-09-20 22:24:28 UTC

Post by Eric Wong
with Måns sox,

Sorry, copy+paste error since I can't type 'å' properly :x

Post by Eric Wong
The patch looks sane, and if OpenBSD is anyway building with this, it's
probably good to pick it up.

Yes, the OpenBSD port of SoX is using this.
(I am not sure whether Alex actually commited yet.)
http://marc.info/?l=openbsd-ports&m=147395657332262&w=2
Changes to sndio.c should not affect anything else in SoX I believe.

Thanks, it's sitting in my "pu" branch on git://80x24.org/sox
with Alex credited as the author.

commit ea4f3fb3c6de93e10786fbfc1cb1bdf9006bbcdb
Author: Alexandre Ratchov <***@caoua.org>
Date: Tue Sep 20 22:00:40 2016 +0000

sndio: handle 24-bit samples properly on OpenBSD

Reported-by: Jan Stary <***@stare.cz>
cf. http://marc.info/?l=openbsd-ports&m=147395657332262&w=2

------------------------------------------------------------------------------

Jan Stary

2016-09-21 05:45:48 UTC

Post by Eric Wong
with Måns sox,

Sorry, copy+paste error since I can't type 'å' properly :x

Post by Eric Wong
The patch looks sane, and if OpenBSD is anyway building with this, it's
probably good to pick it up.

Yes, the OpenBSD port of SoX is using this.
(I am not sure whether Alex actually commited yet.)
http://marc.info/?l=openbsd-ports&m=147395657332262&w=2
Changes to sndio.c should not affect anything else in SoX I believe.

Thanks, it's sitting in my "pu" branch on git://80x24.org/sox
with Alex credited as the author.

Thank you.

What is the relation of this git branch at git://80x24.org/sox
to the actual SoX repository at git://git.code.sf.net/p/sox/code ?

Post by Eric Wong
commit ea4f3fb3c6de93e10786fbfc1cb1bdf9006bbcdb
Date: Tue Sep 20 22:00:40 2016 +0000
sndio: handle 24-bit samples properly on OpenBSD

That's a bit misleading: it will properly handle
whatever bitwidth SoX asks for. The 24 vs 32 was
just the initiating problem.

------------------------------------------------------------------------------

Eric Wong

2016-09-21 06:58:52 UTC

Post by Eric Wong
Thanks, it's sitting in my "pu" branch on git://80x24.org/sox
with Alex credited as the author.

Thank you.
What is the relation of this git branch at git://80x24.org/sox
to the actual SoX repository at git://git.code.sf.net/p/sox/code ?

Just another git repository. I don't have rights on SF,
nor do I much care for the URL[*]

And I believe git URLs should be treated equally (so I reject
the Embrace, Extend, ... concept of git hosts which add
proprietary features to git).

I'm just keeping my git repo on git://80x24.org/sox up-to-date
so Chris et al. can more easily resume maintenance someday
without having to dig through bug tracker + archives.

[*] Except sf.net makes the usability mistake of having
"code" as the basename, making the user type more when
cloning (or end up with a directory named "code").
Back in the day, gitorious.org made a similar mistake,
which I think hurt their adoption.

Post by Eric Wong
commit ea4f3fb3c6de93e10786fbfc1cb1bdf9006bbcdb
Date: Tue Sep 20 22:00:40 2016 +0000
sndio: handle 24-bit samples properly on OpenBSD

That's a bit misleading: it will properly handle
whatever bitwidth SoX asks for. The 24 vs 32 was
just the initiating problem.

Oops. Can you propose a standalone commit message which
I can copy and use directly? Thanks.

(I much prefer commit messages written with the patch
submission itself, basically, same conventions as
Linux kernel and git.git development).

------------------------------------------------------------------------------

Jan Stary

2016-09-21 07:19:26 UTC

Post by Eric Wong
commit ea4f3fb3c6de93e10786fbfc1cb1bdf9006bbcdb
Date: Tue Sep 20 22:00:40 2016 +0000
sndio: handle 24-bit samples properly on OpenBSD

That's a bit misleading: it will properly handle
whatever bitwidth SoX asks for. The 24 vs 32 was
just the initiating problem.

Oops. Can you propose a standalone commit message which
I can copy and use directly? Thanks.

I can't speak for Alex, but I suppose
"Ask OpenBSD's sndio to use bits_per_sample"
would be appropriate.

Jan

------------------------------------------------------------------------------

Alexandre Ratchov

2016-09-21 11:42:17 UTC

Post by Eric Wong
commit ea4f3fb3c6de93e10786fbfc1cb1bdf9006bbcdb
Date: Tue Sep 20 22:00:40 2016 +0000
sndio: handle 24-bit samples properly on OpenBSD

That's a bit misleading: it will properly handle
whatever bitwidth SoX asks for. The 24 vs 32 was
just the initiating problem.

Oops. Can you propose a standalone commit message which
I can copy and use directly? Thanks.

I can't speak for Alex, but I suppose
"Ask OpenBSD's sndio to use bits_per_sample"
would be appropriate.

Agreed. Without this diff, the requested bits_per_sample (part of
the encoding) was not taken into account: the code used the signal
precision instead, which is not the same all the time.

------------------------------------------------------------------------------

Måns Rullgård

2016-09-21 11:19:59 UTC

Post by Eric Wong
commit ea4f3fb3c6de93e10786fbfc1cb1bdf9006bbcdb
Date: Tue Sep 20 22:00:40 2016 +0000
sndio: handle 24-bit samples properly on OpenBSD

That's a bit misleading: it will properly handle
whatever bitwidth SoX asks for. The 24 vs 32 was
just the initiating problem.
From what I can tell, it makes a difference only for non-power-of-2

sample widths. 24 is the only commonly used such value.

--
Måns Rullgård

------------------------------------------------------------------------------

Alexandre Ratchov

2016-09-21 11:42:56 UTC