[vlc-devel] [PATCH 11/18] input: use vlc_viewpoint_from/to_euler

[Alexandre Janniaux]

Hi,
On Thu, Apr 01, 2021 at 08:02:10AM +0200, Steve Lhomme wrote:
> On 2021-03-31 11:25, Alexandre Janniaux wrote:
> > From: Alexandre Janniaux <alexandre.janniaux at gmail.com>
> >
> > ... and also clip pitch to avoid singularities in locations where a
>
> It looks like a commit title that was cut by GitHub.

Not really, github doesn´t wrap at 38 characters. I just
didn´t duplicate the short commit message because it brings
no value here.

> > relative viewpoint update is used. Otherwise, conversions between
> > viewpoints and euler angles might lead to corrupted values and blinking
> > pitch angles.
> > ---
> >   src/input/input.c | 22 ++++++++++++++++++----
> >   1 file changed, 18 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/input/input.c b/src/input/input.c
> > index 69552bc3e5..b134d4f097 100644
> > --- a/src/input/input.c
> > +++ b/src/input/input.c
> > @@ -1758,6 +1758,10 @@ static void ControlNav( input_thread_t *p_input, int i_type )
> >       if( b_viewpoint_ch && viewpoint_updated )
> >       {
> > +        /* We need to clip the pitch here to avoid singularity poles, otherwise
> > +         * the next vlc_viewpoint_to_euler might reverse the pitch. */
> > +        pitch = VLC_CLIP(pitch, -85.f, 85.f);
>
> These arbitrary values should have a name. And why not use -90, 90 ? In
> Matroska you can have pitch values of -90 to 90, both included. Same for MP4
> [1]. Does a pitch of 270 turns to 85 ?
>
> [1] https://github.com/google/spatial-media/blob/master/docs/spherical-video-v2-rfc.md

They are arbitrary values, so well, it´s quite hard to name them.
They are basically the closest values to the singularity points
where conversion to/from euler angles doesn´t explode. This is
explained in the description of the commit.

We can support pitch with singularity values only if there is no
singularities, meaning only if we don´t have roll, but it would
mean that we´d have multiple conversion function for the two
convention used and flag to indicate which convention is in use.

Note also that this is the input code for handling controls and
for relative updates of the viewpoint, so it has nothing to do
immediately with the original pose. It´s also converting from
Euler quaternions so initial pitch values don´t really make
sense at this point, a pitch of 270 doesn´t exist in this
convention since it would alias other pitch/yaw values.

This patch does change the previous behaviour, since previous
behaviour was not clipped. But because there was no conversion
before it could just work as long as the user didn´t change roll.

Regards,
--
Alexandre Janniaux
Videolabs

> > +
> >           priv->viewpoint_changed = true;
> >           vlc_viewpoint_from_euler( &priv->viewpoint, yaw, pitch, roll );
> >           ViewpointApply( p_input );
> > @@ -2148,10 +2152,20 @@ static bool Control( input_thread_t *p_input,
> >               else
> >               {
> >                   priv->viewpoint_changed = true;
> > -                priv->viewpoint.yaw   += param.viewpoint.yaw;
> > -                priv->viewpoint.pitch += param.viewpoint.pitch;
> > -                priv->viewpoint.roll  += param.viewpoint.roll;
> > -                priv->viewpoint.fov   += param.viewpoint.fov;
> > +                float previous[3], update[3];
> > +                vlc_viewpoint_to_euler(&priv->viewpoint, &previous[0],
> > +                                       &previous[1], &previous[2]);
> > +                vlc_viewpoint_to_euler(&param.viewpoint, &update[0],
> > +                                       &update[1], &update[2]);
> > +
> > +                /* We need to clip the pitch here to avoid singularity poles,
> > +                 * otherwise the next vlc_viewpoint_to_euler might reverse
> > +                 * the pitch. */
> > +                vlc_viewpoint_from_euler(&priv->viewpoint,
> > +                        previous[0] + update[0],
> > +                        VLC_CLIP(previous[1] + update[1], -85.f, 85.f),
> > +                        previous[2] + update[2]);
> > +                priv->viewpoint.fov += param.viewpoint.fov;
> >               }
> >               ViewpointApply( p_input );
> > --
> > 2.31.0
> >
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> >
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel