Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for vr goggle phone holders (Split display rendering of video) #92

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

0TheRain0
Copy link

@0TheRain0 0TheRain0 commented Jun 19, 2021

Addresses #9
This adds a "VR" mode to support vr goggle phone holders, like Google Cardboard etc. For testing, I used this one: https://www.amazon.com/dp/B08SBM1HMW?psc=1&ref=ppx_yo2_dt_b_product_details
It makes use of code from Google's "Grafika" project (see https://github.com/google/grafika) in order to do OpenGL ES operations and render the video twice with good performance. This code is in the "gles" folder.
A preferences setting was added for "Enable VR Goggle Mode"
I'm not sure how you are translating strings for this project, so my strings are currently in English only. Let me know if there is something I should do to translate them, I could use google translate or something.
Screenshot_20210618-191058
Screenshot_20210618-191026

vrdemo.mov

@0TheRain0 0TheRain0 changed the title Add support for vr goggle phone holders (Split display rendering of video) Add support for vr goggle phone holders (Split display rendering of video) https://github.com/fpvout/DigiView-Android/issues/9 Jun 19, 2021
@0TheRain0 0TheRain0 changed the title Add support for vr goggle phone holders (Split display rendering of video) https://github.com/fpvout/DigiView-Android/issues/9 Add support for vr goggle phone holders (Split display rendering of video) Jun 19, 2021
@0TheRain0 0TheRain0 force-pushed the feature/vr_support branch 2 times, most recently from 205aa7c to 4e5d080 Compare June 19, 2021 18:07
@jlucidar
Copy link
Member

Hi @0TheRain0 ! Thanks for your contribution !

Just a few questions on this :

  • Why do you import the grafika lib in code ? couldn't this be imported by gradle ? I'd like to keep external dependencies outside of this repo if possible. (checking out at the lib it says it's not stable, do you think there are some other alternatives, that we could trust ?)
  • Do you think it's possible to move most of the logic ot a separate class ? I don't want to overcomplicate the videoReaderExoPlayer class. this class may be replaced in the future and will be refactored, so keeping it simple in there would make it easier to maintain :) Maybe abstracting the surface rendering type is a good idea ?

don't worry about translations yet, we will do a translation round before next release :)

Thanks a lot !

@D3VL-Jack
Copy link
Member

D3VL-Jack commented Jul 14, 2021

Hey @0TheRain0 Amazing work here!
We'd like to get this out with the next release if possible, quite a few people are asking for this awesome feature!

I can't seem to locate you in our Discord server, any chance you could reply with your username (or @ me in the server @d3vl_jack), please, we can discuss more in dev-chat!

Thanks!

@0TheRain0
Copy link
Author

@Jack-Rogers @jlucidar I'm not on the discord - i'll try to get on there later today! @jlucidar with regards to your requests - on the topic of including grafika as a dependency instead of putting parts of it's code in the code base - unfortunately it's not a library and can't be used that way. Also, it's not a code base that is being actively developed, so it won't be getting updates. The license and source is cited in the files though, and it's ok to include the sources in your own project from what I know. As far as separating out this code from the videoReaderExoPlayer class, I can look into that. Also I need to make a fix for the aspect of the two images. Will try to get that done soon.

…ode to sub package "vr". Set two surfaces to correct aspect for video format.
@0TheRain0
Copy link
Author

@jlucidar @Jack-Rogers I moved most of the VR code into a new custom view class "VrView" and it along with the gles code to a subpackage "vr". So now most of this is consolidated to that package. I also put in a fix for the aspect - when I had tested this while my buddy was flying it made me feel kind of queezy before and the image looked a little off... but with this fix it looks much better in the goggles.
I don't know if you noticed but I made some formatting and indentation corrections in videoReaderExoPlayer as well.. but if you'd like and if that makes this confusing to read, I can undo that portion of my changes.
Screenshot_20210717-130618

@jlucidar jlucidar linked an issue Jul 19, 2021 that may be closed by this pull request
@jlucidar jlucidar added this to the v1.0.0 milestone Jul 19, 2021
@jlucidar jlucidar added the enhancement New feature or request label Jul 19, 2021
@jlucidar
Copy link
Member

Neat! it looks way better! thanks for your efforts.
I will check your branch out today, and do some testing to see if everything is ok, And will merge it if it works. I saw that some dependencies might be cleaned up but I can deal with that :)

@badgio440
Copy link

badgio440 commented Sep 9, 2021

Will this be added to the app in an update? This feature will be amazing!!

Comment on lines +76 to +86
surfaceViewLeft.getHolder().addCallback(videoSurfaceCallbackLeft);
surfaceViewRight.getHolder().addCallback(videoSurfaceCallbackRight);
surfaceViewLeft.setVisibility(View.GONE);
surfaceViewRight.setVisibility(View.GONE);
surfaceViewLeft.post(new Runnable() {
@Override
public void run() {
surfaceViewLeft.setVisibility(View.VISIBLE);
surfaceViewRight.setVisibility(View.VISIBLE);
}
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @0TheRain0 , didn't know about the Grafika repo, really a hidden gem.
Your VrView works well, though I don't have a phone holder with lenses to test.

What is the idea behind this code? To retrigger the surfaceCreated?
If we don't set to GONE first, could it trigger the callback already before the view is set to GONE?

I'm just registering on view inflation, and expect the surfaceView to be later ready, while this is a racecondition, in practice it works well for me and it inverts the dependency, so that the VrView doesn't have to know about the player.

d4rken-org/fpv-dvca@06d7565#diff-c3426db1c549d5574f8210d209aa690c2fda7d05d83b3f98548ea81e7b3fca8b

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@d4rken if someone else would like to take ownership of this code change and make the adjustments needed I would appreciate it. I no longer have time to work on it.

@fichek fichek added the help wanted Extra attention is needed label Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: compatibility with VR goggles
6 participants