'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/]
