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

360videos with device motion #7

Closed
wants to merge 7 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@carolanitz
Copy link
Member

carolanitz commented Mar 1, 2018

It's still work in progress but I wanted to share this.
I'm still having the problem that when I hold the phone exactly opposite of the starting point I see the exact start frame upside down again and currently the gesturerecognizers are completely overwritten.

360videos with devicemotion

@carolanitz carolanitz force-pushed the carolanitz:360videosWithDeviceMotion branch Mar 1, 2018

@carolanitz

This comment has been minimized.

Copy link
Member

carolanitz commented Mar 1, 2018

@Mikanbu Mikanbu requested review from Mikanbu and removed request for Mikanbu Mar 1, 2018

@lazerwalker

This comment has been minimized.

Copy link

lazerwalker commented Mar 1, 2018

I'm really rusty at this sort of math, so take everything I'm saying with a grain of salt, but that rotation problem sounds a bit like the sorts of behaviors you see when encountering gimbal lock.

If that's the case, the solution would be to switch from using Euler angles to Quaternions. The math is a bit of a pain, but they are exposed as part of the CMAttitude data.

@carolanitz

This comment has been minimized.

Copy link
Member

carolanitz commented Mar 1, 2018

yes the problem is less the iPhone side of things.
The libvlc core takes pitch, yaw and role and I mentioned to @tguillem already that I would like to switch to quaternions but that would mean we need to adjust it on all platform. Hence I try to to avoid that :D

@carolanitz

This comment has been minimized.

Copy link
Member

carolanitz commented Mar 14, 2018

before merging this PR still needs to address the following:

  • work together with gesture recognizers
  • have a disabled rotation lock

@carolanitz carolanitz force-pushed the carolanitz:360videosWithDeviceMotion branch Apr 24, 2018

@carolanitz carolanitz requested review from Mikanbu and fkuehne Apr 24, 2018

@carolanitz

This comment has been minimized.

Copy link
Member

carolanitz commented Apr 24, 2018

This works now with gesture recognizer and devicemotion BUT it feels weird once you use devicemotion again after panning. I can't say why that is exactly but I feel like that could go into a follwo up commit

@magwyz

This comment has been minimized.

Copy link
Contributor

magwyz commented May 3, 2018

Hum, I thought my patch was working well...
Did you try to add the support from something else or my patch is broken?
I am not sure to see what is the issue on your video.

@carolanitz

This comment has been minimized.

Copy link
Member

carolanitz commented May 3, 2018

@magwyz Yes your patch worked fine for the devicemotion alone. But once I add panning to the mix and change the starting point for the devicemotion it seems to experience some axe switching :( The video is old btw. The patch is a commit btw. I just removed the file itself that I commited by accident

@magwyz

This comment has been minimized.

Copy link
Contributor

magwyz commented May 3, 2018

@carolanitz Panning is moving the video with the fingers, right?
About the starting point, you want to add a set constant angle to the rotations because you do not like the current starting point, right?

@carolanitz

This comment has been minimized.

Copy link
Member

carolanitz commented May 3, 2018

yes panning is moving with fingers and I don't want to add a set angle to the rotations. I'm just saving the location the user panned to and then add the devicemotion diff to that position. but that seems to mess up roll and yaw somehow

@carolanitz

This comment has been minimized.

Copy link
Member

carolanitz commented May 3, 2018

@magwyz It's okay for a first version but I will create a ticket for followup work.

carolanitz and others added some commits Mar 1, 2018

DeviceMotion: use quaternions and convert them ourself
Do not use the Euler angles provided by CoreMotion as they are not defined in the same reference frame as libvlc.

Signed-off-by: Carola Nitz <nitz.carola@googlemail.com>
360Video: moved currentMediaIs360 to playbackController
adjusted Bool properties from nonatomic to assign
turned interfacelock off for 360Videos
DeviceMotion: resolved jumping & not sensitive enough panning
lastEuler.pitch was accidentally set twice which resulted in jumping and
removing the screenscale + adjusting the zoomfactor makes panning and zooming more responsive

@carolanitz carolanitz force-pushed the carolanitz:360videosWithDeviceMotion branch to fc80bfe May 3, 2018

@magwyz

This comment has been minimized.

Copy link
Contributor

magwyz commented May 3, 2018

@carolanitz Ok, I understand what you want to achieve but I have no fix in mind when I read the MR. Do as you want for the first version. If you want, I will also be able to have a look when I go back to work next week and have access to the Mac and a device.

@carolanitz

This comment has been minimized.

Copy link
Member

carolanitz commented May 3, 2018

@magwyz that's basically what we decided on :) it already feels so much better than what we have currently 😄

@magwyz

This comment has been minimized.

Copy link
Contributor

magwyz commented May 3, 2018

@carolanitz Sorry, I don't get it. What have you decided? What is better than currently?

@carolanitz

This comment has been minimized.

Copy link
Member

carolanitz commented May 3, 2018

That we merge it in as is and these changes from this Pull request are better than what we currently have for 360 in the shipped Appstore app. We can then follow up with a patch release to make it perfect once you're back

@magwyz

This comment has been minimized.

Copy link
Contributor

magwyz commented May 3, 2018

ok, fair enough. :-)

carolanitz added some commits May 7, 2018

VLCMovieViewController: 360 videos fix rotationlock
on iPad with iOS 9 and higher shouldAutorotate is never called since all orientations are supported by default when
we support multitasking. This lead to no rotationlock when interfacelock was enabled or a 360 video was played.
We're now using supportedInterfaceOrientations: and lock 360 videos to portrait and for interfacelock we save the current orientation and return it.

@carolanitz carolanitz force-pushed the carolanitz:360videosWithDeviceMotion branch to 6e87a1e May 11, 2018

@carolanitz

This comment has been minimized.

Copy link
Member

carolanitz commented May 17, 2018

merged with :
3d5dc23 VLCMovieViewController: 360 videos fix rotationlock
f125c56 360 video: adjust values when they get out of bounds
7da7983 DeviceMotion: resolved jumping & not sensitive enough panning
df0d504 DeviceMotion: make panning and devicemotion interact with each other
4a38e44 360Video: moved currentMediaIs360 to playbackController
9563d3a DeviceMotion: use quaternions and convert them ourself
82f8e9d Devicemotion: add devicemotion for 360 videos

@carolanitz carolanitz closed this May 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment