GNOME Bugzilla – Bug 794817
msdk: Add Dmabuf Import support in encoder and vpp elements
Last modified: 2018-05-30 23:35:14 UTC
The dmabuf-export patches are already here #793707 We need to add dmabuf-import support in msdk plugins, mainly to get the benefit of zero copy in v4l2src + msdk encode pipeline. See the comments: https://bugzilla.gnome.org/show_bug.cgi?id=793707#c17 https://bugzilla.gnome.org/show_bug.cgi?id=793707#c23 https://bugzilla.gnome.org/show_bug.cgi?id=793707#c24 https://bugzilla.gnome.org/show_bug.cgi?id=793707#c25
The main issue is that Msdk requires all FDs(+ underlined VASurfaces) which the pipeline is going to use, prior to invoking the Initialize routine (MSDKInit).
Nicolas, I would like to know more about the idea you mentioned in: https://bugzilla.gnome.org/show_bug.cgi?id=793707#c18 " Question, if upstream was to "commit" it's decided allocation, would that help ? In recent research, we found that delaying allocation to when first buffer comes increases startup latency (specially with CMA), so it's something we are considering, optional of course, for backward compatibility. To help understand, this would mimic the caps query followed by caps event model. " Do you mean we can get the pool from v4l2 in the downstream element? Wondering how we can get the list of FDs allocated by v4l2 in msdk element.
Our experiments and research continues. So far, it would seem that the pool would be "notified" after being activated, because activating a pool may fail in cases where a fallback is possible. On our side, we don't have this extreme requirement, we just need to know how many buffers, so that we can optimize and make sure buffer mapping is constant. We still tolerate more buffers being added. V4L2 with UVC driver as an example have this CREATE_BUFS feature, that can be triggered to increase the buffer pool size if it's needed (instead of blocking on the pool).
So upstream send a "notification", probably an event with pool information; then downstream can acqure_buffer/get_fd/release_buffer on all the pre-allocated buffers. right? Do you have any code which I can peek into (for the notification experiments)?
Yes, that's mostly it, as you can get the number of allocated buffers, you can get them all out using acquire, though I think we should make it safe and use GST_BUFFER_POOL_ACQUIRE_FLAG_DONTWAIT. Adding Guillaume to the loop, as he is likely to provide an code. We are stuck with gst-omx, and need to fix the memory model before we can do any of this safely.
Our use case in gst-omx is to ensure that an input port has as many buffers allocated that the output of its upstream component (to prevent buffers starvation). Here is what I have so far: https://gitlab.collabora.com/cassidy/gst-omx/commit/58bf10b30c39bff0e0f8d1e59d086df459e8ba18 Basically I use a custom downstream event that an element can send once its buffers have been allocated. This event contains the pool (so you can fetch all the allocated fds for your use case) and the number of allocated buffers (to cover cases where upstream isn't using a pool and we are just interested in the number of buffers and not their content). It's really a WIP experimentation patch so far. Nothing is set in the stone and I'm not even sure that's the proper way to fix my use case. I hope to find some time to resume this work soon. Would this kind of event work for you as well?
(In reply to Guillaume Desmottes from comment #6) > Our use case in gst-omx is to ensure that an input port has as many buffers > allocated that the output of its upstream component (to prevent buffers > starvation). > > Here is what I have so far: > https://gitlab.collabora.com/cassidy/gst-omx/commit/ > 58bf10b30c39bff0e0f8d1e59d086df459e8ba18 > > Basically I use a custom downstream event that an element can send once its > buffers have been allocated. This event contains the pool (so you can fetch > all the allocated fds for your use case) and the number of allocated buffers > (to cover cases where upstream isn't using a pool and we are just interested > in the number of buffers and not their content). > > It's really a WIP experimentation patch so far. Nothing is set in the stone > and I'm not even sure that's the proper way to fix my use case. I hope to > find some time to resume this work soon. > > Would this kind of event work for you as well? I believe it should work with msdk use case too.
I opened bug #795747 about adding this new event to the official allocation API.
sreerenj: see my comment on bug #795747 : could you please try using the proposed API to check if it fits your needs as well?
(In reply to Guillaume Desmottes from comment #9) > sreerenj: see my comment on bug #795747 : could you please try using the > proposed API to check if it fits your needs as well? Sure I'll, but no time this week!
(In reply to Guillaume Desmottes from comment #9) > sreerenj: see my comment on bug #795747 : could you please try using the > proposed API to check if it fits your needs as well? It would be nice if you/someone can add the support in v4l2src so that it will easy to test with other components :)
(In reply to sreerenj from comment #11) > (In reply to Guillaume Desmottes from comment #9) > > sreerenj: see my comment on bug #795747 : could you please try using the > > proposed API to check if it fits your needs as well? > > > It would be nice if you/someone can add the support in v4l2src so that it > will easy to test with other components :) Sure, I attached it to bug #795747
Patches to enable dmabuf-import in gst-msdk vpp element is here: https://github.com/sreerenjb/gst-plugins-bad/commits/msdkvpp-dmabuf-import-dynamic-memid I'm currently patching the media-driver to make it work with v4l2src. :(
Nicolas, Does the v4l2src respect the size requirement from downstream in dmabuf export mode? Based on my testing it is not and I am wondering whether it is because of any specific constraints? https://cgit.freedesktop.org/gstreamer/gst-plugins-good/tree/sys/v4l2/gstv4l2object.c#n4254
Just to recap our IRC discussion, V4L2 Kernel API does not any API that really match this. There is ways that user space could tell it's preferred stride, with guaranty it will be used. UVC driver does not implement that (even though it's doable). Downstream, in every cases, should be ready to copy (fallback path) if the incoming alignment or stride does not match it's own requirement.
(In reply to Nicolas Dufresne (ndufresne) from comment #15) > Just to recap our IRC discussion, V4L2 Kernel API does not any API that > really match this. There is ways that user space could tell it's preferred > stride, with guaranty it will be used. UVC driver does not implement that > (even though it's doable). Downstream, in every cases, should be ready to > copy (fallback path) if the incoming alignment or stride does not match it's > own requirement. Thank you for the details, it is unfortunate that we can't do the zero-copy here. But interestingly it is possible with v4l2+gstreamer-vaapi+intel-vaapi-driver where gstreamer-vaapi + intel-vaapi-driver is happy to take whatever upstream is providing. This is not the case with the new media-driver.: Here is the bug: https://github.com/intel/media-driver/issues/169 What I can think of is a work-around: 1: By default msdk elements check for the padding/size restrictions of incoming buffers from v4l2src and do the copy. The code is already there, needs to add some sanity check. 2: We can still make media-driver work with this by fooling it, just tell it that the size of memory is large enough :( This works in basic test cases, but I don't know when it is going to fail. So maybe put this path under a property which requires explicit enablement if the user really wants to experiment with it. I believe it worth a shot since gstreamer-vaapi+ intel-vaapi-driver works like this for a while by without checking the padding restrictions.
Created attachment 372467 [details] [review] msdk: Add method to export dmabuf to VASurface
Created attachment 372468 [details] [review] msdk: Add method to replace interal VASurface of mfxFrameSurface
Created attachment 372469 [details] [review] msdk: vpp: Add supprot for dmabuf-import
Created attachment 372470 [details] [review] msdk: enc: Add support for dmabuf-import
Review of attachment 372467 [details] [review]: pushed as commit: 62d2d8ebf91ae7f8554e67a15a1cc8a22f549a3c
Review of attachment 372468 [details] [review]: pushed as commit: a7b7939dd7eae38009ff194c403d4b9a5ca30a44
Review of attachment 372469 [details] [review]: pushed as commit: a972d76784261ccb0bc1197d216f5b1906a7f8f4
Review of attachment 372470 [details] [review]: pushed as commit: 57b987526068dc24874fcf99e0d572d268c79741