At Wed, 6 Feb 2013 18:18:07 -0800,
Vinod Koul wrote:
>> 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...
In the way above, NEXT_TRACK is no longer a trigger op, but it's
rather an operational option. This would set a flag something like
compr->drain_mode = SND_COMPR_DRAIN_RESTART;
while its default is SND_COMPR_DRAIN_STOP.
(Or, set compr->next_track_after_drain = true, or whatever.)
Though, I'm also not sure whether the scenario above improves the
situation better...
Takashi