[alsa-devel] [PATCH] compress: add support for gapless playback
Takashi Iwai
tiwai at suse.de
Mon Feb 11 14:41:18 CET 2013
At Mon, 11 Feb 2013 04:22:45 -0800,
Vinod Koul wrote:
>
(snip)
> --- a/include/uapi/sound/asound.h
> +++ b/include/uapi/sound/asound.h
> @@ -271,7 +271,9 @@ typedef int __bitwise snd_pcm_state_t;
> #define SNDRV_PCM_STATE_PAUSED ((__force snd_pcm_state_t) 6) /* stream is paused */
> #define SNDRV_PCM_STATE_SUSPENDED ((__force snd_pcm_state_t) 7) /* hardware is suspended */
> #define SNDRV_PCM_STATE_DISCONNECTED ((__force snd_pcm_state_t) 8) /* hardware is disconnected */
> -#define SNDRV_PCM_STATE_LAST SNDRV_PCM_STATE_DISCONNECTED
> +#define SNDRV_PCM_STATE_NEXT_TRACK ((__force snd_pcm_state_t) 9) /* stream will move to next track */
Isn't this better to be set as another flag instead of the trigger
command? This changes the runtime->state, and it may make things
complicated. For example, what happens if user triggers NEXT_TRACK,
then PAUSE_PUSH and PAUSE_RELEASE?
> +#define SNDRV_PCM_STATE_PARTIAL_DRAIN ((__force snd_pcm_state_t) 10) /* stream is draining partially */
> +#define SNDRV_PCM_STATE_LAST SNDRV_PCM_STATE_PARTIAL_DRAIN
Well, I'd hesitate to add this for PCM, since there is no counterpart
for PCM (yet). Until then, let's avoid touching the PCM API
definition but only compress API.
> enum {
> SNDRV_PCM_MMAP_OFFSET_DATA = 0x00000000,
> diff --git a/include/uapi/sound/compress_offload.h b/include/uapi/sound/compress_offload.h
> index 05341a4..1d63993 100644
> --- a/include/uapi/sound/compress_offload.h
> +++ b/include/uapi/sound/compress_offload.h
> @@ -30,7 +30,7 @@
> #include <sound/compress_params.h>
>
>
> -#define SNDRV_COMPRESS_VERSION SNDRV_PROTOCOL_VERSION(0, 1, 0)
> +#define SNDRV_COMPRESS_VERSION SNDRV_PROTOCOL_VERSION(0, 1, 1)
> /**
> * struct snd_compressed_buffer: compressed buffer
> * @fragment_size: size of buffer fragment in bytes
> @@ -121,6 +121,32 @@ struct snd_compr_codec_caps {
> struct snd_codec_desc descriptor[MAX_NUM_CODEC_DESCRIPTORS];
> };
>
> +enum {
> + SNDRV_COMPRESS_ENCODER_PADDING = 1,
> + SNDRV_COMPRESS_ENCODER_DELAY = 2,
> +};
> +
> +/**
> + * struct snd_compr_keyvalue: compressed stream key/value pairs
> + * @key: key id
> + * @value: key value
> + */
> +
> +struct snd_compr_keyvalue {
> + __u32 key;
> + __u32 value;
> +};
> +
> +/**
> + * struct snd_compr_metadata: compressed stream metadata
> + * @count: number of keys
> + * @keys: pointer to count keys
> + */
> +struct snd_compr_metadata {
> + __u32 count;
> + struct snd_compr_keyvalue *keys;
> +};
Do you really want that? Then you'll have to provide 32/64bit
ioctl conversion functions as well. I wouldn't take that mess but let
user repeat single key/value ioctls.
Instead of relying completely on the driver side implementation, you
can keep a metadata array in snd_compr_stream or snd_compr
internally. Then the ioctl handler would be just like:
static int compr_stream_set_metadata(struct snd_compr_stream *stream, void __user * arg)
{
struct snd_compr_keyvalue kv;
if (!stream->ops->set_metadata)
return -ENXIO;
if (copy_from_user(&kv, arg, sizeof(kv)))
return -EFAULT;
if (kv > SNDRV_COMPRESS_METADATA_LAST)
return -EINVAL;
stream->metadata[kv.key] = kv.value;
return stream->ops->set_metadata(stream);
}
As a bonus, we can reduce even get_metadata ops implementation in each
driver:
static int compr_stream_get_metadata(struct snd_compr_stream *stream, void __user * arg)
{
if (!stream->ops->set_metadata)
return -ENXIO;
if (copy_from_user(&kv, arg, sizeof(kv)))
return -EFAULT;
if (kv > SNDRV_COMPRESS_METADATA_LAST)
return -EINVAL;
kv.value = stream->metadata[kv.key];
if (copy_to_user(arg, &kv, sizeof(kv))
return -EFAULT;
return 0;
}
Of course, this assumes that stream->metadata[] is initialized at each
open properly.
Takashi
> +
> /**
> * compress path ioctl definitions
> * SNDRV_COMPRESS_GET_CAPS: Query capability of DSP
> @@ -145,6 +171,10 @@ struct snd_compr_codec_caps {
> struct snd_compr_codec_caps)
> #define SNDRV_COMPRESS_SET_PARAMS _IOW('C', 0x12, struct snd_compr_params)
> #define SNDRV_COMPRESS_GET_PARAMS _IOR('C', 0x13, struct snd_codec)
> +#define SNDRV_COMPRESS_SET_METADATA _IOW('C', 0x14,\
> + struct snd_compr_metadata)
> +#define SNDRV_COMPRESS_GET_METADATA _IOW('C', 0x15,\
> + struct snd_compr_metadata)
> #define SNDRV_COMPRESS_TSTAMP _IOR('C', 0x20, struct snd_compr_tstamp)
> #define SNDRV_COMPRESS_AVAIL _IOR('C', 0x21, struct snd_compr_avail)
> #define SNDRV_COMPRESS_PAUSE _IO('C', 0x30)
> @@ -152,10 +182,14 @@ struct snd_compr_codec_caps {
> #define SNDRV_COMPRESS_START _IO('C', 0x32)
> #define SNDRV_COMPRESS_STOP _IO('C', 0x33)
> #define SNDRV_COMPRESS_DRAIN _IO('C', 0x34)
> +#define SNDRV_COMPRESS_NEXT_TRACK _IO('C', 0x35)
> +#define SNDRV_COMPRESS_PARTIAL_DRAIN _IO('C', 0x36)
> /*
> * TODO
> * 1. add mmap support
> *
> */
> #define SND_COMPR_TRIGGER_DRAIN 7 /*FIXME move this to pcm.h */
> +#define SND_COMPR_TRIGGER_NEXT_TRACK 8
> +#define SND_COMPR_TRIGGER_PARTIAL_DRAIN 9
> #endif
> diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> index ad11dc9..4928209 100644
> --- a/sound/core/compress_offload.c
> +++ b/sound/core/compress_offload.c
> @@ -262,9 +262,14 @@ static ssize_t snd_compr_write(struct file *f, const char __user *buf,
>
> stream = &data->stream;
> mutex_lock(&stream->device->lock);
> - /* write is allowed when stream is running or has been steup */
> + /* write is allowed when stream is running or has been steup
> + * also stream cna be written when next_track info has been setup
> + * or its has partially drained
> + */
> if (stream->runtime->state != SNDRV_PCM_STATE_SETUP &&
> - stream->runtime->state != SNDRV_PCM_STATE_RUNNING) {
> + stream->runtime->state != SNDRV_PCM_STATE_RUNNING &&
> + stream->runtime->state != SNDRV_PCM_STATE_NEXT_TRACK &&
> + stream->runtime->state != SNDRV_PCM_STATE_PARTIAL_DRAIN) {
> mutex_unlock(&stream->device->lock);
> return -EBADFD;
> }
> @@ -288,6 +293,9 @@ static ssize_t snd_compr_write(struct file *f, const char __user *buf,
> stream->runtime->state = SNDRV_PCM_STATE_PREPARED;
> pr_debug("stream prepared, Houston we are good to go\n");
> }
> + if (stream->runtime->state == SNDRV_PCM_STATE_NEXT_TRACK ||
> + stream->runtime->state == SNDRV_PCM_STATE_PARTIAL_DRAIN)
> + stream->runtime->state = SNDRV_PCM_STATE_RUNNING;
>
> mutex_unlock(&stream->device->lock);
> return retval;
> @@ -514,6 +522,78 @@ out:
> return retval;
> }
>
> +static int
> +snd_compr_copy_metadata(struct snd_compr_metadata **arg, unsigned long user)
> +{
> + struct snd_compr_metadata _mdata, *mdata;
> + int len;
> +
> + if (copy_from_user(&_mdata, (void __user *)user,
> + sizeof(_mdata)))
> + return -EFAULT;
> +
> + len = sizeof(_mdata.count) + _mdata.count * sizeof(*_mdata.keys);
> +
> + mdata = kmalloc(len, GFP_KERNEL);
> + if (!mdata)
> + return -ENOMEM;
> +
> + if (copy_from_user(mdata, (void __user *)arg, len)) {
> + kfree(mdata);
> + return -EFAULT;
> + }
> + *arg = mdata;
> + return len;
> +}
> +
> +static int
> +snd_compr_set_metadata(struct snd_compr_stream *stream, unsigned long arg)
> +{
> + struct snd_compr_metadata *mdata;
> + int retval;
> +
> + if (!stream->ops->set_metadata)
> + return -ENXIO;
> + /*
> + * we should allow parameter change only when stream has been
> + * opened not in other cases
> + */
> + retval = snd_compr_copy_metadata(&mdata, arg);
> + if (retval <= 0)
> + return retval;
> +
> + retval = stream->ops->set_metadata(stream, mdata);
> +
> + kfree(mdata);
> + return retval;
> +}
> +
> +static int
> +snd_compr_get_metadata(struct snd_compr_stream *stream, unsigned long arg)
> +{
> + struct snd_compr_metadata *mdata;
> + int retval, len;
> +
> + if (!stream->ops->get_metadata)
> + return -ENXIO;
> +
> + len = snd_compr_copy_metadata(&mdata, arg);
> + if (len <= 0)
> + return len;
> + retval = stream->ops->get_metadata(stream, mdata);
> + if (retval != 0)
> + goto out;
> +
> + if (copy_to_user((void __user *)arg, mdata, len)) {
> + retval = -EFAULT;
> + goto out;
> + }
> +
> +out:
> + kfree(mdata);
> + return retval;
> +}
> +
> static inline int
> snd_compr_tstamp(struct snd_compr_stream *stream, unsigned long arg)
> {
> @@ -594,6 +674,38 @@ static int snd_compr_drain(struct snd_compr_stream *stream)
> return retval;
> }
>
> +static int snd_compr_partial_drain(struct snd_compr_stream *stream)
> +{
> + int retval;
> +
> + /* we can partially drain streams, only when next track info has been
> + * passed to the dsp
> + */
> + if (stream->runtime->state != SNDRV_PCM_STATE_NEXT_TRACK)
> + return -EPERM;
> +
> + retval = stream->ops->trigger(stream, SND_COMPR_TRIGGER_PARTIAL_DRAIN);
> + if (retval != 0)
> + return retval;
> + stream->runtime->state = SNDRV_PCM_STATE_PARTIAL_DRAIN;
> + return 0;
> +}
> +
> +static int snd_compr_next_track(struct snd_compr_stream *stream)
> +{
> + int retval;
> +
> + /* only a running stream can transition to next track */
> + if (stream->runtime->state != SNDRV_PCM_STATE_RUNNING)
> + return -EPERM;
> +
> + retval = stream->ops->trigger(stream, SND_COMPR_TRIGGER_NEXT_TRACK);
> + if (retval != 0)
> + return retval;
> + stream->runtime->state = SNDRV_PCM_STATE_NEXT_TRACK;
> + return 0;
> +}
> +
> static long snd_compr_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
> {
> struct snd_compr_file *data = f->private_data;
> @@ -623,6 +735,12 @@ static long snd_compr_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
> case _IOC_NR(SNDRV_COMPRESS_GET_PARAMS):
> retval = snd_compr_get_params(stream, arg);
> break;
> + case _IOC_NR(SNDRV_COMPRESS_SET_METADATA):
> + retval = snd_compr_set_metadata(stream, arg);
> + break;
> + case _IOC_NR(SNDRV_COMPRESS_GET_METADATA):
> + retval = snd_compr_get_metadata(stream, arg);
> + break;
> case _IOC_NR(SNDRV_COMPRESS_TSTAMP):
> retval = snd_compr_tstamp(stream, arg);
> break;
> @@ -644,6 +762,13 @@ static long snd_compr_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
> case _IOC_NR(SNDRV_COMPRESS_DRAIN):
> retval = snd_compr_drain(stream);
> break;
> + case _IOC_NR(SNDRV_COMPRESS_PARTIAL_DRAIN):
> + retval = snd_compr_partial_drain(stream);
> + break;
> + case _IOC_NR(SNDRV_COMPRESS_NEXT_TRACK):
> + retval = snd_compr_next_track(stream);
> + break;
> +
> }
> mutex_unlock(&stream->device->lock);
> return retval;
> --
> 1.7.0.4
>
More information about the Alsa-devel
mailing list