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
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.

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.


_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

Reply via email to