sound: Implement AFMT_FLOAT support

[christos]

Summary

Even though the OSS manual [1] advises against using AFMT_FLOAT, there
are applications that expect the sound driver to support it, and might
not work properly without it.

This patch adds AFMT_F32_LE|BE (as well as AFMT_FLOAT for OSS
compatibility) in sys/soundcard.h and implements AFMT_F32_LE|BE <->
AFMT_S32_LE|BE conversion functions. As a result, applications can
write/read floats to/from sound(4), but internally, because sound(4)
works with integers, we convert floating point samples to integer ones,
before doing any processing.

[1] http://manuals.opensound.com/developer/AFMT_FLOAT.html

PR: 157050, 184380, 264973, 280612, 281390
Sponsored by: The FreeBSD Foundation
MFC after: 1 week

Test Plan

Test with D47639 for AFMT_F32_LE (if your machine is little endian). To test AFMT_F32_BE explicitly, you can apply the following patch to beep(1) instead:

diff --git a/usr.bin/beep/beep.c b/usr.bin/beep/beep.c
index 151236b4825b..96e13cc86e9d 100644
--- a/usr.bin/beep/beep.c
+++ b/usr.bin/beep/beep.c
@@ -152,7 +152,7 @@ usage(void)
 int
 main(int argc, char **argv)
 {
-       int32_t *buffer;
+       float *buffer;
        size_t slope;
        size_t size;
        size_t off;
@@ -208,9 +208,9 @@ main(int argc, char **argv)
        if (ioctl(f, SOUND_PCM_WRITE_CHANNELS, &c) != 0)
                errx(1, "ioctl SOUND_PCM_WRITE_CHANNELS(1) failed");

-       c = AFMT_S32_NE;
+       c = AFMT_F32_BE;
        if (ioctl(f, SNDCTL_DSP_SETFMT, &c) != 0)
-               errx(1, "ioctl SNDCTL_DSP_SETFMT(AFMT_S32_NE) failed");
+               errx(1, "ioctl SNDCTL_DSP_SETFMT(AFMT_F32_BE) failed");

        if (ioctl(f, SNDCTL_DSP_SPEED, &sample_rate) != 0)
                errx(1, "ioctl SNDCTL_DSP_SPEED(%d) failed", sample_rate);
@@ -251,7 +251,15 @@ main(int argc, char **argv)
                else if (off > (size - slope))
                        sample = sample * (size - off - 1) / (float)slope;

-               buffer[off] = sample * 0x7fffff00;
+               int32_t tmp;
+               memcpy(&tmp, &sample, sizeof(tmp));
+               tmp = ((uint32_t)tmp & 0x000000ff) << 24 |
+                   (tmp & 0x0000ff00) << 8 |
+                   (tmp & 0x00ff0000) >> 8 |
+                   (tmp & 0xff000000) >> 24;
+               memcpy(&sample, &tmp, sizeof(sample));
+
+               buffer[off] = sample;
        }

        if (write(f, buffer, size * sizeof(buffer[0])) !=

[Alexander88207_protonmail.com]

Hello,

i can confirm so far that it works fine with emulators/wine.

I did took 2 FMOD games for testing (Way of the Hunter and Roboquest) that required AFMT_FLOAT.

Thank you for your work!

[emaste]

sys/dev/sound/pcm/feeder_format.c

58

I'm a bit worried about a possible performance impact, and we might need to hoist the fpu_kern_enter / fpu_kern_leaave up a level, but we can profile it and see what the impact actually is.

[dev_submerge.ch]

Very welcome, thanks for tackling this @christos. Please note that there are sound cards supporting float format, and I intend to experiment with that some time in the future (probably needs bitperfect for now).

sys/dev/sound/pcm/feeder_format.c

58

I'm a bit worried about a possible performance impact, and we might need to hoist the fpu_kern_enter / fpu_kern_leaave up a level, but we can profile it and see what the impact actually is.

+1. Also the way this function is called through a function pointer for every sample seems quite inefficient, at least I don't see how it could be inlined there. Means a restructuring could help all format conversions, not just float.

Although I see moving format conversions out to userland as the better option in the middle to long term, anyway.

[christos]

sys/dev/sound/pcm/feeder_format.c

58

I will second Florian's idea about optimizing the conversion framework in general, as long as we keep doing this in the kernel and not userland.

For the fpu_kern(9) functions specifically, I think they could be moved around the do-while in feed_format_feed(), as Ed suggested in private, although I haven't tested this yet. This will make the code a bit uglier, because we'll have to check for AFMT_FLOAT in feed_format_feed() so that we can call fpu_kern_enter()/fpu_kern_leave(), but it will most likely give us a good performance boost.

[christos]

sys/dev/sound/pcm/feeder_format.c

58

Also I am wondering whether we need to care about endianness.

[dev_submerge.ch]

sys/dev/sound/pcm/feeder_format.c

58

Also I am wondering whether we need to care about endianness.

Good point. In principle there's AFMT_FLOAT_BE and AFMT_FLOAT_LE, and what we currently support here as AFMT_FLOAT is actually AFMT_FLOAT_NE. I don't think it matters with audio data from userland, that should always be native endian. But it would matter when going to a sound card, e.g. a RME HDSPe card in a PowerPC big endian machine.

I suppose we should have a closer look. For now we could at least define the different endianness formats for floats, similar to the integer formats.

58

I will second Florian's idea about optimizing the conversion framework in general, as long as we keep doing this in the kernel and not userland.

The remark on doing conversion in userland was meant as a perspective on how much effort we should invest here, not as a concrete suggestion - sorry for the confusion.

For the fpu_kern(9) functions specifically, I think they could be moved around the do-while in feed_format_feed(), as Ed suggested in private, although I haven't tested this yet. This will make the code a bit uglier, because we'll have to check for AFMT_FLOAT in feed_format_feed() so that we can call fpu_kern_enter()/fpu_kern_leave(), but it will most likely give us a good performance boost.

If my assumptions about the code are correct, we do two function calls in feed_format_feed() (read and write) for each sample, with no inlining at all. That would be terribly inefficient, for all formats. Ideally we could specialize the whole read-write-loop instead of just the read / write functions, thereby speed up all format conversions and include the fpu_kern_enter()/fpu_kern_leave() in the float specializations. Unfortunately, this creates a whole lot of specializations for all format combinations.

Does that make sense? What is a good way to measure this in the kernel?

[christos]

• Depend on D47932.
• Implement little and big endian support.
• Make AFMT_FLOAT usable from dev.pcm.X.play|rec.vchanformat.

[christos]

sys/dev/sound/pcm/feeder_format.c

58

I think we can move the discussion to D47932. I cleaned up the framework quite a lot IMHO so we can work on it easier now.

[christos]

Remove early return in pcm_sample_read().

[christos]

Bump.

[christos]

Fix shifting error I accidentally introduced while rewriting some parts.

[christos]

Use __bswap32() instead of doing the LE <-> BE conversion manually.

As per kevans@ and imp@ on IRC, since v is an int32_t, it shouldn't be a
problem that __bswap32() expects and returns a uint32_t, because the return
value is implicitly casted.

[imp]

sys/dev/sound/pcm/feeder_format.c

141

Why not just bswap32?

258

Bswap32?

300

Bswap32?

[dev_submerge.ch]

I have a hard time to apply these interdependent patches cleanly to my tree. @christos, do you update all affected patches in the series when doing a rebase?

sys/dev/sound/pcm/feeder_format.c

141

There's discussion relevant to this in D47932.