On 23.11.2017 00:27, Takashi Sakamoto wrote:
On Nov 23 2017 04:17, Maciej S. Szmigiero wrote:
(..)
--- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -236,7 +236,11 @@ typedef int __bitwise snd_pcm_format_t; #define SNDRV_PCM_FORMAT_DSD_U32_LE ((__force snd_pcm_format_t) 50) /* DSD, 4-byte samples DSD (x32), little endian */ #define SNDRV_PCM_FORMAT_DSD_U16_BE ((__force snd_pcm_format_t) 51) /* DSD, 2-byte samples DSD (x16), big endian */ #define SNDRV_PCM_FORMAT_DSD_U32_BE ((__force snd_pcm_format_t) 52) /* DSD, 4-byte samples DSD (x32), big endian */ -#define SNDRV_PCM_FORMAT_LAST SNDRV_PCM_FORMAT_DSD_U32_BE +#define SNDRV_PCM_FORMAT_S20_4LE ((__force snd_pcm_format_t) 53) /* in four bytes */ +#define SNDRV_PCM_FORMAT_S20_4BE ((__force snd_pcm_format_t) 54) /* in four bytes */ +#define SNDRV_PCM_FORMAT_U20_4LE ((__force snd_pcm_format_t) 55) /* in four bytes */ +#define SNDRV_PCM_FORMAT_U20_4BE ((__force snd_pcm_format_t) 56) /* in four bytes */ +#define SNDRV_PCM_FORMAT_LAST SNDRV_PCM_FORMAT_U20_4BE
In my opinion, for this type of definition, it's better to declare left/right-adjusted or padding side. (Of course, silence definition is already a hint, however the lack of information forces developers to have a careful behaviour to handle entries on the list.> (I note that in current ALSA PCM interface there's no way to deliver MSB/LSB-first information about sample format.)
No other sound format includes this information in its name so if we name these formats SNDRV_PCM_FORMAT_{S, U}20LSB_4 they are going to have it inconsistent with every other one (I assume you meant to include such information in a format name?).
But information about whether this format is MSB or LSB justified can be added in a comment so the situation is clear for other developers from the definition without needing to read the actual processing code.
Additionally, alsa-lib includes some codes related to the definition[1]. If you'd like to thing goes well out of ALSA SoC part, it's better to submit changes to the library as well.
[1] http://git.alsa-project.org/?p=alsa-lib.git;a=blob;f=src/pcm/pcm_misc.c;h=54...
I have alsa-lib changes ready for these formats - they were needed to test these patches, will post them when this is merged on the kernel side (in case some changes are needed which affect both).
Regards
Takashi Sakamoto
Best regards, Maciej Szmigiero