[vlc-devel] [PATCH] lib: map position enum to the correct subpicture bitwise mask values

[Rémi Denis-Courmont]

Le 2014-05-23 01:23, Mark Lee a écrit :
> libvlc_media_player_set_video_title_display() was wrongly using the
> enum value directly, leading to the video title appearing in the 
> wrong
> position for some values
> ---
>  lib/media_player.c | 36 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/lib/media_player.c b/lib/media_player.c
> index 67bbde7..fb7d758 100644
> --- a/lib/media_player.c
> +++ b/lib/media_player.c
> @@ -1449,8 +1449,42 @@ void
> libvlc_media_player_set_video_title_display( libvlc_media_player_t
> *p_mi, l
>  {
>      if ( position != libvlc_position_disable )
>      {
> +        int64_t pos;
> +        switch (position) {
> +            case libvlc_position_center:
> +                pos = 0;
> +                break;
> +            case libvlc_position_left:
> +                pos = SUBPICTURE_ALIGN_LEFT;
> +                break;
> +            case libvlc_position_right:
> +                pos = SUBPICTURE_ALIGN_RIGHT;
> +                break;
> +            case libvlc_position_top:
> +                pos = SUBPICTURE_ALIGN_TOP;
> +                break;
> +            case libvlc_position_top_left:
> +                pos = SUBPICTURE_ALIGN_TOP | SUBPICTURE_ALIGN_LEFT;
> +                break;
> +            case libvlc_position_top_right:
> +                pos = SUBPICTURE_ALIGN_TOP | SUBPICTURE_ALIGN_RIGHT;
> +                break;
> +            case libvlc_position_bottom:
> +                pos = SUBPICTURE_ALIGN_BOTTOM;
> +                break;
> +            case libvlc_position_bottom_left:
> +                pos = SUBPICTURE_ALIGN_BOTTOM | 
> SUBPICTURE_ALIGN_LEFT;
> +                break;
> +            case libvlc_position_bottom_right:
> +                pos = SUBPICTURE_ALIGN_BOTTOM | 
> SUBPICTURE_ALIGN_RIGHT;
> +                break;
> +            default:
> +                pos = 0;
> +                break;

I think a lookup table (preceded by a boundary check) makes more sense 
than a switch here. Also there should be no need for int64. Otherwise 
OK.

> +        }
> +
>          var_SetBool( p_mi, "video-title-show", true );
> -        var_SetInteger( p_mi, "video-title-position", position );
> +        var_SetInteger( p_mi, "video-title-position", pos );
>          var_SetInteger( p_mi, "video-title-timeout", timeout );
>      }
>      else

-- 
Rémi Denis-Courmont