[pulseaudio-discuss] [PATCH v9] pipe-source: [DRAFT] implement autosuspend behavior

Georg Chini georg at chini.tk
Fri Feb 23 08:38:28 UTC 2018

On 22.02.2018 22:01, Raman Shishniou wrote:
> On 02/22/2018 10:18 PM, Georg Chini wrote:
>>> -        /* Hmm, nothing to do. Let's sleep */
>>> -        pollfd->events = (short) (u->source->thread_info.state == PA_SOURCE_RUNNING ? POLLIN : 0);
>>> +        /* Post data to source, discard data or wait for state transition to be complete */
>>> +        if (PA_SOURCE_IS_OPENED(u->source->thread_info.state)) {
>>> +
>>> +            if (u->writer_connected && u->corkfd >= 0) {
>>> +                pa_assert_se(pa_close(u->corkfd) == 0);
>>> +                u->corkfd = -1;
>>> +            }
>>> +
>>> +            if (u->memchunk.length > 0) {
>>> +                pa_source_post(u->source, &u->memchunk);
>>> +                pa_memblock_unref(u->memchunk.memblock);
>>> +                u->memchunk.memblock = pa_memblock_new(u->core->mempool, u->pipe_size);
>>> +                u->memchunk.length = 0;
>>> +            }
>>> +
>>> +        } else if (u->suspended_by_user)
>> You have to open the corkfd here as well to avoid POLLHUP spam if the writer
>> disconnects during user suspend.
> During user suspend we are doing normal polling. If the writer will be disconnected
> we'll see it by reading 0 bytes and corkfd will be opened.

See below, I would like to avoid continuous polling during user suspend.

>>> +            u->memchunk.length = 0;
>>> +
>>> +
>>> +        pollfd->events = u->memchunk.length ? 0 : POLLIN;
>> How do you wake up from autosuspend? When the writer disconnects,
>> memchunk.length will be 0 and so you do no longer listen for POLLIN.
>> I guess you need to know the current suspend cause here:
> 	> user suspended -> events = 0
>> (only auto suspended + no writer connected) or running -> events = POLLIN
>> only auto suspended + writer connected (=wake up transition) -> events = 0
> Yes. This is what I do, keep data in memchunk and wait while source will be resumed.
Yeah, looks I am too dump to understand a simple comparison ... sorry. 
(Shame on
me, I know I have been making a lot of silly mistakes during my reviews 
and your
knowledge of PA is better than mine. I hope I did not annoy you too much.)

But now I have another issue:
You are polling the pipe and running the loop even if the source is user 
This seems like a waste of CPU (even more than accepting some POLLIN spam
during wakeup transition). I know you do it to discard data that is 
written during
suspend, but I wonder if there is no better way to discard that data 
without running
the loop permanently.
I am thinking of draining the pipe in the SET_STATE handler. If you are 
events = 0 and open the corkfd on user suspend, nothing except messages
should wake us up. Now, when the state changes to running, you can drain the
pipe in the SET_STATE handler. The thread loop will just run through on 
the first
iteration after user suspend, because revents = 0 and chunk.length = 0. Now,
because the source is open again, you can set events = POLLIN.
After that, you are back to normal.
You can safely assume writer_connected=false during user suspend (you do
not notice when the writer disconnects if you set events = 0). If the writer
is still connected after suspend, writer_connected will get set when you 
the first data. It will cause an unnecessary unsuspend message, but this 
have no effect because neither the suspend cause nor the state change.

I would also suggest to use a flag like set_pollin in the comparison, 
set and reset
the flag in the appropriate places and explain why in a comment. This is 
one of
the situations, where a little bit more code could make the concept 
clearer. I don't
mind keeping it as is however, if you think it's not worth the effort.

More information about the pulseaudio-discuss mailing list