[FFmpeg-devel] [PATCH] lavfi: add a libplacebo filter

[Niklas Haas]

On Tue, 09 Mar 2021 23:55:36 +0100 Lynne <dev at lynne.ee> wrote:
> It's simpler because it makes no effort to integrate with anything
> we use.

To be clear, I agree that integration with the hwcontext is the ultimate
goal. I'm just arguing about the usefulness of allowing
vf_libplacebo to exist as-is until then. The amount of extra code to
support a CPU filter path is negligible, so why not merge it as-is
rather than let it rot? Do you have a good reason to NAK this patch
as-is other than "it's slower than it could be"?

> There is, it's just hidden in libplacebo. If you integrate with the
> framework code like in the previous PR, there's no duplication.
> And no, we will not be removing the current Vulkan filters and
> we intend to continue expanding them. Not even if you add similar
> filters to libplacebo.

The duplication in libplacebo exists already for reasons independent of
ffmpeg, so there's no wasted effort here. And this duplication of logic
will fundamentally continue existing even if we do integrate with
hwcontext_vulkan, because libplacebo performs its own vulkan calls one
way or the other.

(Also, as I argued before, it's not strictly speaking a duplication
because the context creation code in libplacebo is technically different
from the one in libavutil - notably, libplacebo enables a different set
of extensions and features.)

> It's a good approach since API users work with VkImages. It's more
> complex because it's explicit.

It may be a good approach in principle but it doesn't seem to be working
in practice, which is my concern. The downside of vulkan's explicitness
is that it's harder to share vulkan state across API or ABI boundaries,
and as the synchronization issue highlights, getting this API right
seems to be nontrivial enough that it's preventing me from using
hwcontext_vulkan.

Khronos specifically points out that they intended for existing
platform-specific resource sharing mechanisms (fds and handles) to be
used to communicate VkImages across process boundaries or between
VkDevice instances, which is why VK_KHR_external_memory exists.

While libplacebo and libavfilter may technically share the same address
space, the differing requirements between the two seems a valid reason
to have separate VkDevices and thus use explicit resource sharing
extensions rather than communicating VkImage pointers directly.

> With an extension. Only supported on Linux. Only supported on recent
> AMD cards and drivers. Which is also buggy and crashes drivers. And
> the Intel implementation has been sitting as a PR for more than 3 years!
> You may have the privilege of using it because you lucked out on
> hardware, but the rest of us are not so lucky.

(I'm not sure what you mean - what extension is required? Are you talking
about DRM modifier support inside vulkan? Those work fine on intel, and
now also AMD.)

That said, I changed my mind and agree that something inherently
Linux-only is probably not a long-term solution, so we definitely do
need some vendor-agnostic vulkan-based sharing API. Whether it's based
on sharing VkImages directly or by sharing vulkan memory objects
indirectly is not as important as that it works. But I guess let's not
confuse the issue of how to fix hwcontext_vulkan with the issue of
whether it's okay to merge this patch as-is for the time being.