diff mbox

snd_pcm_set_params faild on set_buffer_time_near and set_period_time_near

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

Commit Message

Martin Geier July 24, 2015, 11:16 a.m. UTC 
Patch with fixed name and description.

Martin


On 24.07.2015 10:21, Takashi Iwai wrote:
> 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
>>

Comments

Raymond Yau July 25, 2015, 2:01 a.m. UTC | #1 
2015-07-24 19:16 GMT+08:00 Martin Geier <martin.geier@streamunlimited.com>:

> Patch with fixed name and description.
>
> Martin
>
>
>
> On 24.07.2015 10:21, Takashi Iwai wrote:
>
>> 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
>>>>>
>>>>

What is the period time and buffer time after applied your patch ?

Are latency always <= 50ms ?
Martin Geier July 27, 2015, 7:12 a.m. UTC | #2 
Hi Raymond,

Output parameters with custom logs:
set_buffer_time_near: -22
set period time: 12517
get period size: 276
set buffer size: 1104
get buffer size: 1104

ACCESS:  RW_INTERLEAVED
FORMAT:  S16_LE
SUBFORMAT:  STD
SAMPLE_BITS: 16
FRAME_BITS: 32
CHANNELS: 2
RATE: 22050
PERIOD_TIME: (12517 12518)
PERIOD_SIZE: 276
PERIOD_BYTES: 1104
PERIODS: 4
BUFFER_TIME: (50068 50069)
BUFFER_SIZE: 1104
BUFFER_BYTES: 4416

What do you mean, are latency always <= 50ms? In what cases?

Martin

On 25.07.2015 04:01, Raymond Yau wrote:
>
>
> 2015-07-24 19:16 GMT+08:00 Martin Geier 
> <martin.geier@streamunlimited.com 
> <mailto:martin.geier@streamunlimited.com>>:
>
>     Patch with fixed name and description.
>
>     Martin
>
>
>
>     On 24.07.2015 10:21, Takashi Iwai wrote:
>
>         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
>
>
>
> What is the period time and buffer time after applied your patch ?
>
> Are latency always <= 50ms ?
Raymond Yau July 27, 2015, 8:36 a.m. UTC | #3 
2015-7-27 ??3:12? "Martin Geier" <martin.geier@streamunlimited.com>???
>
> Hi Raymond,
>
> Output parameters with custom logs:
> set_buffer_time_near: -22
> set period time: 12517
> get period size: 276
> set buffer size: 1104
> get buffer size: 1104
>
>
> ACCESS:  RW_INTERLEAVED
> FORMAT:  S16_LE
> SUBFORMAT:  STD
> SAMPLE_BITS: 16
> FRAME_BITS: 32
> CHANNELS: 2
> RATE: 22050
> PERIOD_TIME: (12517 12518)
> PERIOD_SIZE: 276
> PERIOD_BYTES: 1104
> PERIODS: 4
> BUFFER_TIME: (50068 50069)
> BUFFER_SIZE: 1104
> BUFFER_BYTES: 4416
>
> What do you mean, are latency always <= 50ms? In what cases?

http://www.alsa-project.org/alsa-doc/alsa-lib/group___p_c_m.html#ga45d50841b307f2156fce1857bfac228c

Set the hardware and software parameters in a simple way.
Parameters:pcm PCM handle
format required PCM format
access required PCM access
channels required PCM channels
rate required sample rate in Hz
soft_resample 0 = disallow alsa-lib resample stream, 1 = allow resampling
latency required overall latency in us

Refer to your result seem latency only close to 50ms
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 latency failed

When method snd_pcm_set_params sets sample rate to 22050 and latency to 50000 
to davinci soc driver method snd_pcm_hw_params_set_buffer_time_near fails
and variable params is already changed in the method so the next method 
snd_pcm_hw_params_set_period_time_near fails also.

Signed-off-by: Martin Geier <martin.geier@streamunlimited.com>
---
 src/pcm/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, params_saved;
         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 */
+	params_saved = *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 = params_saved;
         	/* 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