pulseaudio: fix PaPulseAudio_Initialize unlocking #852
Conversation
illuusio
force-pushed
the
pulseaudio-papulseaudio_initialize-848
branch
2 times, most recently
from
a53789e to
ba8f5f0
Compare
illuusio
force-pushed
the
pulseaudio-papulseaudio_initialize-848
branch
3 times, most recently
from
f6fbdfb to
96fa25d
Compare
|
@illuusio Changes look good to me. I also gave it a quick test and it worked. |
| @@ -589,6 +597,7 @@ PaError PaPulseAudio_Initialize( PaUtilHostApiRepresentation ** hostApi, | |||
| break; | |||
| case PA_CONTEXT_TERMINATED: | |||
| case PA_CONTEXT_FAILED: | |||
| result = paUnanticipatedHostError; | |||
There was a problem hiding this comment.
I think there's a missing call to PaUtil_SetLastHostErrorInfo or PA_PULSEAUDIO_SET_LAST_HOST_ERROR here. perhaps separate calls for each case.
There was a problem hiding this comment.
I'll work this out in other PR as I noticed that error reporting is not up-to-date. I this is mandatory I'll add it in here but it would be easier to have own PR to make sure every Error report is correct not just this one.
There was a problem hiding this comment.
It's fine (and good) to do additional cleanup later, but this PR adds a paUnanticipatedHostError. Our policy is to not accept PRs that introduce new problems, so we'd appreciate it if you could fix this case in this PR please.
There was a problem hiding this comment.
Ok now it should be solved
There was a problem hiding this comment.
Thanks, but now the first parameter to PA_PULSEAUDIO_SET_LAST_HOST_ERROR is not correct. It should be the native error code definitely not a PortAudio error code. For this PR, you could just set it to 0 if you don't have anything better. For further info check the definition of PA_PULSEAUDIO_SET_LAST_HOST_ERROR in pa_linux_pulseaudio_internal.h and then the definition of PaUtil_SetLastHostErrorInfo in pa_util.h.
Also, initiliaze is mis-spelled on line 606.
While I'm here, note that you'll need to fix the PA_PULSEAUDIO_SET_LAST_HOST_ERROR in a later PR to pass the correct host api type.
There was a problem hiding this comment.
I've correct the errors and spelling and actually hostapi type is fixed in another PR but I can fix it here if it's needed.
There was a problem hiding this comment.
I've removed those new error codes and now it behaves as it was before only it goes to error (problem is that it just return PaNoError) but as it did worked like that lately it should be ok.
illuusio
force-pushed
the
pulseaudio-papulseaudio_initialize-848
branch
from
96fa25d to
233152a
Compare
illuusio
force-pushed
the
pulseaudio-papulseaudio_initialize-848
branch
2 times, most recently
from
33d8b89 to
029df8a
Compare
Make sure that if PaPulseAudio_Initialize fails that a) Mainloop is unlocked if HostAPI is initialized b) Unlocking is done before freeing c) It's done only once Also move allocations group freeing code to correct place now there only one place where it's called
illuusio
force-pushed
the
pulseaudio-papulseaudio_initialize-848
branch
from
029df8a to
8efc877
Compare
When PaPulseAudio_Initialize fails it does not currently do it very well as bug report #848 tells. Now Only free lock if it's initialized enough to be unlocked.
Unlocking is done after allocations are done. That is because Locking is just after allocations and next
goto errorshould have allocated memory and drop to that. Also now Group freeing code is one place.Resolves #848