[pulseaudio-discuss] [PATCH 6/8] pass pa_suspend_cause_t to SINK/SOURCE_SET_STATE handlers
georg at chini.tk
Thu Feb 22 08:13:24 UTC 2018
On 22.02.2018 08:47, Tanu Kaskinen wrote:
> 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
> 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.
Well, I don't mind either way. For Raman's patches it is not necessary
to update the suspend cause in the I/O-thread since the old and new
cause are accessible within the SET_STATE handler (because the main
thread is waiting). On the other hand I like your idea because it makes
the sink/source code more generic and removes possible error causes.
More information about the pulseaudio-discuss