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

Alan Horstmann gineera at aspect135.co.uk
Tue Nov 6 04:33:49 EST 2018


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


More information about the Portaudio mailing list