GNOME Bugzilla – Bug 761059
kmssink: add new plugin and element
Last modified: 2016-10-31 14:17:32 UTC
This is simple video sink that use libdrm/libkms API to render frames. The element uses planes to render through drmModeSetPlane(). It has been tested in an Exynos4412 board and in a Freescale I.MX6 board.
Created attachment 319620 [details] [review] kmssink: add plugin and sink element
Created attachment 319631 [details] [review] kmssink: add plugin and sink element This is simple video sink that use libdrm/libkms API to render frames. The element uses planes to render through drmModeSetPlane(). It has been tested in an Exynos4412 board and in a Freescale I.MX6 board.
Review of attachment 319631 [details] [review]: ::: sys/kms/gstkmssink.c @@ +54,3 @@ + GST_PAD_SINK, + GST_PAD_ALWAYS, + GST_STATIC_CAPS (GST_VIDEO_CAPS_MAKE (GST_VIDEO_FORMATS_ALL))); Frome the utility functions I can see that there exist a very limited list of formats. Maybe we should stick with that here ? @@ +60,3 @@ +{ + static const char *drivers[] = { "i915", "radeon", "nouveau", "vmwgfx", + "exynos", "amdgpu", "imx-drm", "rockchip", "atmel-hlcdc" That's not exactly elegant, any better idea ? It's in udev for sure, hence also found in /sys/class/drm/card0/device/uevent . What if you have two cards on your system ? @@ +544,3 @@ + pool = NULL; + if (need_pool) { + if (!gst_kms_sink_ensure_pool (self, caps, size)) Caching pools makes certain renegotiation scenario fails. Create a new pool for each propose_allocation() call here. The allocator can be reused without problems. ::: sys/kms/gstkmsutils.c @@ +43,3 @@ + /* DEF_FMT (XRGB1555, ???), */ + /* DEF_FMT (XBGR1555, ???), */ + DEF_FMT (ARGB8888, ARGB), I believe this is not the correct match. Normally, DRM formats are expressed from a register point of view, while GStreamer format are defined as seen in the buffer array. For that reason, in little endian, DRM_FORMAT_ARGB8888 matches GST_VIDEO_FORMAT_BGRA, and for big endiant it matches GST_VIDEO_FORMAT_ARGB.
Review of attachment 319631 [details] [review]: ::: sys/kms/gstkmssink.c @@ +54,3 @@ + GST_PAD_SINK, + GST_PAD_ALWAYS, + GST_STATIC_CAPS (GST_VIDEO_CAPS_MAKE (GST_VIDEO_FORMATS_ALL))); Thanks a lot for your promptly review, Nicolas! Agree, the formats should be taken from the explicitly supported ones. @@ +60,3 @@ +{ + static const char *drivers[] = { "i915", "radeon", "nouveau", "vmwgfx", + "exynos", "amdgpu", "imx-drm", "rockchip", "atmel-hlcdc" Are you suggesting to use GstDeviceProvider as in v4l2??? @@ +544,3 @@ + pool = NULL; + if (need_pool) { + if (!gst_kms_sink_ensure_pool (self, caps, size)) Can you share me a bug report or a discussion about this renegotiation fail? Because we are using this pool caching in gstreamer-vaapi. ::: sys/kms/gstkmsutils.c @@ +43,3 @@ + /* DEF_FMT (XRGB1555, ???), */ + /* DEF_FMT (XBGR1555, ???), */ + DEF_FMT (ARGB8888, ARGB), Thanks for the catch!
By the way, I have a hackish support for dmabuf-import using the DMA-BUF/PRIME buffer sharing support, but I think it should be added in a next iteration.
(In reply to Víctor Manuel Jáquez Leal from comment #4) > Review of attachment 319631 [details] [review] [review]: > ::: sys/kms/gstkmssink.c > @@ +60,3 @@ > +{ > + static const char *drivers[] = { "i915", "radeon", "nouveau", "vmwgfx", > + "exynos", "amdgpu", "imx-drm", "rockchip", "atmel-hlcdc" > > Are you suggesting to use GstDeviceProvider as in v4l2??? I didn't go that far in my reflection, but a DeviceProvider would fit well here. So a combination of "device" property and a device monitor. But I would not block merging just for that. I understand this element is mostly for specific use case, and require per-display adaptation. So let's say if you have time. We probably want a default that just work in any case. > > @@ +544,3 @@ > + pool = NULL; > + if (need_pool) { > + if (!gst_kms_sink_ensure_pool (self, caps, size)) > > Can you share me a bug report or a discussion about this renegotiation fail? > Because we are using this pool caching in gstreamer-vaapi. It's here, I have ported ximagesink, xvimagesink and glimagessink so far. On the v4l2sink side, I need a bit or rework of the allocator and pool, but that's coming. For kmssink it's trivial really. The main difference, is that instead of checking if the GstBuffer::pool is yours, you check if the GstBuffer::pool is of the right type. https://bugzilla.gnome.org/show_bug.cgi?id=748344
(In reply to Víctor Manuel Jáquez Leal from comment #5) > By the way, I have a hackish support for dmabuf-import using the > DMA-BUF/PRIME buffer sharing support, but I think it should be added in a > next iteration. That's really nice !
Not sure I understand the device provider aspect of this discussion, what would that solve? Is this a sink one would want to present to a user in a GUI interface as an option, for example?
(In reply to Tim-Philipp Müller from comment #8) > Not sure I understand the device provider aspect of this discussion, what > would that solve? Is this a sink one would want to present to a user in a > GUI interface as an option, for example? The interface to open a drm device is drmOpen(name), where name is a string. In the code, there is an hardcoded list of names, and we try each of them until it works. The point is that you can have multiple display drivers on one platform (specially on PC), in which case you are left with no way to choose. If you make the parallel with v4l2sink, a device provider/monitor exist, so you can enumerate the v4l2 output device and configure the sink to use the selected one. Additionally, there is a device property that can let you manually select a device. It's not in my opinion a good reason not to merge. It's just a nice to have feature, just like the announced dmabuf support.
Created attachment 320170 [details] [review] kmssink: add plugin and sink element This is simple video sink that use libdrm/libkms API to render frames. The element uses planes to render through drmModeSetPlane(). It has been tested in an Exynos4412 board and in a Freescale I.MX6 board.
This new iteration of the element has a ton of changes :) 1) check for gstreamer-video-1.0 pkg-config instead of linking with -lgstvideo-1.0 directly 2) add some documentation 3) add two properties: driver-name and connector-id 4) the pad template now it is generated according with the supported color formats 5) the buffer pool is not cached anymore 6) the allowed caps are now restricted the frame size to the min width,height and max width,height framebuffer reported by the driver 7) the RGB color formats are set according to the endianness Now, I have two more patches and I don't know if I should add them here, or wait to this patch to be pushed and open new bugs: one for the display ratio and the dmabuf-import one.
Created attachment 320437 [details] [review] kmssink: add plugin and sink element This is simple video sink that use libdrm/libkms API to render frames. The element uses planes to render through drmModeSetPlane(). It has been tested in an Exynos4412 board and in a Freescale I.MX6 board.
Created attachment 320438 [details] [review] kmssink: calculate display ratio Get the aspect ratio given the information provided by libdrm, and with it calculate the display ratio.
Created attachment 320439 [details] [review] kmssink: wip: add support for dmabuf-import This is a work-in-progress. It offers a mechanism for importing dmabuf into a kms buffer using the PRIME kernel interface. It has been tested with a Freescale I.MX6 board.
Review of attachment 320439 [details] [review]: As weird as it sound, I'm not exactly a fan of the io-mode properties, even though, I agree they can be handy. Though, how hard would it be to check if the incoming frame is containing dmabuf and choose the right method automatically ? I don't see any setup like in v4l2 that makes this obviously non-trivial. (not a review, just a question ;-P)
(In reply to Nicolas Dufresne (stormer) from comment #15) > Review of attachment 320439 [details] [review] [review]: > > As weird as it sound, I'm not exactly a fan of the io-mode properties, even > though, I agree they can be handy. Though, how hard would it be to check if > the incoming frame is containing dmabuf and choose the right method > automatically ? I don't see any setup like in v4l2 that makes this obviously > non-trivial. (not a review, just a question ;-P) I experimented with different approaches but none of them worked. Though, I ought to say, that my test environment for v4l2videc with dmabuf is constrained to GStreamer 1.4.5. I have pending to try with master and try to negotiate the feature memory:DMABuf and play with the bufferpool's options... Is that already landed in v4l2viddec? My general idea is: If the feature memory:DMABuf is negotiated, we know that it could be either to import or export buffers. If upstream request a buffer pool option, say GST_BUFFER_POOL_OPTION_DMABUF, so the sink will offer a buffer pool with exported KMS buffers to PRIME dmabufs (that's missing in the current implementation). Otherwise (no GST_BUFFER_POOL_OPTION_DMABUF of the buffer pools is not used in upstream) the buffers will be imported following the current mechanism.
So far, we went forward without the memory:DMABuf feature, as we could not agree on the design of it. It was always filled with ambiguities. glupload implements DMABuf importation without any needs for negotiation. This is done by simply inspecting the incoming memories. For a sink like this one, there is only 3 case so it's rather simple. - The buffer you received is one from a pool you proposed (just use it) - The buffer you received is a DMABuf (try to prime it) - None of the above, of PRIME failed, just copy
Using kmssink on i.MX6 with Kernel 4.1.0 I encountered the problem, that it won't work when there is a tee in the pipeline. Tee blocks allocation queries and kmssink may not have a copy path yet, to workaround this. Hints from Nicolas: * check the handle_frame function, maybe it only accepts buffer that comes from their own buffer pool, and can't copy to appropriate memory, used of that off-tree element so far only use zero-copy path See following pipeline for example: # GST_DEBUG=kmssink:6 gst-launch-1.0 -v -m videotestsrc ! tee ! queue ! kmssink connector=25 device-name=imx-drm Setting pipeline to PAUSED ... 0:00:00.279195334 606 0xb8ca00 DEBUG kmssink gstkmssink.c:583:gst_kms_sink_start:<kmssink0> open DRM device: imx-drm Pipeline is PREROLLING ... Got message #9 from element "kmssink0" (state-changed): GstMessageStateChanged, old-state=(GstState)GST_STATE_NULL, new-state=(GstState)GST_STATE_READY, pending-state=(GstState)GST_STATE_VOID_PENDING; Got message #10 from element "queue0" (state-changed): GstMessageStateChanged, old-state=(GstState)GST_STATE_NULL, new-state=(GstState)GST_STATE_READY, pending-state=(GstState)GST_STATE_VOID_PENDING; Got message #11 from element "tee0" (state-changed): GstMessageStateChanged, old-state=(GstState)GST_STATE_NULL, new-state=(GstState)GST_STATE_READY, pending-state=(GstState)GST_STATE_VOID_PENDING; Got message #12 from element "videotestsrc0" (state-changed): GstMessageStateChanged, old-state=(GstState)GST_STATE_NULL, new-state=(GstState)GST_STATE_READY, pending-state=(GstState)GST_STATE_VOID_PENDING; Got message #13 from element "pipeline0" (state-changed): GstMessageStateChanged, old-state=(GstState)GST_STATE_NULL, new-state=(GstState)GST_STATE_READY, pending-state=(GstState)GST_STATE_PAUSED; Got message #15 from pad "queue0:src" (stream-status): GstMessageStreamStatus, type=(GstStreamStatusType)GST_STREAM_STATUS_TYPE_CREATE, owner=(GstElement)"\(GstQueue\)\ queue0", object=(GstTask)"\(GstTask\)\ queue0:src"; Got message #16 from element "queue0" (state-changed): GstMessageStateChanged, old-state=(GstState)GST_STATE_READY, new-state=(GstState)GST_STATE_PAUSED, pending-state=(GstState)GST_STATE_VOID_PENDING; Got message #17 from element "tee0" (state-changed): GstMessageStateChanged, old-state=(GstState)GST_STATE_READY, new-state=(GstState)GST_STATE_PAUSED, pending-state=(GstState)GST_STATE_VOID_PENDING; Got message #20 from pad "videotestsrc0:src" (stream-status): GstMessageStreamStatus, type=(GstStreamStatusType)GST_STREAM_STATUS_TYPE_CREATE, owner=(GstElement)"\(GstVideoTestSrc\)\ videotestsrc0", object=(GstTask)"\(GstTask\)\ videotestsrc0:src"; Got message #21 from element "videotestsrc0" (state-changed): GstMessageStateChanged, old-state=(GstState)GST_STATE_READY, new-state=(GstState)GST_STATE_PAUSED, pending-state=(GstState)GST_STATE_VOID_PENDING; Got message #22 from pad "queue0:src" (stream-status): /GstPipeline:pipeline0/GstVideoTestSrc:videotestsrc0.GstPad:src: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)320\,\ height\=\(int\)240\,\ framerate\=\(fraction\)30/1\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ interlace-mode\=\(string\)progressive" /GstPipeline:pipeline0/GstTee:tee0.GstTeePad:src_0: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)320\,\ height\=\(int\)240\,\ framerate\=\(fraction\)30/1\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ interlace-mode\=\(string\)progressive" GstMessageStreamStatus, type=(GstStreamStatusType)GST_STREAM_STATUS_TYPE_ENTER, owner=(GstElement)"\(GstQueue\)\ queue0", object=(GstTask)"\(GstTask\)\ queue0:src"; /GstPipeline:pipeline0/GstQueue:queue0.GstPad:src: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)320\,\ height\=\(int\)240\,\ framerate\=\(fraction\)30/1\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ interlace-mode\=\(string\)progressive" /GstPipeline:pipeline0/GstQueue:queue0.GstPad:src: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)320\,\ height\=\(int\)240\,\ framerate\=\(fraction\)30/1\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ interlace-mode\=\(string\)progressive" /GstPipeline:pipeline0/GstKMSSink:kmssink0.GstPad:sink: caps = "video/x-raw\,\ format\=\(string\)I420\,\ width\=\(int\)320\,\ height\=\(int\)240\,\ framerate\=\(fraction\)30/1\,\ pixel-aspect-ratio\=\(fraction\)1/1\,\ interlace-mode\=\(string\)progressive" 0:00:00.306441334 606 0xb3f8f0 INFO kmssink gstkmssink.c:282:gst_kms_sink_find_mode_and_plane:<kmssink0> connector mode = 1280x800 0:00:00.306594334 606 0xb3f8f0 DEBUG kmssink gstkmssink.c:403:gst_kms_sink_show_frame:<kmssink0> crtc id = 18 / plane id = 20 / buffer id = 115283 0:00:00.306695000 606 0xb3f8f0 WARN kmssink gstkmssink.c:429:gst_kms_sink_show_frame:<kmssink0> error: drmModeSetPlane failed: No such file or directory (2) Got message #23 from pad "videotestsrc0:src" (stream-status): GstMessageStreamStatus, type=(GstStreamStatusType)GST_STREAM_STATUS_TYPE_ENTER, owner=(GstElement)"\(GstVideoTestSrc\)\ videotestsrc0", object=(GstTask)"\(GstTask\)\ videotestsrc0:src"; Got message #24 from element "pipeline0" (stream-start): GstMessageStreamStart, group-id=(uint)0; Got message #30 from element "kmssink0" (error): GstMessageError, gerror=(GError)NULL, debug=(string)"gstkmssink.c\(429\):\ gst_kms_sink_show_frame\ \(\):\ /GstPipeline:pipeline0/GstKMSSink:kmssink0:\012drmModeSetPlane\ failed:\ No\ such\ file\ or\ directory\ \(2\)"; ERROR: from element /GstPipeline:pipeline0/GstKMSSink:kmssink0: GStreamer encountered a general resource error. Additional debug info: gstkmssink.c(429): gst_kms_sink_show_frame (): /GstPipeline:pipeline0/GstKMSSink:kmssink0: drmModeSetPlane failed: No such file or directory (2) ERROR: pipeline doesn't want to preroll. Setting pipeline to NULL ... /GstPipeline:pipeline0/GstKMSSink:kmssink0.GstPad:sink: caps = "NULL" /GstPipeline:pipeline0/GstQueue:queue0.GstPad:src: caps = "NULL" /GstPipeline:pipeline0/GstQueue:queue0.GstPad:sink: caps = "NULL" /GstPipeline:pipeline0/GstTee:tee0.GstTeePad:src_0: caps = "NULL" /GstPipeline:pipeline0/GstTee:tee0.GstPad:sink: caps = "NULL" /GstPipeline:pipeline0/GstVideoTestSrc:videotestsrc0.GstPad:src: caps = "NULL" Freeing pipeline ...
(In reply to Tobias from comment #18) > Using kmssink on i.MX6 with Kernel 4.1.0 I encountered the problem, that it > won't work when there is a tee in the pipeline. Tee blocks allocation > queries and kmssink may not have a copy path yet, to workaround this. Tobias, looking the log it seems that you are not using the kmssink code posted in this bug, which is my latest code. Can you test it. Perhaps it won't work as expected, but at least (I hope) it won't end with that error.
In dmabuf-import mode this plugin unrefs the input buffer right after queueing it via SETPLANE. I suppose it should poll for a vblank event instead and only release the previous buffer once the current buffer is actually presented? With a fast DMABUF source I can currently see it rendering into the current scanout buffer because it got the buffer handed back too early.
Created attachment 321241 [details] [review] kmssink: add plugin and sink element This is simple video sink that use libdrm/libkms API to render frames. The element uses planes to render through drmModeSetPlane(). It has been tested in an Exynos4412 board and in a Freescale I.MX6 board.
Created attachment 321242 [details] [review] kmssink: calculate display ratio Get the aspect ratio given the information provided by libdrm, and with it calculate the display ratio.
Created attachment 321243 [details] [review] kmssink: add dmabuf support This patch will enable the import of dmabufs into a KMS buffer using the PRIME kernel interface. It has been tested with a Freescale I.MX6 board.
V3: According to the comment #17 this iteration removes the need of "io-mode". Now the sink detects if the incoming buffer is a dmabuf one, and adapts the acquired buffer. Also, if dmabuf import fails, the buffer restores its bo. 1. The kmssink sets the allocator to the bufferpool 2. The buffer pool doesn't need to know the drm fd anymore 3. Use of G_ADD_PRIVATE in G_DEFINE_TYPE_WITH_CODE to setup the private structures 4. Remove the io-mode enum and property 5. The allocator does a dup of the drm fd
(In reply to Philipp Zabel from comment #20) > In dmabuf-import mode this plugin unrefs the input buffer right after > queueing it via SETPLANE. I suppose it should poll for a vblank event > instead and only release the previous buffer once the current buffer is > actually presented? With a fast DMABUF source I can currently see it > rendering into the current scanout buffer because it got the buffer handed > back too early. Thanks. Do you think a drmWaitVBlank after SetPlane is enough?
That alone is not enough. I think gst_kms_sink_show_frame() needs to keep a reference to the input GstBuffer * buf until after the next flip completes. We not only have to avoid to release the input buffer while it is pending for a flip, but also as long as it is scanned out.
Created attachment 321759 [details] [review] kmssink: add plugin and sink element This is simple video sink that use libdrm/libkms API to render frames. The element uses planes to render through drmModeSetPlane(). It has been tested in an Exynos4412 board and in a Freescale I.MX6 board.
Created attachment 321760 [details] [review] kmssink: calculate display ratio Get the aspect ratio given the information provided by libdrm, and with it calculate the display ratio.
Created attachment 321761 [details] [review] kmssink: add dmabuf support This patch will enable the import of dmabufs into a KMS buffer using the PRIME kernel interface. It has been tested with a Freescale I.MX6 board.
Created attachment 321762 [details] [review] kmssink: log driver information
Created attachment 321763 [details] [review] kmssink: query DRM driver capabilites If the driver does not support dumb buffers, the sink stops. If the driver does not support prime import, the method is skipped.
Created attachment 321764 [details] [review] kmssink: wait for page flip This patch request for drmModePageFlip() for the used CRTC, and handles a GstPoll to wait for the page flip event.
When not using dmabuf, I see kmssink add and alternate between two drm framebuffers: gst_kms_sink_show_frame:<kmssink0> displaying fb 62 gst_kms_sink_show_frame:<kmssink0> displaying fb 63 gst_kms_sink_show_frame:<kmssink0> displaying fb 62 gst_kms_sink_show_frame:<kmssink0> displaying fb 63 ... But when using dmabuf, the first fb gets removed by gst_allocator_memory_reset() called from gst_kms_allocator_dmabuf_import() and it keeps displaying the same fb, as the gst_buffer_pool_acquire_buffer() call in gst_kms_sink_get_input_buffer() keeps returning the same destination buffer: gst_kms_allocator_dmabuf_import:<KMSMemory::allocator> resetting bo-based buffer gst_kms_sink_import_dmabuf:<kmssink0> imported prime fd 43 gst_kms_sink_show_frame:<kmssink0> displaying fb 62 gst_kms_sink_import_dmabuf:<kmssink0> imported prime fd 45 gst_kms_sink_show_frame:<kmssink0> displaying fb 62 gst_kms_sink_import_dmabuf:<kmssink0> imported prime fd 47 gst_kms_sink_show_frame:<kmssink0> displaying fb 62 gst_kms_sink_import_dmabuf:<kmssink0> imported prime fd 43 ...
(In reply to Philipp Zabel from comment #33) > When not using dmabuf, I see kmssink add and alternate between two drm > framebuffers: > > gst_kms_sink_show_frame:<kmssink0> displaying fb 62 > gst_kms_sink_show_frame:<kmssink0> displaying fb 63 > gst_kms_sink_show_frame:<kmssink0> displaying fb 62 > gst_kms_sink_show_frame:<kmssink0> displaying fb 63 > ... > > But when using dmabuf, the first fb gets removed by > gst_allocator_memory_reset() called from gst_kms_allocator_dmabuf_import() > and it keeps displaying the same fb, as the gst_buffer_pool_acquire_buffer() > call in gst_kms_sink_get_input_buffer() keeps returning the same destination > buffer: > > gst_kms_allocator_dmabuf_import:<KMSMemory::allocator> resetting bo-based > buffer > gst_kms_sink_import_dmabuf:<kmssink0> imported prime fd 43 > gst_kms_sink_show_frame:<kmssink0> displaying fb 62 > gst_kms_sink_import_dmabuf:<kmssink0> imported prime fd 45 > gst_kms_sink_show_frame:<kmssink0> displaying fb 62 > gst_kms_sink_import_dmabuf:<kmssink0> imported prime fd 47 > gst_kms_sink_show_frame:<kmssink0> displaying fb 62 > gst_kms_sink_import_dmabuf:<kmssink0> imported prime fd 43 > ... Yes. But, sorry, what's problem there?
The same fb is pushed every time, causing the same physical address to be flipped to in the imx-drm driver.
(In reply to Philipp Zabel from comment #35) > The same fb is pushed every time, causing the same physical address to be > flipped to in the imx-drm driver. For sure I'm missing something, but on the imx.6 where I'm running the test, the same fb id is rendered all the time (from an imported prime fd) and the video plays normally. That's why I can't understand. Is the same frame shown all the time?
Strange. Here the same physical address is shown all the time. I am using v4l2videodec as a dmabuf source. It uses three buffers in my case, so I see about every third buf buffer on the screen, albeit without synchronisation.
Philipp, could you try increasing the min buffers in the buffer pool??? in gst_kms_sink_create_pool(): + gst_buffer_pool_config_set_params (config, caps, size, 0, 0); - gst_buffer_pool_config_set_params (config, caps, size, 2, 0); Or three. Perhaps we should get the minimal buffers of the offered buffer pool and set them in our buffer pool.
Increasing to 2 causes it to stutter, alternating between the first two buffers. After increasing min buffers to 3, I get smooth playback with the correct frames. So that works, but I suspect that even when getting the minimal buffers from the offered pool, things will still break when the source decides to change the order or adds additional buffers due to latency.
Created attachment 321941 [details] [review] enable Y42B support
Created attachment 321942 [details] [review] enable NV16 support
Created attachment 321944 [details] [review] enable UYVY, YUY2, and YVYU support
(In reply to Philipp Zabel from comment #39) > Increasing to 2 causes it to stutter, alternating between the first two > buffers. After increasing min buffers to 3, I get smooth playback with the > correct frames. > > So that works, but I suspect that even when getting the minimal buffers from > the offered pool, things will still break when the source decides to change > the order or adds additional buffers due to latency. (I don't know the terms in kms, sorry, just giving the theory here) When you import a DMABuf, you have to make sure to keep the upstream DMABuf alive until it is no longer being used as a scannout. This requires a minimum amount of buffers. When correctly implemented, 2 "importation buffers" are enough, but might be slow due to cache being badly used. In an ideal situation, you need 1 "importation buffer" per DMABuf, so you use the same envelope for the same DMABuf. This is probably what you get "by accident" when setting it to 3. This should not rely on bufferpool order or anything. In GL space, it's an EGLImage that represent the "importation buffer". What we do, is that we cache which ELGImage was used for which upstream DMABuf. This solves the cache issue, we can then bind the EGLImage to a texture to obtain a rendering.
Created attachment 323486 [details] [review] kmssink: add plugin and sink element This is simple video sink that use libdrm/libkms API to render frames. The element uses planes to render through drmModeSetPlane(). It has been tested in an Exynos4412 board and in a Freescale I.MX6 board.
Created attachment 323487 [details] [review] kmssink: calculate display ratio Get the aspect ratio given the information provided by libdrm, and with it calculate the display ratio.
Created attachment 323488 [details] [review] kmssink: add dmabuf support This patch will enable the import of dmabufs into a KMS buffer using the PRIME kernel interface. If the driver does not support prime import, the method is skipped. It has been tested with a Freescale I.MX6 board.
Created attachment 323489 [details] [review] kmssink: wait for page flip or vblank This patch requests for drmModePageFlip() for the used CRTC, if the kernel module suppports async page flip. If it does not, the element requests for a vblank event. A GstPoll waits for the event to happen.
Created attachment 323490 [details] [review] kmssink: keep last rendered buffer in memory
Created attachment 323491 [details] [review] kmssink: enable Y42B (planar YUV 4:2:2) format
Created attachment 323492 [details] [review] kmssink: enable NV16 (chroma-interleaved YUV 4:2:2) format
Created attachment 323493 [details] [review] kmssink: enable UYVY, YUY2, and YVYU (interleaved YUV 4:2:2) formats
This is another boost to kmssink. The dmabuf handling was rewritten completely following Nicolas recommendations. I'm still not sure about the pageflip handling because of a possible overwrite of the CRTC buffer (should I use there -1 as fb id?) Also, I haven't tested this code in master, since I have to use 1.4.5 for my tests for dmabuf (yes, I backed port the parent buffer meta). There are more improvements, bug and mem leak fixes. Philipp, it would be lovely if you test this code in your setup and tell how it goes. Thanks!
Thanks! From a quick look the buffer flipping seems to work well now. One thing I noticed with a 1920x1200 monitor that very incorrectly reports its physical size as 160mm x 90mm, is that the drmModeSetPlane fails with -ENOSPC because it tries do scale a 320x264 region of the 320x240 input buffer up to 1920x1200 (which imx-drm doesn't support anyway).
Hello, tested flipping with gstreamer 1.6.3 and linux 4.1 using Phillips imx-scaler driver, works. One question: would is make sense to split the vsync code, so that - request vblank after every buffer - wait for vblank beginning with the second buffer this should shorten the blocking time - or do I miss somthing Thanks for your work Markus
Created attachment 325163 [details] [review] kmssink: add plugin and sink element This is simple video sink that use libdrm/libkms API to render frames. The element uses planes to render through drmModeSetPlane(). It has been tested in an Exynos4412 board and in a Freescale I.MX6 board.
Created attachment 325164 [details] [review] kmssink: calculate display ratio Get the aspect ratio given the information provided by libdrm, and with it calculate the display ratio.
Created attachment 325165 [details] [review] kmssink: add dmabuf support This patch will enable the import of dmabufs into a KMS buffer using the PRIME kernel interface. If the driver does not support prime import, the method is skipped. It has been tested with a Freescale I.MX6 board.
Created attachment 325166 [details] [review] kmssink: wait for page flip or vblank This patch requests for drmModePageFlip() for the used CRTC, if the kernel module suppports async page flip. If it does not, the element requests for a vblank event. A GstPoll waits for the event to happen.
Created attachment 325167 [details] [review] kmssink: keep last rendered buffer in memory
Created attachment 325168 [details] [review] kmssink: enable Y42B (planar YUV 4:2:2) format
Created attachment 325169 [details] [review] kmssink: enable NV16 (chroma-interleaved YUV 4:2:2) format
Created attachment 325170 [details] [review] kmssink: enable UYVY, YUY2, and YVYU (interleaved YUV 4:2:2) formats
This new round just fixes a bug for unref the allocator at stop() (In reply to Markus Niebel from comment #54) > One question: would is make sense to split the vsync code, so that > > - request vblank after every buffer > - wait for vblank beginning with the second buffer > > this should shorten the blocking time - or do I miss somthing Yes, it makes sense, but I would left that optimization after the merge in upstream :) Nicolas, Sebastian, Tim: I can haz perms to push this?
(In reply to Víctor Manuel Jáquez Leal from comment #63) > This new round just fixes a bug for unref the allocator at stop() > > (In reply to Markus Niebel from comment #54) > > One question: would is make sense to split the vsync code, so that > > > > - request vblank after every buffer > > - wait for vblank beginning with the second buffer > > > > this should shorten the blocking time - or do I miss somthing > > Yes, it makes sense, but I would left that optimization after the merge in > upstream :) > > > Nicolas, Sebastian, Tim: I can haz perms to push this? Please go ahead. My initial comments were addressed and replied already. This patchset is clean and development will be made easier upstream. As we just started 1.9, this feels like the perfect timing.
Attachment 325163 [details] pushed as 620e1d2 - kmssink: add plugin and sink element Attachment 325164 [details] pushed as 1aee6cd - kmssink: calculate display ratio Attachment 325165 [details] pushed as c419d17 - kmssink: add dmabuf support Attachment 325166 [details] pushed as b29f7d0 - kmssink: wait for page flip or vblank Attachment 325167 [details] pushed as 7d06cf3 - kmssink: keep last rendered buffer in memory Attachment 325168 [details] pushed as bdb62b2 - kmssink: enable Y42B (planar YUV 4:2:2) format Attachment 325169 [details] pushed as 360e934 - kmssink: enable NV16 (chroma-interleaved YUV 4:2:2) format Attachment 325170 [details] pushed as 2f51985 - kmssink: enable UYVY, YUY2, and YVYU (interleaved YUV 4:2:2) formats