Re: [cdi-devel] [PATCH] CDI.audio

[Matthew Iselin <matthew@xxxxxxxxxxxxxx>]

A few thoughts from me.

On Thu, Nov 18, 2010 at 7:45 AM, Kevin Wolf <kevin@xxxxxxxxxx> wrote:
> On Wed, Nov 17, 2010 at 09:04:22PM +0100, max@xxxxxxxxxx wrote:
>> From: Max Reitz <max@xxxxxxxxxx>
>>

>> +    /**
>> +     * \german
>> +     * Ist dieser Wert ungleich 0, so handelt es sich um ein Aufnahmegerät,
>> +     * es kann nur gelesen werden. Ist er 0, dann können nur Töne abgespielt
>> +     * werden.
>> +     * \endgerman
>> +     * \english
>> +     * If this value is not 0, this device is a recording device, thus one can
>> +     * only read data. Otherwise, only play back is possible.
>> +     * \endenglish
>> +     */
>> +    int record;
>
> Hm, shouldn't that be a property of a stream/channel? It seems odd to
> have two devices appearing in the system for one physical sound card,
> just to have one for playback and one for recording. It's like having
> a different devices for reading from a disk than for writing to it.

What about a system with a sound card that offers three inputs and six
outputs? To the OS all three inputs should appear as unique devices
unless they are not separate on the sound card.

Something to consider.

>> +
>> +    /**
>> +     * \german
>> +     * Abspielgeräte können mehrere logische Kanäle anbieten, die vom Gerät
>> +     * zusammengemixt werden. Ist dem nicht der Fall oder handelt es sich um
>> +     * ein Aufnahmegerät, dann ist die Größe dieser Liste genau 1. Sie enthält
>> +     * Elemente des Typs struct cdi_audio_stream.
>> +     * \endgerman
>> +     * \english
>> +     * Play back devices may provide several logical streams which are mixed by
>> +     * the device. If this is either not true or it is a recording device, the
>> +     * list's size is exactly 1. It contains elements of type
>> +     * struct cdi_audio_stream.
>> +     * \endenglish
>> +     */
>> +    cdi_list_t streams;
>
> If you want to keep the restriction enforced and still have only one
> device for everything, you could have a cdi_list_t playback_streams and
> a struct cdi_audio_stream* record_stream, which may be NULL.

Maybe this is something for CDI/mixer if that path is chosen. I'm
wondering now what does the mixing here on devices that don't do
hardware mixing.

Other than that though, this looks good.

Matt