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

[Georg Chini]

On 23.02.2018 11:03, Raman Shishniou wrote:
> On 02/23/2018 11:38 AM, Georg Chini wrote:
>
>> But now I have another issue:
>> You are polling the pipe and running the loop even if the source is user suspended.
>> 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 setting
>> 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 read
>> the first data. It will cause an unnecessary unsuspend message, but this will
>> 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.
>>
> We will face two main problems if we do something like that:
>
> First problem - we don't know how writer will react to full pipe.
> If it open pipe in non-blocking mode, it will get EAGAIN on every
> write() while pipe stays full. If it open pipe in blocking mode,
> it will just stuck at write() until user unsuspend the source.
> I think both behaviors are bad for writer - it should contain a
> special code to deal with it.

I guess that should be the problem of the writer. If it is intended
to write to a pipe, it must be able to deal with the situation that
the pipe is full.

>
> The second problem is hidden now because I temporary dropped
> a part of code that keep frame boundaries. If we drain the pipe
> as soon as user resume the source - we'll loose frame boundaries.
> Audio stream will be broken for any case except s8/u8 mono.
> Actually we have to read every time while suspended by user and drop
> whole chunk except for not completely read last frame, move it to
> the head of memchunk and do next read() at position where this frame ends.

We could work around this I think. You just need to have the last
read fragment available in the SET_STATE handler. Then you do
not loose frame boundaries, because you continue to read where
you have stopped.

>
> BTW, currently pipe-source PA just crashed if I try to write s24le to pipe:
>
> I decided to do not open a bug because almost whole pipe-source should
> be rewritten, and this is what I'm doing now.
>
Yes, makes sense. If your patch fixes a crash bug, it has a good
chance to get into 12.0 (which it would not have otherwise).