* [PATCH 1/3] ALSA: pcm: use helper functions to refer parameters as constants
2017-06-09 0:34 [PATCH 0/3] ALSA: pcm: Takashi Sakamoto
@ 2017-06-09 0:34 ` Takashi Sakamoto
2017-06-09 0:34 ` [PATCH 2/3] ALSA: pcm: calculate non-mask/non-interval parameters always when possible Takashi Sakamoto
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Takashi Sakamoto @ 2017-06-09 0:34 UTC (permalink / raw)
To: tiwai, perex; +Cc: alsa-devel
To fixup some parameters, ALSA PCM core refers the other parameters as
constants. There're some macros for this purpose.
This commit replaces codes with them.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
sound/core/pcm_native.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 422ee4629698..87a507f12f2f 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -444,8 +444,8 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params)
{
struct snd_pcm_hardware *hw;
- struct snd_interval *i = NULL;
- struct snd_mask *m = NULL;
+ const struct snd_interval *i;
+ const struct snd_mask *m;
int err;
params->info = 0;
@@ -470,13 +470,13 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream,
return err;
if (!params->msbits) {
- i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS);
+ i = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS);
if (snd_interval_single(i))
params->msbits = snd_interval_value(i);
}
if (!params->rate_den) {
- i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE);
+ i = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_RATE);
if (snd_interval_single(i)) {
params->rate_num = snd_interval_value(i);
params->rate_den = 1;
@@ -492,8 +492,8 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream,
SNDRV_PCM_INFO_MMAP_VALID);
}
if (!params->fifo_size) {
- m = hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT);
- i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS);
+ m = hw_param_mask_c(params, SNDRV_PCM_HW_PARAM_FORMAT);
+ i = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_CHANNELS);
if (snd_mask_single(m) && snd_interval_single(i)) {
err = substream->ops->ioctl(substream,
SNDRV_PCM_IOCTL1_FIFO_SIZE, params);
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] ALSA: pcm: calculate non-mask/non-interval parameters always when possible
2017-06-09 0:34 [PATCH 0/3] ALSA: pcm: Takashi Sakamoto
2017-06-09 0:34 ` [PATCH 1/3] ALSA: pcm: use helper functions to refer parameters as constants Takashi Sakamoto
@ 2017-06-09 0:34 ` Takashi Sakamoto
2017-06-09 0:34 ` [PATCH 3/3] ALSA: pcm: move fixup of info flag after selecting single parameters Takashi Sakamoto
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Takashi Sakamoto @ 2017-06-09 0:34 UTC (permalink / raw)
To: tiwai, perex; +Cc: alsa-devel
A structure for parameters of PCM runtime has parameters which are
not classified as mask/interval type. They are decided only when
corresponding normal parameters have unique values.
* struct snd_pcm_hw_params.msbits
* struct snd_pcm_hw_params.rate_num
* struct snd_pcm_hw_params.rate_den
* struct snd_pcm_hw_params.fifo_size
Current implementation of hw_params ioctl sometimes doesn't decide these
parameters even if corresponding parameters are fixed, because these
parameters are evaluated before a call of snd_pcm_hw_params_choose().
This commit adds a helper function to process the parameters and call it
in proper positions.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
sound/core/pcm_native.c | 70 +++++++++++++++++++++++++++++++------------------
1 file changed, 44 insertions(+), 26 deletions(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 87a507f12f2f..dfe6113a6a60 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -440,12 +440,45 @@ static int constrain_params_by_rules(struct snd_pcm_substream *substream,
return 0;
}
+static int fixup_unreferenced_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params)
+{
+ const struct snd_interval *i;
+ const struct snd_mask *m;
+ int err;
+
+ if (!params->msbits) {
+ i = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS);
+ if (snd_interval_single(i))
+ params->msbits = snd_interval_value(i);
+ }
+
+ if (!params->rate_den) {
+ i = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_RATE);
+ if (snd_interval_single(i)) {
+ params->rate_num = snd_interval_value(i);
+ params->rate_den = 1;
+ }
+ }
+
+ if (!params->fifo_size) {
+ m = hw_param_mask_c(params, SNDRV_PCM_HW_PARAM_FORMAT);
+ i = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_CHANNELS);
+ if (snd_mask_single(m) && snd_interval_single(i)) {
+ err = substream->ops->ioctl(substream,
+ SNDRV_PCM_IOCTL1_FIFO_SIZE, params);
+ if (err < 0)
+ return err;
+ }
+ }
+
+ return 0;
+}
+
int snd_pcm_hw_refine(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params)
{
struct snd_pcm_hardware *hw;
- const struct snd_interval *i;
- const struct snd_mask *m;
int err;
params->info = 0;
@@ -469,20 +502,6 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream,
if (err < 0)
return err;
- if (!params->msbits) {
- i = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS);
- if (snd_interval_single(i))
- params->msbits = snd_interval_value(i);
- }
-
- if (!params->rate_den) {
- i = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_RATE);
- if (snd_interval_single(i)) {
- params->rate_num = snd_interval_value(i);
- params->rate_den = 1;
- }
- }
-
hw = &substream->runtime->hw;
if (!params->info) {
params->info = hw->info & ~(SNDRV_PCM_INFO_FIFO_IN_FRAMES |
@@ -491,16 +510,7 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream,
params->info &= ~(SNDRV_PCM_INFO_MMAP |
SNDRV_PCM_INFO_MMAP_VALID);
}
- if (!params->fifo_size) {
- m = hw_param_mask_c(params, SNDRV_PCM_HW_PARAM_FORMAT);
- i = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_CHANNELS);
- if (snd_mask_single(m) && snd_interval_single(i)) {
- err = substream->ops->ioctl(substream,
- SNDRV_PCM_IOCTL1_FIFO_SIZE, params);
- if (err < 0)
- return err;
- }
- }
+
params->rmask = 0;
return 0;
}
@@ -517,6 +527,8 @@ static int snd_pcm_hw_refine_user(struct snd_pcm_substream *substream,
return PTR_ERR(params);
err = snd_pcm_hw_refine(substream, params);
+ if (err >= 0)
+ err = fixup_unreferenced_params(substream, params);
if (copy_to_user(_params, params, sizeof(*params))) {
if (!err)
err = -EFAULT;
@@ -596,6 +608,10 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
if (err < 0)
goto _error;
+ err = fixup_unreferenced_params(substream, params);
+ if (err < 0)
+ goto _error;
+
if (substream->ops->hw_params != NULL) {
err = substream->ops->hw_params(substream, params);
if (err < 0)
@@ -3621,6 +3637,8 @@ static int snd_pcm_hw_refine_old_user(struct snd_pcm_substream *substream,
}
snd_pcm_hw_convert_from_old_params(params, oparams);
err = snd_pcm_hw_refine(substream, params);
+ if (err >= 0)
+ err = fixup_unreferenced_params(substream, params);
snd_pcm_hw_convert_to_old_params(oparams, params);
if (copy_to_user(_oparams, oparams, sizeof(*oparams))) {
if (!err)
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] ALSA: pcm: move fixup of info flag after selecting single parameters
2017-06-09 0:34 [PATCH 0/3] ALSA: pcm: Takashi Sakamoto
2017-06-09 0:34 ` [PATCH 1/3] ALSA: pcm: use helper functions to refer parameters as constants Takashi Sakamoto
2017-06-09 0:34 ` [PATCH 2/3] ALSA: pcm: calculate non-mask/non-interval parameters always when possible Takashi Sakamoto
@ 2017-06-09 0:34 ` Takashi Sakamoto
2017-06-09 1:45 ` [PATCH 0/3] ALSA: pcm: Takashi Sakamoto
2017-06-09 7:19 ` Takashi Iwai
4 siblings, 0 replies; 6+ messages in thread
From: Takashi Sakamoto @ 2017-06-09 0:34 UTC (permalink / raw)
To: tiwai, perex; +Cc: alsa-devel
When drivers register no flags about information of PCM hardware, ALSA
PCM core fixups it roughly. Currently, this operation places in a
function snd_pcm_hw_refine(). It can be moved to a function
fixup_unreferenced_params() because it doesn't affects operations
between these two functions.
This idea is better to bundle codes with similar purposes and this commit
achieves it.
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
sound/core/pcm_native.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index dfe6113a6a60..3293db0172db 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -472,13 +472,21 @@ static int fixup_unreferenced_params(struct snd_pcm_substream *substream,
}
}
+ if (!params->info) {
+ params->info = substream->runtime->hw.info;
+ params->info &= ~(SNDRV_PCM_INFO_FIFO_IN_FRAMES |
+ SNDRV_PCM_INFO_DRAIN_TRIGGER);
+ if (!hw_support_mmap(substream))
+ params->info &= ~(SNDRV_PCM_INFO_MMAP |
+ SNDRV_PCM_INFO_MMAP_VALID);
+ }
+
return 0;
}
int snd_pcm_hw_refine(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params)
{
- struct snd_pcm_hardware *hw;
int err;
params->info = 0;
@@ -502,16 +510,8 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream,
if (err < 0)
return err;
- hw = &substream->runtime->hw;
- if (!params->info) {
- params->info = hw->info & ~(SNDRV_PCM_INFO_FIFO_IN_FRAMES |
- SNDRV_PCM_INFO_DRAIN_TRIGGER);
- if (!hw_support_mmap(substream))
- params->info &= ~(SNDRV_PCM_INFO_MMAP |
- SNDRV_PCM_INFO_MMAP_VALID);
- }
-
params->rmask = 0;
+
return 0;
}
EXPORT_SYMBOL(snd_pcm_hw_refine);
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/3] ALSA: pcm:
2017-06-09 0:34 [PATCH 0/3] ALSA: pcm: Takashi Sakamoto
` (2 preceding siblings ...)
2017-06-09 0:34 ` [PATCH 3/3] ALSA: pcm: move fixup of info flag after selecting single parameters Takashi Sakamoto
@ 2017-06-09 1:45 ` Takashi Sakamoto
2017-06-09 7:19 ` Takashi Iwai
4 siblings, 0 replies; 6+ messages in thread
From: Takashi Sakamoto @ 2017-06-09 1:45 UTC (permalink / raw)
To: alsa-devel
Oops, I sent this patchset with cover-letter without good subject line,
sorry...
On Jun 9 2017 09:34, Takashi Sakamoto wrote:
> This is a part of layout of the structure.
>
> struct snd_pcm_hw_params {
> ...
> unsigned int info;
> unsigned int msbits;
> unsigned int rate_num;
> unsigned int rate_den;
> snd_pcm_uframes_t fifo_size;
> ...
> };
>
> For example, although the 'msbits', 'rate_num' and 'rate_den' fields are
> filled in a result of HW_REFINE command conditionally, they're never
> filled in a result of HW_PARAMS command. This seems a bug.
I note that there's a case for HW_PARAMS that the result includes proper
values to these parameters. When drivers register constrain rules
relevant to them, the members are filled.
Regards
Takashi Sakamoto
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 0/3] ALSA: pcm:
2017-06-09 0:34 [PATCH 0/3] ALSA: pcm: Takashi Sakamoto
` (3 preceding siblings ...)
2017-06-09 1:45 ` [PATCH 0/3] ALSA: pcm: Takashi Sakamoto
@ 2017-06-09 7:19 ` Takashi Iwai
4 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2017-06-09 7:19 UTC (permalink / raw)
To: Takashi Sakamoto; +Cc: alsa-devel
On Fri, 09 Jun 2017 02:34:37 +0200,
Takashi Sakamoto wrote:
>
> Hi,
>
> This patchset is a revised version of my former RFC.
>
>
> [alsa-devel] [PATCH RFC 00/21] ALSA: pcm: add tracepoints for PCM params operation
> http://mailman.alsa-project.org/pipermail/alsa-devel/2017-May/120548.html
>
> In ALSA PCM interface, applications can get hardware capability as
> 'struct snd_pcm_hw_params' type of data returned by a call of ioctl(2)
> with SNDRV_PCM_IOCTL_HW_REFINE/SNDRV_PCM_IOCTL_HW_PARAMS' commands.
> Results of these commands should be the same in a certain conditions but
> actually it's not.
>
> This is a part of layout of the structure.
>
> struct snd_pcm_hw_params {
> ...
> unsigned int info;
> unsigned int msbits;
> unsigned int rate_num;
> unsigned int rate_den;
> snd_pcm_uframes_t fifo_size;
> ...
> };
>
> For example, although the 'msbits', 'rate_num' and 'rate_den' fields are
> filled in a result of HW_REFINE command conditionally, they're never
> filled in a result of HW_PARAMS command. This seems a bug.
>
> This patchset attempts to fix the bug. This affects userspace applications
> in cotents of the result, but it's acceptrable with a merit for the
> applications to get enough parameters.
>
> Takashi Sakamoto (3):
> ALSA: pcm: use helper functions to refer parameters as constants
> ALSA: pcm: calculate non-mask/non-interval parameters always when
> possible
> ALSA: pcm: move fixup of info flag after selecting single parameters
Applied all three patches now. Thanks.
Takashi
^ permalink raw reply [flat|nested] 6+ messages in thread