[alsa-devel] locking looks odd
David Henningsson
diwic at ubuntu.com
Thu Sep 1 08:27:07 CEST 2016
(Sorry if this email reaches you twice, had some issues)
On 2016-08-17 19:46, Jaroslav Kysela wrote:
> Dne 16.8.2016 v 23:03 Samuel Thibault napsal(a):
>> Hello,
>>
>> We are having odd issues with libasound 1.1.2 which we didn't have with
>> libasound 1.1.1, more precisely
>>
>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=833950
>>
>> so I'm having a look at the locking API introduced in 1.1.2, and there
>> are some oddities:
>>
>> - snd_pcm_new seems to initialize pcm->thread_safe to 0 by default, this
>> does not seem safe. The attached patch initializes it to 1, which
>> fixes the bug in our tests.
>>
>> - snd_pcm_hw_open_fd forces it to 1, thus ignoring what snd_pcm_new set.
> The thread_safe has this meaning:
>
> 0 - the pcm plugin is not thread safe
> 1 - the pcm plugin is thread safe (actually only the hw plugin)
> -1 - disable thread safety
>
> Your patch does not look correct. It's necessary to determine where the
> mutex is locked the second time (use gdb and backtrace for all threads
> for that). Note that plugins may be chained.
Still Samuel's point about -1 being ignored is valid, so I just sent a
patch about that.
But I'm quite sceptic about having that environment variable in the
first place - it seems to me that new apps will start to rely on
alsa-lib doing the locking for them, second a blog post comes along that
tells people to set that environment variable to 0 to maximize
performance, third those apps will crash and the user doesn't understand
why.
>
>> - one can find both __snd_pcm_lock and snd_pcm_lock functions, what is
>> the expected difference between them?
> __snd_pcm_lock/unlock is for forced lock
>
> snd_pcm_lock/unlock skips locking for safe plugins (only hw plugin)
These are quite confusing names, one would expect them to be the same
(snd_pcm_lock being a compatibility wrapper around some internal
__snd_pcm_lock).
I'm not sure about better names, but maybe something like
snd_pcm_lock_all and snd_pcm_lock_ts0 would at least indicate that they
are different functions.
More information about the Alsa-devel
mailing list