Sorry for the slow review here; I meant to reply earlier but had a computer part die on me...
On 2/18/22 11:27, Rémi Bernon wrote:
@@ -62,6 +63,12 @@ NTSTATUS wg_transform_destroy(void *args) { struct wg_transform *transform = args;
- if (transform->container)
gst_element_set_state(transform->container, GST_STATE_NULL);- if (transform->container)
g_object_unref(transform->container);
These conditions can be collapsed.
Actually I'm not much of a fan of calling destruction functions from the error path of the corresponding creation function like this; I find it tends to lead to problems later.
@@ -111,6 +122,13 @@ NTSTATUS wg_transform_create(void *args) gst_pad_set_element_private(transform->my_sink, transform); gst_pad_set_chain_function(transform->my_sink, transform_sink_chain_cb);
- status = STATUS_UNSUCCESSFUL;
- gst_element_set_state(transform->container, GST_STATE_PAUSED);
- ret = gst_element_get_state(transform->container, NULL, NULL, -1);
- if (ret == GST_STATE_CHANGE_FAILURE)
goto done;
I'd rather set "status" inside the if block here.
(Although given that we never interpret the status value, there doesn't seem to be much point in setting it to anything specific; i.e. we could just always set it to STATUS_UNSUCCESSFUL and have done.)