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:
- 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.
- 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
- Send SNDRV_COMPRESS_NEXT_TRACK
- 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