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).
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
