feat: introduce description() for devices

[roderickvd]

Introduces DeviceTrait::description() and deprecates DeviceTrait::name() to clarify the distinction between human-readable descriptions and unique device identifiers.

Motivation

The current name() implementation was inconsistent across platforms - sometimes returning unique IDs, sometimes human-readable names, and sometimes a mix of both. In this state, examples don't work as expected when --device foo is given on the command line, and neither will user applications that match on device name.

For example: on ALSA, name() called self.to_string() which combined the PCM ID and description. For a device with PCM ID "hw:0,0" and description "HD Audio", name() returned "hw:0,0 (HD Audio)". When a user passed --device hw:0,0, that string comparison failed.

In conclusion, this mixing of PCM ID and description broke device selection.

Solution

This split makes the API clearer: use description() for displaying to users, and id() for uniquely identifying devices. With this change:

• id() returns "alsa:hw:0,0" in our ALSA example - just the identifier, no description mixed in.
• description() returns "HD Audio" - just the human-readable name

Device selection does expect alsa:hw:0,0 instead of just hw:0,0. Through the power of #1014 that's precisely what we want, because if there are more hosts available on the platform, then we have devices unique to their named scopes.

Backwards compatibility

To ease upgrading, v0.16 and earlier users relying on name() can still use that and will get the same results as they did before. In our ALSA example: just "hw:0,0" without the "alsa:" prefix.

[roderickvd]

cc: @xephyris - currently the device selection was quite broken and this PR leverages your stable IDs to fix that. I'd like to fix this breakage and agree on how this should work before v0.17 - your perspectives are appreciated!

[xephyris]

This seems to be a great idea, however one suggestion I have is if we are doing a description() method, would it be possible to have it return more data about the device?

A struct for this with potentially more data about things such as device type (Input/Output), channel information, or some other important details as well as the human readable device name seems more fitting of a description() method, especially because right now there seems to be significant overlap between the name() and description() methods.
I think for 0.17 release, it doesn't have to have too much information, however this would leave ample room for expansion in the future.

Another not too significant change is I think we might want to create a new DeviceDescriptionError instead of reusing DeviceNameError, as it reduces any confusion that could arise from finding out where this error came from.

What are your thoughts?

[roderickvd]

Great idea. As well as address, type (headphones…) for those that support it. It could become a struct with a bunch of Options and a fmt::Display implementation for a default to_string() implementation.

[roderickvd]

Working on it. Gonna be a big one - anything that stretches across all hosts is.
Do feel free to comment on this type I've cooked up:

pub struct DeviceDescription {
    /// Human-readable device name (always available).
    name: String,

    /// Device manufacturer or vendor name (macOS, Windows).
    manufacturer: Option<String>,

    /// Driver name (ALSA, ASIO).
    /// Example: ASIO4ALL v2
    driver: Option<String>,

    /// Categorization of device type (defaults to `Unknown` if not available).
    /// Examples: BuiltIn, Usb, Bluetooth, WiredHeadset, ... , Unknown
    device_type: DeviceType,

    /// Connection/transport type (defaults to `Unknown` if not available).
    /// Examples: BuiltIn, Usb, Bluetooth, Pci, FireWire, ... , Unknown
    bus_type: BusType,

    /// Direction: input, output, or duplex (defaults to `Unknown` if not known).
    direction: DeviceDirection,

    /// Physical address or connection identifier (Android).
    /// Example: 00:1A:7D:DA:71:13
    address: Option<String>,

    /// Additional description lines with detailed information (ALSA, WASAPI).
    ///
    /// On ALSA, this contains the multi-line description from hints.
    /// On WASAPI, this may contain additional device description info.
    extended: Vec<String>,

    /// Name of the audio interface/adapter (WASAPI).
    /// Example: Focusrite USB Audio
    interface_name: Option<String>,

    /// Container identifier for grouping devices from same hardware (WASAPI).
    container_id: Option<String>,
}

[roderickvd]

@xephyris on Discord, @jwagner wrote:

The DeviceId enum looks a bit clunky I think that might as well be (HostId, String) given that the only exceptions to that are aaudio, and null which can be trivially mapped to a string. And all ids need to be mappable to strings anyways.

Which I think is an interesting thought. Do you see any reason why we wouldn't flatten it like that?

[xephyris]

I think there are benefits to both approaches, and I would be willing to make a pull request to change the id system if the general consensus is to have a (HostId, String) structure.

The main benefits I see from such a method would be greater simplicity for the DeviceId enum and an easier path towards adding more APIs.

The main benefit of the current method, would be human readability and also API specific implementations. While it is a bit clunky, I think it being more specific here can help to prevent errors that could arise from incorrect host matching.

Also just a question, how would we be able to implement HostId matching if we were to do this implementation? From my limited testing it seems like if I do a match host.id(), only the supported APIs would show up, like in the case of macOS, the only error I get is that the "non-exhaustive patterns: HostId::CoreAudio not covered".

[jwagner]

Hey @xephyris, I totally agree that both approaches have their merits. I'm not quite sure what you mean with 'incorrect host matching' and 'HostId matching'?

[xephyris]

By host matching I mean when we are trying to determine what API is being used currently or what host type the id belongs to.

For example, right now for the from_string() method in DeviceId, we are matching to a specific API member

match platform {
    "wasapi" => Ok(DeviceId::WASAPI(data.to_string())),
    "asio" => Ok(DeviceId::ASIO(data.to_string())),
    "coreaudio" => Ok(DeviceId::CoreAudio(data.to_string())),
    "alsa" => Ok(DeviceId::ALSA(data.to_string())),
    "aaudio" => {
        let id = data.parse().map_err(|_| DeviceIdError::BackendSpecific {
            err: BackendSpecificError {
                description: format!("Failed to parse aaudio device id: {}", data),
            },
        })?;
        Ok(DeviceId::AAudio(id))
    }
    "jack" => Ok(DeviceId::Jack(data.to_string())),
    "webaudio" => Ok(DeviceId::WebAudio(data.to_string())),
    "emscripten" => Ok(DeviceId::Emscripten(data.to_string())),
    "ios" => Ok(DeviceId::IOS(data.to_string())),
    "null" => Ok(DeviceId::Null),
    &_ => todo!("implement DeviceId::FromStr for {platform}"),
 }

I'm just unsure about how would such an implementation work with HostId's, as there doesn't seem to be an effective way to match it, as only the device's supported platform APIs seems to show up.
For HostId matching errors, that would mainly be on the user end, as there doesn't seem to be anywhere with a concrete list of all the possible HostIds, so I think by making the enum with fixed types, errors in matching HostIds could be avoided.

HostId also doesn't seem to currenly support to_string(), although that could be easily resolved.

What are your thoughts? If there are any misunderstandings that I have, I would be happy to learn more.

[jwagner]

I think from_string could then be (HostId::from_string, data). Assuming HostId has from_string, if not it can be added.

But maybe I'm also missing distinction between what a Host(Id) and a platform is.

I didn't consider the case where you would want to deserialize a DeviceId from an unsupported Host. If that is something we want to support, and we don't want unsupported Hosts to show up in the HostId enum then another enum is required.

On a side note I think we might want to turn the

            &_ => todo!("implement DeviceId::FromStr for {platform}"),

into a Err(DeviceIdError::UnsupportedPlatform).

This would avoid a panic when trying to read a value that came from a newer version, like when an older version of an application reads a state from a newer version.

[roderickvd]

@xephyris @jwagner I gave the DeviceId simplification to (HostId, String a go in c77da59. I think this is a good middle ground. Let me know what you think.

[jwagner]

Looks good to me @roderickvd. I think the trade-off that it is no longer possible to parse an id from an unsupported host is good, given the simplification it brings to the code, but I'm biased. :)

[xephyris]

This definitely looks cleaner and simpler, however I believe the issue I mentioned above isn't quite addressed.

Is there any way to easily add in something that lists all of the supported hosts? This is mainly for extending outside of cpal, however I still feel like it is important, especially for the project I'm working on, cpvc. It would be useful to get a list of some sort that lets me process DeviceIds differently based on the API, the HostId method slightly complicates this as there doesn't seem to be any way to match with HostIds, especially ones that aren't supported in the platform.

Right now I'm on macOS and if I try to match by HostId, the only member is CoreAudio.

pub fn from_cpal_id(device_id: DeviceId) -> Self {
        match device_id.0 {
            HostId::CoreAudio => todo!(),
        }
    }

If I try to add another HostId, such as Jack, I get an error.

no variant or associated item named `Jack` found for enum `cpal::HostId` in the current scope
variant or associated item not found in `cpal::HostId`

If there is an easy approach to resolve this, that doesn't require a bulky enum, that'd be great, but as of now, I still lean towards the old approach.

[roderickvd]

You could now match on the string representation:

match device_id.0.to_string().as_str() {
    "coreaudio" => todo!(),
    "jack" => todo!(),
    // ...
}

[xephyris]

That could work nicely, although one thing to point out is it does leave more room for error, as the strings could get messed up when coding leading to undesired behavior.

Maybe we could add a list of all the possible hosts in the documentation? That way it is easier to tell what APIs are supported, although it is less concrete than an enum.

[roderickvd]

Can do. Risk is that the documentation will go stale when we add a host and forget to update the documentation, but that's just what it is.

[roderickvd]

This PR has become quite extensive, so I'm gonna merge it to let it simmer for a while. That's a call out to everyone to test it out, thanks!