[vlc-devel] [PATCH 3/8] libvlc: add the API needed to set/get the viewpoint in 360° videos

[Steve Lhomme]

On Fri, Sep 9, 2016 at 9:37 AM, Steve Lhomme <robux4 at gmail.com> wrote:
> On Thu, Sep 8, 2016 at 5:50 PM, Rémi Denis-Courmont <remi at remlab.net> wrote:
>> Le jeudi 8 septembre 2016, 17:35:30 Steve Lhomme a écrit :
>>> ---
>>>  include/vlc/libvlc_media_player.h | 28 ++++++++++++++++++++++++++++
>>>  lib/media_player.c                |  3 +++
>>>  lib/video.c                       | 37
>>> +++++++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+)
>>>
>>> diff --git a/include/vlc/libvlc_media_player.h
>>> b/include/vlc/libvlc_media_player.h index 2718a33..222b25a 100644
>>> --- a/include/vlc/libvlc_media_player.h
>>> +++ b/include/vlc/libvlc_media_player.h
>>> @@ -104,6 +104,16 @@ typedef struct libvlc_audio_output_device_t
>>>  } libvlc_audio_output_device_t;
>>>
>>>  /**
>>> + * Viewpoint for video outputs
>>> + */
>>> +typedef struct libvlc_video_viewpoint_t
>>> +{
>>> +    float f_yaw;   /**< view point yaw in degrees ]-180;180] */
>>> +    float f_pitch; /**< view point pitch in degrees ]-180;180] */
>>> +    float f_roll;  /**< view point roll in degrees ]-180;180] */
>>> +} libvlc_video_viewpoint_t;
>>> +
>>> +/**
>>>   * Marq options definition
>>>   */
>>>  typedef enum libvlc_video_marquee_option_t {
>>> @@ -1127,6 +1137,24 @@ LIBVLC_API char *libvlc_video_get_aspect_ratio(
>>> libvlc_media_player_t *p_mi ); LIBVLC_API void
>>> libvlc_video_set_aspect_ratio( libvlc_media_player_t *p_mi, const char
>>> *psz_aspect );
>>>
>>>  /**
>>> + * Get current video viewpoint.
>>> + *
>>> + * \param p_mi the media player
>>> + * \return the video viewpoint or NULL if unspecified
>>> + * (the result must be released with free() or libvlc_free()).
>>> + */
>>> +LIBVLC_API libvlc_video_viewpoint_t *libvlc_video_get_viewpoint(
>>> libvlc_media_player_t *p_mi ); +
>>> +/**
>>> + * Set new video viewpoint information.
>>> + *
>>> + * \param p_mi the media player
>>> + * \param psz_aspect new video viewpoint or NULL to reset to default
>>> + */
>>> +LIBVLC_API void libvlc_video_set_viewpoint( libvlc_media_player_t *p_mi,
>>> +                                            const libvlc_video_viewpoint_t
>>> *p_viewpoint ); +
>>> +/**
>>>   * Get current video subtitle.
>>>   *
>>>   * \param p_mi the media player
>>> diff --git a/lib/media_player.c b/lib/media_player.c
>>> index 69d3523..197c9be 100644
>>> --- a/lib/media_player.c
>>> +++ b/lib/media_player.c
>>> @@ -640,6 +640,9 @@ libvlc_media_player_new( libvlc_instance_t *instance )
>>>      var_Create (mp, "zoom", VLC_VAR_FLOAT | VLC_VAR_DOINHERIT);
>>>      var_Create (mp, "aspect-ratio", VLC_VAR_STRING);
>>>      var_Create (mp, "crop", VLC_VAR_STRING);
>>> +    var_Create (mp, "viewpoint-yaw", VLC_VAR_FLOAT | VLC_VAR_DOINHERIT);
>>> +    var_Create (mp, "viewpoint-pitch", VLC_VAR_FLOAT | VLC_VAR_DOINHERIT);
>>> +    var_Create (mp, "viewpoint-roll", VLC_VAR_FLOAT | VLC_VAR_DOINHERIT);
>>>      var_Create (mp, "deinterlace", VLC_VAR_INTEGER);
>>>      var_Create (mp, "deinterlace-mode", VLC_VAR_STRING);
>>>
>>> diff --git a/lib/video.c b/lib/video.c
>>> index b2c9b34..bee4e6d 100644
>>> --- a/lib/video.c
>>> +++ b/lib/video.c
>>> @@ -281,6 +281,43 @@ void libvlc_video_set_aspect_ratio(
>>> libvlc_media_player_t *p_mi, free (pp_vouts);
>>>  }
>>>
>>> +libvlc_video_viewpoint_t *libvlc_video_get_viewpoint( libvlc_media_player_t
>>> *p_mi )
>>> +{
>>> +    libvlc_video_viewpoint_t *p_viewpoint =
>>> malloc(sizeof(libvlc_video_viewpoint_t));
>>> +    if (p_viewpoint == NULL)
>>> +        return NULL;
>>
>> If you want to be able to expand the structure later, this is not enough due
>> to libvlc_video_set_viewpoint. And if you don't care, then malloc() seems
>> overkill.
>
> I don't understand the issue. You mean expand the structure and remain
> libvlc ABI backward compatible ? Is that something we're trying to
> achieve ? Also if you expand the structure more memory is allocated
> but it's still the same free() that has to be used and should know
> about it.
>
> Do you suggest using a get for each of the 3 values ? The viewpoint
> will still has to be set with all values at once otherwise you'll end
> up with weird transitions between states. And if it's not via a
> structure than adding a value will break backward compatibility
> anyway.

I added a new parameter (and there might be another one later). The
non backward compatible thing I see is passing the "viewpoint" data in
a string with separators. Adding more values changes the string and
thus newer/older parsers may have a problem parsing the data. This is
not really related to the structure though. Expanding the structure in
a backward compatible way is still possible.

The reason why all values are merged into a single value is that it
doesn't make sense to update one direction and not the others at once.
That would produce some kind of "scale" movements when changing the
viewpoint. Now the string format may not be the best thing. I'll try
to use a VLC_VAR_ADDRESS instead. That will avoid the costly parsing
step at each change.

>>> +
>>> +    p_viewpoint->f_yaw   = var_GetFloat( p_mi,"viewpoint-yaw" );
>>> +    p_viewpoint->f_pitch = var_GetFloat( p_mi,"viewpoint-pitch" );
>>> +    p_viewpoint->f_roll  = var_GetFloat( p_mi,"viewpoint-roll" );
>>> +    return p_viewpoint;
>>> +}
>>> +
>>> +void libvlc_video_set_viewpoint( libvlc_media_player_t *p_mi,
>>> +                                 const libvlc_video_viewpoint_t
>>> *p_viewpoint ) +{
>>> +    if (p_viewpoint == NULL)
>>> +        return;
>>> +
>>> +    var_SetFloat(p_mi, "viewpoint-yaw", p_viewpoint->f_yaw);
>>> +    var_SetFloat(p_mi, "viewpoint-pitch", p_viewpoint->f_pitch);
>>> +    var_SetFloat(p_mi, "viewpoint-roll", p_viewpoint->f_roll);
>>> +
>>> +    char psz_viewpoint[3 * 13];
>>> +    sprintf( psz_viewpoint, "%.7f:%.7f:%.7f",
>>> +             p_viewpoint->f_yaw, p_viewpoint->f_pitch, p_viewpoint->f_roll
>>> ); +    size_t n;
>>> +    vout_thread_t **pp_vouts = GetVouts (p_mi, &n);
>>> +    for (size_t i = 0; i < n; i++)
>>> +    {
>>> +        vout_thread_t *p_vout = pp_vouts[i];
>>> +
>>> +        var_SetString (p_vout, "viewpoint", psz_viewpoint);
>>> +        vlc_object_release (p_vout);
>>> +    }
>>> +    free (pp_vouts);
>>> +}
>>> +
>>>  int libvlc_video_get_spu( libvlc_media_player_t *p_mi )
>>>  {
>>>      input_thread_t *p_input_thread = libvlc_get_input_thread( p_mi );
>>
>> --
>> 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