snd_uaudio: skip duplicate configurations

[christos]

Summary

If the user has supplied non-default values through
hw.usb.uaudio.default_*, snd_uaudio(4) will create a duplicate entry if
it matches the same configuration twice. This is because
uaudio_chan_fill_info() will create both the user-supplied
configuration, as well as the ones it automatically detects.

Sponsored by: The FreeBSD Foundation
MFC after: 2 weeks

Test Plan

Before:

# sysctl hw.usb.uaudio.default_rate=44100
[...]
uaudio0: Play[0]: 44100 Hz, 2 ch, 32-bit S-LE PCM format, 2x2ms buffer.
uaudio0: Play[0]: 96000 Hz, 2 ch, 32-bit S-LE PCM format, 2x2ms buffer.
uaudio0: Play[0]: 88200 Hz, 2 ch, 32-bit S-LE PCM format, 2x2ms buffer.
uaudio0: Play[0]: 48000 Hz, 2 ch, 32-bit S-LE PCM format, 2x2ms buffer.
uaudio0: Play[0]: 44100 Hz, 2 ch, 32-bit S-LE PCM format, 2x2ms buffer.
uaudio0: Record[0]: 44100 Hz, 2 ch, 32-bit S-LE PCM format, 2x2ms buffer.
uaudio0: Record[0]: 96000 Hz, 2 ch, 32-bit S-LE PCM format, 2x2ms buffer.
uaudio0: Record[0]: 88200 Hz, 2 ch, 32-bit S-LE PCM format, 2x2ms buffer.
uaudio0: Record[0]: 48000 Hz, 2 ch, 32-bit S-LE PCM format, 2x2ms buffer.
uaudio0: Record[0]: 44100 Hz, 2 ch, 32-bit S-LE PCM format, 2x2ms buffer.

After:

# sysctl hw.usb.uaudio.default_rate=44100
[...]
uaudio0: Play[0]: 44100 Hz, 2 ch, 32-bit S-LE PCM format, 2x2ms buffer.
uaudio0: Play[0]: 96000 Hz, 2 ch, 32-bit S-LE PCM format, 2x2ms buffer.
uaudio0: Play[0]: 88200 Hz, 2 ch, 32-bit S-LE PCM format, 2x2ms buffer.
uaudio0: Play[0]: 48000 Hz, 2 ch, 32-bit S-LE PCM format, 2x2ms buffer.
uaudio0: Record[0]: 44100 Hz, 2 ch, 32-bit S-LE PCM format, 2x2ms buffer.
uaudio0: Record[0]: 96000 Hz, 2 ch, 32-bit S-LE PCM format, 2x2ms buffer.
uaudio0: Record[0]: 88200 Hz, 2 ch, 32-bit S-LE PCM format, 2x2ms buffer.
uaudio0: Record[0]: 48000 Hz, 2 ch, 32-bit S-LE PCM format, 2x2ms buffer.

[dev_submerge.ch]

The only case here is a duplicate sample rate, right? I'm uneasy to do this in the descriptor loop (why?).

I think it's much simpler and less invasive to integrate this with the changes in D43679. Would that be ok for you? I'll update it anyway today.

[christos]

In D43767#998111, @dev_submerge.ch wrote:

The only case here is a duplicate sample rate, right?

For my device yeah, but I'm not sure if more sophisticated devices would print configs with multiple different channels or bits, which is why I added a check for channels and bits as well.

I'm uneasy to do this in the descriptor loop (why?).
I think it's much simpler and less invasive to integrate this with the changes in D43679. Would that be ok for you? I'll update it anyway today.

We could add an inner loop in uaudio_chan_fill_info() indeed. The reason I did it in the descriptor loop was so that we only check against valid values (the loop is placed after we've made sure the rate/channel/bit config is supported), whereas in uaudio_chan_fill_info() we'd get check against non-supported values as well.

That being said, even if we move the loop in uaudio_chan_fill_info(), I think it's better to have 2 separate patches as these patches are not directly related.

[dev_submerge.ch]

In D43767#998113, @christos wrote:

In D43767#998111, @dev_submerge.ch wrote:

The only case here is a duplicate sample rate, right?

For my device yeah, but I'm not sure if more sophisticated devices would print configs with multiple different channels or bits, which is why I added a check for channels and bits as well.

I'm uneasy to do this in the descriptor loop (why?).
I think it's much simpler and less invasive to integrate this with the changes in D43679. Would that be ok for you? I'll update it anyway today.

We could add an inner loop in uaudio_chan_fill_info() indeed. The reason I did it in the descriptor loop was so that we only check against valid values (the loop is placed after we've made sure the rate/channel/bit config is supported), whereas in uaudio_chan_fill_info() we'd get check against non-supported values as well.

That being said, even if we move the loop in uaudio_chan_fill_info(), I think it's better to have 2 separate patches as these patches are not directly related.

No need for an inner loop. It's a simple if clause to skip the default value when we iterate through the sample rates. Which is practically the same as I'm doing with the other default values there.

[christos]

In D43767#998119, @dev_submerge.ch wrote:

In D43767#998113, @christos wrote:

In D43767#998111, @dev_submerge.ch wrote:

The only case here is a duplicate sample rate, right?

For my device yeah, but I'm not sure if more sophisticated devices would print configs with multiple different channels or bits, which is why I added a check for channels and bits as well.

I'm uneasy to do this in the descriptor loop (why?).
I think it's much simpler and less invasive to integrate this with the changes in D43679. Would that be ok for you? I'll update it anyway today.

We could add an inner loop in uaudio_chan_fill_info() indeed. The reason I did it in the descriptor loop was so that we only check against valid values (the loop is placed after we've made sure the rate/channel/bit config is supported), whereas in uaudio_chan_fill_info() we'd get check against non-supported values as well.

That being said, even if we move the loop in uaudio_chan_fill_info(), I think it's better to have 2 separate patches as these patches are not directly related.

No need for an inner loop. It's a simple if clause to skip the default value when we iterate through the sample rates. Which is practically the same as I'm doing with the other default values there.

True. It might indeed be better to merge this change with your patch.

[dev_submerge.ch]

Integrated into D43679. The parameter iteration shouldn't produce duplicate entries anymore.