Hi,
On Nov 30 2017 07:17, Maciej S. Szmigiero wrote:
The previous patch has added 20-bit PCM formats SND_PCM_FORMAT_{S,U}20 to
alsa-lib.
We need to extend the linear format conversion code with handling of these
sample formats so they can also be utilized by applications that only
recognize the more typical ones like SND_PCM_FORMAT_S16.
Since the conversion arrays are indexed by a format bit width divided by 8
the easiest way to handle these formats is to treat them like they were
40-bit wide (the next free integer multiple of 8 bits).
This doesn't create a collision risk with a future format since there can't
really be a 40-bit sample format that occupies 4 bytes.
Make sure we use the getput conversion method for these formats since a
direct conversion from / to them is not supported.
While we are at it let's also add asserts on a format bit width value
in snd_pcm_linear_get_index() and snd_pcm_linear_put_index() received from
snd_pcm_format_width() so we won't try to access memory outside a
conversion array if for some reason this function had returned a value that
is not in the expected range.
Signed-off-by: Maciej S. Szmigiero <mail@xxxxxxxxxxxxxxxxxxxxx>
---
src/pcm/pcm_linear.c | 16 +++++++++++++---
src/pcm/pcm_route.c | 6 ++++--
src/pcm/plugin_ops.h | 50 ++++++++++++++++++++++++++++++++++++++++++++++----
3 files changed, 63 insertions(+), 9 deletions(-)
diff --git a/src/pcm/pcm_linear.c b/src/pcm/pcm_linear.c
index 0d61e927323f..e4973a308004 100644
--- a/src/pcm/pcm_linear.c
+++ b/src/pcm/pcm_linear.c
@@ -90,6 +90,7 @@ int snd_pcm_linear_get_index(snd_pcm_format_t src_format, snd_pcm_format_t dst_f
endian = 0;
pwidth = snd_pcm_format_physical_width(src_format);
width = snd_pcm_format_width(src_format);
+ assert(width >= 8 && width <= 32);
...
> @@ -121,6 +125,7 @@ int snd_pcm_linear_put_index(snd_pcm_format_t
src_format, snd_pcm_format_t dst_f
endian = 0;
pwidth = snd_pcm_format_physical_width(dst_format);
width = snd_pcm_format_width(dst_format);
+ assert(width >= 8 && width <= 32);
if (pwidth == 24) {
switch (width) {
case 24:
I can understand your intention to insert these assert(), while it's
better not to include them in this series of patch.
A call of 'snd_pcm_format_physical_width()' and 'snd_pcm_format_width()'
returns 4 for 'SNDRV_PCM_FORMAT_IMA_ADPCM'. Furthermore, both of them
returns -EINVAL for 'SNDRV_PCM_FORMAT_MPEG', 'SNDRV_PCM_FORMAT_GSM' and
'SNDRV_PCM_FORMAT_SPECIAL'. They can bring wrong calculation to the
resampling function, in the worse case result in any fault.
However, this is another issue, at least out of your aim for this
patchset. It's a kind of bug for wider influence. I think to have
another opportunity to discuss about the way to handle such special
formats in the resampling layer. Would you please post this patchset
again without these assertions?
(Additionally, I suspect that current implementation of the resampling
layer itself is not enough proper for non-PCM samples such as float, DSD
and so on.)
And next time, please follow a below instruction.
1.use '--subject-prefix' option with 'alsa-lib][PATCH v3' for
git-format-patch so that subject of your patches includes better
information for reviewers.
(If it's for kernel stuffs, just use '--v')
2. use '--cover-letter' option for git-format-patch so that you can
write changelog of your successive work.
3.use '--thread' option for git-send-email (or add 'sendemail.thread'
entry into your local repository) so that posted patches spin in one
message thread.
Thanks
Takashi Sakamoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
