GNOME Bugzilla – Bug 779790
waylandsink: add offset setting for multiplane dmabuf
Last modified: 2017-03-14 15:10:56 UTC
Created attachment 347528 [details] [review] add offset setting for multiplane dmabuf By default, the offset value of each plane is taken from GstVideoInfo, which is corresponding to the color format. There are cases where this value is different from the actual memory offset. When the memory is allocated for each plane separately, The value of the offset of each memory block (from GstMem) should be used.
Review of attachment 347528 [details] [review]: You definitely spotted a bug here, good catch. The solution isn't quite right. In fact, the memory offset in GStreamer is currently always 0, but we should eventually account for it. That needs to be implement in v4l2 (were such offset is signalled) and accounted in glupload, kmssink, vl42sink and possibly vaapisink. Considering the spread, a helper would be nice. Better approach is to use gst_buffer_find_memory (buffer, offset, ...). The offset is what you get from the video meta. This is generic, and will work regardless if you have 1 or more DMABuf. The length parameter is tricky, and is unimplemented (using 1) in GstVideoMeta::default_map(). The returned skip is the actual offset on the memory. The final offset would then be mem->offset + skip (regardless the plane). I think in most correct implementation, we simply use skip, which is always correct for now, but won't always be.
Created attachment 347818 [details] [review] waylandsink: fix memory offset calculation for multi-plane dmabuf buffers The offset calculation was indeed wrong. In my older implementations I was using 0 for the offset, which worked at least (while the current implementation doesn't work for sure). The correct solution is indeed mem->offset + skip, which works regardless of how many file descriptors & planes there are. I think the attached patch is doing this correctly now. Nicolas?
Review of attachment 347818 [details] [review]: Yes, this is the right method I believe. This probably fixes YUV handling with latest Weston. On PC, the best way to test is to use vivid capture driver, loaded with mplane=2. And then: v4l2src io-mode=dmabuf ! video/x-raw,format... ! waylandsink As mention, I would error-out if a plane is not found though. ::: ext/wayland/wllinuxdmabuf.c @@ +113,3 @@ + zwp_linux_buffer_params_v1_add (params, fd, i, m->offset + skip, + stride, 0, 0); + } I think we should handle the else/error case here.
Created attachment 347914 [details] [review] waylandsink: fix memory offset calculation for dmabuf buffers Added error handling.
Created attachment 347915 [details] [review] glupload: adjust memory offset calculation for dmabuf buffers This applies the same logic to glupload. The code there already uses gst_buffer_find_memory, but the memory offset is not being accounted for.
Created attachment 347916 [details] [review] kmssink: adjust memory offset calculation for dmabuf buffers Same for kmssink.
Comment on attachment 347916 [details] [review] kmssink: adjust memory offset calculation for dmabuf buffers lgtm
Review of attachment 347914 [details] [review]: Looks good now.
commit 4757ec88604835b5e3b6ce32ba6ddb5e6b6611b4 Author: George Kiagiadakis <george.kiagiadakis@collabora.com> Date: Mon Mar 13 16:06:53 2017 +0200 kmssink: adjust memory offset calculation for dmabuf buffers The data in the dmabuf fd may not start from byte 0, therefore we need to inform DRM about this additional offset. https://bugzilla.gnome.org/show_bug.cgi?id=779790 commit 20d4aca0d44b83347c12c24e770a5b578afcfc20 Author: George Kiagiadakis <george.kiagiadakis@collabora.com> Date: Mon Mar 13 15:48:33 2017 +0200 glupload: adjust memory offset calculation for dmabuf buffers The data in the dmabuf fd may not start from byte 0, therefore we need to inform EGL about this additional offset. https://bugzilla.gnome.org/show_bug.cgi?id=779790 commit 5e9ce3313f4acd26b6672e7729f2a3e4f95ff798 Author: George Kiagiadakis <george.kiagiadakis@collabora.com> Date: Mon Mar 13 12:46:47 2017 +0200 waylandsink: fix memory offset calculation for dmabuf buffers https://bugzilla.gnome.org/show_bug.cgi?id=779790