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

[Takashi Iwai <tiwai@suse.de>]

At Thu, 7 Feb 2013 11:37:38 +0000,
Richard Fitzgerald wrote:
> 
> > > 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.
> 
> Ok, so let's keep the partial drain but split out the NEXT_TRACK hint so we keep
> the two operations separate. It's clearer to understand. NEXT_TRACK tells DSP to
> prepare for data for new track, PARTIAL_DRAIN asks DSP when it has played all
> data of current track. I don't think that application _must_ drain when doing
> gapless, only if it cares to know when DSP reaches the join between tracks.
> Possible problem - application calls NEXT_TRACK and PARTIAL_DRAIN when DSP has
> already reached end of current track, so what does PARTIAL_DRAIN mean in this
> case? Which track is it draining? Maybe define that it always refers to the
> track _before_ the most recent NEXT_TRACK call?
> 
> Also can we make metadata a key-pair list as Takashi suggested, with unlimited
> length. Something like
> 
> struct snd_compr_metadata_pair {
> 	enum snd_compr_metadata_key key;
> 	u32 value;
> };
> 
> struct snd_compr_metadata {
> 	int count; /* number of actual entries in following array */
> 	struct snd_compr_metadata_pair *pairs;
> };
> 
> Should we union the u32 value with a u8* buf to allow for any case where the
> value needs to be a buffer of binary data?

Oh no, don't do it.  Never.

In general,
 - Never put types that depending on the architecture in struct passed
   to ioctls (e.g. long, pointer)
 - Never use bit fields for ABI
 - If 64bit value is used, think of alignment; safer to add packed
   attribute

So, in general, use only u32, s16, or whatever, the types with
explicit size.


Takashi