[Bug 789843] API: GstPromise - object with promise/future-like semantics

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=789843

Olivier Crête <olivier.crete at ocrete.ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #362870|none                        |needs-work
             status|                            |

--- Comment #2 from Olivier Crête <olivier.crete at ocrete.ca> ---
Review of attachment 362870:
 --> (https://bugzilla.gnome.org/review?bug=789843&attachment=362870)

The API seems relatively sane. The only thing I'm not so happy about is the
change function being called synchronously with the mutex held, is that really
required? I'd feel better having it called from another thread, doing something
like gst_element_call_async(), it could even be using the same thread pool.

It may also be nice to have a vaarg version of gst_promise_reply_...() which
calls gst_structure_new_valist() internally to save a line.

::: gst/gstpromise.c
@@ +40,3 @@
+ *
+ * A #GstPromise can be created with gst_promise_new(), replied to
+ * with gst_promise_reply(), interrupted with gst_promise_reply() and

I guess you mean "interrupted with gst_promise_interrupt()" ?

@@ +157,3 @@
+    GST_LOG ("%p replied", promise);
+
+    promise->promise = s;

Maybe you want to also do gst_structure_set_parent_refcount() ? Or does it make
no sense at all?

::: gst/gstpromise.h
@@ +69,3 @@
+  GstStructure         *promise;
+
+  /*< private >*/

Shouldn't this be in a hidden struct part of the struct? Maybe using the
GstXXXImpl trick used by GstBuffer. I would hide all of the implementation
details, including the structure and the result and only allow access to the
accessor function.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.