[pulseaudio-discuss] [PATCH v8] pipe-source: implement autosuspend option

Raman Shishniou rommer at ibuffed.com
Wed Feb 21 14:33:04 UTC 2018

On 02/21/2018 05:00 PM, Georg Chini wrote:
> On 21.02.2018 12:50, Raman Shishniou wrote:
>> On 02/21/2018 02:24 PM, Georg Chini wrote:
>>> On 21.02.2018 12:22, Raman Shishniou wrote:
>>>> On 02/21/2018 12:13 PM, Raman Shishniou wrote:
>>>>> On 02/21/2018 09:39 AM, Georg Chini wrote:
>>>>>> On 21.02.2018 06:05, Georg Chini wrote:
>>>>>>> On 21.02.2018 05:55, Georg Chini wrote:
>>>>>>>> On 20.02.2018 22:34, Raman Shishniou wrote:
>>>>>>>>> On 02/20/2018 11:04 PM, Georg Chini wrote:
>>>>>>>>>> On 20.02.2018 19:49, Raman Shishniou wrote:
>>>>>>>>>>> On 02/20/2018 07:02 PM, Georg Chini wrote:
>>>>>>>>>>>> On 20.02.2018 16:38, Raman Shyshniou wrote:
>>>>>>>>>>>>> Currently the pipe-source will remain running even if no
>>>>>>>>>>>>> writer is connected and therefore no data is produced.
>>>>>>>>>>>>> This patch adds the autosuspend=<bool> option to prevent this.
>>>>>>>>>>>>> Source will stay suspended if no writer is connected.
>>>>>>>>>>>>> This option is enabled by default.
>>>>>>>>>>>>> ---
>>>>>>>>>>>>>       src/modules/module-pipe-source.c | 279 +++++++++++++++++++++++++++++----------
>>>>>>>>>>>>>       1 file changed, 212 insertions(+), 67 deletions(-)
>>>>>>>>>>> I think I need post a simple pseudo code of new thread loop because it
>>>>>>>>>>> was completely rewritten. There are too many changes in one patch.
>>>>>>>>>>> It can be difficult to see the whole picture of new main loop.
>>>>>>>>>> Well, I applied the patch and looked at the result. I still don't like the approach.
>>>>>>>>>> I would propose this:
>>>>>>>>>> auto_suspended = false;
>>>>>>>>>> revents = 0
>>>>>>>>>> events = POLLIN
>>>>>>>>>> for (;;) {
>>>>>>>>>>          /* This is the part that is run when the source is opened
>>>>>>>>>>           * or auto suspended
>>>>>>>>>>          if (SOURCE_IS_OPENED(source) || auto_suspended) {
>>>>>>>>>>              /* Check if we wake up from user suspend */
>>>>>>>>>>              if (corkfd >= 0 && !auto_suspended) {
>>>>>>>>>>                   len = 0
>>>>>>>>>>                   close pipe for writing
>>>>>>>>>>              }
>>>>>>>>>>              /* We received POLLIN or POLLHUP or both */
>>>>>>>>>>              if (revents) {
>>>>>>>>>>                 /* Read data from pipe */
>>>>>>>>>>                 len = read data
>>>>>>>>>>                 /* Got data, post it */
>>>>>>>>>>                 if (len > 0) {
>>>>>>>>>>                     if (auto_suspend) {
>>>>>>>>>>                         send unsuspend message
>>>>>>>>>>                         auto_suspend = false
>>>>>>>>>>                    }
>>>>>>>>>>                    post data
>>>>>>>>> We cannot post data here because source still suspended. Sending resume message is not enough
>>>>>>>>> to immediately resume the source. We need to wait several poll runs until it will be resumed.
>>>>>>>>> (source->thread_info.state changed in this thread, i.e. during poll run). But we will see
>>>>>>>>> POLLIN and/or POLLHUP each run if we don't remove pipe fd from polling.
>>>>>>>> Why do we have to wait? The source will be unsuspended on the next rtpollrun.
>>>>>>>> I do not see why we cannot already push data. Or does something get lost?
>>>>>>> Why would we receive POLLIN on each run? We read the data from the pipe.
>>>>>>> If you think the data should not be posted, you can just skip posting and discard
>>>>>>> the data. According to your pseudo-code it is done like tis in your previous patch.
>>>>>> I should not write mails before I have woken up completely ... I see what you mean
>>>>>> now (and I also see that you do not discard data as I thought). But I still believe you
>>>>>> can post the data before the source gets unsuspended. What is the difference if the
>>>>>> samples are stored in the source or in the source output? Anyway we are talking
>>>>>> about a time frame of (very probably) less than 1 ms between sending the message
>>>>>> and receiving it. To ensure that the loop works as expected, auto_suspended should
>>>>>> be set/reset in the suspend/unsuspend message and not directly in the thread function.
>>>>>> POLLHUP spam cannot happen because corkfd will be opened on the first POLLHUP.
>>>>>> POLLIN spam cannot happen when auto_suspend is set/reset from the message
>>>>>> handler.
>>>>> Not, I can't post it here. The source may not be resumed at all after we send a resume message.
>>>>> Not within 1 ms, not within next hour. It can be autosuspended and suspended by user manually
>>>>> after it. I that case we read data and should discard it instead of posting (as you propose).
>>>>> But that algorithm will post data to suspended source while it suspended by user.
>>>>> Also auto_suspended can't be set/reset in suspend/resume message handler because it called from
>>>>> main context and accessed from thread context.
>>>>> That's why I read data and wait while source will be resumed before posting.
>>>> I just looked into pa_source_post() code:
>>>> void pa_source_post(pa_source*s, const pa_memchunk *chunk) {
>>>>       pa_source_output *o;
>>>>       void *state = NULL;
>>>>       pa_source_assert_ref(s);
>>>>       pa_source_assert_io_context(s);
>>>>       pa_assert(PA_SOURCE_IS_LINKED(s->thread_info.state));
>>>>       pa_assert(chunk);
>>>>       if (s->thread_info.state == PA_SOURCE_SUSPENDED)
>>>>           return;
>>>> ...
>>>> There are only 3 valid states of source to post data:
>>>> static inline bool PA_SOURCE_IS_LINKED(pa_source_state_t x) {
>>>>       return x == PA_SOURCE_RUNNING || x == PA_SOURCE_IDLE || x == PA_SOURCE_SUSPENDED;
>>>> }
>>>> And if the source is suspended:
>>>> if (s->thread_info.state == PA_SOURCE_SUSPENDED)
>>>>           return;
>>>> If we read some data, send resume and try to post, chunk will be just discarded
>>>> in pa_source_post().
>>>> So we must to wait source->thread_info.state will be changed to RUNNING or IDLE
>>>> before posting any data. And the only way to wait - call some pa_rtpoll_run()
>>>> and check the source state to be valid for posting after each call. Again,
>>>> we must stop polling pipe while we waiting because we can get endless loop
>>>> if source stays suspended for long time after we send a resume message.
>>>> I think my algorithm implemented in this patch is the simplest way to achieve this.
>>> Well, your code is not doing the right thing either. When the source gets user
>>> suspended, there will be some (trailing) data you read from the pipe. Now you
>>> use this data as an indicator, that the source got suspended. When the source
>>> gets unsuspended, the first thing you do is post the trailing data that was read
>>> when the source was suspended. And only after that you start polling the pipe
>>> again
>> I can't track the suspend reason in i/o thread right now. It's not copied to
>> thread_info in pa_source struct along with state during state changes.
>> Tanu proposed a patches that will pass pa_suspend_cause_t to SINK/SOURCE_SET_STATE
>> handlers and set_state() callbacks. It we add suspend_cause to thread_info too,
>> there will be easy way to discard data if we are suspended by user:
>> if (PA_SOURCE_IS_OPENED(u->source->thread_info.state)) {
>>      ... post data ...
>>      chunk.length = 0;
>> } else if (PA_SUSPEND_APPLICATION is not in thread_info->suspend_cause) {
>>      ... discard data ...
>>      chunk.length = 0;
>> }
> I see another problem. If, during suspend, a writer connects and
> disconnects again, the pipe may be full of old data after we resume.
> So I guess we have to read data from the pipe continuously and
> discard it while the source is suspended.

Right now yes. This is what original code does.

But if we'll track suspend cause, pending data will be discarded right after
our unsuspend message will be processed, i.e

{i/o thread} send message to main thread ->
  {main thread} pipe_source_process_msg() will send message to i/o thread ->
    {i/o thread} source_process_msg() will process message from main thread
                                      and change thread_info.

If PA_SUSPEND_APPLICATION not in suspend_cause but thread still suspended,
all pending data will be discarded and pipe will be continuously monitored.
All futher data will be posted or discarded while PA_SUSPEND_APPLICATION not in

So we will stop pipe polling only for short period to wait our unsuspend message will
be processed.


More information about the pulseaudio-discuss mailing list