[Bug 727886] GstVideoGLTextureUploadMeta assumes no color conversion is done during upload

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=727886
  GStreamer | gst-plugins-base | git

--- Comment #31 from Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> 2014-04-22 14:06:05 UTC ---
(In reply to comment #30)
> After my argument in comment #29, and considering the above, I want to make a
> new proposal:
> 
> * Add indeed a new GstVideoGLTextureUploadMeta2 which will be defined as:

I think we agreed that if we where to create a new meta, we would give it a
better name. An example could be GstVideoGLTextureMapMeta.

> 
> struct _GstVideoGLTextureUploadMeta2 {
>   GstMeta       meta;
> 
>   GstVideoGLTextureOrientation texture_orientation;
>   GstVideoInfo texture_info;

Any rational for the texture_ prefix ?

>   GstStructure *features;
> 
>   /* <private> */
>   GstBuffer *buffer;
>   GstVideoGLTextureUpload upload;
> 
>   gpointer      user_data;
>   GBoxedCopyFunc user_data_copy;
>   GBoxedFreeFunc user_data_free;
> };
> 
> * The number of textures *must* be equal to the number of planes of the video
> format set in texture_info.

Why ? Won't that prevent few platforms from using it ?

> * The texture GL format (GL_RGBA, GL_RGB, etc) can be determined from the
> plane's bit depth, as set in texture_info - I think the sink is not really
> interested in the format, it is interested in how the data is stored in the
> texture (8bit -> A, 16bit -> RG, 24bit -> RGB, 32bit -> RGBA / see also
> CoglTextureComponents for example)

Though you need to guaranty to never try with a miss-matching depth.

> * The size of each texture can be calculated using
> GST_VIDEO_INFO_COMP_WIDTH/HEIGHT

The dimension in pixels, to clarify.

> 
> * The "features" structure should contain a list of additional features that
> the sink needs to enable in order to use the uploaded texture. These can be one
> of:
> 
> - texture_size_includes_stride (G_TYPE_BOOLEAN): TRUE if the texture size
> includes the paddings of the buffer (as in vivante) and the sink needs to
> display only the top-left subregion of the texture that equals the video size
> as negotiated with the caps
> - <anything else that may come up later> :)

That is not seems very elegant, maybe some more research is needed, better
rationals.

> 
> * Sinks should advertise through the allocation query what they support using a
> structure named "GstVideoGLTextureUploadMeta2" with the following well-known
> fields:
> 
> - format (list - G_TYPE_STRING): the list of texture formats that are supported
> - orientation (list - G_TYPE_INT): the list of texture orientations
> (GstVideoGLTextureOrientation) that are supported
> - feature.* : reserved namespace for special features that the sink can
> support; these should be the same as the fields of the "features" structure
> (see above), but prefixed with "feature.". For example:
> feature.texture_size_includes_stride (G_TYPE_BOOLEAN)
> - gst.gl.* : whatever GstGL needs to use (see comment #29)

Looks good, maybe a special attention to ensure this can be intersected /
subset, so we can have generic code to check if it's compatible ?

> 
> * Upstream elements that produce buffers should decide based on the allocation
> query how to best match what the sink supports, but they should add the upload
> meta anyway, whether the sink supports all the features they need or not. The
> features field of the upload meta should be populated with all the features
> that are *mandatory* in order for the upload to work properly and nothing else
> 
> * Sinks that support the upload meta should look at the format and the features
> in the upload meta and decide themselves if they are able to use it or not. If
> there is a certain feature in the "features" field that the sink does not know
> about, then it *must not* use the upload meta.
> 
> The idea is that with the "features" field, all the sinks that are going to
> implement GstVideoGLTextureUploadMeta2 must implement a logic block that will
> iterate over all the fields in the "features" structure and will enable their
> handling. But if there is at least one field in "features" that the sink does
> not know how to handle, then it must fall back to not using the upload meta,
> assuming that this feature is required. This way we can later add more features
> and/or change the semantics by using fields of the "features" structure to
> indicate whatever we need without dropping backwards compatibility. This also
> solves the dreadful problem with the tee.

That why I'd like to see a reuse of the caps intersection and subset features
here (see gst_structure_is_subset(), gst_structure_can_intersect()).

> 
> * Now in case we have GstVideoGLTextureUploadMeta2 negotiated as a
> GstCapsFeature, the negotiation should happen entirely with caps and the
> "features" structure of the upload meta can be ignored, although it should
> still be there for convenience. The fields of the allocation query structure
> should be distributed in caps as follows:
> 
> - format: becomes a field in the caps (which exists already in the video/x-raw
> caps, but it changes semantics here, describing the format of the texture
> instead of the buffers)
> - orientation: becomes a field in the caps
> - feature.*: become GstCapsFeatures that are advertised additionally to
> "meta:GstVideoGLTextureUploadMeta2". I propose then prefixing them with
> "meta:GstVideoGLTextureUploadMeta2:" instead of "feature." so that they become
> meta:GstVideoGLTextureUploadMeta2:* or something similar... Obviously the sink
> here will need to advertise all possible combinations of them in different caps
> structures. Unfortunately I see is no other way to ensure that they all exist
> in both upstream and downstream elements.
> - gst.gl*: these should stay in the allocation query structure.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.