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

[Georg Chini]

On 21.02.2018 20:15, Raman Shishniou wrote:
> On 02/21/2018 09:41 PM, Raman Shuishniou wrote:
>> 21.02.2018 20:07, Georg Chini пишет:
>>> Maybe you misunderstood me. What I mean, is that the pipe can be
>>> opened for writing as long as we are suspended. So it open when
>>> we see that the source is suspended or when we auto suspend. Close
>>> it as soon as the source gets unsuspended. This will avoid POLLHUP
>>> during suspend completely. While auto suspended, we additionally have
>>> to listen for POLLIN.
>>> This way we can only get POLLHUP (or POLLIN with no data) when the
>>> source is running.
>>>
>> I think I understand. We need to keep our writer opened while transition
>> from autosuspended to opened state and just set events = 0.

Exactly. I was also thinking about how to decide if data can be discarded
or needs to be posted. On resume, the SET_STATE handler could set a
just_unsuspended flag and when we know auto suspend is the only
suspend cause, we can additionally set a resume_from_auto_suspend
flag when we send the unsuspend message. In the thread function we
could then check if

just_unsuspended && resume_from_autosuspend -> post pending data, reset 
flags
just_unsuspended && !resume_from_auto_suspend -> discard data, reset flags
!just_unsuspended -> post data

But that's only an idea.

>>
>> But what I don't understand - why you so hate the freeing and allocating
>> rtpoll_item during this transition?

I don't hate it. However I think it looks weird and none of the other thread
functions does something like this. If somebody looks at the code in a
year or so, it will be hard to figure out why it was done that way.
This is a general thing - using similar patterns where possible, adding
comments and sometimes even writing more code just for clarity
make the the software better maintainable.

>>
>> I'll try ro rewrite the patch (again) while we waiting for Tanu to apply
>> his patches.

Sorry. Meanwhile I have done this often enough with my patches when
Tanu disagreed. I know it's really annoying but in the end those discussions
improve the quality of the code because we will end up with something
we both can live with (and that will probably really do what was intended).

> Also, did I still need to make this behaviour optional?
>
Yes, I think it should still be optional, but it is OK to make it default.
The reason is that we keep backwards compatibility wherever
possible.