[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 #20 from George Kiagiadakis <kiagiadakis.george at gmail.com> 2014-04-15 09:45:26 UTC ---
(In reply to comment #19)
> (In reply to comment #18)
> > http://cgit.collabora.com/git/user/gkiagia/gst-plugins-base.git/log/?h=gluploadmeta
> 
> I'd prefer simply ->format rather then uploaded_format. Also, for backward

ok

> compatiblity, you need to handle the case this is set to
> GST_VIDEO_FORMAT_UNKNOWN (zero) in your implementation and need to be
> documented.

I do handle the case where it is set to UNKNOWN and it is documented in
gstvideometa.h

> > http://cgit.collabora.com/git/user/gkiagia/cogl.git/log/?h=gluploadmeta
> 
> Saying you have texture support for GST_VIDEO_FORMATS_ALL is most likely not
> true. It should instead list the item you have in your switch, which I suppose
> match BASE_SINK_CAPS.

This is a good argument. I was thinking that there can be 2 cases for the caps
which have meta:GstVideoGLTextureUploadMeta:

1) We declare that the "format" field of those caps represents the "uploaded"
format of the data, so in coglsink we advertise:

video/x-raw, format={ AYUV, YV12, I420, RGBA, BGRA, RGB, BGR, NV12 };
video/x-raw(meta:GstVideoGLTextureUploadFormat), format={ AYUV, YV12, I420,
RGBA, BGRA, RGB, BGR, NV12 }

and when it gets negotiated with imxvpudec using the caps with the meta, format
is the uploaded format (RGBA), although the buffers are actually I420:

video/x-raw(meta:GstVideoGLTextureUploadFormat), format=RGBA

2) We declare that the "format" field of those caps represents the actual
buffer format and we only negotiate the uploaded format using the allocation
query. Therefore coglsink advertises:

video/x-raw, format={ AYUV, YV12, I420, RGBA, BGRA, RGB, BGR, NV12 };
video/x-raw(meta:GstVideoGLTextureUploadFormat), format={ ANY }

and gets negotiated with imxvpudec using:

video/x-raw(meta:GstVideoGLTextureUploadFormat), format=I420

What I have implemented here is the second, because it looks cleaner. You only
deal with the logic of the uploaded format negotiation in one place and you
keep the information about what format the buffers actually are.

> Implementation of GstVideoGLTextureUploadMetaParams looks wrong to me and isn't
> documented anywhere.

Why is it wrong? It needs to be documented, though, that's true.

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