[Bug 779146] dmabuf: be able to negotiate tiled surfaces

[GStreamer (GNOME Bugzilla)]

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

--- Comment #28 from Scott D Phillips <scott.d.phillips at intel.com> ---
(In reply to Nicolas Dufresne (stormer) from comment #26)
> Review of attachment 347945 [details] [review]:
> 
> ::: gst-libs/gst/allocators/gstdmabufmeta.c
> @@ +25,3 @@
> + *
> + * #GstDmabufMeta carries metadata that goes along with
> + * dmabuf memory in the buffer, like drm modifier.
> 
> Document the negotiation process.
> 
> @@ +133,3 @@
> +
> +void
> +gst_query_add_allocation_dmabuf_meta (GstQuery * query, guint64
> drm_modifier)
> 
> This is confusing name, does it add the allocation meta, or it add a
> modifier. To be honnest, I have never see a meta working like this. Why not
> simply pass an array of guint64 here ? Also, undocumented.

Sure, I can change it to an array. I suppose it will be a length and then an
array pointer, because there is no possible sentinel value for the end of an
array of modifiers (as zero is valid and not necessarily the last choice in
everyone's preference list). But yes, I can see that being more ergonomic than
what is there currently.

> ::: gst-libs/gst/allocators/gstdmabufmeta.h
> @@ +41,3 @@
> +  GstMeta parent;
> +
> +  guint64 drm_modifier;
> 
> Modifiers are applied per plane in other API. This must be incorrect.

Which API? There is just a single modifier with ADDFB2. From drm_mode.h:

> Note that even though it looks like we have a modifier
> per-plane, we in fact do not. The modifier for each plane must
> be identical. Thus all combinations of different data layouts
> for multi plane formats must be enumerated as separate
> modifiers.

> @@ +44,3 @@
> +};
> +
> +GST_EXPORT
> 
> We don't currently use these, please remove until further notice.

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