[vlc-devel] [PATCH] libvlc: rename the aspect ratio setter/getter and use a fraction

[Rémi Denis-Courmont]

Le keskiviikkona 29. toukokuuta 2019, 17.16.00 EEST Steve Lhomme a écrit :
> Using sample in the name is more accurate than just an "aspect ratio", even
> though that's probably the one most people know.
> 
> ---
> how do we document APIs that are removed ?
> ---
>  include/vlc/libvlc_media_player.h | 16 ++++++++++++----
>  lib/libvlc.sym                    |  4 ++--
>  lib/media_player.c                |  2 +-
>  lib/video.c                       | 28 +++++++++++++++++++++-------
>  4 files changed, 36 insertions(+), 14 deletions(-)
> 
> diff --git a/include/vlc/libvlc_media_player.h
> b/include/vlc/libvlc_media_player.h index 10e302529d..179597e80f 100644
> --- a/include/vlc/libvlc_media_player.h
> +++ b/include/vlc/libvlc_media_player.h
> @@ -1486,23 +1486,31 @@ LIBVLC_API float libvlc_video_get_scale(
> libvlc_media_player_t *p_mi ); */
>  LIBVLC_API void libvlc_video_set_scale( libvlc_media_player_t *p_mi, float
> f_factor );
> 
> +typedef struct libvlc_aspect_ratio_t
> +{
> +    int numerator; /**< aspect ratio numerator */
> +    int denominator; /**< aspect ratio denominator, 0 if the source default
> is used */
> +} libvlc_aspect_ratio_t;

AFAIK, denominator is unsigned, and structure seems overkill when there could 
just as well be two parameters.

> +
>  /**
>   * Get current video aspect ratio.
>   *
>   * \param p_mi the media player
> - * \return the video aspect ratio or NULL if unspecified
> + * \param out the video aspect ratio
>   * (the result must be released with free() or libvlc_free()).
>   */
> -LIBVLC_API char *libvlc_video_get_aspect_ratio( libvlc_media_player_t *p_mi
> );
> +LIBVLC_API void libvlc_video_get_sample_aspect_ratio(
> libvlc_media_player_t *p_mi, +                                             
>         libvlc_aspect_ratio_t *out );

How do you communicate unset A/R here?

> 
>  /**
>   * Set new video aspect ratio.
>   *
>   * \param p_mi the media player
> - * \param psz_aspect new video aspect-ratio or NULL to reset to default
> + * \param aspect new video aspect-ratio or NULL to reset to default
>   * \note Invalid aspect ratios are ignored.
>   */
> -LIBVLC_API void libvlc_video_set_aspect_ratio( libvlc_media_player_t *p_mi,
> const char *psz_aspect );
> +LIBVLC_API void
> libvlc_video_set_sample_aspect_ratio( libvlc_media_player_t *p_mi, +       
>                                               const libvlc_aspect_ratio_t
> *aspect );
> 
>  /**
>   * Create a video viewpoint structure.
> diff --git a/lib/libvlc.sym b/lib/libvlc.sym
> index 67b8efcba0..1ae26de315 100644
> --- a/lib/libvlc.sym
> +++ b/lib/libvlc.sym
> @@ -218,7 +218,7 @@ libvlc_toggle_fullscreen
>  libvlc_track_description_list_release
>  libvlc_video_get_adjust_float
>  libvlc_video_get_adjust_int
> -libvlc_video_get_aspect_ratio
> +libvlc_video_get_sample_aspect_ratio
>  libvlc_video_get_crop_geometry
>  libvlc_video_get_size
>  libvlc_video_get_cursor
> @@ -236,7 +236,7 @@ libvlc_video_get_track_count
>  libvlc_video_get_track_description
>  libvlc_video_set_adjust_float
>  libvlc_video_set_adjust_int
> -libvlc_video_set_aspect_ratio
> +libvlc_video_set_sample_aspect_ratio
>  libvlc_video_set_callbacks
>  libvlc_video_set_crop_geometry
>  libvlc_video_set_deinterlace
> diff --git a/lib/media_player.c b/lib/media_player.c
> index fd085c3793..ee7ff2e050 100644
> --- a/lib/media_player.c
> +++ b/lib/media_player.c
> @@ -618,7 +618,7 @@ libvlc_media_player_new( libvlc_instance_t *instance )
>      var_Create (mp, "fullscreen", VLC_VAR_BOOL);
>      var_Create (mp, "autoscale", VLC_VAR_BOOL | VLC_VAR_DOINHERIT);
>      var_Create (mp, "zoom", VLC_VAR_FLOAT | VLC_VAR_DOINHERIT);
> -    var_Create (mp, "aspect-ratio", VLC_VAR_STRING);
> +    var_Create (mp, "aspect-ratio", VLC_VAR_COORDS);
>      var_Create (mp, "crop", VLC_VAR_STRING);
>      var_Create (mp, "deinterlace", VLC_VAR_INTEGER | VLC_VAR_DOINHERIT);
>      var_Create (mp, "deinterlace-mode", VLC_VAR_STRING |
> VLC_VAR_DOINHERIT); diff --git a/lib/video.c b/lib/video.c
> index a318df0098..278c75eb9f 100644
> --- a/lib/video.c
> +++ b/lib/video.c
> @@ -229,18 +229,31 @@ void libvlc_video_set_scale( libvlc_media_player_t
> *p_mp, float f_scale ) free (pp_vouts);
>  }
> 
> -char *libvlc_video_get_aspect_ratio( libvlc_media_player_t *p_mi )
> +void libvlc_video_get_sample_aspect_ratio( libvlc_media_player_t *p_mi,
> +                                          libvlc_aspect_ratio_t *out )
>  {
> -    return var_GetNonEmptyString (p_mi, "aspect-ratio");
> +    int32_t num,den;
> +    var_GetCoords(p_mi, "aspect-ratio", &num, &den);
> +    out->numerator   = num;
> +    out->denominator = den;
>  }
> 
> -void libvlc_video_set_aspect_ratio( libvlc_media_player_t *p_mi,
> -                                    const char *psz_aspect )
> +void libvlc_video_set_sample_aspect_ratio( libvlc_media_player_t *p_mi,
> +                                           const libvlc_aspect_ratio_t
> *aspect ) {
> -    if (psz_aspect == NULL)
> -        psz_aspect = "";
> -    var_SetString (p_mi, "aspect-ratio", psz_aspect);
> 
> +    char *psz_aspect;
> +    if (aspect == NULL)
> +    {
> +        var_SetCoords(p_mi, "aspect-ratio", 0, 0);
> +        psz_aspect = "";
> +    }
> +    else
> +    {
> +        if (asprintf(&psz_aspect, "%d:%d", aspect->numerator,
> aspect->denominator) == -1 ) +            return;
> +        var_SetCoords(p_mi, "aspect-ratio", aspect->numerator,
> aspect->denominator); +    }
>      size_t n;
>      vout_thread_t **pp_vouts = GetVouts (p_mi, &n);
>      for (size_t i = 0; i < n; i++)
> @@ -251,6 +264,7 @@ void libvlc_video_set_aspect_ratio(
> libvlc_media_player_t *p_mi, vout_Release(p_vout);
>      }
>      free (pp_vouts);
> +    free(psz_aspect);
>  }
> 
>  libvlc_video_viewpoint_t *libvlc_video_new_viewpoint(void)


-- 
Реми Дёни-Курмон
http://www.remlab.net/