[pulseaudio-discuss] [PATCH 6/8] pass pa_suspend_cause_t to SINK/SOURCE_SET_STATE handlers

[Tanu Kaskinen]

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