vvvv / VL.StandardLibs

A collection of standard libraries for vvvv including VL.Stride, VL.Skia, VL.ImGui, msgpack.org[VL]
https://visualprogramming.net
GNU Lesser General Public License v3.0
40 stars 15 forks source link

Re-implement Skia versions #67

Closed azeno closed 2 years ago

azeno commented 4 years ago

Currently VideoIn and VideoPlayer output Stride textures to make full use of hardware acceleration.

If we bring back the version returning images the hardware acceleration doesn't make much sense as it requires a download from the GPU. So instead we could work with a non-accelerated pipeline in those cases.

YanYas commented 3 years ago

I agree. on the projects I've worked on with this library Stride hasn't been required and the results were using a Skia renderer.

You may have to split the repo in two where the core MediaFoundation is a module for the different output repos.

azeno commented 3 years ago

Now that we use ANGLE as rendering back-end for Skia, we can create Skia images out of the given Direct3D11 textures directly -> no GPU download -> same performance as in Stride. So the only remaining question would be how to organize the packages.

(1) One package

We deliver one package called VL.Video.MediaFoundation containing two nodes VideoPlayer (Stride) and VideoPlayer (Skia).

+ Only one package. Easier to install. + The code is easy to maintain - the package is independent from the core product - For an export to work, the package would need to reference both Skia and Stride increasing the output size. - Video playback not available per default - user needs to install the package first

(2) A package per rendering back-end

We deliver a package per rendering back-end VL.Video.MediaFoundation.Stride and VL.Video.MediaFoundation.Skia each containing the respective VideoPlayer node. Both would reference the core package VL.Video.MediaFoundation containing support nodes like video settings.

+ Each package comes with the dependencies it actually needs - smaller footprint on export + The code is easy to maintain - the packages are independent from the core product - Might be confusing for beginners as the names Skia or Stride are probably meaningless - but so would be MediaFoundation - Video playback not available per default - user needs to install the package first

(2a) @joreg brought an alternative naming scheme with the back-end in front Both VL.Stride.Video.MediaFoundation, VL.Skia.Video.MediaFoundation referencing VL.MediaFoundation. The Video prefix in the core package was left-out to avoid users installing the base package.

+ Thinking was that with this scheme the packages are easier to find - The pattern can only be applied by us / it assumes that the core and support packages come from the same owner/group

(3) One core package with back-ends delivering the nodes

We have one core package VL.Video.MediaFoundation with only back-end independent nodes (like the video setting nodes). The actual nodes are delivered by the back-ends themself.

+ Video playback comes out of the box - On export SharpDX.MediaFoundation.dll (500kb) would always be included even if not used - but that is true for a lot of external libraries - Changes in the core package would require updates in the rendering packages

In all cases we need to discuss whether a VideoPlayer uses (Back-end) as version or is placed in a sub-category?

tebjan commented 3 years ago

Since we should deliver video playback with vvvv, I'd go with the two package approach. It could even have a common core package if it makes sense code-wise.

It could even make sense to make these references to VL.Stride or VL.Skia because adding an extra package reference only for video playback seems cumbersome. Skia and Stride should both have video playback out of the box...

azeno commented 3 years ago

Ohh I didn't think of that. Good idea to reference the back-end specific ones by default from the main drawing libraries itself. That removes the issue of having to know the package name. And yes, a common package will make a lot of sense. The back-end specific code is tiny compared to the rest.

azeno commented 3 years ago

Hmm, on a second thought I'm not sure that idea works out as we'd introduce a cyclic dependency. For example VL.Skia -> VL.Video... -> VL.Skia? We'd need to add a VL.Skia.Core package to break the dependency cycle.

Or place the specific nodes in the VL.Stride and VL.Skia packages directly and only reference the VL.Video.MediaFoundation.Core package.

tebjan commented 3 years ago

Or place the specific nodes in the VL.Stride and VL.Skia packages directly and only reference the VL.Video.MediaFoundation.Core package.

that sounds easy enough. the only issue could be that the Stride and Skia packages would need an update when something was fixed or changed on video playback. a core and two specific packages might be the cleanest in terms of references, but cumbersome to use...

joreg commented 3 years ago

"the Stride and Skia packages would need an update when something was fixed or changed on video playback"

but until the parent packs are updated, ppl could still update the video packs individually. which makes the requirement to update the parents less pressing..

joreg commented 3 years ago

how much does an un-need mediafoundation reference contribute to an export?

als MF is windows only.. how does that influence this discussion?

azeno commented 3 years ago

The SharpDX.MediaFoundation.dll is around 500kb

And MF Windows only - true. But currenty we're hiding that fact in other packages as well. The whole thing should probably be called VL.Video and VL.Video.MediaFoundation would be something which gets plugged-in when running on Windows?

joreg commented 3 years ago

uk, so trying to sum up the options again:

either way have a core VL.MediaFoundation pack

option 1

pros

cons

option 2

pros

cons


currently (in 2021.4 previews) we basically have option 1, only simplified by the fact that you don't have to distinguish between stride and skia. staying with this option but adding backends to pack names would be most consistent i believe. you'd still type "video" in the nodebrowser but now see both "stride" and "skia" options and you'd have to choose the right one. consistent and clearly understandable once you learn the one thing: the difference btwn stride and skia and when to use which.

also think it further. someone will add a VL.OpenCV or VL.Vulkan one day and then we could continue consistently with a VL.Vulkan.Video.MediaFoundation....

if one day we get GStreamer working cross-platform it could simply be added in a \Video category as part of VL.Skia and VL.Stride

azeno commented 3 years ago

You brought up a new naming scheme in your last post where you put the rendering into the front. Not sure about that. Most of the code is in VL.Video.MediaFoundation. Then there's some specialized code specific to rendering / mapping the produced texture on to a surface native to that rendering back-end. So to me it feels off that suddendly that code becomes a subpart (naming wise) of the rendering engines.

joreg commented 3 years ago

yes, thats for readability. i thought all VL.Stride sub-libs would best start with VL.Stride and then .Sub, for findability and sortability.

and VL.Video.MediaFoundation would be renamed to VL.MediaFoundation, because it's shorter and more obscure, which is good, because nobody should use it directly.

joreg commented 3 years ago

yes, thats for readability. i thought all VL.Stride sub-libs would best start with VL.Stride and then .Sub, for findability and sortability.

and VL.Video.MediaFoundation would be renamed to VL.MediaFoundation, because it's shorter and more obscure, which is good, because nobody should use it directly.

azeno commented 3 years ago

Just to be on the same page - that naming scheme wouldn't be possible for anyone else, right? We as the maintainers of the VL.Stride.* space wouldn't be very happy if someone else uploaded a package with that name. So anyone else would need to go with the other naming scheme - will this fall on our heads at some point?

joreg commented 3 years ago

"We as the maintainers of the VL.Stride.* space wouldn't be very happy if someone else uploaded a package with that name." but could we even prevent that?

"will this fall on our heads at some point?" wouldn't we have to control the whole VL.* namespace to prevent this?

i think nuget even has a means to reserve a namespace, but i haven't looked to deep into this and didn't think about it in this particular case.

azeno commented 3 years ago

I just updated my post above to add our latest thoughts.

azeno commented 3 years ago

Maybe it helps for the discussion to bring in another example: https://github.com/vvvv/VL.CEF It currently uses option (1) and suffers from the same issues. Option (3) would be a no-go because the CEF dependency is huge (>100mb). So only (2) and (2a) would be viable in that case.

joreg commented 3 years ago

we've decided to leave this as is for 2021.4 but there is a new idea (to be picked up after 2021.4):

azeno commented 2 years ago

Having GPU support for IImage and a subsequent implicit conversion to SKImage and Texture is not that simple - when setting up a texture we need the graphics context, but Stride and Skia use a different one. So we'd first need to find a way to share the same context between the two APIs. We therefor dropped that idea for now and went with the explicit approach providing versions for both render APIs in one package.