diff mbox

snd_pcm_set_params faild on set_buffer_time_near and set_period_time_near

Message ID 55B1EEAE.3080307@streamunlimited.com (mailing list archive)
State New, archived
Headers show

Commit Message

Martin Geier July 24, 2015, 7:52 a.m. UTC 
Thanks for help,

in attachment is patch with suggest change.
Is the patch acceptable?

Martin

On 23.07.2015 10:11, Takashi Iwai wrote:
> On Tue, 21 Jul 2015 10:52:01 +0200,
> Martin Geier wrote:
>> Hallo,
>>
>> I tried to configure alsa device with snd_pcm_set_params function,
>> requested
>> parameters was:
>> channel 2, rate: 22050, soft_resample: 1, latency: 50000
>>
>> Function snd_pcm_hw_params_set_buffer_time_near and
>> snd_pcm_hw_param_set_near
>> is called with:
>> min: 50000, max: 50000, mindir: 0, maxdir: -1
>>
>> and input params was:
>>
>> ACCESS:  RW_INTERLEAVED
>> FORMAT:  S16_LE
>> SUBFORMAT:  STD
>> SAMPLE_BITS: 16
>> FRAME_BITS: 32
>> CHANNELS: 2
>> RATE: 22050
>> PERIOD_TIME: (362 743039)
>> PERIOD_SIZE: [8 16384]
>> PERIOD_BYTES: [32 65536]
>> PERIODS: [2 19]
>> BUFFER_TIME: (725 1486078)
>> BUFFER_SIZE: [16 32768]
>> BUFFER_BYTES: [64 131072]
>> TICK_TIME: ALL
>>
>> unfortunately, this function fails, on function snd_pcm_hw_param_set_first:
>>
>> ALSA ERROR hw_params: set_near (BUFFER_TIME)
>>              value = 50000 : Invalid argument
>> ACCESS:  RW_INTERLEAVED
>> FORMAT:  S16_LE
>> SUBFORMAT:  STD
>> SAMPLE_BITS: 16
>> FRAME_BITS: 32
>> CHANNELS: 2
>> RATE: 22050
>> PERIOD_TIME: (4580 5533)
>> PERIOD_SIZE: NONE
>> PERIOD_BYTES: [404 488]
>> PERIODS: 10
>> BUFFER_TIME: (50022 50023)
>> BUFFER_SIZE: 1103
>> BUFFER_BYTES: 4412
>> TICK_TIME: ALL
>>
>> This shouldn't be problem because in snd_pcm_set_params is
>> snd_pcm_hw_params_set_period_time_near called.
>> This function should be called with same params as
>> snd_pcm_hw_params_set_buffer_time_near
>> but the input params are:
>>
>> snd_pcm_hw_param_set_near: min: 12500, max: 12500, mindir: 0, maxdir: -1
>> hw_params: snd_pcm_hw_param_set_near (PERIOD_TIME)
>> ACCESS:  RW_INTERLEAVED
>> FORMAT:  S16_LE
>> SUBFORMAT:  STD
>> SAMPLE_BITS: 16
>> FRAME_BITS: 32
>> CHANNELS: 2
>> RATE: 22050
>> PERIOD_TIME: (4580 5533)
>> PERIOD_SIZE: NONE
>> PERIOD_BYTES: [404 488]
>> PERIODS: 10
>> BUFFER_TIME: (50022 50023)
>> BUFFER_SIZE: 1103
>> BUFFER_BYTES: 4412
>> TICK_TIME: ALL
>>
>> because in snd_pcm_hw_param_set_near is
>>
>> if (last)
>>       err = snd_pcm_hw_param_set_last(pcm, params, var, val, dir);
>> else
>>       err = snd_pcm_hw_param_set_first(pcm, params, var, val, dir);
>> if (err < 0)
>>       dump_hw_params(params, "set_near", var, *val, err);
>> return err;
>>
>> and in error part is only debug output and not restore original parameters.
>>
>> Environment:
>> alsa-lib 1.0.27.1
>> TI AM335x soc
>>
>> Simple fix is attached, but I am not sure if it is correct.
> Thanks for analysis and patch.  It's however rather a bug in
> snd_pcm_set_params().  There is no guarantee to keep the old value in
> snd_pcm_set_*(), thus the caller needs to take care of it instead.
>
>
> Takashi
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Comments

Takashi Iwai July 24, 2015, 8:21 a.m. UTC | #1 
On Fri, 24 Jul 2015 09:52:14 +0200,
Martin Geier wrote:
> 
> Thanks for help,
> 
> in attachment is patch with suggest change.
> Is the patch acceptable?

I prefer a bit clearer name like params_saved or such, but yes, it's
what I had in my mind, too.  Could you tidy it up and resubmit with
the more description -- why it's needed and what does it?  Last but
not least, it'd be helpful if you give also signed-off-by tag.


thanks,

Takashi

> 
> Martin
> 
> On 23.07.2015 10:11, Takashi Iwai wrote:
> > On Tue, 21 Jul 2015 10:52:01 +0200,
> > Martin Geier wrote:
> >> Hallo,
> >>
> >> I tried to configure alsa device with snd_pcm_set_params function,
> >> requested
> >> parameters was:
> >> channel 2, rate: 22050, soft_resample: 1, latency: 50000
> >>
> >> Function snd_pcm_hw_params_set_buffer_time_near and
> >> snd_pcm_hw_param_set_near
> >> is called with:
> >> min: 50000, max: 50000, mindir: 0, maxdir: -1
> >>
> >> and input params was:
> >>
> >> ACCESS:  RW_INTERLEAVED
> >> FORMAT:  S16_LE
> >> SUBFORMAT:  STD
> >> SAMPLE_BITS: 16
> >> FRAME_BITS: 32
> >> CHANNELS: 2
> >> RATE: 22050
> >> PERIOD_TIME: (362 743039)
> >> PERIOD_SIZE: [8 16384]
> >> PERIOD_BYTES: [32 65536]
> >> PERIODS: [2 19]
> >> BUFFER_TIME: (725 1486078)
> >> BUFFER_SIZE: [16 32768]
> >> BUFFER_BYTES: [64 131072]
> >> TICK_TIME: ALL
> >>
> >> unfortunately, this function fails, on function snd_pcm_hw_param_set_first:
> >>
> >> ALSA ERROR hw_params: set_near (BUFFER_TIME)
> >>              value = 50000 : Invalid argument
> >> ACCESS:  RW_INTERLEAVED
> >> FORMAT:  S16_LE
> >> SUBFORMAT:  STD
> >> SAMPLE_BITS: 16
> >> FRAME_BITS: 32
> >> CHANNELS: 2
> >> RATE: 22050
> >> PERIOD_TIME: (4580 5533)
> >> PERIOD_SIZE: NONE
> >> PERIOD_BYTES: [404 488]
> >> PERIODS: 10
> >> BUFFER_TIME: (50022 50023)
> >> BUFFER_SIZE: 1103
> >> BUFFER_BYTES: 4412
> >> TICK_TIME: ALL
> >>
> >> This shouldn't be problem because in snd_pcm_set_params is
> >> snd_pcm_hw_params_set_period_time_near called.
> >> This function should be called with same params as
> >> snd_pcm_hw_params_set_buffer_time_near
> >> but the input params are:
> >>
> >> snd_pcm_hw_param_set_near: min: 12500, max: 12500, mindir: 0, maxdir: -1
> >> hw_params: snd_pcm_hw_param_set_near (PERIOD_TIME)
> >> ACCESS:  RW_INTERLEAVED
> >> FORMAT:  S16_LE
> >> SUBFORMAT:  STD
> >> SAMPLE_BITS: 16
> >> FRAME_BITS: 32
> >> CHANNELS: 2
> >> RATE: 22050
> >> PERIOD_TIME: (4580 5533)
> >> PERIOD_SIZE: NONE
> >> PERIOD_BYTES: [404 488]
> >> PERIODS: 10
> >> BUFFER_TIME: (50022 50023)
> >> BUFFER_SIZE: 1103
> >> BUFFER_BYTES: 4412
> >> TICK_TIME: ALL
> >>
> >> because in snd_pcm_hw_param_set_near is
> >>
> >> if (last)
> >>       err = snd_pcm_hw_param_set_last(pcm, params, var, val, dir);
> >> else
> >>       err = snd_pcm_hw_param_set_first(pcm, params, var, val, dir);
> >> if (err < 0)
> >>       dump_hw_params(params, "set_near", var, *val, err);
> >> return err;
> >>
> >> and in error part is only debug output and not restore original parameters.
> >>
> >> Environment:
> >> alsa-lib 1.0.27.1
> >> TI AM335x soc
> >>
> >> Simple fix is attached, but I am not sure if it is correct.
> > Thanks for analysis and patch.  It's however rather a bug in
> > snd_pcm_set_params().  There is no guarantee to keep the old value in
> > snd_pcm_set_*(), thus the caller needs to take care of it instead.
> >
> >
> > Takashi
> > _______________________________________________
> > Alsa-devel mailing list
> > Alsa-devel@alsa-project.org
> > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 
> -- 
> *StreamUnlimited*
> High Tech Campus Vienna, Gutheil-Schoder-Gasse 10, A-1102 Vienna, Austria
> Levocska 9, 851 01 Bratislava, Slovakia
> Email: martin.geier@streamunlimited.com
> www.streamunlimited.com
> 
> Meet us at:
> IFA - Berlin, 4-9 September 2015
> HK Electronics - Hong Kong, 13-16 October 2015
> CEDIA - Dallas, 14-17 October 2015
> [1.2  <text/html; windows-1252 (7bit)>]
> 
> >From 2d4e94d974678bbd4826b33c59c275d1f34edc20 Mon Sep 17 00:00:00 2001
> From: Martin Geier <martin.geier@streamunlimited.com>
> Date: Fri, 24 Jul 2015 09:30:57 +0200
> Subject: [PATCH] restore hw params on set rate failed
> 
> ---
>  pcm.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
> index ca4d416..b6de73e 100644
> --- a/src/pcm/pcm.c
> +++ b/src/pcm/pcm.c
> @@ -7883,7 +7883,7 @@ int snd_pcm_set_params(snd_pcm_t *pcm,
>                         int soft_resample,
>                         unsigned int latency)
>  {
> -        snd_pcm_hw_params_t *params;
> +        snd_pcm_hw_params_t *params, pparams;
>          snd_pcm_sw_params_t *swparams;
>          const char *s = snd_pcm_stream_name(snd_pcm_stream(pcm));
>          snd_pcm_uframes_t buffer_size, period_size;
> @@ -7936,9 +7936,11 @@ int snd_pcm_set_params(snd_pcm_t *pcm,
>  		return -EINVAL;
>  	}
>  	/* set the buffer time */
> +	pparams = *params;
>  	err = INTERNAL(snd_pcm_hw_params_set_buffer_time_near)(pcm, params, &latency, NULL);
>  	if (err < 0) {
>  	        /* error path -> set period size as first */
> +			*params = pparams;
>          	/* set the period time */
>          	period_time = latency / 4;
>          	err = INTERNAL(snd_pcm_hw_params_set_period_time_near)(pcm, params, &period_time, NULL);
> -- 
> 1.9.1
>
diff mbox

Patch

From 2d4e94d974678bbd4826b33c59c275d1f34edc20 Mon Sep 17 00:00:00 2001
From: Martin Geier <martin.geier@streamunlimited.com>
Date: Fri, 24 Jul 2015 09:30:57 +0200
Subject: [PATCH] restore hw params on set rate failed

---
 pcm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
index ca4d416..b6de73e 100644
--- a/src/pcm/pcm.c
+++ b/src/pcm/pcm.c
@@ -7883,7 +7883,7 @@  int snd_pcm_set_params(snd_pcm_t *pcm,
                        int soft_resample,
                        unsigned int latency)
 {
-        snd_pcm_hw_params_t *params;
+        snd_pcm_hw_params_t *params, pparams;
         snd_pcm_sw_params_t *swparams;
         const char *s = snd_pcm_stream_name(snd_pcm_stream(pcm));
         snd_pcm_uframes_t buffer_size, period_size;
@@ -7936,9 +7936,11 @@  int snd_pcm_set_params(snd_pcm_t *pcm,
 		return -EINVAL;
 	}
 	/* set the buffer time */
+	pparams = *params;
 	err = INTERNAL(snd_pcm_hw_params_set_buffer_time_near)(pcm, params, &latency, NULL);
 	if (err < 0) {
 	        /* error path -> set period size as first */
+			*params = pparams;
         	/* set the period time */
         	period_time = latency / 4;
         	err = INTERNAL(snd_pcm_hw_params_set_period_time_near)(pcm, params, &period_time, NULL);
-- 
1.9.1