[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Embedded-pv-devel] [Xen-devel] [PATCH v8] This is ABI for the two halves of a Para-virtual sound driver to communicate with each to other.



Thank you all for the review and comments!
I guess only these are not addressed (or still need comments):

> 1. You can only say how many channels a stream has, not what the
> channels correspond to (e.g., "left", "right" for a 2 channel stereo
> stream; or "left-front", "rear-right", etc. for a 6 channel 5.1 surround
> sound stream).

PV driver may be configured to have number of channels which are
not supported by the back's HW. In that case this is up to back how to treat
those. If we introduce some specific ordering or mapping that will only be
a recommendation to the back, which it may follow.
Thus, I would stick to what we have now

> 3. For the PCM formats you need to precisely specify the format or
> reference external specifications that do so.  For example, "mpeg" is a
> bit vague as I'm sure this standard body has produced many different
> audio formats.  The specifications don't need to be freely available,
> just correctly referenced.

I do understand the intent here, but I blieve we cannot make the protocol
being simple and fit all at once. So, I would suggest leaving it as is
at least for now
or removing it.

> 4. 8 bits for stream index, operation and (particularly) pcm format feel
> a little limiting.
8 bits mean you can have 256 streams, operations and formats (which are
indices). I would prefer to stay with 8 bits

> 5. For the read/write operations is the stream buffer considered to be
> circular?  I think it probably should be.
I guess it shouldn't. The reason of having offset is the fact that software on
front side may mmap part of the buffer and tell the driver the offset of
the "dirty" region of the buffer.

Please find updated list of TODOs for the next version:

1. Change frontend-id to frontend_id
2. Have a single sound card configured with bunch of
different devices/streams
3. State that sample rates and formats expressed as decimals w/o any
particular ordering
4. Put description of migration/disconnection state
5. Change __attribute__((packed)) to __packed (scripts/checkpatch.pl)
6. Padding to fit cache line? What is its size (ARM/x86...) - need to discuss
7. Change GPL header to BSD
8. Remove #ifdef __KERNEL
9. Remove "Multiple PV drivers are allowed in the domain at the same time."
10. Support a single card as device/stream configuration allows fine
tuning already
11. Explicitly say which indices in XenStore configuration are contiguous
12. For strings "If not defined then use frontend's default." add more
description:
"This default is depends on concrete PV frontend implementation"
13. Make names of cards/devices optional
14. Remove PCM_FORMAT_SPECIAL
15. Change volume units from dBm to dB

Thank you,
Oleksandr

_______________________________________________
Embedded-pv-devel mailing list
Embedded-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/cgi-bin/mailman/listinfo/embedded-pv-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.