Rename stream Format types to StreamConfig and other related renamings.
#371
Conversation
This implements the changes described at RustAudio#370. This commit implements only the `null` and `alsa` backends - the rest will be implemented in follow-up commits. Closes RustAudio#370.
mitchmindtree
force-pushed
the
stream_config
branch
from
ee4e24f to
8785f63
Compare
|
I've also added a impl From<SupportedStreamConfig> for StreamConfigimplementation, but I'm unsure if it would be better to have an inherent impl on fn config(&self) -> StreamConfiginstead? Perhaps both? My thinking is the inherent impl may 1. make it easier to avoid type inference ambiguity and 2. reduce the need for |
mitchmindtree
force-pushed
the
stream_config
branch
from
8785f63 to
e139bfa
Compare
mitchmindtree
force-pushed
the
stream_config
branch
from
e139bfa to
07b66f5
Compare
msiglreith
left a comment
msiglreith
left a comment
There was a problem hiding this comment.
No strong opinion here on the name changes, even though aligning it with khronos speech would be appealing (Info/Properties/Capabilities)
| /// Please note that `Device`s may become invalid if they get disconnected. Therefore all the | ||
| /// methods that involve a device return a `Result` allowing the user to handle this case. | ||
| pub trait DeviceTrait { | ||
| /// The iterator type yielding supported input stream formats. |
There was a problem hiding this comment.
format should be replaced, same below
There was a problem hiding this comment.
Agh I knew I'd miss one somewhere
src/lib.rs
Outdated
|
|
||
| impl SupportedStreamConfig { | ||
| /// Construct a `SupportedStreamConfig` from an existing `StreamConfig`. | ||
| pub fn from_config(conf: &StreamConfig, fmt: SampleFormat) -> Self { |
There was a problem hiding this comment.
supported confuses me a bit here as is it guarenteed that it is actually supported by the backend if I can just craft it myself?
There was a problem hiding this comment.
Good point! I remember this crossing my mind also now that you mention it, but I didn't get around to addressing it.
Perhaps we should make it so that SupportedStreamConfig's can only be constructed via either the Device or SupportedStreamConfigRange, make this from_config constructor pub(crate), and expose the fields via methods instead? This way any instance of a SupportedStreamConfig can assumed to actually be supported (edit: from the Device that created it that is... nothing stopping the user using it with a non-supporting device)?
It's worth keeping in mind that users intending to use build_input/output_stream_raw currently must use a SupportedStreamConfig, so it would be especially important that Device::supported_input/output_formats is accurate and thorough to ensure we aren't restricting users from opening streams that are in fact supported by the device and host. Alternatively, to avoid this issue we could change the build_input/output_stream_raw API to accept a StreamConfig and SampleFormat as separate arguments, rather than expecting a SupportedStreamConfig.
There was a problem hiding this comment.
The proposed change to the raw stream APIs seems natural, but bear in mind that this is a type with public fields, which can therefore be constructed by hand regardless.
This should give the user a higher confidence that, if they have a `SupportedStreamConfig` format type that it is actually supported. Also updates the `raw` stream builder methods to take a `StreamConfig` and `SampleFormat` as separate arguments for flexibility. **Backends Updated** - [x] null - [x] alsa - [ ] emscripten - [ ] coreaudio - [ ] wasapi - [ ] asio
Restrict the ways in which `SupportedStreamConfig/Range`s can be constructed
This rebases RustAudio#372, addressing the recent changes introduced by RustAudio#397, RustAudio#395, and RustAudio#371 in the process. TODO: - [ ] Complete implementation of `callback` and `playback` timestamps in the output stream callback.
This rebases RustAudio#372, addressing the recent changes introduced by RustAudio#397, RustAudio#395, and RustAudio#371 in the process. TODO: - [ ] Complete implementation of `callback` and `playback` timestamps in the output stream callback.
3 participants
This implements the changes described at #370.
Closes #370.
Backends implemented:
Edit: here is the full list of renamings to make it easier for downstream crates to adapt:
Formattype'sdata_typefield renamed tosample_format.Shape->StreamConfig- The configuration input required to build a stream.Format->SupportedStreamConfig- Describes a single supported stream configuration.SupportedFormat->SupportedStreamConfigRange- Describes a range of supported configurations.Device::default_input/output_format->Device::default_input/output_config.Device::supported_input/output_formats->Device::supported_input/output_configs.Device::SupportedInput/OutputFormats->Device::SupportedInput/OutputConfigs.SupportedFormatsError->SupportedStreamConfigsErrorDefaultFormatError->DefaultStreamConfigErrorBuildStreamError::FormatNotSupported->BuildStreamError::StreamConfigNotSupported