Re: [Xen-devel] [PATCH v3] sndif: add ABI for Para-virtual sound

[Oleksandr Dmytryshyn <oleksandr.dmytryshyn@xxxxxxxxxxxxxxx>]

On Thu, Jan 22, 2015 at 12:24 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> On 22.01.15 at 10:56, <oleksandr.dmytryshyn@xxxxxxxxxxxxxxx> wrote:
>> +/*
>> + * PCM FORMATS
>> + *
>> + * XENSND_PCM_FORMAT_<format>[_<endian>]
>> + *
>> + * format: <S/U/FLOAT>[<bits>] or <name>
>> + *     S - signed, U - unsigned, FLOAT - float
>> + *     bits - 8, 16, 24, 32 or absent
>> + *     name - MU_LAW, GSM, etc.
>> + *
>> + * endian: <LE/BE>, may be absent
>> + *     LE - Little endian, BE - Big endian
>> + */
>> +#define XENSND_PCM_FORMAT_S8            0
>> +#define XENSND_PCM_FORMAT_U8            1
>> +#define XENSND_PCM_FORMAT_S16_LE        2
>> +#define XENSND_PCM_FORMAT_S16_BE        3
>> +#define XENSND_PCM_FORMAT_U16_LE        4
>> +#define XENSND_PCM_FORMAT_U16_BE        5
>> +#define XENSND_PCM_FORMAT_S24_LE        6
>> +#define XENSND_PCM_FORMAT_S24_BE        7
>> +#define XENSND_PCM_FORMAT_U24_LE        8
>> +#define XENSND_PCM_FORMAT_U24_BE        9
>> +#define XENSND_PCM_FORMAT_S32_LE        10
>> +#define XENSND_PCM_FORMAT_S32_BE        11
>> +#define XENSND_PCM_FORMAT_U32_LE        12
>> +#define XENSND_PCM_FORMAT_U32_BE        13
>> +#define XENSND_PCM_FORMAT_FLOAT_LE      14 /* 4-byte float, IEEE-754 
>> 32-bit, */
>> +#define XENSND_PCM_FORMAT_FLOAT_BE      15 /* range -1.0 to 1.0             
>>  */
>
> I think these should carry 32 in their names. I also wonder why
> it needs to be FLOAT instead of just F.
I'll rename those defines in the next patch-set.

>> + * Request open - open an pcm stream for playback or capture:
>> + *     0    1     2     3     4     5     6     7  octet
>> + * +-----+-----+-----+-----+-----+-----+-----+-----+
>> + * |       operation       |       stream_id       |
>> + * +-----+-----+-----+-----+-----+-----+-----+-----+
>> + * |      pcm_format       |      pcm_channes      |
>
> pcm_channel(s)?
I'll fix this in the next patch-set

>> + * All responce packets have the same length (76 bytes)
>
> response
I'll fix this in the next patch-set

>> + * Responce for all requests exept the XENSND_GET_VOLUME:
>> + *     0    1     2     3     4     5     6     7  octet
>> + * +-----+-----+-----+-----+-----+-----+-----+-----+
>> + * |       stream_id       |       operation       |
>
> Is there a really need for echoing the operation, i.e. isn't the echoed
> ID sufficient? Because if you'd be able to drop this, request and
> response sizes would match up, avoiding wasted space on the
> shared page(s).
In our implementation there is no matter if is echoed the operation or no.
Frontend sends one request and waits a response. So I can simply remove
the echoed operation. But in the future if this echoed the operation
will be needed
it can be simply restored.


>> + * +-----+-----+-----+-----+-----+-----+-----+-----+
> Jan
>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel