gsoc-2021 final review: sound mixer improvements

[christos]

Summary

mixer(3): OSS mixer library.
mixer(8): complete rewrite of the old mixer(8) utility.
sound(4): new ioctls and sysctls.
/etc/rc.d/mixer: modified to run with the new mixer(8).

Wiki article: https://wiki.freebsd.org/SummerOfCode2021Projects/SoundMixerImprovements
This project has been part of Google Summer of Code 2021.

[• hselasky]

NOTE: The kernel changes in this diff are already in 14-current.

[• hselasky]

mixer_lib/Makefile

8 ↗

(On Diff #94024)

What requires "-lm" ?

mixer_lib/mixer.h

35 ↗

(On Diff #94024)

Why do you need to include <math.h> ?

[christos]

mixer_lib/Makefile

8 ↗

(On Diff #94024)

The MIX_VOLDENORM macro.

mixer_lib/mixer.h

35 ↗

(On Diff #94024)

For the MIX_VOLDENORM macro.

[christos]

Removed useless files.

[• hselasky]

mixer_lib/mixer.h

68 ↗

(On Diff #94052)

Can we change this to:

#define MIX_VOLDENORM(v)	((int)((v) * 100.0f + 0.5f))

roundf() handles some special cases like negative values and infinite values. I think we don't need that.

Then we can get rid of the libm dependency and math.h .

[christos]

Removed math.h dependency.

[christos]

mixer.3: updated MIX_VOLDENORM definition.

[• hselasky]

mixer_lib/mixer.h

26 ↗

(On Diff #94166)

__BEGIN_DECLS

Should be after the include of sys/cdefs.h

The FBSDID is only for C-files. Please remove and change it into:

• $FreeBSD$

At the end of the copyright notice. See other files for an example.

36 ↗

(On Diff #94166)

I would prefer 1U << (n) instead of 1 << n

[christos]

Added $FreeBSD$ tags.

[dmitryluhtionov_gmail.com]

mixer_lib/mixer.c

264 ↗

(On Diff #94166)

Post-assignment check

mixer_lib/mixer.h

104 ↗

(On Diff #94166)

Why mixer_close() return any value ?
In all the code it is called as (void)mixer_close(m);

mixer_prog/mixer_prog.c

289 ↗

(On Diff #94166)

dunit not a unsigned integer

349 ↗

(On Diff #94166)

mixer_get_ctl() may return NULL
cp->name - can be deference pointer

[dmitryluhtionov_gmail.com]

mixer_prog/mixer_prog.c

289 ↗

(On Diff #94166)

maybe it makes sense to make it like unsigned int ?

[christos]

mixer(3): fixed post assignment check in mixer_remove_ctl.
mixer(8): added error handling in initctls.

mixer_lib/mixer.c

264 ↗

(On Diff #94166)

Fixed.

mixer_lib/mixer.h

104 ↗

(On Diff #94166)

The library is meant to be used by mixer programs in general, so other
programs might want to check the return value of mixer_close().

mixer_prog/mixer_prog.c

349 ↗

(On Diff #94166)

Edited initctls() so we can continue only if everything went correctly in the control initialization.

[dmitryluhtionov_gmail.com]

mixer_prog/mixer_prog.c

316 ↗

(On Diff #94236)

mix_ctl_t *
mixer_get_ctl(struct mix_dev *d, int id)
{
	mix_ctl_t *cp;

	TAILQ_FOREACH(cp, &d->ctls, ctls) {
		if (cp->id == id)
			return (cp);
	}
	errno = EINVAL;

	return (NULL);
}

below

	warn("%s.%s=%.2f:%.2f", 
			    m->dev->name, cp->name, v.left, v.right);

without any checks

[christos]

mixer_prog/mixer_prog.c

316 ↗

(On Diff #94236)

warn() is called in case mixer_set_vol() fails -- this doesn't
have to do with what mixer_get_ctl() returns. As I said in the
previous comments, these calls to mixer_get_ctl() are
guaranteed to work since error checks are done in inictls.

[• hselasky]

I'm pulling this in. Let's see what happens.