On Mon, Feb 11, 2013 at 02:41:18PM +0100, Takashi Iwai wrote:
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?
hmmm, that makes sense. Also it would make things much easier on all the transistions
+#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.
If we do above then we may not need this :)
+/**
- 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.
Ah, i forgot the damn pointer :-)
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:
how do we decide the size of array and what if we have more params in future
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); }
And when do we clear this? metadata for gapless, though a same stream session gets updated for every track.
Right now I am inlcined to put a single value, and be done with it or update older structs with bunch of padding for future use :)
Of course, this assumes that stream->metadata[] is initialized at each open properly.
Yes and it cant be cleared later for next track, perhpas we clear it when we get NEXT_TRACK signalled from user space?
-- ~Vinod