[Portaudio] pa_linux_alsa.c:3636 Assertion failed (with hack fix)

Ross Bencina rossb-lists at audiomulch.com
Tue Nov 6 08:17:33 EST 2018


Hi Alan, Phil,

I'm OK with the new patch. It's a step forward.

A possible improvement: Check, and if the error is not EPIPE (or 
whatever else is expected) log a warning, something like "warning: 
unexpected error (-42) -- recovering as if XRUN". That would at least 
alert us to other conditions that need to be handled (and make it easier 
to test unplugging USB devices etc).

Ross.




On 6/11/2018 8:33 PM, Alan Horstmann wrote:
> Hi Phil,
> 
> Bear in mind, without this change any negative return from
> snd_pcm_poll_descriptors() will fail the assert; for Portaudio to have worked
> with asserts enabled those error returns must be extremely rare!  When I
> asked on the Alsa-devel list it seemed various plugins could potentially give
> different error return values, but there was no suggestion that they needed
> handling differently.  So I judged at this stage a catch-all approach should
> at least be an improvement on the existing code which does not handle the
> negative returns at all!  Unplugging of USB devices does create tricky
> situations indeed.
> 
> Alan
> 
> On Sunday 04 November 2018 20:52, Phil Burk wrote:
>> Thanks for working on this.
>> If all negative error codes are treated as xruns, then might important
>> errors be ignored?
>> What happens if a USB device is unplugged when running?
>> Maybe we should check for specific codes, eg. EPIPE.
>>
>> Phil Burk
>>
>>
>> On Sun, Nov 4, 2018 at 7:53 AM Alan Horstmann <gineera at aspect135.co.uk>
>>
>> wrote:
>>> Hi Ross & others,
>>>
>>> Thanks for casting an eye over this, and you are right the handling is
>>> not quite correct.
>>>
>>> Having looked over at it again, where lower level errors are considered
>>> to be
>>> due to an Xrun, this is not treated as paUnanticipatedHostError at the
>>> next
>>> level up, but rather as a recoverable situation.  So I have amended the
>>> patch
>>> along these lines, not setting an error in the higher function, but still
>>> exiting early and triggering recovery.
>>>
>>> As for earlier discussion about return types, it is somewhat a matter of
>>> style, but the PaAlsaStreamComponent_BeginPolling() function is very
>>> short (probably inlined by the compiler) and PaError is not really
>>> appropriate. By
>>> returning the actual Alsa error code there is the potential if needed to
>>> respond differently to different Alsa errors (although my understanding
>>> ATM
>>> is that recovery action is the appropriate response for all).
>>>
>>> So, here is a version-3 patch - a retest would be appreciated!
>>>
>>> Regards
>>>
>>> Alan
>>>
>>> On Wednesday 17 October 2018 11:01, you wrote:
>>>> Hi Alan,
>>>>
>>>> I am no fan of those PA_ENSURE et al macros, however the PA_ENSURE
>>>> macro usage that you removed sets the `result` local variable prior to
>>>> jumping to the error: label. I think you should do the same and set
>>>> `result` to something valid prior to `goto error`.
>>>>
>>>> As a side note: it looks to me as though PaAlsaStream_WaitForFrames has
>>>> the potential to go into an infinite loop if the PA_ENSURE in the
>>>> error: section fails (since it will just `goto error;` again). But
>>>> don't let me distract you.
>>>>
>>>> Ross.
>>>>
>>>> On 16/10/2018 5:54 AM, Alan Horstmann wrote:
>>>>> On Monday 15 October 2018 04:40, Chameleon wrote:
>>>>>> I can confirm that it works (by deliberately causing an xrun by
>>>>>> taking too long in the audiocallback on my system) and from reading
>>>>>> the code
>>>
>>> it
>>>
>>>>>> looks correct. I can also confirm that I'm using alsa's dmix plugin.
>>>>>>
>>>>>> The only small thing is that PaAlsaStreamComponent_BeginPolling(..)
>>>
>>> has
>>>
>>>>>> a return type of PaError, which may at some point be used with
>>>>>> Pa_GetErrorText(..) which would then return "Invalid error code" on
>>>
>>> the
>>>
>>>>>> alsa/system error code. A (potential) cosmetic bug on a bug fix that
>>>>>> stops a library crash, so I offer it entirely as an observation and
>>>
>>> not
>>>
>>>>>> as an issue that requires attention.
>>>>>
>>>>> Thanks for the confirmation.  Indeed, 'dmix' was mentioned as one
>>>
>>> plugin
>>>
>>>>> that would return EPIPE to indicate XRUN.
>>>>>
>>>>> As far as the return type is concerned, that was something I had also
>>>>> seen as slightly untidy, but left it in haste to minimise changes.
>>>
>>> But,
>>>
>>>>> since the return would not be checked by PA_ENSURE(.), it would
>>>>> really
>>>
>>> be
>>>
>>>>> better to clean it up to an int, and remove 'result' as per this
>>>>> version-2 patch, attached.  Could you check this one instead, please?
>>>>>
>>>>> Phil, Ross, are you happy with this change?
>>>>>
>>>>> Regards
>>>>>
>>>>> Alan
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Portaudio mailing list
>>>>> Portaudio at lists.columbia.edu
>>>>> https://lists.columbia.edu/mailman/listinfo/portaudio
>>>
>>> _______________________________________________
>>> Portaudio mailing list
>>> Portaudio at lists.columbia.edu
>>> https://lists.columbia.edu/mailman/listinfo/portaudio
> _______________________________________________
> Portaudio mailing list
> Portaudio at lists.columbia.edu
> https://lists.columbia.edu/mailman/listinfo/portaudio
> 
> 


More information about the Portaudio mailing list