Pulse: Stop calling user callback before stream start #142
Conversation
|
@kinetiknz since Paul is on PTO can you please have a look? |
|
Try run of the patch: |
|
5526dd2 is unrelated (owned_critical_section isn't even used in the pulse backend!) so please split it out. Also, as I've asked before, please stop making updates to the copy in Gecko's tree directly, it's a recipe for losing changes in situations like this. I'm also not happy that this wrapper has been made recursive in the pthreads implementation ( please don't propagate use of this wrapper until this is resolved) - the correct solution is to remove the use of recursive locks in the audiounit backend. The Windows backend explicitly prohibits recursion; the behaviour should match across platforms as much as possible, and I do not want this code polluted with recursive locks (un)intentionally. cc @padenot |
|
d1d57d1 needs a reference to the BMO bug that you're fixing and some rationale for why this change is being made added to the commit message. |
| WRAP(pa_stream_set_write_callback)(stm->output_stream, stream_write_callback, stm); | ||
| // De-register write cb here and register after PA is on ready state | ||
| // This will avoid calling the callback before stream start | ||
| WRAP(pa_stream_set_write_callback)(stm->output_stream, NULL, stm); |
There was a problem hiding this comment.
There should be no callback registered at this point, so why do we need to set it to NULL explicitly?
achronop
force-pushed
the
pulse-setup-cb
branch
from
5526dd2
to
62f5abe
Compare
|
@kinetiknz: I pushed a new version according to your comments. I've changed a little the way to avoid the pre buffering callback. Let me know what you think. |
| @@ -120,6 +122,7 @@ struct cubeb_stream { | |||
| pa_sample_spec input_sample_spec; | |||
| int shutdown; | |||
| float volume; | |||
| int state; | |||
There was a problem hiding this comment.
Make this a cubeb_state since that's what it holds.
|
Thanks, looks good. Merged in b42b7597 with state's type changed. |
PulseAudio uses pre buffering which means that fires once write callback during the setup before stream uncork. This is responsible for a huge number of warnings during linux debug testing described by Bug 1286041.
This PR solves the issue by hooking the write callback after stream uncork. For this reason the user callback must be called once manually at that point. In order to be called by the same thread as the rest of them it is done through a defer event.