On Wed, 2018-02-21 at 11:59 +0200, Tanu Kaskinen wrote:
> 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).

I took the liberty of pushing these patches to master after all,
because as I'm removing SET_STATE handlers from all modules, rebasing
the patches would become cumbersome.

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

Reply via email to