On 16.03.2018 19:26, Tanu Kaskinen wrote:
On Fri, 2018-03-16 at 11:15 +0100, Georg Chini wrote:
On 13.03.2018 18:40, Tanu Kaskinen wrote:
The suspend cause isn't yet used by any of the callbacks. 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 | 7 +++-
src/modules/alsa/alsa-source.c | 7 +++-
src/modules/bluetooth/module-bluez4-device.c | 4 +-
src/modules/bluetooth/module-bluez5-device.c | 4 +-
src/modules/echo-cancel/module-echo-cancel.c | 2 +-
src/modules/module-combine-sink.c | 7 +++-
src/modules/module-equalizer-sink.c | 2 +-
src/modules/module-esound-sink.c | 7 +++-
src/modules/module-ladspa-sink.c | 2 +-
src/modules/module-null-sink.c | 2 +-
src/modules/module-null-source.c | 2 +-
src/modules/module-pipe-sink.c | 2 +-
src/modules/module-remap-sink.c | 2 +-
src/modules/module-sine-source.c | 2 +-
src/modules/module-solaris.c | 14 ++++++-
src/modules/module-tunnel-sink-new.c | 7 +++-
src/modules/module-tunnel-source-new.c | 7 +++-
src/modules/module-virtual-sink.c | 2 +-
src/modules/module-virtual-surround-sink.c | 2 +-
src/modules/oss/module-oss.c | 14 ++++++-
src/modules/raop/raop-sink.c | 7 +++-
src/pulsecore/sink.c | 57
+++++++++++++++++++++-------
src/pulsecore/sink.h | 2 +-
src/pulsecore/source.c | 57
+++++++++++++++++++++-------
src/pulsecore/source.h | 2 +-
25 files changed, 168 insertions(+), 55 deletions(-)
...
static void pa_sink_volume_change_push(pa_sink *s);
@@ -429,19 +434,47 @@ static int sink_set_state(pa_sink *s, pa_sink_state_t
state, pa_suspend_cause_t
* current approach of not setting any suspend cause works well enough.
*/
if (s->set_state_in_main_thread) {
- ret = s->set_state_in_main_thread(s, state, suspend_cause);
- /* set_state_in_main_thread() is allowed to fail only when resuming. */
- pa_assert(ret >= 0 || resuming);
+ if ((ret = s->set_state_in_main_thread(s, state, suspend_cause)) < 0) {
+ /* set_state_in_main_thread() is allowed to fail only when
resuming. */
+ pa_assert(resuming);
+
+ /* If resuming fails, we set the state to SUSPENDED and
+ * suspend_cause to 0. */
+ state = PA_SINK_SUSPENDED;
+ suspend_cause = 0;
+ state_changed = state != s->state;
+ suspend_cause_changed = suspend_cause != s->suspend_cause;
+ suspending = PA_SINK_IS_OPENED(s->state);
+ resuming = false;
+
+ if (!state_changed && !suspend_cause_changed)
+ return ret;
+ }
}
I think the above is not correct. When set_state_in_main_thread() fails,
the state of the sink will be SUSPENDED before and after the call. We
know it was suspended before and if resume fails it will still be suspended
after the call. So if the (failing) set_state() forgot to set the state back
to SUSPENDED, we should just do so.
The set_state_in_io_thread() callback is never responsible for setting
the state, and as you explained, the state isn't changing, so there
isn't anything to set anyway.
Because we don't have a state
change, it does not make sense to send notifications if the handler
falsely set the state to IDLE or RUNNING. This leaves only suspend
changes for further processing. Same comment applies for the source
side.
Since the state isn't changing, I should change this
+ state = PA_SINK_SUSPENDED;
+ suspend_cause = 0;
+ state_changed = state != s->state;
+ suspend_cause_changed = suspend_cause != s->suspend_cause;
+ suspending = PA_SINK_IS_OPENED(s->state);
+ resuming = false;
to this:
+ suspend_cause = 0;
+ state_changed = false;
+ suspend_cause_changed = suspend_cause != s->suspend_cause;
+ resuming = false;
Yes, that should be OK.
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
