Move SampleFormat check out of the main stream loop

[mitchmindtree]

Currently CPAL requires matching on an UnknownTypeBuffer enum upon every iteration of the audio loop, despite being able to query compatible sample formats prior to running the loop using Voice::format.

The beep example demonstrates how each format might be handled in this manner:

    loop {
        match channel.append_data(32768) {
            cpal::UnknownTypeBuffer::U16(mut buffer) => {
                for (sample, value) in buffer.chunks_mut(format.channels.len()).zip(&mut data_source) {
                    let value = ((value * 0.5 + 0.5) * std::u16::MAX as f32) as u16;
                    for out in sample.iter_mut() { *out = value; }
                }
            },

            cpal::UnknownTypeBuffer::I16(mut buffer) => {
                for (sample, value) in buffer.chunks_mut(format.channels.len()).zip(&mut data_source) {
                    let value = (value * std::i16::MAX as f32) as i16;
                    for out in sample.iter_mut() { *out = value; }
                }
            },

            cpal::UnknownTypeBuffer::F32(mut buffer) => {
                for (sample, value) in buffer.chunks_mut(format.channels.len()).zip(&mut data_source) {
                    for out in sample.iter_mut() { *out = value; }
                }
            },
        }

        channel.play();
    }

In any serious audio applications, the work contained in each branch would likely be required to be abstracted into some function, where the application either:

• Does all its work in one format and then converts the final buffer to the stream's sample format or
• Remains entirely generic over the sample type, using some Sample trait.

In either case, we likely end up using some generic function within the branching like so:

    loop {
        match channel.append_data(32768) {
            cpal::UnknownTypeBuffer::U16(mut buffer) =>
                fill_buffer_with_data::<u16>(&mut buffer, &mut data_source),
            cpal::UnknownTypeBuffer::I16(mut buffer) =>
                fill_buffer_with_data::<i16>(&mut buffer, &mut data_source),
            cpal::UnknownTypeBuffer::F32(mut buffer) =>
                fill_buffer_with_data::<f32>(&mut buffer, &mut data_source),
        }

        channel.play();
    }

Considering we already know whether or not certain formats are supported before we start the stream, we should be able to move this branching to occur before the stream even begins:

fn main() {
    let endpoint = cpal::get_default_endpoint().expect("Failed to get default endpoint");

    if run_stream::<f32>(&end_point, &mut data_source).is_ok() {}
    else if run_stream::<i32>(&end_point, &mut data_source).is_ok() {}
    else if run_stream::<i16>(&end_point, &mut data_source).is_ok() {}
    ...
    else {
        panic!("No compatible audio stream formats found for the device");
    }
}

This could of course be refactored to only try different sample formats if some specific UnsupportedFormat error is returned, or by iterating on the EndPoint's supported formats, matching on them and attempting to run the stream that way.

I'm curious to get your thoughts on this, as the current in-loop matching seems both unnecessary and to question whether the sample_format field in the Format struct yielded by endpoint.get_supported_formats_list() is useful at all?

[mitchmindtree]

This has become impossible to implement since the introduction of the EventLoop type, as all streams must now be handled within a single callback, and each stream might be of some different sample format depending on the device it is associated with. #278 may make this possible again, but some serious thought has to be put into how to achieve this while being able to support web audio APIs (see this comment).

[ishitatsuyuki]

I think I'll implement this sometime after the EventLoop deal, but one question is: is there a "default" format that we should expose? I think on most platforms you can choose arbitrary format, but if we need to provide "default" then it would be polymorphic, which doesn't play well with generic API design.

[mitchmindtree]

I'm currently looking into implementing this now that #354 has landed.

As a part of this change, I'm thinking about how to best approach the "callback" API for the new Device::build_input_stream/build_output_stream methods.

Rather than the current signature that looks like this:

FnMut(crate::StreamData) + Send + 'static

I'm imagining we could go with something like this.

Input:

FnMut(InputData<T>, &StreamInfo) + Send + 'static

Output:

FnMut(OutputData<T>, &StreamInfo) + Send + 'static

where the *Data type represents the audio data and StreamInfo provides access to other potentially useful information about the stream such as timing information or flags.

The reason I'm thinking we provide a type around the "data" is in case we decide to add support for non-interleaved data in the future. Currently on platforms that only support non-interleaved data (ASIO comes to mind, not sure if there are others) we do the conversion to interleaved behind the scenes, but I can imagine we might want to provide a more transparent API for this in the future.

As for StreamInfo, I might leave this out for now. I plan on looking into adding stream timing in a follow-up API - I might add the StreamInfo type then. I'm also open to better names for this type if anything comes to mind.

[mitchmindtree]

I wonder if it is also worth removing the data_type: SampleFormat field from the Format struct? The Format type is solely used to specify the desired format when calling build_input/output_stream, however with these changes the desired SampleFormat can be inferred from the sample type used within the user's data callback function.

[mitchmindtree]

On second thoughts, the Format type is also used to specify the default format via the Device::default_input/output_format methods.

One option would be to remove the data_type field and have the default_input/output_format return a (Format, SampleFormat)?

An alternative option might be to leave the Format type as is, and instead change the build_input/output_stream methods to take sample rate and number of channels as individual arguments.

Otherwise, we could just leave it as is and assert_eq!(format.data_type, T::FORMAT) at the beginning of each of the build_input/output_stream methods.

[Ralith]

I don't think this has been fixed; as of #359 every relevant example still contains SampleFormat check or an equivalent unwrap in the main loop.

[mitchmindtree]

For anyone following I've responded at #359 here. I'm happy to re-open this if it's decided this is still worth tackling.

[Ralith]

A complete proposal for an improved API has been drafted at #366.