[Bug 776927] gl: gldownload: convert GstGLMemory to GstDmaBuf

[GStreamer (GNOME Bugzilla)]

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

Nicolas Dufresne (stormer) <nicolas at ndufresne.ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nicolas at ndufresne.ca

--- Comment #25 from Nicolas Dufresne (stormer) <nicolas at ndufresne.ca> ---
(In reply to Matt Fischer from comment #24)
> (In reply to Nicolas Dufresne (stormer) from comment #23)
> > You need to check if downstream supports GstVideoMeta. If not, you may check
> > (option) that the list os strides and offsets matches the default set to
> > GstVideoInfo when doing gst_video_info_set_format(). If that does not match,
> > dmabuf exportation is not possible, return NULL.
> > 
> How does one check if downstream supports a given meta?  I was under the
> impression that you just attached whatever meta you wanted, and hoped that
> downstream paid attention to it.  Since I am already relying on there being
> a GstVideoMeta on the incoming buffer, I had thought that it would be safe
> to assume that I could just create one on the outgoing buffer as well.

I miss that in the review, you indeed need to check if the vmeta pointer is
null, otherwise your code may crash. To check if downstream supports
GstVideoMeta, you need to look into the allocation query. See
"propose_allocation" function in _UploadMethod structure. There is a
"decide_query" pointer that contains this information. Then you can get the
information using:

  gst_query_find_allocation_meta(query, GST_VIDEO_META_API_TYPE, NULL)

> > We have function tables already, so you don't endup doing get_proc_address()
> > every image plane.
> > 
> I'm not sure I understand what you're asking me to do here.  Can you clarify?

You should not be calling get_proc_address() for each frame, but cache those
function pointer. That being said, I just noticed Mathew started doing that, so
you took that from existing code. We can fix that later, it's just slightly
inefficient.

> 
> > Why ? What's the problem ? and why checking this so late ? We already
> > support multiple FD when importing. I don't think this is very useful if you
> > limit this to packed formats.
> >
> This is just a comment about multiple planes within the dmabuf export.  A
> planar image will have multiple GstMemory objects, each of which will get
> its own EGLImage and its own dmabuf.  See the loop in _try_export_dmabuf()
> to see how it will loop for multiple planes.

Ok, that looks correct, nevermind then ;-P

> 
> As far as I understand, that will correctly handle the planar cases.  If we
> try to handle the case where each of those individual EGLImages also exports
> multiple dmabufs, I wasn't clear how we would handle the N * M dmabuf
> handles that would result.  Can you comment on what we ought to do in that
> case?

Well, in theory we could support N Texture/Plane coming out as 1 DMABuf. But in
practice, I doubt it's really going to happen, and it's probably complicated in
the current code, so nothing you should worry about.

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