Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wip] Add dmabuf feedback #1278

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Consolatis
Copy link
Member

@Consolatis Consolatis commented Dec 4, 2023

Fixes

This turned out to be a bit more complex because the wlr_renderer_init_wl_display() call we used to do does not store or return the wlr_linux_dmabuf_v1 pointer anywhere (which we need to supply to wlr_scene_set_linux_dmabuf_v1()).

So instead of using wlr_renderer_init_wl_display() we just do it manually. This is also what the the sway scene graph patches do. We could also just use both but that causes duplicated globals warnings in wlroots which might cause further hard to detect / debug issues.

As for how to actually test this.. I have no idea.

Todo:

  • remove logging
  • find a way to actually test this

@yuiiio
Copy link

yuiiio commented Dec 24, 2023

can test this with weston-simple-dmabuf-feedback.
and output is same as sway, so works fine.
(weston-simple-dmabuf-feedback needs revert this commit 34400d7d1686d53ceaf79ff1142349a1a171db68 cause assertion error, even with sway.
created sway issue swaywm/sway#7879 maybe bug in impl xdg-shell and wlroots or labwc has same logic)
but, not related this MR :)

@jp7677 jp7677 mentioned this pull request Feb 3, 2024
@jp7677
Copy link
Contributor

jp7677 commented Feb 29, 2024

Accidentally I stumbled upon the dmabuf-wayland video output in mpv, usable by e.g. mpv -v --no-config --hwdec=vaapi --vo=dmabuf-wayland <video-file> (or without hwdec).
This output mode bails out without this PR. but works nicely with this PR applied. The mpv output looks normal as far as I can judge. Also the labwc verbose output shows no failing statements from this PR.
So I would also lean towards "This PR works as intended".

@ahesford
Copy link
Member

The dmabuf-wayland output has always worked for me. (It mangles HDR colors, but that's expected because it does no color management.)

@jp7677
Copy link
Contributor

jp7677 commented Feb 29, 2024

ah yeah sorry. I though to be clever to just set the additions from this PR into comments for testing, but missed the fact that I had to also revert wlr_renderer_init_wl_shm back into wlr_renderer_init_wl_display. This resulted in mpv saying [vo/dmabuf-wayland/wayland] Compositor doesn't support the zwp_linux_dmabuf_v1 (ver. 4) protocol! which tricked me, but clearly my mistake. Next time it is git stash again...

Sorry for the noise.

@Consolatis
Copy link
Member Author

Consolatis commented Feb 29, 2024

I remember reading an anecdote about dmabuf feedback causing an overflow of the wayland connection buffer and thus wlroots killing the client. AFAIR there were talks about wlroots potentially sending too many tranches but I didn't follow up on that. That is another reason why I am a bit skeptical about this PR (I don't see it improving anything other than being able to say "look, we support a new feature" but it might actually cause issues that weren't there before).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants