<div dir="ltr">Hi <span style="font-family:arial,sans-serif;font-size:13px;white-space:nowrap">Tanu Kaskinen,</span><br><div><br></div><div style>I rewrote the patch by following your feedback.</div><div style>I kept "<span style="font-family:arial,sans-serif;font-size:13px">pa_operation_new_with_state_</span><span style="font-family:arial,sans-serif;font-size:13px">callback" ("</span><span style="font-family:arial,sans-serif;font-size:13px">pa_operation_new_with_cancel_</span><span style="font-family:arial,sans-serif;font-size:13px">callback" previously) refactoring as the same for a while because the argument favonia provided.</span></div>
<div style><span style="font-family:arial,sans-serif;font-size:13px">If you insist that we should remove pa_operation_new_with_state_callback, </span></div><div style><span style="font-family:arial,sans-serif;font-size:13px">I will send another patch with that removed.</span></div>
<div style>Thanks</div><div style><br></div><div style><br></div><div style><br></div><div><br></div><div><div>diff --git a/src/map-file b/src/map-file</div><div>index a20314c..91d61c2 100644</div><div>--- a/src/map-file</div>
<div>+++ b/src/map-file</div><div>@@ -220,6 +220,7 @@ pa_msleep;</div><div> pa_operation_cancel;</div><div> pa_operation_get_state;</div><div> pa_operation_ref;</div><div>+pa_operation_set_state_callback;</div><div> pa_operation_unref;</div>
<div> pa_parse_sample_format;</div><div> pa_path_get_filename;</div><div>diff --git a/src/pulse/internal.h b/src/pulse/internal.h</div><div>index c5bdcb1..aee4398 100644</div><div>--- a/src/pulse/internal.h</div><div>+++ b/src/pulse/internal.h</div>
<div>@@ -234,6 +234,8 @@ struct pa_operation {</div><div> pa_operation_state_t state;</div><div> void *userdata;</div><div> pa_operation_cb_t callback;</div><div>+ void *state_userdata;</div><div>+ pa_operation_notify_cb_t state_callback;</div>
<div> </div><div> void *private; /* some operations might need this */</div><div> };</div><div>@@ -249,6 +251,7 @@ void pa_command_stream_event(pa_pdispatch *pd, uint32_t command, uint32_t tag, p</div><div> void pa_command_client_event(pa_pdispatch *pd, uint32_t command, uint32_t tag, pa_tagstruct *t, void *userdata);</div>
<div> void pa_command_stream_buffer_attr(pa_pdispatch *pd, uint32_t command, uint32_t tag, pa_tagstruct *t, void *userdata);</div><div> </div><div>+pa_operation *pa_operation_new_with_state_callback(pa_context *c, pa_stream *s, pa_operation_cb_t callback, void *userdata, pa_operation_notify_cb_t state_callback, void *state_userdata);</div>
<div> pa_operation *pa_operation_new(pa_context *c, pa_stream *s, pa_operation_cb_t callback, void *userdata);</div><div> void pa_operation_done(pa_operation *o);</div><div> </div><div>diff --git a/src/pulse/operation.c b/src/pulse/operation.c</div>
<div>index fe160a3..891269e 100644</div><div>--- a/src/pulse/operation.c</div><div>+++ b/src/pulse/operation.c</div><div>@@ -26,13 +26,14 @@</div><div> #include <pulse/xmalloc.h></div><div> #include <pulsecore/macro.h></div>
<div> #include <pulsecore/flist.h></div><div>+#include <pulse/fork-detect.h></div><div> </div><div> #include "internal.h"</div><div> #include "operation.h"</div><div> </div><div> PA_STATIC_FLIST_DECLARE(operations, 0, pa_xfree);</div>
<div> </div><div>-pa_operation *pa_operation_new(pa_context *c, pa_stream *s, pa_operation_cb_t cb, void *userdata) {</div><div>+pa_operation *pa_operation_new_with_state_callback(pa_context *c, pa_stream *s, pa_operation_cb_t cb, void *userdata, pa_operation_notify_cb_t state_cb, void *state_userdata) {</div>
<div> pa_operation *o;</div><div> pa_assert(c);</div><div> </div><div>@@ -47,6 +48,8 @@ pa_operation *pa_operation_new(pa_context *c, pa_stream *s, pa_operation_cb_t cb</div><div> o->state = PA_OPERATION_RUNNING;</div>
<div> o->callback = cb;</div><div> o->userdata = userdata;</div><div>+ o->state_callback = state_cb;</div><div>+ o->state_userdata = state_userdata;</div><div> </div><div> /* Refcounting is strictly one-way: from the "bigger" to the "smaller" object. */</div>
<div> PA_LLIST_PREPEND(pa_operation, c->operations, o);</div><div>@@ -55,6 +58,10 @@ pa_operation *pa_operation_new(pa_context *c, pa_stream *s, pa_operation_cb_t cb</div><div> return o;</div><div> }</div><div>
</div><div>+pa_operation *pa_operation_new(pa_context *c, pa_stream *s, pa_operation_cb_t cb, void *userdata) {</div><div>+ return pa_operation_new_with_state_callback(c, s, cb, userdata, NULL, NULL);</div><div>+}</div>
<div>+</div><div> pa_operation *pa_operation_ref(pa_operation *o) {</div><div> pa_assert(o);</div><div> pa_assert(PA_REFCNT_VALUE(o) >= 1);</div><div>@@ -91,6 +98,8 @@ static void operation_unlink(pa_operation *o) {</div>
<div> o->stream = NULL;</div><div> o->callback = NULL;</div><div> o->userdata = NULL;</div><div>+ o->state_callback = NULL;</div><div>+ o->state_userdata = NULL;</div><div> }</div><div> </div>
<div> static void operation_set_state(pa_operation *o, pa_operation_state_t st) {</div><div>@@ -104,6 +113,9 @@ static void operation_set_state(pa_operation *o, pa_operation_state_t st) {</div><div> </div><div> o->state = st;</div>
<div> </div><div>+ if (o->state_callback)</div><div>+ o->state_callback(o, o->state_userdata);</div><div>+</div><div> if ((o->state == PA_OPERATION_DONE) || (o->state == PA_OPERATION_CANCELED))</div>
<div> operation_unlink(o);</div><div> </div><div>@@ -130,3 +142,17 @@ pa_operation_state_t pa_operation_get_state(pa_operation *o) {</div><div> </div><div> return o->state;</div><div> }</div><div>+</div><div>
+void pa_operation_set_state_callback(pa_operation *o, pa_operation_notify_cb_t cb, void *userdata) {</div><div>+ pa_assert(o);</div><div>+ pa_assert(PA_REFCNT_VALUE(o) >= 1);</div><div>+</div><div>+ if (pa_detect_fork())</div>
<div>+ return;</div><div>+</div><div>+ if (o->state == PA_OPERATION_DONE || o->state == PA_OPERATION_CANCELED)</div><div>+ return;</div><div>+</div><div>+ o->state_callback = cb;</div><div>+ o->state_userdata = userdata;</div>
<div>+}</div><div>diff --git a/src/pulse/operation.h b/src/pulse/operation.h</div><div>index b6b5691..20c3636 100644</div><div>--- a/src/pulse/operation.h</div><div>+++ b/src/pulse/operation.h</div><div>@@ -34,6 +34,9 @@ PA_C_DECL_BEGIN</div>
<div> /** An asynchronous operation object */</div><div> typedef struct pa_operation pa_operation;</div><div> </div><div>+/** A generic callback for operation cancelation */</div><div>+typedef void (*pa_operation_notify_cb_t) (pa_operation *o, void *userdata);</div>
<div>+</div><div> /** Increase the reference count by one */</div><div> pa_operation *pa_operation_ref(pa_operation *o);</div><div> </div><div>@@ -50,6 +53,10 @@ void pa_operation_cancel(pa_operation *o);</div><div> /** Return the current status of the operation */</div>
<div> pa_operation_state_t pa_operation_get_state(pa_operation *o);</div><div> </div><div>+/** Set the callback function that is called when the operation</div><div>+ * is canceled due to disconnection. \since 3.0 */</div>
<div>+void pa_operation_set_state_callback(pa_operation *o, pa_operation_notify_cb_t cb, void *userdata);</div><div>+</div><div> PA_C_DECL_END</div><div> </div><div> #endif</div></div></div><div class="gmail_extra"><br><br>
<div class="gmail_quote">On Sat, Jan 5, 2013 at 12:25 AM, Favonia <span dir="ltr"><<a href="mailto:favonia@gmail.com" target="_blank">favonia@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Fri, Jan 4, 2013 at 7:50 AM, Tanu Kaskinen <<a href="mailto:tanuk@iki.fi">tanuk@iki.fi</a>> wrote:<br>
> Ok, would you be willing to write an updated patch?<br>
I'm CC'ing one of my teammates who might be able to do it now.<br>
<br>
> Some feedback about the first version:<br>
><br>
> * It would be better to call the callback from operation_set_state<br>
> rather than from context_unlink().<br>
That's probably true. We'll look at it. Thanks.<br>
<br>
> * pa_operation_new_with_cancel_callback() seems unnecessary. There's no<br>
> need to initialize the callback to anything else than NULL when creating<br>
> a new operation object.<br>
This is indeed unnecessary under the current practice. As far as I<br>
remember, all current APIs which return a new pa_operation object only<br>
provide a way to subscribe to the result of successful execution.<br>
Perhaps I was hoping that, in some future version we can also set up<br>
the new callback when created. We can certainly comment out this<br>
function for now. However, in the long run, refactoring with this<br>
function will help, I think.<br>
<br>
> * Every new function in the public API need to be added to<br>
> src/map-file, otherwise applications won't be able to use the function<br>
> (linking will fail).<br>
I see. Sorry for the mistake.<br>
<span class="HOEnZb"><font color="#888888"><br>
Favonia<br>
</font></span></blockquote></div><br></div>