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

Tue Apr 22 04:55:52 PDT 2014

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

--- Comment #30 from George Kiagiadakis <kiagiadakis.george at gmail.com> 2014-04-22 11:55:48 UTC ---
So basically there have been some arguments on irc which are not written here:

* We agreed that adding texture size information on the meta is probably ok.
Nicolas argued, though, that one should avoid using the fragment shader to
implement this cropping, due to 16-bit float constraints on some platforms that
will lead to bad video quality.

* Nicolas argued that upstream should not add the upload meta on buffers if
downstream does not claim support for all the features that upstream needs
(i.e. the texture format being different than the caps format or the texture
size being different than the video size), so that we have backwards
compatibility with sinks that are not updated to handle this information. The
problem however is that adding a tee before the video sink will drop the
allocation query and there will be no upload meta added although it may be
supported by the sink. Carlos argued here that we should probably do a
GstVideoGLTextureUploadMeta2 in order to solve the compatibility problem.

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:

struct _GstVideoGLTextureUploadMeta2 {
  GstMeta       meta;

  GstVideoGLTextureOrientation texture_orientation;
  GstVideoInfo texture_info;
  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.
* 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)
* The size of each texture can be calculated using
GST_VIDEO_INFO_COMP_WIDTH/HEIGHT

* 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> :)

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

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

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