On Thu, 2018-02-22 at 08:25 +0100, Georg Chini wrote: > On 19.02.2018 15:48, Tanu Kaskinen wrote: > > The suspend cause isn't yet used by any of the handlers. The alsa sink > > and source will use it to sync the mixer when the SESSION suspend cause > > is removed. Currently the syncing is done in pa_sink/source_suspend(), > > and I want to change that, because pa_sink/source_suspend() shouldn't > > have any alsa specific code. > > --- > > src/modules/alsa/alsa-sink.c | 11 +++++++++-- > > src/modules/alsa/alsa-source.c | 11 +++++++++-- > > src/modules/bluetooth/module-bluez4-device.c | 22 ++++++++++++++++++---- > > src/modules/bluetooth/module-bluez5-device.c | 22 ++++++++++++++++++---- > > src/modules/echo-cancel/module-echo-cancel.c | 2 +- > > src/modules/module-combine-sink.c | 10 +++++++++- > > src/modules/module-equalizer-sink.c | 2 +- > > src/modules/module-esound-sink.c | 11 +++++++++-- > > src/modules/module-ladspa-sink.c | 2 +- > > src/modules/module-null-sink.c | 6 ++++-- > > src/modules/module-null-source.c | 2 +- > > src/modules/module-pipe-sink.c | 9 ++++++--- > > src/modules/module-remap-sink.c | 2 +- > > src/modules/module-sine-source.c | 2 +- > > src/modules/module-solaris.c | 23 ++++++++++++++++++----- > > src/modules/module-tunnel-sink-new.c | 12 ++++++++++-- > > src/modules/module-tunnel-source-new.c | 12 ++++++++++-- > > src/modules/module-virtual-sink.c | 2 +- > > src/modules/module-virtual-surround-sink.c | 2 +- > > src/modules/oss/module-oss.c | 24 ++++++++++++++++++------ > > src/modules/raop/raop-sink.c | 9 ++++++++- > > src/pulsecore/sink.c | 15 +++++++++------ > > src/pulsecore/sink.h | 23 +++++++++++++++++++++++ > > src/pulsecore/source.c | 15 +++++++++------ > > src/pulsecore/source.h | 23 +++++++++++++++++++++++ > > 25 files changed, 218 insertions(+), 56 deletions(-) > > > > I would rename pa_sink_message_set_state to pa_sink_set_state_message > because the first name sounds like a function name. Ok, pa_sink_set_state_message sounds better to me too. > Also there may be > some changes required due to the new patch 2 of the series. Otherwise > looks good. Raman suggested adding suspend_cause to thread_info. That's not as simple as it seems, because the suspend cause has to be updated also when resuming fails, but pa_sink_process_msg() isn't called when resuming fails, and thread_info.suspend_cause would be set in pa_sink_process_msg(). The way I'd solve the suspend_cause updating problem is to add a new callback set_state_in_io_thread() that is called from the core SET_STATE handler, and all modules would use the callback instead of handling the SET_STATE message. Having the callback allows the core SET_STATE handler to manage the state updating so that thread_info.suspend_cause is always updated no matter what happens in the set_state_in_io_thread() callback. This would also get rid of the awkward rule that the SET_STATE handlers must call pa_sink_process_msg() on success but not on failure. Adding suspend_cause to thread_info is not necessary, so one option is to just do nothing, but the refactoring seems beneficial, so I'd prefer to do that. -- Tanu https://liberapay.com/tanuk https://www.patreon.com/tanuk
