protocol-native: Early request bandaid for high latency sink/source

As it is implemented, the early request mode can in some cases be counter-productive. The mode is designed to give the client a steady request/report rate of small-ish chunks (A somewhat silly client requirement but at least Flash and Firefox break horribly when you break this.).

Unfortunately PulseAudio does not have any mechanism for telling a sink/source how often it should request/report data. So a more blunt hack was applied where the entire latency is restricted to the fragment size.

So far so good, but where the current code breaks down is when the sink cannot satisfy this tiny latency request. We then "report" to the client what we can guarantee by setting the fragment size to the sink's/source's full buffer size/latency.

This severely changes the resulting buffer attributes from what the client requested, and in practice breaks applications. The most prominent user of this feature is the ALSA plugin, and it doesn't even have a mechanism of adapting to the server giving back something different than what was requested.

So long term, the whole early request mode needs to be implemented in a better way. Either the sink's/source's need to grow the ability to control request/report rate. Or we put some form of timer based emulation in front of them on behalf of these clients.

Short term, we should change the behaviour of what happens when we cannot guarantee a fragment rate. Instead of giving the client really shitty buffering parameters as a result, we should just keep the requested attributes and do things on a best-effort basic. Basically how things would behave if the client didn't have the early request bit at all.

The attached patch does just that, as well as expand on the comment about how the early request thing is implemented.

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=66962

diff --git a/src/pulsecore/protocol-native.c b/src/pulsecore/protocol-native.c
index 96df383da6b1c61d9b066dfb5e80b62e468966a5..606bf25cbc1cdee9ae3e5cf69a486a2f94ec50fe 100644 (file)
--- a/src/pulsecore/protocol-native.c
+++ b/src/pulsecore/protocol-native.c
@@ -555,8 +555,11 @@ static void fix_record_buffer_attr_pre(record_stream *s) {
     if (s->early_requests) {
 
         /* In early request mode we need to emulate the classic
-         * fragment-based playback model. We do this setting the source
-         * latency to the fragment size. */
+         * fragment-based playback model. Unfortunately we have no
+         * mechanism to tell the source how often we want it to send us
+         * data. The next best thing we can do is to set the source's
+         * total buffer (i.e. its latency) to the fragment size. That
+         * way it will have to send data at least that often. */
 
         source_usec = fragsize_usec;
 
@@ -584,10 +587,12 @@ static void fix_record_buffer_attr_pre(record_stream *s) {
 
     if (s->early_requests) {
 
-        /* Ok, we didn't necessarily get what we were asking for, so
-         * let's tell the user */
+        /* Ok, we didn't necessarily get what we were asking for. We
+         * might still get the proper fragment interval, we just can't
+         * guarantee it. */
 
-        fragsize_usec = s->configured_source_latency;
+        if (fragsize_usec != s->configured_source_latency)
+            pa_log_debug("Could not configure a sufficiently low latency. Early requests might not be satisifed.");
 
     } else if (s->adjust_latency) {
 
@@ -963,8 +968,11 @@ static void fix_playback_buffer_attr(playback_stream *s) {
     if (s->early_requests) {
 
         /* In early request mode we need to emulate the classic
-         * fragment-based playback model. We do this setting the sink
-         * latency to the fragment size. */
+         * fragment-based playback model. Unfortunately we have no
+         * mechanism to tell the sink how often we want to be queried
+         * for data. The next best thing we can do is to set the sink's
+         * total buffer (i.e. its latency) to the fragment size. That
+         * way it will have to query us at least that often. */
 
         sink_usec = minreq_usec;
         pa_log_debug("Early requests mode enabled, configuring sink latency to minreq.");
@@ -1013,10 +1021,12 @@ static void fix_playback_buffer_attr(playback_stream *s) {
 
     if (s->early_requests) {
 
-        /* Ok, we didn't necessarily get what we were asking for, so
-         * let's tell the user */
+        /* Ok, we didn't necessarily get what we were asking for. We
+         * might still get the proper fragment interval, we just can't
+         * guarantee it. */
 
-        minreq_usec = s->configured_sink_latency;
+        if (minreq_usec != s->configured_sink_latency)
+            pa_log_debug("Could not configure a sufficiently low latency. Early requests might not be satisifed.");
 
     } else if (s->adjust_latency) {