[PATCH] winegstreamer: Fixes for video flipping

[Andrew Eikum]

Thanks very much for working on this, I'm glad to have more eyes
looking at it. Comments in-line below.

On Sat, Jul 16, 2016 at 02:51:10AM +1000, Jan Schmidt wrote:
> The videoflip element doesn't handle all formats,
> so some formats like Intel Indeo 3 will fail.
> Add videoconvert to do format conversion when it's
> needed.
> 

Shouldn't the decodebin handle this during format negotiation when the
videoflip is connected to it?

> Fix some refcount issues too - add a new element
> to a bin takes ownership of the floating reference,
> so there's no need to keep pointers around to unref
> later.
> 

Good catch. Could you separate this out into a different, stand-alone
patch from the other changes?

> Fix state setting of new elements - always just
> set them to PAUSED. gst_element_sync_state_with_parent()
> can be problematic, and we only need these in paused
> or playing anyway.
> 

I'm not sure about this part. It makes sense to me for all members of
a container to be in the same state. Why is it better for these to be
PAUSED while others are PLAYING? If it's needed, this should be its
own patch, too.

> @@ -846,13 +855,16 @@ static void init_new_decoded_pad(GstElement *bin, GstPad *pad, GSTImpl *This)
>  
>          gst_util_set_object_arg(G_OBJECT(pin->flipfilter), "method", "vertical-flip");
>  
> -        gst_bin_add(GST_BIN(This->container), pin->flipfilter);
> -        gst_element_sync_state_with_parent(pin->flipfilter);
> +        gst_element_set_state (vconv, GST_STATE_PAUSED);
> +        gst_bin_add(GST_BIN(This->container), vconv); /* bin takes ownership */
> +        gst_element_set_state (pin->flipfilter, GST_STATE_PAUSED);
> +        gst_bin_add(GST_BIN(This->container), pin->flipfilter); /* bin takes ownership */
>  
> -        pin->flip_sink = gst_element_get_static_pad(pin->flipfilter, "sink");
> +        gst_element_link (vconv, pin->flipfilter);
> +
> +        pin->flip_sink = gst_element_get_static_pad(vconv, "sink");
>          if(!pin->flip_sink){
>              WARN("Couldn't find sink on flip filter\n");
> -            gst_object_unref(pin->flipfilter);

I believe this leaves a dangling element in the container. Should
these lines be replaced with calls to gst_bin_remove() instead?

Andrew