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

[Vinod Koul <vinod.koul@intel.com>]

On Thu, Feb 07, 2013 at 01:15:54AM +0000, Richard Fitzgerald wrote:
> > +- partial drain
> > +This is called when end of file is reached. The userspace can inform DSP that
> > +EOF is reached and now DSP can start skipping padding delay. Also next write
> > +data would belong to next track
> 
> We're really doing two different tasks inside the "partial drain". The name,
> and the reason we are doing two tasks in one, comes from a particular higher-
> layer scenario, but there's no reason the driver API need use the same
> terminology as one particular application of the functionality.
> The two tasks are:
> 
> 1) Tell the DSP that we have sent all data for the current track and following
> data will be for the next track. This lets the DSP lay down a marker for where
> it should strip padding at the end of a track, and know it should be expecting
> more data for another track to follow gaplessly where it must strip the
> encoder delay from the start.
> 
> 2) Ask for notification when DSP has reached the changeover point between the
> playback of the two tracks
> 
> I think it would be more logical and less confusing not to combine the two into
> a single ioctl. Instead, add only one new ioctl specifically to provide the
> track changeover hint, something like SNDRV_COMPRESS_NEXT_TRACK (meaning DSP
> should expect data for next track to follow). Don't add a new drain, just use
> the existing SNDRV_COMPRESS_DRAIN - the driver/DSP can make a decision whether
> it needs to do something special with the drain if we have told it that we
> will be sending it some more data for a following track.
> 
> So the SNDRV_COMPRESS_PARTIAL_DRAIN in this patch would become
> 
> 1) Send SNDRV_COMPRESS_NEXT_TRACK
> 2) Send SNDRV_COMPRESS_DRAIN
The problem would be in that case the defination of SNDRV_COMPRESS_DRAIN which
expects the decoder to completely drain its buffers and come to complete halt.
This would also mean the framework will treat a drained stream as stopped and
needs a new start. Certainly we dont want that in this case. So we can't use
SNDRV_COMPRESS_DRAIN to indicate. Yes we can put conditional check but IMO that
would overtly complicate this. If we are not doing proper drian lets not
call it that.

But I think I like the idea of splitting the two up to do a cleaner interface.
Let me check this...

> When we reach the final track we just do SNDRV_COMPRESS_DRAIN to wait for it
> to finish.
> 
> If we setup a next track by doing SNDRV_COMPRESS_NEXT_TRACK and then change
> our mind and decide that this is going to be the last track, we do a
> SNDRV_COMPRESS_DRAIN without sending any next track data, then we do
> SNDRV_COMPRESS_STOP
Nope that is where we would have issue. You dont call SNDRV_COMPRESS_STOP for
compressed streams. You have to wait till DSP has decoded and rendered data for
the last track which can be done by using SNDRV_COMPRESS_DRAIN only.
SNDRV_COMPRESS_STOP should not be called in this case.

--
~Vinod