Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pulseaudio: Add function to handle Pulseaudio pa_operation #924

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

illuusio
Copy link
Collaborator

@illuusio illuusio commented Jun 9, 2024

This PR unifies handling on pa_operation. It does not add or remove anything just adds function that should make easier not to get into infinite loop at the start.

@illuusio illuusio changed the title pulseaudio: Add macro for handling Pulseaudio pa_operation Pulseaudio: Add macro for handling Pulseaudio pa_operation Jun 9, 2024
@illuusio illuusio requested a review from philburk June 9, 2024 07:31
@illuusio illuusio force-pushed the update-macro-defs branch 2 times, most recently from 7272950 to f6a745a Compare June 9, 2024 07:38
@RossBencina
Copy link
Collaborator

There does not appear to be a reason for this to be a macro, could you make it a regular function please?

@illuusio illuusio changed the title Pulseaudio: Add macro for handling Pulseaudio pa_operation Pulseaudio: Add function to handle Pulseaudio pa_operation Jun 25, 2024
@illuusio
Copy link
Collaborator Author

Ok I've reworked macro to function and made needed changes to code.

src/hostapi/pulseaudio/pa_linux_pulseaudio_cb.c Outdated Show resolved Hide resolved
src/hostapi/pulseaudio/pa_linux_pulseaudio_internal.h Outdated Show resolved Hide resolved
src/hostapi/pulseaudio/pa_linux_pulseaudio_cb.c Outdated Show resolved Hide resolved
src/hostapi/pulseaudio/pa_linux_pulseaudio.c Show resolved Hide resolved

while( pa_operation_get_state( localOperation ) == PA_OPERATION_RUNNING )
{
pa_threaded_mainloop_wait( hostapi->mainloop );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to:
https://freedesktop.org/software/pulseaudio/doxygen/thread-mainloop_8h.html#ac96d19260567d0406350f1b3a14c4bed

the call to pa_threaded_mainloop_wait() needs to be under a lock.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok made needed changes to have locking when using wait

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot find the commits where the locks were added.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad! Not pushed stuff as you mentioned. Now done it and rebased.

@RossBencina RossBencina added the src-pulseaudio PulseAudio Host API in src/hostapi/pulseaudio label Jul 12, 2024
@illuusio
Copy link
Collaborator Author

illuusio commented Aug 5, 2024

@philburk this could be reviewed again is it ok to merge

illuusio and others added 3 commits August 24, 2024 09:46
Now pa_operation are handled multiple ways. Add function to make
sure that they are handled always same.
As function does not return value it should be void type

Co-authored-by: Ross Bencina <rossb@audiomulch.com>
As function does not return value it should be void type

Co-authored-by: Ross Bencina <rossb@audiomulch.com>
@illuusio
Copy link
Collaborator Author

illuusio commented Aug 24, 2024

Ok I fixed some locking problem which was caused when stopping device but now it does not hang because of double locking.

…_wait

Mainloop should be locked when using pa_threaded_mainloop_wait
make changes that it happens
@@ -897,6 +911,7 @@ static PaError RequestStop( PaPulseAudio_Stream * stream,
PaError ret = paNoError;
PaPulseAudio_HostApiRepresentation *pulseaudioHostApi = stream->hostapi;
pa_operation *pulseaudioOperation = NULL;
int waitLoop = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The waitLoop variable seems unused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
src-pulseaudio PulseAudio Host API in src/hostapi/pulseaudio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants