After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 779787 - waylandsink: handling y-flipped dmabuf
waylandsink: handling y-flipped dmabuf
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.11.1
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-03-09 07:31 UTC by Shinya Saito
Modified: 2018-11-03 14:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
add y-flip flag for dmabuf (4.82 KB, patch)
2017-03-09 07:31 UTC, Shinya Saito
none Details | Review

Description Shinya Saito 2017-03-09 07:31:42 UTC
Created attachment 347525 [details] [review]
add y-flip flag for dmabuf

By default, waylandsink does not set the y-flip flag on wl_buffers backed by dmabufs.  Depending on the dmabuf source, this can cause the image to be  displayed upside down. 

In the patch, a y-flip property is added to waylandsink, but I don't know if this is the best way. Maybe the Gstbuffer should hold the information of y-flip, but I
am not sure.
Comment 1 Nicolas Dufresne (ndufresne) 2017-03-09 13:06:39 UTC
Upstream can use an orientation tag, or a affine transform meta to signal this.

Is this feature limited to DMABuf bug, or can we y-flip shm too ? Also, what does y flip means?
Comment 2 Nicolas Dufresne (ndufresne) 2017-03-09 16:31:14 UTC
Can you explain a use case why you need to expose this as a property ? The intent for this flag is for user to not have to care, while a property is for user control.
Comment 3 Damian Hobson-Garcia 2017-03-13 04:16:07 UTC
Please allow me to add some background to this discussion.

The y-flip setting is for dmabuf wayland protocol only.  Shm buffers do not have this flag.

The need for the flag comes from the fact that OpenGL generally has the origin (0,0) at the bottom-left corner, while other sources typically, but not necessarily have it at the top-left, which causes a problem when the two different types of buffers are exported and imported via dmabuf. (The GL texture2D functions take this into account for buffers that are imported via a virtual memory address).  

I have discussed this with Shinya and we both agree with you that the ideally the user shouldn't have to care about this setting and should be set automatically by the meta-data.  However we couldn't find any meta-data in use that seemed to capture the necessary information.  I think that the orientation tag can be used if the input comes from a device with at top-left origin, but its also necessary to determine if the buffer has been written by OpenGL (or another boffom-left origin source) and not modified elsewhere in the pipeline. We didn't want to propose some new meta-data for just this one case, so we opted for the property instead.  

Also, the "polarity" of the flag changed with Weston 1.12.  Before that y-flip being set would mean that the origin was in the top-left, but after 1.12, it now means that it is in the bottom-left,  (cf. https://cgit.freedesktop.org/wayland/weston/commit/?id=319397e050e2b4833e10093ccefd8ad77a6ef78d) so a different setting will be necessary depending on the version of weston installed on the system.  I suppose thought that this is something that could be addressed with the configure scripts.
Comment 4 Nicolas Dufresne (ndufresne) 2017-03-13 14:26:31 UTC
(In reply to Damian Hobson-Garcia from comment #3)
> Please allow me to add some background to this discussion.
> 
> The y-flip setting is for dmabuf wayland protocol only.  Shm buffers do not
> have this flag.

This makes it clear that the property approach is not acceptable.

> 
> The need for the flag comes from the fact that OpenGL generally has the
> origin (0,0) at the bottom-left corner, while other sources typically, but
> not necessarily have it at the top-left, which causes a problem when the two
> different types of buffers are exported and imported via dmabuf. (The GL
> texture2D functions take this into account for buffers that are imported via
> a virtual memory address).  
> 
> I have discussed this with Shinya and we both agree with you that the
> ideally the user shouldn't have to care about this setting and should be set
> automatically by the meta-data.  However we couldn't find any meta-data in
> use that seemed to capture the necessary information.  I think that the
> orientation tag can be used if the input comes from a device with at
> top-left origin, but its also necessary to determine if the buffer has been
> written by OpenGL (or another boffom-left origin source) and not modified
> elsewhere in the pipeline. We didn't want to propose some new meta-data for
> just this one case, so we opted for the property instead.  

It is still not clear why it matter if the y-flipped dmabuf is from GL or another driver behaving the same way. Though, you need some consideration, since most DMABuf are mappable. Let's say you want the following pipeline:

  weirdodmabuexporter ! textoverlay text=allo ! waylandsink

Depending on the memory controller behaviour, with the property approach, you may end up with an up-side down text being printed (at least until text offload is implement in waylandsink). That means, the property approach is just not doing the right thing, since it forces the application to know every details about the specific platform. As textoverlay don't currently know how to compensate (it could though), you need a solution that weird force a copy (or texture2D) on your dmabuf exporter (or make the negotiation) fail.

The right approach here entirely depend on the following factor:
1. Is the memory controller fixing it on mmap ?
2. Is the DMABuf mappable ?

For each combination there exist a "right" approach, which is not a property. Some options are:

- GstVideoAffineTransformationMeta
- GstDmabufMeta (as proposed by bug #779146)
- DMAbuf caps feature + GstDmabufMeta (non-mappable)

> 
> Also, the "polarity" of the flag changed with Weston 1.12.  Before that
> y-flip being set would mean that the origin was in the top-left, but after
> 1.12, it now means that it is in the bottom-left,  (cf.
> https://cgit.freedesktop.org/wayland/weston/commit/
> ?id=319397e050e2b4833e10093ccefd8ad77a6ef78d) so a different setting will be
> necessary depending on the version of weston installed on the system.  I
> suppose thought that this is something that could be addressed with the
> configure scripts.

This is not our problem if there was a bug in a unstable protocol implementation at some random point in time. If you want to hack support for that in your product, it's your call. It does not make sense upstream.
Comment 5 Damian Hobson-Garcia 2017-03-31 03:57:37 UTC
Apologies for the late response.

(In reply to Nicolas Dufresne (stormer) from comment #4)
> (In reply to Damian Hobson-Garcia from comment #3)

> This is not our problem if there was a bug in a unstable protocol
> implementation at some random point in time. If you want to hack support for
> that in your product, it's your call. It does not make sense upstream.

I see your point.

> 
> The right approach here entirely depend on the following factor:
> 1. Is the memory controller fixing it on mmap ?
> 2. Is the DMABuf mappable ?
> 

After poking around some more through the GL plugin code, it looks like dmabuf is only used when uploading to GL textures, and that buffers are mmapp'ed (which "fixes" the orientation) and presented as shm buffer to the wayland compositor.  Given that, combined with the fact any hacks to address the  "polarity" issue don't belong upstream, I can't really think of any case where the ZWP_LINUX_BUFFER_PARAMS_V1_FLAGS_Y_INVERT flag needs to be set. So maybe this patch (or any variation of it) isn't really needed after all. Sorry for wasting your time.
Comment 6 GStreamer system administrator 2018-11-03 14:05:33 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/530.