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 785006 - DirectViv texture upload method is broken
DirectViv texture upload method is broken
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 781652 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2017-07-17 02:38 UTC by Jan Schmidt
Modified: 2018-11-03 14:11 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Jan Schmidt 2017-07-17 02:38:01 UTC
If the DirectVIV texture upload method is available, it adds caps transformations to the glupload negotiated caps set, but might not be able to perform them when buffers arrive if the buffers don't contain GstMemory from a physical memory allocator.

That means that this use case no longer works:

gst-launch-1.0 videotestsrc ! glupload ! glimagesink

glupload will negotiate to transform I420 to RGBA, but fail to do it. Instead, the raw memory upload will output buffers that contain I420 GstVideoMeta, with RGBA caps and result in:

Got context from element 'sink': gst.gl.GLDisplay=context, gst.gl.GLDisplay=(GstGLDisplay)"\(GstGLDisplayX11\)\ gldisplayx11-0";

** (gst-launch-1.0:9095): CRITICAL **: gst_video_frame_map_id: assertion 'info->finfo->format == meta->format' failed
0:00:00.722662716  9095   0x8c1bb0 ERROR            glimagesink gstglimagesink.c:1610:prepare_next_buffer: Failed to map video frame.
ERROR: from element /GstPipeline:pipeline0/GstGLImageSinkBin:glimagesinkbin0/GstGLImageSink:sink: Failed to convert multiview video buffer
Additional debug info:
gstglimagesink.c(1720): gst_glimage_sink_prepare (): /GstPipeline:pipeline0/GstGLImageSinkBin:glimagesinkbin0/GstGLImageSink:sink
ERROR: pipeline doesn't want to preroll.
Setting pipeline to NULL ...
Freeing pipeline ...

The DirectVIV method really needs a Caps Feature on the received caps to confirm that the buffers it will be receiving are going to work.
Comment 1 Sebastian Dröge (slomo) 2017-07-17 07:05:15 UTC
A caps feature would also need gstreamer-imx to be updated accordingly. Did you try that and have a patch?
Comment 2 Sebastian Dröge (slomo) 2017-07-17 07:05:38 UTC
And a caps feature seems problematic here, as this still works like normal sysmem.
Comment 3 Jan Schmidt 2017-07-17 10:00:06 UTC
I think there's 2 solutions:

1) Add a CapsFeature that's conditionally negotiated, and glupload only supports DirectVIV when the caps feature is present, but upstream also (2nd preference) negotiates plain video/x-raw caps without it
2) Add a random field to all video/x-raw caps that marks the stream as 'is-physical-memory=true' - and hope that field is passed on appropriately by intermediate elements, and deleted if they copy the data to a non-physical buffer.


1) Seems slightly more complicated to implement, but more likely to not randomly fail.
Comment 4 Sebastian Dröge (slomo) 2017-07-17 11:56:03 UTC
2) seems like not a good idea. Let's go with 1)

Can we do 1) but let glupload accept DirectVIV without the caps feature, and it just outputs always caps with the caps feature?
Comment 5 Jan Schmidt 2017-07-17 12:00:17 UTC
No? The problem is that without an incoming caps feature, glupload doesn't actually know if it can produce 'RGBA' caps from 'I420' input until it receives a buffer, but by then it's already claimed that it can.
Comment 6 Sebastian Dröge (slomo) 2017-07-17 12:10:06 UTC
Oh right...
Comment 7 Nicolas Dufresne (ndufresne) 2017-07-17 13:32:21 UTC
We should delay the caps negotiation then, until we know how we are uploading the first buffer. This seems also needed for the direct dmabuf uploader here. I thought this was already supported.

It also means we need to handle caps renegotiation on uploader fallback now.
Comment 8 Jan Schmidt 2017-07-17 13:49:11 UTC
More generally, you're saying that glupload needs to be able to renegotiate on receiving a buffer - since one buffer supporting directviv/dmabuf doesn't imply the next does (upstream might change).

I don't think basetransform has ever supported that, has it? Caps transforms are always based on upstream/downstream caps, not on the contents of a current buffer?
Comment 9 Nicolas Dufresne (ndufresne) 2017-07-17 14:59:49 UTC
(In reply to Jan Schmidt from comment #8)
> More generally, you're saying that glupload needs to be able to renegotiate
> on receiving a buffer - since one buffer supporting directviv/dmabuf doesn't
> imply the next does (upstream might change).

Yes, though, if that changes without a new allocation query, we will only degrade the to a slow path, as this only happens in real-life when upstream starts copying to vmalloc memory. It happens on certain threshold when let's say no more CMA is allowed, so we start "preserving" the CMA by copying out to vmalloc memory.

> 
> I don't think basetransform has ever supported that, has it? Caps transforms
> are always based on upstream/downstream caps, not on the contents of a
> current buffer?

I believe gst_base_transform_update_src_caps(), since 1.6.
Comment 10 Matthew Waters (ystreet00) 2017-07-20 02:03:01 UTC
(In reply to Jan Schmidt from comment #8)
> More generally, you're saying that glupload needs to be able to renegotiate
> on receiving a buffer - since one buffer supporting directviv/dmabuf doesn't
> imply the next does (upstream might change).

There is already some allowance for this with the GST_GL_UPLOAD_RECONFIGURE return value that can be signalled from uploaders.
Comment 11 Matthew Waters (ystreet00) 2017-07-20 02:04:56 UTC
*** Bug 781652 has been marked as a duplicate of this bug. ***
Comment 12 GStreamer system administrator 2018-11-03 14:11:01 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/583.