[pulseaudio-discuss] [PATCH] pacat: make pacat respond to cork/uncork events

[Colin Guthrie]

'Twas brillig, and Lu Guanqun at 23/08/11 09:30 did gyre and gimble:
> Hi Col,
> 
> On Tue, Aug 23, 2011 at 04:22:48PM +0800, Colin Guthrie wrote:
>> 'Twas brillig, and Lu Guanqun at 23/08/11 07:18 did gyre and gimble:
>>> Pacat remembers the number of cork requests, and then cork/uncork the stream
>>> accordingly.
>>>
>>> With this change, it makes below test script work correctly:
>>>
>>> 	pacat -p --property=media.role="music" <long-sound> &
>>> 	sleep 2
>>> 	pacat -p --property=media.role="phone" <short-sound>
>>> 	wait
>>
>>
>>
>> Interesting, this is likely good as a test bed/example.
> 
> Thanks. I was trying module-cork-music-on-phone and came up with this
> little script. :)
> 
>>
>> That said, I'm not sure we should be sending the operations every time.
>>
>> Perhaps we should only send a cork req when the count moves form 0->1
>> and an uncork when the count moves from 1->0?
> 
> At my first attempt, I'm only doing something like below:
> 
> diff --git a/src/utils/pacat.c b/src/utils/pacat.c
> index f687402..41817ff 100644
> --- a/src/utils/pacat.c
> +++ b/src/utils/pacat.c
> @@ -408,6 +410,12 @@ static void stream_event_callback(pa_stream *s, const char *name, pa_proplist *p
>  
>      t = pa_proplist_to_string_sep(pl, ", ");
>      pa_log("Got event '%s', properties '%s'", name, t);
> +
> +    if (pa_streq(name, PA_STREAM_EVENT_REQUEST_CORK))
> +        pa_operation_unref(pa_stream_cork(s, 1, NULL, NULL));
> +    else if (pa_streq(name, PA_STREAM_EVENT_REQUEST_UNCORK))
> +        pa_operation_unref(pa_stream_cork(s, 0, NULL, NULL));
> +
>      pa_xfree(t);
>  }
>  
> But I read the mailing list and once someone said it's better for client
> to remember how many cork requests it receives and only uncork it when
> it reaches zero. That's why I changed to the current style.
> 
> Do you think my original code is better for this case?

Yes, the latest code is better but what I was meaning was something like:

diff --git a/src/utils/pacat.c b/src/utils/pacat.c
index f687402..2568db5 100644
--- a/src/utils/pacat.c
+++ b/src/utils/pacat.c
@@ -91,6 +91,8 @@ static int32_t latency_msec = 0, process_time_msec = 0;
 static pa_bool_t raw = TRUE;
 static int file_format = -1;

+static int cork_requests = 0;
+
 /* A shortcut for terminating the application */
 static void quit(int ret) {
     pa_assert(mainloop_api);
@@ -408,6 +410,15 @@ static void stream_event_callback(pa_stream *s,
const char *name, pa_proplist *p

     t = pa_proplist_to_string_sep(pl, ", ");
     pa_log("Got event '%s', properties '%s'", name, t);
+
+    if (pa_streq(name, PA_STREAM_EVENT_REQUEST_CORK)) {
+        if (!cork_requests)
+            pa_operation_unref(pa_stream_cork(s, 1, NULL, NULL));
+        cork_requests++;
+    } else if (pa_streq(name, PA_STREAM_EVENT_REQUEST_UNCORK)) {
+        cork_requests--;
+        if (!cork_requests)
+          pa_operation_unref(pa_stream_cork(s, 0, NULL, NULL));
+    }
+
     pa_xfree(t);
 }



(not a real patch!)


This way the actual operation is only sent at the transition of 0->1 and
not 1->2 etc. (and the opposite, it will only be sent when doing 1->0
and not from 2->1).

I think this makes sense, but other opinions are welcomed too.

>> We may at some point have to change the semantics at the server side to
>> deal with the start_corked issue, and thus this is likely more robust in
>> those circumstances.
> 
> It should. However for 'pacat', it doesn't set START_CORKED flag, so I'm
> afraid there's no such problem for 'pacat'.

Aye, but gstreamer does... so the problem is real and thus any "example"
code should probably follow the best practice principles - now we only
need to decide what "best practice" really is :p

Col



-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited [http://www.tribalogic.net/]
Open Source:
  Mageia Contributor [http://www.mageia.org/]
  PulseAudio Hacker [http://www.pulseaudio.org/]
  Trac Hacker [http://trac.edgewall.org/]