Add _Udon_VideoTex global texture support for testing Avatars

llealloo / audiolink

Audio reactive prefabs for VRChat

Other

346 stars 38 forks source link

Add _Udon_VideoTex global texture support for testing Avatars #304

Closed techanon closed 2 months ago

techanon commented 3 months ago

Add support for exporting the YTDLP video player texture to the standard _Udon_VideoTex global texture property.
The texture export logic is baked into the YtdlpPlayer component.

float3 commented 3 months ago

I'll talk to pema and lea to see if this is something we want to have in audiolink

techanon commented 3 months ago

Some additional reasoning for posterity:

The _Udon_VideoTex is an upcoming global shader standard for exposing video player textures to avatars.
Custom and Uber shaders (like poiyomi) are beginning to integrate the standard and it will increase in use.
The AudioLinkAvatar prefab already comes with a video player for testing external media. Including this feature will help adoption and reduce creator friction by enabling testing it in playmode alongside the already existing audio reactive testing.
Efforts to aid adoption by other VideoPlayers (both standalone and prefab) aside from ProTV is starting underway.

float3 commented 3 months ago

@pema99 can you check this out

pema99 commented 2 months ago

The diff looks fine. And I'm fine with the change in principle... but I'm wondering how we can make it clear that this isn't an AudioLink feature, just a convenience provided to the user. Perhaps a link in the UI to some info about it? What do you think @float3

techanon commented 2 months ago

How's this look for the notice?

float3 commented 2 months ago

The diff looks fine. And I'm fine with the change in principle... but I'm wondering how we can make it clear that this isn't an AudioLink feature, just a convenience provided to the user. Perhaps a link in the UI to some info about it? What do you think @float3

ideally I would prefer to not have this in the codebase

techanon commented 2 months ago

I still believe that the accessibility of including this with Audiolink for testing dynamic external content (it's already loading video content) out-weighs the concern for user confusion, especially if proper clear messaging is included as such.

Do you have any alternate ideas on easy implementation into a user's avatar testing flow?

pema99 commented 2 months ago

From a principled point of view, I agree with @float3. From a pragmatic point of view, I agree with @techanon. Perhaps we should hear some of the other frequent contributors out about it.

fundale commented 2 months ago

How's this look for the notice?

May help catch the users attention to the notice better if it were a warning

techanon commented 2 months ago

I don't think escalating it to a warning is necessary here.
Usually a warning signifies some sort of optional action a user can take.
This is simply an informational notice about the context of the feature.
Warnings would confuse more users than it would help imo.

techanon commented 2 months ago

I have added _ST control for testing the tiling/offset options. Demo https://streamable.com/x2v0iw

techanon commented 2 months ago

Don't merge this yet. Found a possible math defect.

techanon commented 2 months ago

Ok, ready for review.

pema99 commented 2 months ago

Conflicts here as well 😅

techanon commented 2 months ago

@pema99 fixed.

pema99 commented 2 months ago

I changed the UI to be slightly more muted. Also, I'm making the default state disabled. It seems like an OK compromise considering several maintainers were on the fence about including this.