[vlc-devel] [PATCH 04/14] core: add viewpoint to change the yaw/pitch/roll view during playback

Steve Lhomme robux4 at gmail.com
Mon Oct 3 11:19:14 CEST 2016


On Sat, Oct 1, 2016 at 4:01 PM, Rémi Denis-Courmont <remi at remlab.net> wrote:
> Le vendredi 30 septembre 2016, 18:36:15 Steve Lhomme a écrit :
>> ---
>>  include/vlc_playlist.h          | 10 +++++++++
>>  include/vlc_vout.h              |  9 ++++++++
>>  include/vlc_vout_display.h      |  7 ++++++
>>  include/vlc_vout_wrapper.h      |  3 +++
>>  lib/audio.c                     |  1 +
>>  src/libvlccore.sym              |  4 ++++
>>  src/playlist/engine.c           | 34 ++++++++++++++++++++++++++++
>>  src/video_output/control.h      |  2 ++
>>  src/video_output/display.c      | 49
>> ++++++++++++++++++++++++++++++++++++++++- src/video_output/video_output.c |
>> 26 ++++++++++++++++++++++
>>  src/video_output/vout_intf.c    | 18 +++++++++++++++
>>  11 files changed, 162 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/vlc_playlist.h b/include/vlc_playlist.h
>> index 4363405..cd1b18a 100644
>> --- a/include/vlc_playlist.h
>> +++ b/include/vlc_playlist.h
>> @@ -29,6 +29,7 @@ extern "C" {
>>  # endif
>>
>>  #include <vlc_events.h>
>> +#include <vlc_vout.h>
>>
>>  TYPEDEF_ARRAY(playlist_item_t*, playlist_item_array_t)
>>
>> @@ -182,6 +183,8 @@ struct playlist_t
>>      playlist_item_t *     p_ml_category; /** < "Library" in CATEGORY view
>> */ playlist_item_t *     p_local_onelevel; /** < "Playlist" in ONELEVEL
>> view */ playlist_item_t *     p_ml_onelevel; /** < "Library" in ONELEVEL
>> view */ +
>> +    vlc_viewpoint_t      viewpoint;
>
> Not sure why playlist needs a viewpoint, but this does not belong here in any
> case.

It should be possible to set the viewpoint before the video starts,
from the command-line for example. To do that the vout will inherit
the value from somewhere, in the case of libvlc that's the media
player, in the case of VLC that's the playlist.

This storage will also be usef between vout creations (although I'm
not sure the vout wrapper does that already for the same input) and
other files loaded which I'm not sure is a good thing or not. It would
probably make sense with a headset where you look somewhere in the
room and different videos happen in the same room setup.

>>  };
>>
>>  /** Helper to add an item */
>> @@ -382,6 +385,13 @@ VLC_API playlist_item_t * playlist_GetPrevLeaf(
>> playlist_t *p_playlist, playlist
>>
>>  VLC_API audio_output_t *playlist_GetAout( playlist_t * );
>>
>> +/*************************
>> + * Viewpoint management
>> + *************************/
>> +VLC_API void playlist_GetViewpoint( playlist_t *, vlc_viewpoint_t
>> *p_viewpoint );
>> +VLC_API void playlist_SetViewpoint( playlist_t *, const
>> vlc_viewpoint_t *p_viewpoint );
>
> Use struct so you don't need promiscuous header inclusions.
>
> But then again, per-playlist viewpoint seems odd. I don't imagine much use
> case for preserving angle across videos. And it won't work with multiple vout
> or multiple simultaneous viewpoints.

Moved to playlist_private_t in forthcoming patch.

>> +
>> +
>>  #define AOUT_VOLUME_DEFAULT             256
>>  #define AOUT_VOLUME_MAX                 512
>>
>> diff --git a/include/vlc_vout.h b/include/vlc_vout.h
>> index 600cb56..4af5c31 100644
>> --- a/include/vlc_vout.h
>> +++ b/include/vlc_vout.h
>> @@ -82,6 +82,12 @@ struct vout_thread_t {
>>  #define VOUT_ALIGN_BOTTOM       0x0008
>>  #define VOUT_ALIGN_VMASK        0x000C
>>
>> +typedef struct {
>> +    float f_yaw;
>> +    float f_pitch;
>> +    float f_roll;
>> +} vlc_viewpoint_t;
>> +
>>  /**************************************************************************
>> *** * Prototypes
>>
>> ***************************************************************************
>> **/ @@ -156,6 +162,9 @@ VLC_API void vout_FlushSubpictureChannel(
>> vout_thread_t *, int );
>>
>>  VLC_API void vout_EnableFilter( vout_thread_t *, const char *,bool , bool
>> );
>>
>> +VLC_API void vout_GetViewpoint( vout_thread_t *, vlc_viewpoint_t
>> *p_viewpoint );
>> +VLC_API void vout_SetViewpoint( vout_thread_t *, const
>> vlc_viewpoint_t *p_viewpoint );
>> +
>>  /**@}*/
>>
>>  #endif /* _VLC_VIDEO_H */
>> diff --git a/include/vlc_vout_display.h b/include/vlc_vout_display.h
>> index 817f770..b4da1e2 100644
>> --- a/include/vlc_vout_display.h
>> +++ b/include/vlc_vout_display.h
>> @@ -30,6 +30,7 @@
>>  #include <vlc_subpicture.h>
>>  #include <vlc_keys.h>
>>  #include <vlc_mouse.h>
>> +#include <vlc_vout.h>
>>  #include <vlc_vout_window.h>
>>
>>  /**
>> @@ -115,6 +116,8 @@ typedef struct {
>>          int den;
>>      } zoom;
>>
>> +    vlc_viewpoint_t viewpoint;
>> +
>>  } vout_display_cfg_t;
>>
>>  /**
>> @@ -175,6 +178,10 @@ enum {
>>       * The cropping requested is stored by video_format_t::i_x/y_offset and
>> * video_format_t::i_visible_width/height */
>>      VOUT_DISPLAY_CHANGE_SOURCE_CROP,   /* const video_format_t *p_source */
>> +
>> +    /* Ask the module to acknowledge/refuse VR/360� viewing direction after
>> +     * being requested externally */
>> +    VOUT_DISPLAY_CHANGE_VIEWPOINT,   /* const vout_display_cfg_t *p_cfg */
>>  };
>>
>>  /**
>> diff --git a/include/vlc_vout_wrapper.h b/include/vlc_vout_wrapper.h
>> index 419bfc8..a2b6675 100644
>> --- a/include/vlc_vout_wrapper.h
>> +++ b/include/vlc_vout_wrapper.h
>> @@ -24,6 +24,7 @@
>>  #ifndef VLC_VOUT_WRAPPER_H
>>  #define VLC_VOUT_WRAPPER_H 1
>>
>> +#include <vlc_vout.h>
>>  #include <vlc_vout_display.h>
>>
>>  /* XXX DO NOT use it outside the vout module wrapper XXX */
>> @@ -93,6 +94,8 @@ void vout_SetDisplayZoom(vout_display_t *, unsigned num,
>> unsigned den); void vout_SetDisplayAspect(vout_display_t *, unsigned num,
>> unsigned den); void vout_SetDisplayCrop(vout_display_t *, unsigned num,
>> unsigned den, unsigned left, unsigned top, int right, int bottom);
>> +void
>> vout_GetDisplayViewpoint(vout_display_t *, vlc_viewpoint_t *p_viewpoint);
>
> This seems useless, unless the vout plugin is expected to unilateraly change
> its view point. And in that later case, the code is incomplete.

There can be more than one control that changes the viewpoint at once
(headset, mouse, keyboard, etc) so for each modification we need to
read whatever the current value is and then modify it.

> So seems wrong either way.
>
>> +void vout_SetDisplayViewpoint(vout_display_t *, const vlc_viewpoint_t
>> *p_viewpoint);
>>
>>  #endif /* VLC_VOUT_WRAPPER_H */
>>
>> diff --git a/lib/audio.c b/lib/audio.c
>> index 3aed6f5..3e09714 100644
>> --- a/lib/audio.c
>> +++ b/lib/audio.c
>> @@ -37,6 +37,7 @@
>>  #include <vlc_common.h>
>>  #include <vlc_input.h>
>>  #include <vlc_aout.h>
>> +#include <vlc_vout.h>
>>  #include <vlc_modules.h>
>>
>>  #include "libvlc_internal.h"
>> diff --git a/src/libvlccore.sym b/src/libvlccore.sym
>> index 8bb2dac..2e7a60d 100644
>> --- a/src/libvlccore.sym
>> +++ b/src/libvlccore.sym
>> @@ -373,6 +373,8 @@ playlist_VolumeSet
>>  playlist_VolumeUp
>>  playlist_MuteSet
>>  playlist_MuteGet
>> +playlist_GetViewpoint
>> +playlist_SetViewpoint
>>  sdp_AddAttribute
>>  sdp_AddMedia
>>  secstotimestr
>> @@ -734,6 +736,8 @@ vout_window_Delete
>>  vout_display_GetDefaultDisplaySize
>>  vout_display_PlacePicture
>>  vout_display_SendMouseMovedDisplayCoordinates
>> +vout_GetViewpoint
>> +vout_SetViewpoint
>>  xml_Create
>>  text_style_Copy
>>  text_style_Create
>> diff --git a/src/playlist/engine.c b/src/playlist/engine.c
>> index 7c00a48..164aa31 100644
>> --- a/src/playlist/engine.c
>> +++ b/src/playlist/engine.c
>> @@ -479,6 +479,9 @@ static void VariablesInit( playlist_t *p_playlist )
>>      var_Create( p_playlist, "mute", VLC_VAR_BOOL );
>>      var_Create( p_playlist, "volume", VLC_VAR_FLOAT );
>>      var_SetFloat( p_playlist, "volume", -1.f );
>> +
>> +    var_Create( p_playlist, "viewpoint", VLC_VAR_ADDRESS );
>> +    var_SetAddress( p_playlist, "viewpoint", &p_playlist->viewpoint );
>
> Why do you need this?

That's the storage that the vout will inherit its initial value from.

>>  }
>>
>>  playlist_item_t * playlist_CurrentPlayingItem( playlist_t * p_playlist )
>> @@ -501,3 +504,34 @@ int playlist_Status( playlist_t * p_playlist )
>>      return PLAYLIST_RUNNING;
>>  }
>>
>> +void playlist_GetViewpoint( playlist_t *p_playlist, vlc_viewpoint_t
>> *p_viewpoint ) +{
>> +    input_thread_t *p_input = pl_priv(p_playlist)->p_input;
>> +    vout_thread_t *p_vout = !p_input ? NULL : input_GetVout( p_input );
>> +
>> +    PL_LOCK;
>> +    if( p_vout )
>> +    {
>> +        vout_GetViewpoint( p_vout, &p_playlist->viewpoint );
>> +        vlc_object_release( (vlc_object_t *)p_vout );
>> +    }
>> +    *p_viewpoint = p_playlist->viewpoint;
>> +    PL_UNLOCK;
>> +}
>> +
>> +void playlist_SetViewpoint(playlist_t *p_playlist, const vlc_viewpoint_t
>> *p_viewpoint) +{
>> +    input_thread_t *p_input = pl_priv(p_playlist)->p_input;
>> +    vout_thread_t *p_vout = !p_input ? NULL : input_GetVout( p_input );
>> +
>> +    PL_LOCK;
>> +    p_playlist->viewpoint = *p_viewpoint;
>> +    if (p_vout)
>> +    {
>> +        vout_SetViewpoint( p_vout, &p_playlist->viewpoint );
>> +        vlc_object_release( (vlc_object_t *)p_vout );
>> +    }
>> +    PL_UNLOCK;
>> +}
>> +
>> +
>> diff --git a/src/video_output/control.h b/src/video_output/control.h
>> index eff9aa1..add2246 100644
>> --- a/src/video_output/control.h
>> +++ b/src/video_output/control.h
>> @@ -58,6 +58,7 @@ enum {
>>      VOUT_CONTROL_CROP_BORDER,           /* border */
>>      VOUT_CONTROL_CROP_RATIO,            /* pair */
>>      VOUT_CONTROL_CROP_WINDOW,           /* window */
>> +    VOUT_CONTROL_VIEWPOINT,             /* viewpoint */
>>  };
>>
>>  typedef struct {
>> @@ -93,6 +94,7 @@ typedef struct {
>>              unsigned width;
>>              unsigned height;
>>          } window;
>> +        vlc_viewpoint_t viewpoint;
>>          const vout_configuration_t *cfg;
>>          subpicture_t *subpicture;
>>      } u;
>> diff --git a/src/video_output/display.c b/src/video_output/display.c
>> index ecf661a..23dc545 100644
>> --- a/src/video_output/display.c
>> +++ b/src/video_output/display.c
>> @@ -373,6 +373,10 @@ struct vout_display_owner_sys_t {
>>          unsigned den;
>>      } crop;
>>
>> +    bool ch_viewpoint;
>> +    vlc_viewpoint_t viewpoint;
>> +    vlc_mutex_t viewpoint_lock;
>> +
>>      /* */
>>      video_format_t source;
>>      filter_chain_t *filters;
>> @@ -850,7 +854,8 @@ bool vout_ManageDisplay(vout_display_t *vd, bool
>> allow_reset_pictures) !ch_wm_state &&
>>  #endif
>>              !osys->ch_sar &&
>> -            !osys->ch_crop) {
>> +            !osys->ch_crop &&
>> +            !osys->ch_viewpoint) {
>>
>>              if (!osys->cfg.is_fullscreen && osys->fit_window != 0) {
>>                  VoutDisplayFitWindow(vd, osys->fit_window == -1);
>> @@ -1034,6 +1039,20 @@ bool vout_ManageDisplay(vout_display_t *vd, bool
>> allow_reset_pictures) osys->crop.den    = crop_den;
>>              osys->ch_crop = false;
>>          }
>> +        if (osys->ch_viewpoint) {
>> +            vout_display_cfg_t cfg = osys->cfg;
>> +
>> +            vlc_mutex_lock(&osys->viewpoint_lock);
>> +            cfg.viewpoint = osys->viewpoint;
>> +
>> +            if (vout_display_Control(vd, VOUT_DISPLAY_CHANGE_VIEWPOINT,
>> &cfg)) { +                msg_Err(vd, "Failed to change Viewpoint");
>> +                osys->viewpoint = cfg.viewpoint;
>> +            }
>> +            osys->cfg.viewpoint = osys->viewpoint;
>> +            osys->ch_viewpoint  = false;
>> +            vlc_mutex_unlock(&osys->viewpoint_lock);
>> +        }
>>
>>          /* */
>>          if (reset_pictures) {
>> @@ -1193,6 +1212,30 @@ void vout_SetDisplayCrop(vout_display_t *vd,
>>      }
>>  }
>>
>> +void vout_GetDisplayViewpoint(vout_display_t *vd, vlc_viewpoint_t
>> *p_viewpoint) +{
>> +    vout_display_owner_sys_t *osys = vd->owner.sys;
>> +
>> +    vlc_mutex_lock(&osys->viewpoint_lock);
>> +    *p_viewpoint = osys->viewpoint;
>> +    vlc_mutex_unlock(&osys->viewpoint_lock);
>
> This ensures that all members of viewpoint are consistent. But it does not
> ensure race-free read-modify-write if that's what you're after.

No only the former. Atomic read/write sequences would be nice when
there are multiple controlers but would make the API a lot more
complex. Requiring lock/unlock before reading writing. I'm not sure we
want that kind of thing in libvlc. That could be added later though.

>> +}
>> +
>> +void vout_SetDisplayViewpoint(vout_display_t *vd, const vlc_viewpoint_t
>> *p_viewpoint) +{
>> +    vout_display_owner_sys_t *osys = vd->owner.sys;
>> +
>> +    vlc_mutex_lock(&osys->viewpoint_lock);
>> +    if (osys->viewpoint.f_yaw   != p_viewpoint->f_yaw ||
>> +        osys->viewpoint.f_pitch != p_viewpoint->f_pitch ||
>> +        osys->viewpoint.f_roll  != p_viewpoint->f_roll) {
>> +        osys->viewpoint = *p_viewpoint;
>> +
>> +        osys->ch_viewpoint = true;
>
> Mixed-up locks.

???

>> +    }
>> +    vlc_mutex_unlock(&osys->viewpoint_lock);
>> +}
>> +
>>  static vout_display_t *DisplayNew(vout_thread_t *vout,
>>                                    const video_format_t *source,
>>                                    const vout_display_state_t *state,
>> @@ -1218,6 +1261,8 @@ static vout_display_t *DisplayNew(vout_thread_t *vout,
>>
>>      vlc_mutex_init(&osys->lock);
>>
>> +    vlc_mutex_init(&osys->viewpoint_lock);
>> +
>>      vlc_mouse_Init(&osys->mouse.state);
>>      osys->mouse.last_moved = mdate();
>>      osys->mouse.double_click_timeout = double_click_timeout;
>> @@ -1289,6 +1334,7 @@ static vout_display_t *DisplayNew(vout_thread_t *vout,
>>
>>      return p_display;
>>  error:
>> +    vlc_mutex_destroy(&osys->viewpoint_lock);
>>      vlc_mutex_destroy(&osys->lock);
>>      free(osys);
>>      return NULL;
>> @@ -1317,6 +1363,7 @@ void vout_DeleteDisplay(vout_display_t *vd,
>> vout_display_state_t *state) vlc_join(osys->event.thread, NULL);
>>          block_FifoRelease(osys->event.fifo);
>>      }
>> +    vlc_mutex_destroy(&osys->viewpoint_lock);
>>      vlc_mutex_destroy(&osys->lock);
>>      free(osys);
>>  }
>> diff --git a/src/video_output/video_output.c
>> b/src/video_output/video_output.c index 7994bec..a3f4908 100644
>> --- a/src/video_output/video_output.c
>> +++ b/src/video_output/video_output.c
>> @@ -540,12 +540,29 @@ void vout_ControlChangeSubMargin(vout_thread_t *vout,
>> int margin) margin);
>>  }
>>
>> +void vout_GetViewpoint( vout_thread_t *vout, vlc_viewpoint_t *p_viewpoint )
>> +{
>> +    vout_GetDisplayViewpoint(vout->p->display.vd, p_viewpoint);
>> +}
>> +
>> +void vout_SetViewpoint(vout_thread_t *vout, const vlc_viewpoint_t
>> *p_viewpoint) +{
>> +    vout_control_cmd_t cmd;
>> +    vout_control_cmd_Init(&cmd, VOUT_CONTROL_VIEWPOINT);
>> +    cmd.u.viewpoint = *p_viewpoint;
>> +
>> +    vout_control_Push(&vout->p->control, &cmd);
>> +}
>> +
>>  /* */
>>  static void VoutGetDisplayCfg(vout_thread_t *vout, vout_display_cfg_t *cfg,
>> const char *title) {
>>      /* Load configuration */
>>      cfg->is_fullscreen = var_CreateGetBool(vout, "fullscreen")
>>
>>                           || var_InheritBool(vout, "video-wallpaper");
>>
>> +    cfg->viewpoint.f_yaw   = 0.0f;
>> +    cfg->viewpoint.f_pitch = 0.0f;
>> +    cfg->viewpoint.f_roll  = 0.0f;
>>      cfg->display.title = title;
>>      const int display_width = var_CreateGetInteger(vout, "width");
>>      const int display_height = var_CreateGetInteger(vout, "height");
>> @@ -1289,6 +1306,12 @@ static void ThreadExecuteCropRatio(vout_thread_t
>> *vout, 0, 0, 0, 0);
>>  }
>>
>> +static void ThreadExecuteViewpoint(vout_thread_t *vout, const
>> vlc_viewpoint_t *p_viewpoint) +{
>> +    msg_Dbg(vout, "ThreadExecuteViewpoint %f %f %f", p_viewpoint->f_yaw,
>> p_viewpoint->f_pitch, p_viewpoint->f_roll); +
>> vout_SetDisplayViewpoint(vout->p->display.vd, p_viewpoint);
>> +}
>> +
>>  static int ThreadStart(vout_thread_t *vout, vout_display_state_t *state)
>>  {
>>      vlc_mouse_Init(&vout->p->mouse);
>> @@ -1548,6 +1571,9 @@ static int ThreadControl(vout_thread_t *vout,
>> vout_control_cmd_t cmd) cmd.u.border.left,  cmd.u.border.top,
>>                  cmd.u.border.right, cmd.u.border.bottom);
>>          break;
>> +    case VOUT_CONTROL_VIEWPOINT:
>> +        ThreadExecuteViewpoint(vout, &cmd.u.viewpoint);
>> +        break;
>>      default:
>>          break;
>>      }
>> diff --git a/src/video_output/vout_intf.c b/src/video_output/vout_intf.c
>> index 11240c8..3b77e35 100644
>> --- a/src/video_output/vout_intf.c
>> +++ b/src/video_output/vout_intf.c
>> @@ -57,6 +57,8 @@ static int AutoScaleCallback( vlc_object_t *, char const
>> *, vlc_value_t, vlc_value_t, void * ); static int ZoomCallback(
>> vlc_object_t *, char const *,
>>                           vlc_value_t, vlc_value_t, void * );
>> +static int ViewpointCallback( vlc_object_t *, char const *,
>> +                              vlc_value_t, vlc_value_t, void * );
>>  static int AboveCallback( vlc_object_t *, char const *,
>>                            vlc_value_t, vlc_value_t, void * );
>>  static int WallPaperCallback( vlc_object_t *, char const *,
>> @@ -252,6 +254,12 @@ void vout_IntfInit( vout_thread_t *p_vout )
>>      var_Change( p_vout, "video-on-top", VLC_VAR_SETTEXT, &text, NULL );
>>      var_AddCallback( p_vout, "video-on-top", AboveCallback, NULL );
>>
>> +    /* Add a variable to indicate if the viewpoint to use to display the
>> video */
>> +    var_Create( p_vout, "viewpoint", VLC_VAR_ADDRESS |
>> VLC_VAR_DOINHERIT );
>> +    text.psz_string = _("Viewpoint");
>> +    var_Change( p_vout, "viewpoint", VLC_VAR_SETTEXT, &text, NULL );
>
> Text on an address variable is pointless: it can't be shown in UI.

OK

>> +    var_AddCallback( p_vout, "viewpoint", ViewpointCallback, NULL );
>> +
>>      /* Add a variable to indicate if the window should be below all others
>> */ var_Create( p_vout, "video-wallpaper", VLC_VAR_BOOL | VLC_VAR_DOINHERIT
>> ); var_AddCallback( p_vout, "video-wallpaper", WallPaperCallback, @@ -309,6
>> +317,7 @@ void vout_IntfReinit( vout_thread_t *p_vout )
>>      var_TriggerCallback( p_vout, "zoom" );
>>      var_TriggerCallback( p_vout, "crop" );
>>      var_TriggerCallback( p_vout, "aspect-ratio" );
>> +    var_TriggerCallback( p_vout, "viewpoint" );
>>
>>      var_TriggerCallback( p_vout, "video-on-top" );
>>      var_TriggerCallback( p_vout, "video-wallpaper" );
>> @@ -602,6 +611,15 @@ static int ZoomCallback( vlc_object_t *obj, char const
>> *name, return VLC_SUCCESS;
>>  }
>>
>> +static int ViewpointCallback(vlc_object_t *obj, char const *cmd,
>> +                             vlc_value_t oldval, vlc_value_t newval, void
>> *data) +{
>> +    vout_SetViewpoint((vout_thread_t *)obj, newval.p_address);
>> +
>> +    VLC_UNUSED(cmd); VLC_UNUSED(oldval); VLC_UNUSED(data);
>> +    return VLC_SUCCESS;
>> +}
>> +
>>  static int AboveCallback( vlc_object_t *obj, char const *name,
>>                            vlc_value_t prev, vlc_value_t cur, void *data )
>>  {
>
> --
> Rémi Denis-Courmont
> http://www.remlab.net/
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel


More information about the vlc-devel mailing list