[pulseaudio-discuss] [PATCH 2/8] null-sink, pipe-sink: prefer thread_info variables in the IO thread

[Tanu Kaskinen]

On Wed, 2018-02-21 at 10:50 +0100, Georg Chini wrote:
> On 21.02.2018 10:43, Tanu Kaskinen wrote:
> > On Wed, 2018-02-21 at 09:40 +0100, Georg Chini wrote:
> > > On 19.02.2018 15:48, Tanu Kaskinen wrote:
> > > > pa_sink_get_state() is supposed to be used from the main thread. In this
> > > > case it doesn't really matter, because the SET_STATE handler is executed
> > > > while the main thread is waiting, but since the state is available also
> > > > in thread_info, let's use that. All other modules use thread_info.state
> > > > too, so at least this change improves consistency.
> > > > ---
> > > >    src/modules/module-null-sink.c | 2 +-
> > > >    src/modules/module-pipe-sink.c | 4 ++--
> > > >    2 files changed, 3 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/src/modules/module-null-sink.c b/src/modules/module-null-sink.c
> > > > index 25b0f3091..c29722fad 100644
> > > > --- a/src/modules/module-null-sink.c
> > > > +++ b/src/modules/module-null-sink.c
> > > > @@ -91,7 +91,7 @@ static int sink_process_msg(
> > > >        switch (code) {
> > > >            case PA_SINK_MESSAGE_SET_STATE:
> > > >    
> > > > -            if (pa_sink_get_state(u->sink) == PA_SINK_SUSPENDED || pa_sink_get_state(u->sink) == PA_SINK_INIT) {
> > > > +            if (u->sink->thread_info.state == PA_SINK_SUSPENDED || u->sink->thread_info.state == PA_SINK_INIT) {
> > > >                    if (PA_PTR_TO_UINT(data) == PA_SINK_RUNNING || PA_PTR_TO_UINT(data) == PA_SINK_IDLE)
> > > >                        u->timestamp = pa_rtclock_now();
> > > >                }
> > > > diff --git a/src/modules/module-pipe-sink.c b/src/modules/module-pipe-sink.c
> > > > index a2074c1f2..fc909af15 100644
> > > > --- a/src/modules/module-pipe-sink.c
> > > > +++ b/src/modules/module-pipe-sink.c
> > > > @@ -111,10 +111,10 @@ static int sink_process_msg(pa_msgobject *o, int code, void *data, int64_t offse
> > > >    
> > > >        switch (code) {
> > > >            case PA_SINK_MESSAGE_SET_STATE:
> > > > -            if (pa_sink_get_state(u->sink) == PA_SINK_SUSPENDED || pa_sink_get_state(u->sink) == PA_SINK_INIT) {
> > > > +            if (u->sink->thread_info.state == PA_SINK_SUSPENDED || u->sink->thread_info.state == PA_SINK_INIT) {
> > > >                    if (PA_PTR_TO_UINT(data) == PA_SINK_RUNNING || PA_PTR_TO_UINT(data) == PA_SINK_IDLE)
> > > >                        u->timestamp = pa_rtclock_now();
> > > > -            } else if (pa_sink_get_state(u->sink) == PA_SINK_RUNNING || pa_sink_get_state(u->sink) == PA_SINK_IDLE) {
> > > > +            } else if (u->sink->thread_info.state == PA_SINK_RUNNING || u->sink->thread_info.state == PA_SINK_IDLE) {
> > > >                    if (PA_PTR_TO_UINT(data) == PA_SINK_SUSPENDED) {
> > > >                        /* Clear potential FIFO error flag */
> > > >                        u->fifo_error = false;
> > > 
> > > As remarked by Raman Shyshniou on one of my patches you could use the
> > > PA_SINK_IS_OPENED() macro instead of
> > > x == PA_SINK_RUNNING || x == PA_SINK_IDLE
> > > 
> > > Otherwise looks good.
> > 
> > Thanks, I'll send v2 for this patch. I pushed patches 1 and 3 to the
> > next branch.
> > 
> 
> Should they not go to master as well?

In order to avoid thinking too much about what should or should not go
to master, I only push things to master that fix important (preferably
blocker) bugs. These two patches aren't important (and not having them
in master shouldn't cause big rebasing problems when the later patches
are applied).

-- 
Tanu

https://liberapay.com/tanuk
https://www.patreon.com/tanuk