stash icon indicating copy to clipboard operation
stash copied to clipboard

Support for videojs-vr with new fields for screen projection and stereo mode and tag based fallback

Open pickleahead opened this issue 3 years ago • 23 comments

Closes #2504

Instead of using tags like in #2659, this approach introduces new fields for screen projection (Flat, Dome, Sphere, ...) and stereo mode (Mono, top bottom and left right).

  • Using a single tag is too vague to properly support various types of projections.
  • Projection and stereo mode have enums and are expandable
  • Projection and stereo mode are set in the scene edit panel
  • The new fields can also be used for general VR support (e.g. DeoVR api) or images in the future.
  • Also includes a fix for applying image filters. Without, the filter function unhides the video element hidden by videojs-vr. The filter is now applied on the canvas in this case.

Things that could be improved:

  • Updating a scene does not reload the player. Changes are not applied instantly.
  • Maybe change the fields in the edit panel or move them to another tab
  • Bulk editing
  • Add support for scrapers

pickleahead avatar Jun 09 '22 15:06 pickleahead

I'm happy to close my branch for yours, but the main issue with this approach is that users with 1000+ VR files would have to go back and manually update each of the scenes one by one to use these new fields you've added. Even users who are adding new scenes in bulk to their stash would likely find manually going through each new scene for these fields frustrating. I agree that this approach is more accurate, but it is also currently less accessible.

Your change also needs to take into account users who view their stash using a VR headset. For these users, you will need to disable the projection setting so they can apply the VR options from their headset instead. Otherwise, the stash settings and headset settings will conflict with each other

cj12312021 avatar Jun 12 '22 17:06 cj12312021

I agree that this approach is more accurate, but it is also currently less accessible.

You could mitigate this by writing a scrapper users could use to populate these new fields based on their existing tags. The scrapper could allow users to specify which tags translate to which screen projection and stereo mode combination.

Your change also needs to take into account users who view their stash using a VR headset. For these users, you will need to disable the projection setting so they can apply the VR options from their headset instead. Otherwise, the stash settings and headset settings will conflict with each other

This issue would need to be addressed in this pull request.

cj12312021 avatar Jun 12 '22 19:06 cj12312021

I'm happy to close my branch for yours, but the main issue with this approach is that users with 1000+ VR files would have to go back and manually update each of the scenes one by one to use these new fields you've added. Even users who are adding new scenes in bulk to their stash would likely find manually going through each new scene for these fields frustrating. I agree that this approach is more accurate, but it is also currently less accessible.

Thank you! I see the issue. I added a tag based fallback for more convenience. So if a scene does not have projection and stereo mode set but has a tag like "VR", "Virtual Reality", etc. the player falls back to a 180° side-by-side projection. This should be correct for most VR content out there. Additionally Tags like "180", "180°", "180_LR", "360", "360°", "360_TB", "360_TOP_BOTTOM", etc. control the projection too.

Tags like "vr", "3d", "180" and "360" are very common. I guess for users who scraped and tagged their vr content before, the player will work just fine out of the box.

I think a scraper or task is a good idea, but it might be too much for this pull request or as a core feature. Bulk editing is something, that I would like to add though.

pickleahead avatar Jun 14 '22 15:06 pickleahead

Your change also needs to take into account users who view their stash using a VR headset. For these users, you will need to disable the projection setting so they can apply the VR options from their headset instead. Otherwise, the stash settings and headset settings will conflict with each other

I disabled videojs-vr for certain user agents like in #2659. @Teda1 Can you explain the issue more (Just for better understanding)?

pickleahead avatar Jun 14 '22 16:06 pickleahead

No worries about the scrapper, it should be pretty easy for most in the community to write and share.

I disabled videojs-vr for certain user agents like in https://github.com/stashapp/stash/pull/2659. @Teda1 Can you explain the issue more (Just for better understanding)?

Thanks. VR headset applications will usually have options that will allow the user to select what projection and stereo mode they want to watch a video at. These options are available for any video you watch on the device, so the options can also be applied to non-VR videos. If you are browsing Stash on a headset and Stash has already applied projection and stereo modes, any mode you attempt to apply from the headset would be added on top of the modes Stash has already applied. This distorts the video and causes severe lag. On the Oculus Browser, you can't enter proper full-screen mode for VR videos until you have selected one of its VR options.

cj12312021 avatar Jun 14 '22 16:06 cj12312021

If there's a development build available I could test this a bit - I have a lot of files with the default download filename that includes things like "LR_180" or "TB_360", so could also use auto-tag to test fallback, so I could test on both browser and a headset or two.

While tags are kind of a clunky way to set the image, I'm not sure how feasible manually setting it on large collections would be - I feel like it would probably be possible for a complicated script to identify if an image is top bottom or left right, but unless there's consistent ratios figuring out the nuances of 360 LR/etc probably aren't feasible.

devloch avatar Jun 24 '22 00:06 devloch

I feel like unless the fields will be used #2105 or stash-box the issue of updating existing libraries should not be dealt in core for now. There is simply no benefit in updating every scene yet. A tag like "vr" would already work most of the time. This is also true for tags like "180" and "360", which are commonly used by sites. However, in cases, when the projection is off, one can easily set the correct values.

Detecting the projection by analysing the image data would be very complicated and slow. In most cases relying on tags and filenames should be enough to set the fields.

pickleahead avatar Jun 26 '22 15:06 pickleahead

I think the current state of this pull request is good enough. If users want the option to set these new fields based on tags, I or someone else in the community can write a scrapper to do that.

cj12312021 avatar Jul 01 '22 04:07 cj12312021

since I just wrote a quick vertical/mobile tagger for https://github.com/stashapp/stash/issues/2677 I realized the VR stuff is mostly the same. Writing a plugin that on hook_scene_create, looks at the scene and does best guess field setting seems pretty simple. So +1, and we can have a plugin that 'autotags' most of the content as it's added, and it's always tweakable that way. I'll submit something soon. so +1 for this patch from me.

scruffynerf avatar Aug 12 '22 05:08 scruffynerf

With the files-refactor changes now in place, we have a separate concept of scenes and video files. My feeling is that the projection and stereo mode would be file-specific and not scene-specific fields. In other words, it might be possible for a scene to have two separate files, one that is not VR-enabled, or it might have a different projection field than the other file.

If my assumption is correct, then projection and stereo mode fields need to be on the file and not the scene. The difficulty there is that we don't currently have the API to edit file objects. This may need to be deferred until we have such functionality in place, or we need some way of setting these details on scene files in the API.

WithoutPants avatar Sep 30 '22 06:09 WithoutPants

Not sure, how common this would be. The only scenario I can think of is for non vr 3D scenes. Someone might store a 2D version for performance reasons.

This would also mess with tags like „VR“, „3D“, etc. We could only rely on file names then.

Storing the data for files is better though.

pickleahead avatar Sep 30 '22 12:09 pickleahead

Yeah, it's possible... I have a few examples of 2Ded 3D scenes, or same scene but done in different projections. It's rare but not unknown.

It's rare though. Like a handful out of 10s of thousands of scenes.

The idea that there are scene tags (describing the content), and file tags (describing the file properties) makes sense. 2 files of the same scene might be encoded differently (which we track), have different resolutions (which we track), and yes, could be different forms of VR (SBS versus TB), or even 2D.

So yes, fields might make more sense than tags, but I don't see a easy way to auto detect this, and it would need to be editable if wrong. It also might be a combo of things (oh this file is SBS, 190 degrees not 180).

At that point, I do wonder if tags are better here.

Can we just do this with file specific tags instead of fields?

scruffynerf avatar Sep 30 '22 13:09 scruffynerf

Guessing all this information from tags is prone to errors and it gets worse with more supported tags and tag formats. Users also wouldn't know, which tags are supported (190? 190 degrees? 190_deg? ...). Support for common tags is very convenient, but in general tags are not reliable.

pickleahead avatar Sep 30 '22 17:09 pickleahead

in general tags are not reliable.

While I agree... How are you going to determine this as a field? If wrong, how does the user edit it? In other words, fields are just as problematic and worse we have no way to edit or change them, so you are required to invent that.
[clarification: I meant the above in reference to the way we determine resolution, encoding etc... In other words, as file properties. If you want an editable field that only applies to one file and not another file, with the same scene, then file specific fields are needed to be done, as opposed to file specific tags. One seems easier than the other... And more useful generally.

Tags have issues, but we use tags now.

scruffynerf avatar Sep 30 '22 17:09 scruffynerf

Also, we now have two competitive VR Stash "docker" solutions that both use tags for these properties, so Tags is doable, and with a 3rd for a built in videojs solution, a consensus as to supported tags could be reached, including aliases etc. Installing aliases and a set of "supported" tags would be trivial, compared to the field solution. Granted, we still need to have tags on files, not just on scenes, but seems like far less of a stretch.

scruffynerf avatar Sep 30 '22 17:09 scruffynerf

I am going to push some improvements soon. In general it is a lot easier to update the data through scene updates for now. We can either update the primary file or all scene files.

I guess an api for file edits is likely to happen at some point (File browser/management, management of scene and gallery files, …). So the forms can be moved to file specific forms later.

I am not opposing file tags. But it is a feature, that should be dealt on its own. There are also other open questions. E.g. would file tags be merged with scene tags? Supporting them in this PR is not an issue. It already works for scene tags.

It also looks like setting the data for files makes auto detection by filename easier. This could be done in file (re-)scanning.

pickleahead avatar Oct 11 '22 13:10 pickleahead

Files aren't really something that should be user-editable in general. Is there a problem with inferring this information from the filename the way that xbvr does it? https://github.com/xbapps/xbvr/blob/b7e6781ecbe81838555f2b470fc9af7edce879a3/pkg/tasks/volume.go#L224

WithoutPants avatar Oct 11 '22 23:10 WithoutPants

File edits changes the hashes. A bad thing. Filename is fine, of course.

I'm now wondering if there is some confusion here about fields versus tags versus "editing" etc.

Rule of thumb: you cannot depend on a filename inside Stash. Period. That's not Stash's business, which is why we record the path and the oshash to have two methods of identification, since either could change for non Stash reasons (or plugins etc...)

If you want info about what/how to display a given file, some of it is inherent to the file (resolution, encoding, for examples), and some aren't easy to detect that way (VR format being a good example, ratio is often but not always 2:1, but not all 2:1 files are VR, and not all SBS are 2:1, and so on.

Tags is the only method right now we can depend on, easy to edit and easy to access

scruffynerf avatar Oct 11 '22 23:10 scruffynerf

File edits changes the hashes. A bad thing. Filename is fine, of course.

Sorry, I meant file entries in the database, not the files themselves.

WithoutPants avatar Oct 11 '22 23:10 WithoutPants

My preference would be to have these extra fields as you may want to be explicit in what you want with auto as the default and fall through to file name and tag detection. Detection based on file names is good and that is what xbvr does and it works fine but as people use the renamer plugins they can potentially mess with the file names to stop this working. Tags also works well, that is what my tool and fl0w's tool does. Stashdb does have tags for these and a lot of the scenes have these tags 180°, 190°, 200°, 220°, 360°, Virtual Reality

There are a few different types of projections commonly used but the pull request covers these. On top of projection Heresphere does support some lens correction, this can be set in the player so we can probably ignore this. HereSphere_JSON_API_Version_1(5).txt

Tweeticoats avatar Oct 12 '22 06:10 Tweeticoats

In my current local state both fields are resolved in the scene resolver (database -> tags -> filename). Without any other changes in the code the result would be written into the database (video_files) on the next scene update. Basically the projection is always detected by tags and filenames for unedited scenes. The form fields are prefilled with the result. The fields can be reset, because they are nullable.

I‘d like to have your opinion first.

Database fields could be omitted in my opinion. Managing the projection would be more complicated on user side though (filenames and correct tagging).

pickleahead avatar Oct 13 '22 11:10 pickleahead

In my current local state both fields are resolved in the scene resolver (database -> tags -> filename). Without any other changes in the code the result would be written into the database (video_files) on the next scene update. Basically the projection is always detected by tags and filenames for unedited scenes. The form fields are prefilled with the result. The fields can be reset, because they are nullable.

I‘d like to have your opinion first.

Database fields could be omitted in my opinion. Managing the projection would be more complicated on user side though (filenames and correct tagging).

File-based fields shouldn't be resolved/set via the scene resolvers. I think there needs to be support added for modifying file values separately before we can do this functionality effectively. Is it feasible to have the first iteration to be read-only and generated from filename/tag first, with the intent to add support for modification of files in a later PR?

WithoutPants avatar Oct 19 '22 00:10 WithoutPants

I fully agree on waiting for file data modification support. This data needs to be editable.

I feel like this approach got way more complicated for videojs-vr only and there are too many things left unclear or things that could be an issue or PR on their own. Detection by filename is less reliable than tags and (scene) tags would be sufficient in most cases for videojs-vr.

Maybe #2659 could be reopened and merged instead. It is way more easy to shift to fields than to rewrite file specific fields and detection later. This way we wouldn‘t rush file mutations and keep the videojs-vr implementation simple.

pickleahead avatar Nov 02 '22 08:11 pickleahead

@WithoutPants what needs to be done to get this feature released? Personally, I think a basic implementation doesn't even need the software to have an understanding of what content is VR. Just a toggle in the the video player to change the video projection would be more than adequate that we have to select each time the page is viewed. What are your thoughts?

Anon247 avatar Feb 19 '23 22:02 Anon247

A manual VR selection button is definitely a more simple and acceptable solution in the shorter term, yes.

WithoutPants avatar Feb 22 '23 04:02 WithoutPants

@pickleahead @Teda1 Please see https://github.com/WithoutPants/stash/tree/vr-proto for an initial prototype for a VR button. I wasn't able to successfully test it, so if you guys want to check it out and add your input, that might get things moving.

WithoutPants avatar Mar 26 '23 02:03 WithoutPants

I’ll find some time this week to check it out.

cj12312021 avatar Mar 27 '23 04:03 cj12312021

@WithoutPants do you want to put up the pull request so we can leave comments there? I'm running your vr-proto branch now, and it's not clear how it's supposed to work. I don't see any VR options.

cj12312021 avatar Mar 27 '23 22:03 cj12312021

@WithoutPants do you want to put up the pull request so we can leave comments there? I'm running your vr-proto branch now, and it's not clear how it's supposed to work. I don't see any VR options.

The VR options are only shown in some VR browsers ((/oculusbrowser|\svr\s/i)). Maybe we need a setting to always show the button, to change the user agent string or to show the button on certain scene tags. Support for VR Devices in browsers is very limited and I guess most VR users prefer using VR players like DeoVR, HereSphere, Whirligig, …

I tested the branch with a few samples and it worked (No VR device). But unfortunately videojs-vr seems to have issues with Three.js at the moment.

I don‘t own an oculus. I could test on steam-vr devices later.

pickleahead avatar Mar 28 '23 08:03 pickleahead

Closing in favour of #3636

WithoutPants avatar May 26 '23 02:05 WithoutPants