Re: [PATCH v2 1/2] ALSA: pcm: add SNDRV_PCM_FORMAT_{S, U}20

[Maciej S. Szmigiero]

Hi,

On 26.11.2017 10:27, Takashi Sakamoto wrote:
> Hi,
> 
(..)
> Before applying this patch:
> 166ÂÂÂÂÂÂÂÂ /* FIXME: the following three formats are not defined properly yet */
> 167ÂÂÂÂÂÂÂÂ [SNDRV_PCM_FORMAT_MPEG] = {
> 168ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .le = -1, .signd = -1,
> 169ÂÂÂÂÂÂÂÂ },
> 170ÂÂÂÂÂÂÂÂ [SNDRV_PCM_FORMAT_GSM] = {
> 171ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .le = -1, .signd = -1,
> 172ÂÂÂÂÂÂÂÂ },
> 173ÂÂÂÂÂÂÂÂ [SNDRV_PCM_FORMAT_SPECIAL] = {
> 174ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .le = -1, .signd = -1,
> 175ÂÂÂÂÂÂÂÂ },
> 
> After applying this patch:
> 
> 166ÂÂÂÂÂÂÂÂ /* FIXME: the following three formats are not defined properly yet */
> 167ÂÂÂÂÂÂÂÂ [SNDRV_PCM_FORMAT_MPEG] = {
> 168ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .le = -1, .signd = -1,
> 169ÂÂÂÂÂÂÂÂ },
> 170ÂÂÂÂÂÂÂÂ [SNDRV_PCM_FORMAT_GSM] = {
> 171ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .le = -1, .signd = -1,
> 172ÂÂÂÂÂÂÂÂ },
> 173ÂÂÂÂÂÂÂÂ [SNDRV_PCM_FORMAT_S20_LE] = {
(..)> 
> I think it good to add an alternative comment for each of entry which is not defined yet, like:
> 
> -> 166ÂÂÂÂÂÂÂÂ /* FIXME: this format is not defined properly yet */
> 167ÂÂÂÂÂÂÂÂ [SNDRV_PCM_FORMAT_MPEG] = {
> 168ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .le = -1, .signd = -1,
> 169ÂÂÂÂÂÂÂÂ },
> -> 170ÂÂÂÂÂÂÂÂ /* FIXME: this format is not defined properly yet */
> 171ÂÂÂÂÂÂÂÂ [SNDRV_PCM_FORMAT_GSM] = {
> 172ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .le = -1, .signd = -1,
> 173ÂÂÂÂÂÂÂÂ },
> 174ÂÂÂÂÂÂÂÂ [SNDRV_PCM_FORMAT_S20_LE] = {
> 175ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .width = 20, .phys = 32, .le = 1, .signd = 1,
> 176ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .silence = {},
> 177ÂÂÂÂÂÂÂÂ },
> 178ÂÂÂÂÂÂÂÂ [SNDRV_PCM_FORMAT_S20_BE] = {
> 179ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .width = 20, .phys = 32, .le = 0, .signd = 1,
> 180ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .silence = {},
> 181ÂÂÂÂÂÂÂÂ },
> 182ÂÂÂÂÂÂÂÂ [SNDRV_PCM_FORMAT_U20_LE] = {
> 183ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .width = 20, .phys = 32, .le = 1, .signd = 0,
> 184ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .silence = { 0x00, 0x00, 0x08, 0x00 },
> 185ÂÂÂÂÂÂÂÂ },
> 186ÂÂÂÂÂÂÂÂ [SNDRV_PCM_FORMAT_U20_BE] = {
> 187ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .width = 20, .phys = 32, .le = 0, .signd = 0,
> 188ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .silence = { 0x00, 0x08, 0x00, 0x00 },
> 189ÂÂÂÂÂÂÂÂ },
> -> 190ÂÂÂÂÂÂÂÂ /* FIXME: this format is not defined properly yet */
> 191ÂÂÂÂÂÂÂÂ [SNDRV_PCM_FORMAT_SPECIAL] = {
> 192ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ .le = -1, .signd = -1,
> 193ÂÂÂÂÂÂÂÂ },
> 

Thanks, fixed now in v3.

> Regards
> 
> Takashi Sakamoto

Best regards,
Maciej Szmigiero