Re: [pulseaudio-discuss] [PATCH v2 3/5] replace sink/source SET_STATE handlers with callbacks
On 13.03.2018 18:40, Tanu Kaskinen wrote: There are no behaviour changes, the code from almost all the SET_STATE handlers is moved with minimal changes to the newly introduced set_state_in_io_thread() callback. The only exception is module-tunnel, which has to call pa_sink_render() after pa_sink.thread_info.state has been updated. The set_state_in_io_thread() callback is called before updating that variable, so moving the SET_STATE handler code to the callback isn't possible. The purpose of this change is to make it easier to get state change handling right in modules. Hooking to the SET_STATE messages in modules required care in calling pa_sink/source_process_msg() at the right time (or not calling it at all, as was the case on resume failures), and there were a few bugs (fixed before this patch). Now the core takes care of ordering things correctly. Another motivation for this change is that there was some talk about adding a suspend_cause variable to pa_sink/source.thread_info. The variable would be updated in the core SET_STATE handler, but that would not work with the old design, because in case of resume failures modules didn't call the core message handler. --- src/modules/alsa/alsa-sink.c | 89 -- src/modules/alsa/alsa-source.c | 89 -- src/modules/bluetooth/module-bluez4-device.c | 172 ++ src/modules/bluetooth/module-bluez5-device.c | 174 ++- src/modules/echo-cancel/module-echo-cancel.c | 31 +++-- src/modules/module-combine-sink.c| 33 +++-- src/modules/module-equalizer-sink.c | 30 +++-- src/modules/module-esound-sink.c | 59 + src/modules/module-ladspa-sink.c | 30 +++-- src/modules/module-null-sink.c | 25 ++-- src/modules/module-null-source.c | 21 ++-- src/modules/module-pipe-sink.c | 45 --- src/modules/module-remap-sink.c | 30 +++-- src/modules/module-sine-source.c | 21 ++-- src/modules/module-solaris.c | 126 ++- src/modules/module-tunnel-sink-new.c | 48 +--- src/modules/module-tunnel-source-new.c | 48 +--- src/modules/module-virtual-sink.c| 30 +++-- src/modules/module-virtual-surround-sink.c | 30 +++-- src/modules/oss/module-oss.c | 152 --- src/modules/raop/raop-sink.c | 121 ++- src/pulsecore/sink.c | 8 ++ src/pulsecore/sink.h | 30 +++-- src/pulsecore/source.c | 8 ++ src/pulsecore/source.h | 32 +++-- 25 files changed, 849 insertions(+), 633 deletions(-) LGTM ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 3/5] replace sink/source SET_STATE handlers with callbacks
On Wed, 2018-03-14 at 01:23 +0300, Raman Shyshniou wrote:
> Hello,
>
> 13.03.2018 20:40, Tanu Kaskinen пишет:
> > diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
>
> ...
> > +/* Called from the IO thread. */
> > +static int sink_set_state_in_io_thread_cb(pa_sink *s, pa_sink_state_t
> > new_state) {
>
> ...
> > +switch (new_state) {
> > +
> > +case PA_SINK_SUSPENDED: {
> > +pa_assert(PA_SINK_IS_OPENED(u->sink->thread_info.state));
>
> ...
> > diff --git a/src/modules/alsa/alsa-source.c b/src/modules/alsa/alsa-source.c
>
> ...
> > +/* Called from the IO thread. */
> > +static int source_set_state_in_io_thread_cb(pa_source *s,
> > pa_source_state_t new_state) {
>
> ...
> > +switch (new_state) {
> > +
> > +case PA_SOURCE_SUSPENDED: {
> > +pa_assert(PA_SOURCE_IS_OPENED(u->source->thread_info.state));
>
> ...
> > diff --git a/src/modules/bluetooth/module-bluez4-device.c
> > b/src/modules/bluetooth/module-bluez4-device.c
>
> ...
> > +case PA_SINK_SUSPENDED:
> > +/* Ignore if transition is PA_SINK_INIT->PA_SINK_SUSPENDED */
> > +if (!PA_SINK_IS_OPENED(u->sink->thread_info.state))
> > +break;
>
> ...
> > +case PA_SOURCE_SUSPENDED:
> > +/* Ignore if transition is PA_SOURCE_INIT->PA_SOURCE_SUSPENDED
> > */
> > +if (!PA_SOURCE_IS_OPENED(u->source->thread_info.state))
> > +break;
>
> I know this is only code movement from SET_STATE handler to callbacks.
> But looking in alsa sink/source these transitions are impossible:
> PA_SINK_INIT->PA_SINK_SUSPENDED
> PA_SOURCE_INIT->PA_SOURCE_SUSPENDED
Those transitions are possible in the bluetooth modules, because in
certain situations they create the sink and source in suspended state.
The alsa sink and source never do that, so for them the transition is
impossible.
--
Tanu
https://liberapay.com/tanuk
https://www.patreon.com/tanuk
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v2 3/5] replace sink/source SET_STATE handlers with callbacks
Hello,
13.03.2018 20:40, Tanu Kaskinen пишет:
diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
...
+/* Called from the IO thread. */
+static int sink_set_state_in_io_thread_cb(pa_sink *s, pa_sink_state_t
new_state) {
...
+switch (new_state) {
+
+case PA_SINK_SUSPENDED: {
+pa_assert(PA_SINK_IS_OPENED(u->sink->thread_info.state));
...
diff --git a/src/modules/alsa/alsa-source.c b/src/modules/alsa/alsa-source.c
...
+/* Called from the IO thread. */
+static int source_set_state_in_io_thread_cb(pa_source *s, pa_source_state_t
new_state) {
...
+switch (new_state) {
+
+case PA_SOURCE_SUSPENDED: {
+pa_assert(PA_SOURCE_IS_OPENED(u->source->thread_info.state));
...
diff --git a/src/modules/bluetooth/module-bluez4-device.c
b/src/modules/bluetooth/module-bluez4-device.c
...
+case PA_SINK_SUSPENDED:
+/* Ignore if transition is PA_SINK_INIT->PA_SINK_SUSPENDED */
+if (!PA_SINK_IS_OPENED(u->sink->thread_info.state))
+break;
...
+case PA_SOURCE_SUSPENDED:
+/* Ignore if transition is PA_SOURCE_INIT->PA_SOURCE_SUSPENDED */
+if (!PA_SOURCE_IS_OPENED(u->source->thread_info.state))
+break;
I know this is only code movement from SET_STATE handler to callbacks.
But looking in alsa sink/source these transitions are impossible:
PA_SINK_INIT->PA_SINK_SUSPENDED
PA_SOURCE_INIT->PA_SOURCE_SUSPENDED
--
Raman
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
