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