[Bug 791453] videocrop: Add GstVideoCropMeta support

[GStreamer (GNOME Bugzilla)]

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

--- Comment #3 from Nicolas Dufresne (stormer) <nicolas at ndufresne.ca> ---
(In reply to Sebastian Dröge (slomo) from comment #2)
> Review of attachment 365331 [details] [review]:
> 
> Looks good to me except for the fixate_caps() hack. Is there no better way,
> maybe by adding some new API to basetransform? :)

Probably, I can open a bug, but I'd like to keep that separate, as it's not
related and this will raise a lot of questions, which we may prefer fixing by
implementing GstAsyncBaseTransform (which will be able to work synchroneously).

> 
> ::: gst/videocrop/gstvideocrop.c
> @@ +704,3 @@
> +    goto done;
> +
> +  if (!gst_pad_push_event (trans->srcpad, gst_event_new_caps (othercaps)))
> 
> gst_base_transform_update_src_caps()? Sending a caps event behind
> basetransform's back is probably not a good idea.

What's undocumented about this helper is that it will mark the srcpad for
reconfigure, so it goes through full renegotiation next time. If you do that in
this case you'll have an infinite recursion.

> 
> @@ +708,3 @@
> +
> +  query = gst_query_new_allocation (othercaps, FALSE);
> +  if (!gst_pad_peer_query (trans->srcpad, query))
> 
> And this would cause two allocation queries to be done, with the result of
> this one being ignored more or less?

The FALSE means I don't need a pool, which reduces the amount of extra work,
and the second query will be faster because the pipeline will already be
drained. It's the only way for now, same thing happens for textoverlay.

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