[vlc-devel] [PATCH 05/17] vulkan: move placebo usage to display.c

[Alexandre Janniaux]

Hi,

On Sat, Apr 17, 2021 at 01:36:52PM +0200, Niklas Haas wrote:
> On Mon, 12 Apr 2021 15:32:21 +0200 Alexandre Janniaux <ajanni at videolabs.io> wrote:
> > The vulkan providers are not supposed to depends upon libplacebo, and
> > only the display is supposed to bring the libplacebo dependency. In
> > addition, many of the libplacebo options are designed for the display.
>
> Something to keep in mind:
>
> Upstream libplacebo has had native support for OpenGL for quite a while
> now, and is currently gaining native D3D11 support. My long-term goal
> for this module was to remove the vulkan dependency completely and just
> make it ingest a `pl_gpu` and a `pl_swapchain`, with platform-specific
> modules providing that capability.
>
> This way, the libplacebo-based renderer can also be used on platforms
> without native vulkan drivers. This commit is going a bit in the
> opposite direction of that, by making `display.c` even more dependent on
> vulkan specifics.

Hi,

I'm not sure I understand. If you move everything into libplacebo,
then it means that modules implementing libplacebo need to link
potentially multiple API (EGL, OpenGL, vulkan, etc) and then even
if the module doesn't expose it, it's actually more dependant on
vulkan and other libraries than with this change.

To be clearer, I'd rather avoid passing placebo objects through a
public interface named vulkan. In addition, to be able to setup
the changes requested in previous review for the Wayland platform,
I would have to link libplacebo to different modules, just to
provide vulkan functionality.

Though, I agree that we could provide some better abstraction for
vulkan support, which would allow to move those variables into a
specific module bringing the VkInstance and base functions, but
I don't think they were at the correct location previously either
in addition of being wrongly prefixed. I am in favour of a more
modular integration of platform-specific code too, but in that way
I'd rather have libplacebo able to use code through abstractions
provided by libvlccore rather than moving all the code into
libplacebo and be hand-tied to libplacebo up to the API exposed
to modules. In the long term, it doesn't prevent a libplacebo
implementation for providing Vulkan support.

If you want to write placebo-specific code, you should probably
write placebo-specific modules (ie. not named «vulkan») to convey
those abstraction, and then you´ll probably have variables like
--placebo-debug instead of --vk-debug so those variables will
disappear. But now that they're here, I can't just remove them
without breaking cmdline and they're mainly used by libplacebo
vulkan objects in display.c anyway. That's not incompatible with
writing this abstraction to move them out of the display, but I
think having libplacebo in one side and the platform specific in
the other side is more preferable than having libplacebo in both
side plus the platform specific on the second side, since in the
first you don't need to link placebo twice, you don't need to
link the rendering API to the libplacebo module and all the
support for rendering APIs is in the same plugin, without
difference between platforms.

Regards,
--
Alexandre Janniaux
Videolabs