[alsa-devel] [RFC/PATCH] control_external: Add ability to specify TLV data.

[Takashi Iwai]

At Sat, 14 Apr 2012 19:36:26 -0700,
Dylan Reid wrote:
> 
> Allow external control plugins to provide TLV data.  This allows
> user-space pcms to specify dB ranges for controls.
> 
> This follows the same model as the ALSA drivers for accessing the
> data.  The code is based on that implementation.  The control can
> provide static data or a callback.  The data is accessed or modified
> in the new snd_ctl_ext_elem_tlv callback.
> 
> Rev bump the protocol version to enable checking if an external
> control supports TLV.
> 
> Signed-off-by: Dylan Reid <dgreid at chromium.org>
> ---
>  include/control_external.h |   17 +++++++++++++++-
>  src/control/control_ext.c  |   46 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 62 insertions(+), 1 deletions(-)
> 
> diff --git a/include/control_external.h b/include/control_external.h
> index 7c066cf..5392ea6 100644
> --- a/include/control_external.h
> +++ b/include/control_external.h
> @@ -60,13 +60,16 @@ typedef struct snd_ctl_ext snd_ctl_ext_t;
>  typedef struct snd_ctl_ext_callback snd_ctl_ext_callback_t;
>  /** Key to access a control pointer */
>  typedef unsigned long snd_ctl_ext_key_t;
> +/** Callback to handle TLV commands. */
> +typedef int (snd_ctl_ext_tlv_rw_t)(snd_ctl_ext_t *ext, snd_ctl_ext_key_t key, int op_flag, unsigned int numid,
> +				   unsigned int *tlv, unsigned int tlv_size);
>  
>  /*
>   * Protocol version
>   */
>  #define SND_CTL_EXT_VERSION_MAJOR	1	/**< Protocol major version */
>  #define SND_CTL_EXT_VERSION_MINOR	0	/**< Protocol minor version */
> -#define SND_CTL_EXT_VERSION_TINY	0	/**< Protocol tiny version */
> +#define SND_CTL_EXT_VERSION_TINY	1	/**< Protocol tiny version */
>  /**
>   * external plugin protocol version
>   */
> @@ -122,6 +125,13 @@ struct snd_ctl_ext {
>  	 * control handle filled by #snd_ctl_ext_create()
>  	 */
>  	snd_ctl_t *handle;
> +	/**
> +	 * optional TLV data for the control.
> +	 */
> +	union {
> +		snd_ctl_ext_tlv_rw_t *c;
> +		const unsigned int *p;
> +	} tlv;
>  
>  	int nonblock;			/**< non-block mode; read-only */
>  	int subscribed;			/**< events subscribed; read-only */
> @@ -245,7 +255,12 @@ typedef enum snd_ctl_ext_access {
>  	SND_CTL_EXT_ACCESS_WRITE = (1<<1),
>  	SND_CTL_EXT_ACCESS_READWRITE = (3<<0),
>  	SND_CTL_EXT_ACCESS_VOLATILE = (1<<2),
> +	SND_CTL_EXT_ACCESS_TLV_READ = (1<<4),
> +	SND_CTL_EXT_ACCESS_TLV_WRITE = (1<<5),
> +	SND_CTL_EXT_ACCESS_TLV_READWRITE = (3<<4),
> +	SND_CTL_EXT_ACCESS_TLV_COMMAND = (1<<6),
>  	SND_CTL_EXT_ACCESS_INACTIVE = (1<<8),
> +	SND_CTL_EXT_ACCESS_TLV_CALLBACK = (1<<28),
>  } snd_ctl_ext_access_t;
>  
>  /**
> diff --git a/src/control/control_ext.c b/src/control/control_ext.c
> index e20d4f3..0a0b397 100644
> --- a/src/control/control_ext.c
> +++ b/src/control/control_ext.c
> @@ -324,6 +324,51 @@ static int snd_ctl_ext_elem_unlock(snd_ctl_t *handle ATTRIBUTE_UNUSED,
>  	return -ENXIO;
>  }
>  
> +static int snd_ctl_ext_elem_tlv(snd_ctl_t *handle, int op_flag,
> +				unsigned int numid,
> +				unsigned int *tlv, unsigned int tlv_size)
> +{
> +	snd_ctl_ext_t *ext = handle->private_data;
> +	snd_ctl_ext_key_t key;
> +	int type, ret;
> +	unsigned int access, count, len;
> +	snd_ctl_elem_id_t id;
> +
> +	/* we don't support TLV on protocol ver 1.0.0 or earlier */
> +	if (ext->version < SNDRV_PROTOCOL_VERSION(1, 0, 0))
> +		return -ENXIO;

Shouldn't this be "<=" ?

Other than that, I like the implementation, looks pretty
straightforward.


thanks,

Takashi


> +
> +	snd_ctl_elem_id_clear(&id);
> +	if (numid > 0) {
> +		ext->callback->elem_list(ext, numid - 1, &id);
> +		id.numid = numid;
> +	} else
> +		id.numid = 0;
> +	key = ext->callback->find_elem(ext, &id);
> +
> +	if (key == SND_CTL_EXT_KEY_NOT_FOUND)
> +		return -ENOENT;
> +	ret = ext->callback->get_attribute(ext, key, &type, &access, &count);
> +	if (ret < 0)
> +		return ret;
> +
> +	if ((op_flag == 0 && (access & SND_CTL_EXT_ACCESS_TLV_READ) == 0) ||
> +	    (op_flag > 0 && (access & SND_CTL_EXT_ACCESS_TLV_WRITE) == 0) ||
> +	    (op_flag < 0 && (access & SND_CTL_EXT_ACCESS_TLV_COMMAND) == 0))
> +		return -ENXIO;
> +	if (access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) {
> +		return ext->tlv.c(ext, key, op_flag, numid, tlv, tlv_size);
> +	} else {
> +		if (op_flag)
> +			return -ENXIO;
> +		len = ext->tlv.p[1] + 2 * sizeof(unsigned int);
> +		if (tlv_size < len)
> +			return -ENOMEM;
> +		memcpy(tlv, ext->tlv.p, len);
> +		return 0;
> +	}
> +}
> +
>  static int snd_ctl_ext_next_device(snd_ctl_t *handle ATTRIBUTE_UNUSED,
>  				   int *device ATTRIBUTE_UNUSED)
>  {
> @@ -429,6 +474,7 @@ static const snd_ctl_ops_t snd_ctl_ext_ops = {
>  	.element_write = snd_ctl_ext_elem_write,
>  	.element_lock = snd_ctl_ext_elem_lock,
>  	.element_unlock = snd_ctl_ext_elem_unlock,
> +	.element_tlv = snd_ctl_ext_elem_tlv,
>  	.hwdep_next_device = snd_ctl_ext_next_device,
>  	.hwdep_info = snd_ctl_ext_hwdep_info,
>  	.pcm_next_device = snd_ctl_ext_next_device,
> -- 
> 1.7.9.rc0
>