From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: snd_pcm_set_params faild on set_buffer_time_near and set_period_time_near Date: Fri, 24 Jul 2015 10:21:17 +0200 Message-ID: References: <55AE0831.2010206@streamunlimited.com> <55B1EEAE.3080307@streamunlimited.com> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 16CA4260558 for ; Fri, 24 Jul 2015 10:21:18 +0200 (CEST) In-Reply-To: <55B1EEAE.3080307@streamunlimited.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Martin Geier Cc: alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org 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 ] > > >From 2d4e94d974678bbd4826b33c59c275d1f34edc20 Mon Sep 17 00:00:00 2001 > From: Martin Geier > 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 >