On 2014-11-24 14:18, Tanu Kaskinen wrote:
On Mon, 2014-11-24 at 13:10 +0100, David Henningsson wrote:
On 2014-11-24 12:04, Tanu Kaskinen wrote:
On Mon, 2014-11-24 at 11:12 +0100, David Henningsson wrote:
On 2014-11-20 04:44, Ricardo Salveti wrote:
Hey,
When investigating the issue we had on Ubuntu Touch, described by bug
https://bugs.launchpad.net/ubuntu-rtm/+source/pulseaudio/+bug/1391230,
I noticed that PulseAudio never releases the resources used by an
active stream if the app gets a SIGSTOP, keeping pulse busy and
consuming cpu until the app resumes or is closed by the user.
On Ubuntu Touch that happens when the application is active playing
audio/video, and the user moves back to the home scopes (Ubuntu Touch
lifecycle will automatically send a SIGSTOP after 5 seconds). When
checking that on my desktop, I also noticed that the same happens (by
forcing a SIGSTOP against mplayer, for example). Pulse only releases
the stream when the app pauses the stream, not necessarily when the
app stops after receiving the signal.
I raised this first with David to understand if it was indeed a valid
use case, and he said that it was indeed something that it was
probably never really considered
(https://bugs.launchpad.net/ubuntu-rtm/+source/pulseaudio/+bug/1391230/comments/8).
So before going and trying to deep dive and find a fix for the issue,
I first wanted to understand from you guys if this is indeed a valid
issue and what would be the best way to get this fixed. I know we're
still using Pulse 4.0 on Ubuntu, but wanted to make sure to get
something that would also be compatible with upstream.
So when I discussed this with Ricardo, my suggestion was that we either
1) (ab)use the cork mechanism to stop/resume the client when we
discover that the client is SIGSTOPped, or
This implies that we implement server-side corking, because currently
the cork status is controlled by the client, and this "stalled" corking
would be controlled by the server.
Even though I have mentioned the "server-side corking" feature before
(as something that we need and use in Tizen), I might not have explained
how I think it should be implemented (Tizen's current implementation is
less flexible than what I'd like). We should have a hashmap of "cork
requests" in sink inputs and source outputs. The stream would be corked
as long as the hashmap isn't empty, i.e. as long as someone thinks that
the stream should be corked. The hashmap keys would be strings
identifying the "corking reasons", so policy modules could easily add
new reasons. The normal client-initiated corking would be one corking
reason, and this stall detection would be another reason. Tizen's policy
module would implement a third reason.
+1 in general for this idea, to consolidate the "stall" concept with the
"server side cork" concept; the fewer concepts on the more use cases,
the better :-)
I wonder what we should do with the "cork requests" though. I feel under
this scheme, we should deprecate them and replace them with a "you have
been (un)corked" message, possibly including the different reasons as
strings, so the client can determine whether the cork happened because
the client requested so, or if it was for some other reason.
I'd vote for not removing the old cork request feature, because there
are applications relying it. If the feature is seen as unnecessary for
new/updated applications, I'm ok with marking it as deprecated in the
documentation.
As for sending some new kind of message like "you've been corked", I
don't think that's necessary. If the introspection API allows pactl to
show the cork status, including corking done by the server (possibly
also including the reason string(s)), then the introspection API should
be sufficient also for applications that need to get notified when their
own stream gets corked by the server.
And if the reasons change, that gives an event/notification on the stream?
Talking about the client API, there's the question that how should
pa_sink_input_info.corked and pa_stream_is_corked(). For backwards
compatibility reasons, I think at least pa_stream_is_corked() should
return 1 only if the stream is corked by the application, and possibly
it would be good to apply the same policy for pa_sink_input_info.corked
too (but I think changing the pa_sink_input_info.corked semantics is
less likely to cause breakage).
Hmm... I realized there's another problem with stopped clients: not only
do their streaming get stalled, but they also stop reading events that
they may be subscribed to. The code looks like the daemon will buffer
data until it runs out of memory, which seems like a bad idea...
Let's assume that there's a simple application that just subscribes to
events and does nothing else ("pactl subscribe" would fit the
description). What to do when such application gets stopped? How do we
notice that it's stopped, if it's not streaming? Should we send a ping
message periodically? Or should we keep track of the amount of queued
data, and if that reaches some threshold, then we consider that client
stopped? Once we have declared the client unresponsive, should we
disconnect it, or just drop subscription events until it becomes
responsive again? In the latter case I think we should notify the
application that events have been dropped.
I think most apps would like to receive buffered events once they are
unstopped, so that should be the best for shorter stops.
But I also think it would be better to disconnect the client in case of
overflow, rather than dropping events. The advanced clients should be
able to deal with disconnect/reconnect and would then re-read the
current state after reconnection.
Or we could generalise this: by dealing with this on the
iochannel/srbchannel level, i e, we could disconnect the client in case
the pstream.send_queue gets too large? That should at least protect the
PulseAudio server from OOM, both for notifications and recording streams.
If the send_queue has more than, say, 100 [1] messages, we start a timer
for 30 [1] seconds. If the send_queue is empty, we stop the timer. If
the timer expires, the client is disconnected. How about that?
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
[1] Subject to bikeshedding
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
