[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 #21 from Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> 2014-04-15 13:15:57 UTC ---
(In reply to comment #20)
> (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

I meant this need to be documented somewhere in docs/design of -base, or in the
Meta doc. Nobody implementing it's own sink will read cogl doc.

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

For video/x-raw(meta:GstVideoGLTextureUploadFormat) the format field means the
texture format you will upload to. If you have negotiated that, you should not
be using map() on the buffer at all.

For video/x-raw, the format means the buffer format (e.g. I420). You may still
have a upload meta, hence the need to add a format in the meta.

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

For that I'd suggest proper review, that will mean, split the gst-plugins-base
part from the cogl and imx part. File bugs for each and attach patch.

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