sound: Remove unused dsp_cdevs[] fields and devices
ClosedPublic
Actions

Authored by christos on Apr 29 2024, 4:00 PM.

Details

Reviewers

markj
emaste
dev_submerge.ch

Commits

rG6f6cc9324131: sound: Remove unused dsp_cdevs[] fields and devices
rG72cbd381075a: sound: Remove unused dsp_cdevs[] fields and devices

Summary

These fields and devices are unused as of e8c0d15a64fa ("sound: Get rid of snd_clone
and use DEVFS_CDEVPRIV(9)").

Sponsored by: The FreeBSD Foundation
MFC after: 1 week

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 57587
Build 54475: arc lint + arc unit

Event Timeline

christos created this revision.Apr 29 2024, 4:00 PM

Herald added a subscriber: imp. · View Herald TranscriptApr 29 2024, 4:00 PM

christos requested review of this revision.Apr 29 2024, 4:00 PM

Harbormaster completed remote builds in B57448: Diff 137846.Apr 29 2024, 4:00 PM

markj accepted this revision.Apr 30 2024, 12:06 AM

This revision is now accepted and ready to land.Apr 30 2024, 12:06 AM

dev_submerge.ch accepted this revision.Apr 30 2024, 10:47 PM

how does e.g. /dev/dspW map to 16-bit now?

In D45013#1026741, @emaste wrote:

how does e.g. /dev/dspW map to 16-bit now?

It doesn't since e8c0d15a64fa, and in fact, I just realized that accessing them doesn't work, as is expected. What we can do is:

Make them /dev/dsp aliases, like the OSS-compat devices.
Remove them.

I am more inclined to do the former, just in case there's a program that uses /dev/dspW, /dev/audio or /dev/dspcd. @dev_submerge.ch what do you think?

In D45013#1026747, @christos wrote:

In D45013#1026741, @emaste wrote:

how does e.g. /dev/dspW map to 16-bit now?

It doesn't since e8c0d15a64fa, and in fact, I just realized that accessing them doesn't work, as is expected. What we can do is:

Make them /dev/dsp aliases, like the OSS-compat devices.

Remove them.

I am more inclined to do the former, just in case there's a program that uses /dev/dspW, /dev/audio or /dev/dspcd. @dev_submerge.ch what do you think?

I've never seen them used by any application, but I concur to keep the aliases for now. We change a lot of things currently, which will produce already enough bug reports when it hits the RELEASEs.

I don't think there's a lot of value in keeping the aliases, if we don't also keep the behaviour -- i.e., if some application does expect to open /dev/dspW and send 16-bit samples without doing any other configuration it's not going to work as expected.

In D45013#1026750, @emaste wrote:

I don't think there's a lot of value in keeping the aliases, if we don't also keep the behaviour -- i.e., if some application does expect to open /dev/dspW and send 16-bit samples without doing any other configuration it's not going to work as expected.

True, but if we delete them the program will just exit, which I don't know if it's any better. I am not very keen on keeping those aliases either however, so I am open to suggestions.

IMO we should either remove them or restore default formats.

In D45013#1026752, @christos wrote:

In D45013#1026750, @emaste wrote:

I don't think there's a lot of value in keeping the aliases, if we don't also keep the behaviour -- i.e., if some application does expect to open /dev/dspW and send 16-bit samples without doing any other configuration it's not going to work as expected.

True, but if we delete them the program will just exit, which I don't know if it's any better. I am not very keen on keeping those aliases either however, so I am open to suggestions.

You're right, unless we can preset format and speed defaults for these clones they are not very useful. We have alternatives for people who want to just cat some wave files, e.g. the vchan sysctls or virtual_oss. I think we can remove them, it's not worth the effort.

In D45013#1027043, @dev_submerge.ch wrote:

In D45013#1026752, @christos wrote:

In D45013#1026750, @emaste wrote:

I don't think there's a lot of value in keeping the aliases, if we don't also keep the behaviour -- i.e., if some application does expect to open /dev/dspW and send 16-bit samples without doing any other configuration it's not going to work as expected.

True, but if we delete them the program will just exit, which I don't know if it's any better. I am not very keen on keeping those aliases either however, so I am open to suggestions.

You're right, unless we can preset format and speed defaults for these clones they are not very useful. We have alternatives for people who want to just cat some wave files, e.g. the vchan sysctls or virtual_oss. I think we can remove them, it's not worth the effort.

No objection. Do you think we should also remove the OSSv4 aliases?

In D45013#1027058, @christos wrote:

No objection. Do you think we should also remove the OSSv4 aliases?

I would keep them, unless there's a reason not to - these are documented as part of OSSv4. The example scripts there make use of these aliases, maybe it's even possible to run them unchanged (didn't try). And it's still the best (only?) reference for people that want to use our OSS API, even if there's quite some differences.

christos retitled this revision from sound: Remove unused dsp_cdevs[] fields to sound: Remove unused dsp_cdevs[] fields and devices.May 1 2024, 9:59 PM

christos edited the summary of this revision. (Show Details)

Remove /dev/dspW, /dev/audio and /dev/dspcd.

This revision now requires review to proceed.May 1 2024, 10:03 PM

Harbormaster completed remote builds in B57521: Diff 137987.May 1 2024, 10:03 PM

Some of the types (SND_DEV_AUDIO, SND_DEV_DSP16, SND_DEV_DSPHW_CD) in sound.h are unused now, I think. Should we get rid of them or mark them deprecated?

Remove unused SND_DEV_* defines from pcm/sound.h. Convert the list to an enum.

Harbormaster completed remote builds in B57587: Diff 138148.May 5 2024, 3:43 PM

Ok for me except for the style, see inline comment. Having separate pcm devices for AC3, SPDIF etc may not be appropriate, but we should probably find a way to integrate these settings into the mixer or a future user space audioctl tool.