[PATCH 2/5] winegstreamer: Create, start and stop a bin container.

[Zebediah Figura]

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.)