Re: [PATCH] compress: add support for gapless playback

[Takashi Iwai <tiwai@suse.de>]

At Wed, 13 Feb 2013 22:33:22 +0530,
Vinod Koul wrote:
> 
> On Wed, Feb 13, 2013 at 05:40:11PM +0100, Takashi Iwai wrote:
> > At Wed, 13 Feb 2013 08:00:34 -0800,
> > Vinod Koul wrote:
> > > 
> > > +static int
> > > +snd_compr_get_metadata(struct snd_compr_stream *stream, unsigned long arg)
> > > +{
> > > +	struct snd_compr_metadata *metadata;
> > > +	int retval;
> > > +
> > > +	if (!stream->ops->get_metadata)
> > > +		return -ENXIO;
> > > +
> > > +	metadata = kmalloc(sizeof(*metadata), GFP_KERNEL);
> > 
> > There is no merit to use kmalloc just for two integers at all.
> > Simply put on stack:
> > 	struct snd_compr_metadata metdata;
> > 	if (copy_from_user(&metadata, (void __user *)arg, sizeof(metadata)))
> > 	...
> while doing this I had thought on this, but my laziness got better of me :(
> > 
> > Ditto for snd_compr_set_metadata().
> > 
> > > +static int snd_compr_partial_drain(struct snd_compr_stream *stream)
> > > +{
> > > +	int retval;
> > > +	if (stream->runtime->state == SNDRV_PCM_STATE_PREPARED ||
> > > +			stream->runtime->state == SNDRV_PCM_STATE_SETUP)
> > > +		return -EPERM;
> > > +	/* stream can be drained only when next track has been signalled */
> > > +	if (stream->next_track == false)
> > > +		return -EPERM;
> > > +
> > > +	retval = stream->ops->trigger(stream, SND_COMPR_TRIGGER_PARTIAL_DRAIN);
> > > +	if (retval)
> > > +		pr_err("Partial drain returned failure\n");
> > 
> > I don't think it's good to spew errors.
> ok
> > 
> > Also, the initialization of new stream fields is missing in other
> > places like prepare callback?  Otherwise the values will retain.
> The stream is created at open. Since it used kzalloc, these variables would be
> init to false.

But you can restart the stream after stopping it without closing the
device, right?


Takashi