wayland: enable screensaver inhibition in GNOME

[jf2048]

Eventually it would be good if GNOME Shell was updated to use the idle-inhibit protocol. But currently, support for it is still unfinished, and there are distros like Ubuntu Jammy LTS that ship with GNOME 42 and will keep that for the next 5 years. Thus those LTS distros will likely never get support for the protocol if it even lands in GNOME 43 or later. In order to get support for screensaver inhibition on those distros, this patch is able to inhibit the screensaver from within a Snap or Flatpak using the Portal API. See some similar code in WebKit as a point of comparison: SleepDisablerGLib.cpp

My use case is for Flatpak and I have tested it works there, but it should also work with the mpv Snap. It will also work outside a sandbox as well, by falling back to org.freedesktop.ScreenSaver.

[CounterPillow]

I generally don't like polluting entire functions with preprocessor'd out code just because a stable distribution might be using GNOME. I didn't look at the actual implementation too closely for now, but those seem like things people would generally not want for the maintainability of the code. But I am just a random scrub so someone else might have a different take.

It also seems like you didn't even compile test this code without GIO, which is not a good first impression to make when submitting this to a project that explicitly shits on "the GN*ME platform" at every opportunity.

[jf2048]

Thank you for your review, this PR is just one example of how this could be done without having to hack Lua scripts, it may not be the best way.

It also seems like you didn't even compile test this code without GIO, which is not a good first impression to make when submitting this to a project that explicitly shits on "the GN*ME platform" at every opportunity.

I changed some code then forgot to test it, please let us avoid the personal comments and stick to only the technical. With this PR I am only interested to improve any outstanding issues here with the GNOME platform. I figure if this causes any issues it is not a hard patch to revert since everything is guarded by preprocessor blocks, that should actually improve the maintainability and make it easy to avoid this code if need be.

[Dudemanguy]

I can't say I like this very much. Do you know what happens if dbus isn't running and this code gets executed?

[jf2048]

I am not particularly happy about it either, but this is what Ubuntu has decided to do. At least, it can be said that it is better than the code in VLC that retains support for even older D-Bus interfaces: dbus.c

Without D-Bus, the proxy creation will return NULL and it will fall back to showing the warning. This can be tested by executing it with no session bus using DBUS_SESSION_BUS_ADDRESS= mpv, or by running it in another compositor like Weston that does not support idle-inhibit or any of the D-Bus interfaces.

[Dudemanguy]

I gave this one some thought, and ultimately, I don't think such a change is acceptable for mpv no matter how we try to rework/refactor it, so I will close this now. Apologies. Essentially, this is just one big giant workaround/hack for one bad environment. We do have some "workarounds for bad environments" in mpv already, but nothing this elaborate. It'd be one thing if the change actually introduced functionality we want in mpv, but this is not that. There is no new wayland functionality here. It's a dbus workaround for a deficiency in one particular desktop environment. The only reason someone would even consider it is because that one DE happens to be popular (i.e. GNOME). If it was some no-name compositor that didn't support idle inhibit, nobody would even care.

With that, I certainly don't think this belongs in wayland_common code or even hypothetically separately in its own file because realistically this is code we'd all want to delete ASAP (if it were to be merged in the first place). If you want to integrate this API with mpv, you can write your own lua script or cplugin to do it (e.g. there is a C plugin for MPRIS). I know this isn't the nicest thing for old LTS distros, but this is the decision they made by enabling wayland by default.

[jf2048]

It'd be one thing if the change actually introduced functionality we want in mpv, but this is not that. There is no new wayland functionality here. It's a dbus workaround for a deficiency in one particular desktop environment.

I am not sure I really understand what you mean here or how to respond to this, just from looking at this code I see mpv still retains support for some very old stuff like D3D9, XV, older MacOS versions, etc. Those are not adding new functionality either, they are just there to support other environments that are also deficient yet popular. But, if Ubuntu is less priority than all of that, then I can understand that.

I do not have much interest to turn this into a plugin, but I figure the patch is here for anyone who comes across it in a github search and wants to do that.

[Dudemanguy]

Well no point in belaboring this, but I don't agree with the assertion that some older versions of things are inherently deficient.

[jf2048]

I guess I was not sure what you meant deficient. Some would say that D3D9 is deficient compared to 10 or 11, etc.