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

[Georg Chini]

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.