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 772838 - vaapi: clutter on wayland: GLTextureUpload negotiated but not available
vaapi: clutter on wayland: GLTextureUpload negotiated but not available
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer-vaapi
unspecified
Other Linux
: Normal normal
: 1.11.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 770025 (view as bug list)
Depends on:
Blocks: 748634
 
 
Reported: 2016-10-13 07:29 UTC by François Guerraz
Modified: 2017-03-06 18:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
debug log (64.17 KB, text/plain)
2016-10-14 08:19 UTC, François Guerraz
  Details
vainfo (1.96 KB, text/plain)
2016-10-14 08:19 UTC, François Guerraz
  Details
log with gst-debug=*:5 (2.16 MB, application/x-xz)
2016-10-14 09:31 UTC, François Guerraz
  Details
Force system memory caps feature instead of GstVideoGLTextureUploadMeta (3.76 KB, patch)
2016-10-18 04:52 UTC, Hyunjun Ko
needs-work Details | Review
vaapidecode: rename member to allowed_sinkpad_caps (3.72 KB, patch)
2016-10-25 16:18 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
vaapidecode: generate source pad caps (3.92 KB, patch)
2016-10-25 16:18 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
vaapidecode: GLTextureUpload if driver supports OpenGL (1.75 KB, patch)
2016-10-25 16:18 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
vaapipostproc: GLTextureUpload if driver supports OpenGL (1.94 KB, patch)
2016-10-25 16:18 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
vaapidecode: texture upload if driver supports GL (1.17 KB, patch)
2017-03-03 21:40 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
vaapipostproc: texture upload if driver supports GL (1.22 KB, patch)
2017-03-03 21:40 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
plugins: retry to create the VA display (2.06 KB, patch)
2017-03-03 21:40 UTC, Víctor Manuel Jáquez Leal
committed Details | Review

Description François Guerraz 2016-10-13 07:29:42 UTC
When in a Wayland session, with gstreamer-vaapi installed, seeking in a video file makes totem freeze.
Comment 1 Víctor Manuel Jáquez Leal 2016-10-14 07:57:50 UTC
Hi François,

In order to define what is happening wee need some more information:

What version of gstreamer-vaapi did you have installed?

Can you paste the output of the command vainfo?

Can you upload a debug log with --gst-debug=vaapi*:5 ??
Comment 2 François Guerraz 2016-10-14 08:17:51 UTC
$ yaourt -Ss gstreamer-vaapi
extra/gstreamer-vaapi 1.9.90+1+g9414815-1 [installed]
    GStreamer Multimedia Framework VAAPI Plugin
Comment 3 François Guerraz 2016-10-14 08:19:02 UTC
Created attachment 337690 [details]
debug log

Note that the last log entry is prior to the seek/freeze. Nothing happens in the log when I click on the seek bar
Comment 4 François Guerraz 2016-10-14 08:19:33 UTC
Created attachment 337691 [details]
vainfo
Comment 5 François Guerraz 2016-10-14 08:20:45 UTC
Also worth mentioning: I have a HiDPI screen with a scaling factor of 2.
Comment 6 Víctor Manuel Jáquez Leal 2016-10-14 09:18:28 UTC
Thanks for your promptly reply!

Can I ask also for a debug log (bigger one) with --gst-debug=*:5 ??

Because looking at the vaapi*:5 log, I don't see any cause for the blocking.
Comment 7 François Guerraz 2016-10-14 09:31:23 UTC
Created attachment 337694 [details]
log with gst-debug=*:5
Comment 8 Hyunjun Ko 2016-10-17 06:14:42 UTC
clutter_gst_source_dispatch is failed because gst-vaapi doesn't support meta_upload method in this case(VaapiDisplayWayland is created) , even though it was negotiated with GstVideoGLTextureUploadMeta.

This is not only about seek, but about playback.
Probably, this issue is same as #770025

See the attached log below.

0:00:00.834319493 m 7925      0x12a0920 INFO        cluttervideosink clutter-gst-video-sink.c:1936:clutter_gst_video_sink_parse_caps:<cluttergstvideosink0> sav                                    ing infos
0:00:00.834335781 m 7925      0x12a0920 DEBUG       cluttervideosink clutter-gst-video-sink.c:2040:clutter_gst_source_dispatch:<cluttergstvideosink0> Trying to                                     upload buffer 0x7f6a84262710 with GL using renderer RGB 32
0:00:00.834478614 m 7925      0x12a0920 mWARN        cluttervideosink clutter-gst-video-sink.c:1371:clutter_gst_rgb32_upload_gl:<cluttergstvideosink0> GL tex                                      ture upload failed
0:00:00.834524977 m 7925      0x12a0920 mWARN        cluttervideosink clutter-gst-video-sink.c:2085:clutter_gst_source_dispatch:<cluttergstvideosink0> Failed                                       to upload buffer
Comment 9 Hyunjun Ko 2016-10-18 04:52:18 UTC
Created attachment 337914 [details] [review]
Force system memory caps feature instead of  GstVideoGLTextureUploadMeta

In some cases, VaapiDisplay doesn't support GL texture upload, 
For example, GstVaapiDisplayWayland is created when working with 
clutterautovideosink on wayland, which doesn't support GL texture upload.

In this kind of scenario, force to use System Memory during negotiation.
Comment 10 Hyunjun Ko 2016-10-18 04:55:52 UTC
(In reply to Hyunjun Ko from comment #9)
> Created attachment 337914 [details] [review] [review]
> Force system memory caps feature instead of  GstVideoGLTextureUploadMeta
> 
> In some cases, VaapiDisplay doesn't support GL texture upload, 
> For example, GstVaapiDisplayWayland is created when working with 
> clutterautovideosink on wayland, which doesn't support GL texture upload.
> 
> In this kind of scenario, force to use System Memory during negotiation.

This is quick fix for this case, and makes it working with software rendering.
But still remains something to do for GL texture upload for performance.
Comment 11 François Guerraz 2016-10-18 05:56:04 UTC
Review of attachment 337914 [details] [review]:

Fixes the crash but appalling performance (a lot worse than software rendering). I recommend not pushing this patch as it would be preferable to fall back to software rendering.
Comment 12 Víctor Manuel Jáquez Leal 2016-10-20 15:10:04 UTC
(In reply to François Guerraz from comment #11)
> Review of attachment 337914 [details] [review] [review]:
> 
> Fixes the crash but appalling performance (a lot worse than software
> rendering).

Can you share any numbers to compare? CPU usage for example? If it is much worse I would agree.
Comment 13 François Guerraz 2016-10-21 05:13:03 UTC
Actually:

With vaapi the cpu usage is about 70% and only a few fps are displayed and the whole desktop slows down to a few FPS (mouse pointer is laggy too).
Without Vaapi cpu usage is 110% *but* it plays smoothly and there is no general UI lag.
Comment 14 Víctor Manuel Jáquez Leal 2016-10-21 07:47:06 UTC
(In reply to François Guerraz from comment #13)
> Actually:
> 
> With vaapi the cpu usage is about 70% and only a few fps are displayed and
> the whole desktop slows down to a few FPS (mouse pointer is laggy too).
> Without Vaapi cpu usage is 110% *but* it plays smoothly and there is no
> general UI lag.

Thanks! That's interesting.

Did you test it with the posted patch? What are the properties of the media? resolution? What's your hardware (besides it is a skylake)?
Comment 15 François Guerraz 2016-10-21 08:37:03 UTC
The machine has an i7-6560U (It's a Dell XPS 13 9350) and I tested the with the proposed patch indeed.
The media is :
Codec: H264 - MPEG-4 AVC (part 10) (avc1)
Resolution: 1920x1090
Frame rate: 29.970178
Decoded format: Planar 4:2:0 YUV
Comment 16 Hyunjun Ko 2016-10-25 07:03:56 UTC
(In reply to Hyunjun Ko from comment #10)
> (In reply to Hyunjun Ko from comment #9)

> This is quick fix for this case, and makes it working with software
> rendering.
> But still remains something to do for GL texture upload for performance.

I filed an issue for this.

https://bugzilla.gnome.org/show_bug.cgi?id=773453

Sounds good to use dmabuf, though I'm not sure how the progress of handling dmabuf on gst-vaapi is going on.
Comment 17 Víctor Manuel Jáquez Leal 2016-10-25 10:16:35 UTC
Well, it seems that the proper fix for this will be when dmabuf support for downstream is added, and that is going to happen after release 1.10.
Comment 18 François Guerraz 2016-10-25 11:20:21 UTC
Ok, would it be possible for gstreamer-vaapi to fail gracefully on Wayland so it's not used at all?
Comment 19 Víctor Manuel Jáquez Leal 2016-10-25 11:40:17 UTC
(In reply to François Guerraz from comment #18)
> Ok, would it be possible for gstreamer-vaapi to fail gracefully on Wayland
> so it's not used at all?

That's tricky, since you can use gstreamer-vaapi using vaapisink, and perhaps other sinks (glimagesink?).

The solution in the middle, afaik, is what Hyunjun is proposing (perhaps with another approach -changing the allowed caps in runtime-), since the system lag, if I understand correctly, is caused by cluttersink because is unable to process the frame rate. Though, I don't know if that is fixable in cluttersink.
Comment 20 Víctor Manuel Jáquez Leal 2016-10-25 16:18:11 UTC
Created attachment 338428 [details] [review]
vaapidecode: rename member to allowed_sinkpad_caps

vaapidecode has a member named allowed_caps, but this name is not enough
explicit. This patch renames allowed_caps to allowed_sinkpad_caps.

No functional changes were included.
Comment 21 Víctor Manuel Jáquez Leal 2016-10-25 16:18:18 UTC
Created attachment 338429 [details] [review]
vaapidecode: generate source pad caps

Just as vaapipostproc, VA decoder's context can be queried to get the possible
raw formats, so, the src caps can negotiate the exact caps that the context
supports.
Comment 22 Víctor Manuel Jáquez Leal 2016-10-25 16:18:25 UTC
Created attachment 338430 [details] [review]
vaapidecode: GLTextureUpload if driver supports OpenGL

When the allowed source pad caps are generated, the GLTextureUpload caps are
only inserted if the driver support OpenGL.
Comment 23 Víctor Manuel Jáquez Leal 2016-10-25 16:18:32 UTC
Created attachment 338431 [details] [review]
vaapipostproc: GLTextureUpload if driver supports OpenGL

Removes GstVideoGLTextureUploadMeta caps feature if the driver
doesn't support opengl.

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>
Comment 24 Víctor Manuel Jáquez Leal 2016-10-25 16:23:53 UTC
Still, it is necessary to have this guard: if the backend doesn't support GL, the GLTextureUpload caps feature should be negotiated.

I have reworked the original idea with patches that configure the allowed caps when negotiating.

Hyunjun, what do you thing?
Comment 25 Hyunjun Ko 2016-10-26 05:32:04 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #24)
> Still, it is necessary to have this guard: if the backend doesn't support
> GL, the GLTextureUpload caps feature should be negotiated.
> 
> I have reworked the original idea with patches that configure the allowed
> caps when negotiating.
> 
> Hyunjun, what do you thing?

I think it makes sense for current design.

But once this patch is adjusted, vaapi elements are never trying to find gl context after the caps feature is removed.

In case of the failure on bug #754680, 
I'm not sure if there might be solution with this patch.
Comment 26 Víctor Manuel Jáquez Leal 2016-10-27 12:16:08 UTC
(In reply to Hyunjun Ko from comment #25)
> (In reply to Víctor Manuel Jáquez Leal from comment #24)
> > Still, it is necessary to have this guard: if the backend doesn't support
> > GL, the GLTextureUpload caps feature should be negotiated.
> > 
> > I have reworked the original idea with patches that configure the allowed
> > caps when negotiating.
> > 
> > Hyunjun, what do you thing?
> 
> I think it makes sense for current design.
> 
> But once this patch is adjusted, vaapi elements are never trying to find gl
> context after the caps feature is removed.
> 
> In case of the failure on bug #754680, 
> I'm not sure if there might be solution with this patch.

That's a good observation.

By the way, do you have found a sound solution for this, or should we go for this workaround for 1.10? We have to have a "fix" tomorrow.
Comment 27 Hyunjun Ko 2016-10-28 01:52:44 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #26)
> By the way, do you have found a sound solution for this, or should we go for
> this workaround for 1.10? We have to have a "fix" tomorrow.

I think we should go with this workaround, because some works should be done on the side of cluttersink, as you know.
Comment 28 Víctor Manuel Jáquez Leal 2016-11-14 11:50:09 UTC
*** Bug 770025 has been marked as a duplicate of this bug. ***
Comment 29 Sjoerd Simons 2016-11-18 20:20:08 UTC
Seems the workaround didn't land in 1.10.x yet making those release unusable under wayland :/
Comment 30 Víctor Manuel Jáquez Leal 2016-11-19 10:39:09 UTC
(In reply to Sjoerd Simons from comment #29)
> Seems the workaround didn't land in 1.10.x yet making those release unusable
> under wayland :/

Hey Sjoerd, long time :)

Yeah, I know. The thing is with the workaround, the experience with some videos is terrible, sluggish. It is better, if you use cluttersink under wayland, not use gstreamer-vaapi.

Though, the real solution, dmabuf sharing, is a work in progress either in gstreamer-vaapi and cluttesink.
Comment 31 Sjoerd Simons 2016-11-21 08:18:01 UTC
Yeah i got some quite mixed results as well (plain clutter-gst seems to work a lot better then totem as well for some reason). I think bad performance is still better then crashing out though especially as it is actually a performance improvement for some cases. I've uploaded the last upstream release to Debian with these patches because of that (as i needed to use vaapi on wayland for some things)


Don't you need to more dynamic caps stuff even if the dma bits and pieces land ?
Comment 32 Víctor Manuel Jáquez Leal 2016-11-25 10:10:09 UTC
(In reply to Sjoerd Simons from comment #31)
> Yeah i got some quite mixed results as well (plain clutter-gst seems to work
> a lot better then totem as well for some reason). I think bad performance is
> still better then crashing out though especially as it is actually a
> performance improvement for some cases. I've uploaded the last upstream
> release to Debian with these patches because of that (as i needed to use
> vaapi on wayland for some things)

OK. I'm going to merge those patches to the branch 1.10

> Don't you need to more dynamic caps stuff even if the dma bits and pieces
> land ?

Yes indeed, the base patches are already merged, but this texture upload checking  is not how we want to do it, so far.
Comment 33 Víctor Manuel Jáquez Leal 2016-11-25 10:12:46 UTC
Comment on attachment 338429 [details] [review]
vaapidecode: generate source pad caps

in branch 1.11
Comment 34 Víctor Manuel Jáquez Leal 2016-11-25 12:19:58 UTC
Pushed on branch 1.10

* 8ef142b vaapipostproc: GLTextureUpload if driver supports OpenGL
* 3ccf7d6 vaapidecode: GLTextureUpload if driver supports OpenGL
* 2fe2bc5 vaapidecode: guard GST_VAAPI_MAKE_GLTEXUPLOAD_CAPS
* f26034c vaapidecode: generate source pad caps
Comment 35 Víctor Manuel Jáquez Leal 2016-11-25 12:20:19 UTC
Comment on attachment 338430 [details] [review]
vaapidecode: GLTextureUpload if driver supports OpenGL

only on branch 1.10
Comment 36 Víctor Manuel Jáquez Leal 2016-11-25 12:20:46 UTC
Comment on attachment 338431 [details] [review]
vaapipostproc: GLTextureUpload if driver supports OpenGL

committed only on branch 1.10
Comment 37 Víctor Manuel Jáquez Leal 2016-11-25 12:23:40 UTC
let's keep the issue open because it is not properly fixed in master until we have dmabuf support
Comment 38 Víctor Manuel Jáquez Leal 2017-03-03 21:15:09 UTC
I have noticed that since bug 777409 decoding video using gstreamer-vaapi under Wayland doesn't work anymore: the negotiated GL context (created or shared) is GLX (!!), and when creating a VAAPI display it looks for a DRI2 connection, as it doesn't exist, the creation of the display fails.
Comment 39 Víctor Manuel Jáquez Leal 2017-03-03 21:40:15 UTC
Created attachment 347170 [details] [review]
vaapidecode: texture upload if driver supports GL

When the allowed source pad caps are generated, the GLTextureUpload caps are
only inserted if the driver support OpenGL.
Comment 40 Víctor Manuel Jáquez Leal 2017-03-03 21:40:19 UTC
Created attachment 347171 [details] [review]
vaapipostproc: texture upload if driver supports GL

Removes GstVideoGLTextureUploadMeta caps feature if the driver
doesn't support opengl.

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>
Comment 41 Víctor Manuel Jáquez Leal 2017-03-03 21:40:23 UTC
Created attachment 347172 [details] [review]
plugins: retry to create the VA display

Particularly in GNOME Wayland, the negotiated or created GL context
defines a GLX environment, but VAAPI fails to create a GLX VA
display because there is no a DRI2 connection.

This patch retries to create the VA display if VA cannot create one
with the GL context parameters. Now using the old list of display
types.

This should also work in the case of systems with two GPU, when the
non-VAAPI has the graphics environment, and the VAAPI-enabled one
shall work headless.
Comment 42 Víctor Manuel Jáquez Leal 2017-03-06 18:54:17 UTC
Attachment 347170 [details] pushed as bce6e14 - vaapidecode: texture upload if driver supports GL
Attachment 347171 [details] pushed as 8d86b3f - vaapipostproc: texture upload if driver supports GL
Attachment 347172 [details] pushed as 3723d20 - plugins: retry to create the VA display
Comment 43 Víctor Manuel Jáquez Leal 2017-03-06 18:55:55 UTC
All committed. But still we need to define an (almost) zero copy path for Wayland based sinks. But that is a future enhancenment.