[vlc-devel] [Patches] bluray menu

Laurent Aimar fenrir at elivagar.org
Tue Feb 14 21:11:18 CET 2012


Hi,

On Tue, Jan 24, 2012 at 12:30:05AM +0100, Hugo Beauzée-Luyssen wrote:
> Here are some new versions for bluray menu patches.

Sorry for the delay.

> Subject: [PATCH 01/11] bluray: Adding some event handling basics.
> 
> ---
>  modules/access/bluray.c |   63 +++++++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 56 insertions(+), 7 deletions(-)
> 
> diff --git a/modules/access/bluray.c b/modules/access/bluray.c
> index 7bfc3f0..443d884 100644
> --- a/modules/access/bluray.c
> +++ b/modules/access/bluray.c
> @@ -80,6 +80,9 @@ static int     blurayDemux  (demux_t *);
>  static int     blurayInitTitles(demux_t *p_demux );
>  static int     bluraySetTitle(demux_t *p_demux, int i_title);
>  
> +static void     blurayHandleEvent( demux_t *p_demux, const BD_EVENT *e );
> +static void     blurayHandleEvents( demux_t *p_demux );

IMHO, It will be better in the long term to avoid forward declaration.

> @@ -169,6 +172,11 @@ static int blurayOpen( vlc_object_t *object )
>          goto error;
>      }
>  
> +    /*
> +     * Initialize the event queue, so we can receive then in blurayDemux(Menu) later.
> +     */
> +    bd_get_event( p_sys->bluray, NULL );
> +
>      /* get title request */
>      if ((pos_title = strrchr(bd_path, ':'))) {
>          /* found character ':' for title information */
> @@ -258,9 +266,17 @@ static int blurayInitTitles(demux_t *p_demux )
>          TAB_APPEND( p_sys->i_title, p_sys->pp_title, t );
>          bd_free_title_info(title_info);
>      }
> +
Cosmetic.
>      return VLC_SUCCESS;
>  }
>  
> +static void blurayUpdateTitle( demux_t *p_demux, int i_title )
> +{
> +    /* read title info and init some values */
> +    p_demux->info.i_title = i_title;
> +    p_demux->info.i_seekpoint = 0;
> +    p_demux->info.i_update |= INPUT_UPDATE_TITLE | INPUT_UPDATE_SEEKPOINT;
> +}
>  
>  /*****************************************************************************
>   * bluraySetTitle: select new BD title
> @@ -282,11 +298,7 @@ static int bluraySetTitle(demux_t *p_demux, int i_title)
>          msg_Err(p_demux, "cannot select bd title '%d'", p_demux->info.i_title);
>          return VLC_EGENERIC;
>      }
> -
> -    /* read title info and init some values */
> -    p_demux->info.i_title = i_title;
> -    p_demux->info.i_seekpoint = 0;
> -    p_demux->info.i_update |= INPUT_UPDATE_TITLE | INPUT_UPDATE_SEEKPOINT;
> +    blurayUpdateTitle( p_demux, i_title );
>  
>      return VLC_SUCCESS;
>  }
> @@ -357,7 +369,7 @@ static int blurayControl(demux_t *p_demux, int query, va_list args)
>          case DEMUX_GET_LENGTH:
>          {
>              int64_t *pi_length = (int64_t*)va_arg(args, int64_t *);
> -            *pi_length = CUR_LENGTH;
> +            *pi_length = p_demux->info.i_title < p_sys->i_title ? CUR_LENGTH : 0;
>              return VLC_SUCCESS;
>          }
>          case DEMUX_SET_TIME:
> @@ -376,7 +388,8 @@ static int blurayControl(demux_t *p_demux, int query, va_list args)
>          case DEMUX_GET_POSITION:
>          {
>              double *pf_position = (double*)va_arg( args, double * );
> -            *pf_position = (double)FROM_TICKS(bd_tell_time(p_sys->bluray))/CUR_LENGTH;
> +            *pf_position = p_demux->info.i_title < p_sys->i_title ?
> +                        (double)FROM_TICKS(bd_tell_time(p_sys->bluray))/CUR_LENGTH : 0.0;
>              return VLC_SUCCESS;
>          }
>          case DEMUX_SET_POSITION:
> @@ -423,6 +436,41 @@ static int blurayControl(demux_t *p_demux, int query, va_list args)
>      return VLC_SUCCESS;
>  }
>  
> +static void     blurayHandleEvent( demux_t *p_demux, const BD_EVENT *e )
> +{
> +    demux_sys_t *p_sys = p_demux->p_sys;
> +
> +    switch( e->event )
> +    {
> +        case BD_EVENT_TITLE:
> +            if ( e->param < p_sys->i_title )
> +                blurayUpdateTitle( p_demux, e->param );
> +            break ;
> +        case BD_EVENT_PLAYITEM:
> +            break ;
> +        case BD_EVENT_AUDIO_STREAM:
> +            break ;
> +        case BD_EVENT_CHAPTER:
> +            p_demux->info.i_update |= INPUT_UPDATE_SEEKPOINT;
> +            p_demux->info.i_seekpoint = 0;
> +            break ;
> +        case BD_EVENT_ANGLE:
> +        case BD_EVENT_IG_STREAM:
> +        default:
> +            msg_Warn( p_demux, "event: %d param: %d", e->event, e->param );
> +            break ;
> +    }
> +}
> +
> +static void    blurayHandleEvents( demux_t *p_demux )
> +{
> +    BD_EVENT    e;
> +
> +    while ( bd_get_event( p_demux->p_sys->bluray, &e ) )
> +    {
> +        blurayHandleEvent( p_demux, &e );
> +    }
> +}
>  
>  #define BD_TS_PACKET_SIZE (192)
>  #define NB_TS_PACKETS (200)
> @@ -436,6 +484,7 @@ static int blurayDemux(demux_t *p_demux)
>          return -1;
>      }
>  
> +    blurayHandleEvents( p_demux );
>      int nread = bd_read(p_sys->bluray, p_block->p_buffer,
>                          NB_TS_PACKETS * BD_TS_PACKET_SIZE);
>      if (nread < 0) {

Otherwise OK.


> Subject: [PATCH 02/11] bluray: Fixing a warning
> 
> ---
>  modules/access/bluray.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/modules/access/bluray.c b/modules/access/bluray.c
> index 443d884..ea1bca8 100644
> --- a/modules/access/bluray.c
> +++ b/modules/access/bluray.c
> @@ -401,7 +401,7 @@ static int blurayControl(demux_t *p_demux, int query, va_list args)
>  
>          case DEMUX_GET_META:
>          {
> -            struct meta_dl *meta = bd_get_meta(p_sys->bluray);
> +            const struct meta_dl *meta = bd_get_meta(p_sys->bluray);
>              if(!meta)
>                  return VLC_EGENERIC;
>  
OK.


> Subject: [PATCH 03/11] bluray: Preparing menu handling.
> 
> ---
>  modules/access/bluray.c |   76 +++++++++++++++++++++++++++++++++++++++-------
>  1 files changed, 64 insertions(+), 12 deletions(-)
> 
> diff --git a/modules/access/bluray.c b/modules/access/bluray.c
> index ea1bca8..9733725 100644
> --- a/modules/access/bluray.c
> +++ b/modules/access/bluray.c
> @@ -35,11 +35,16 @@
>  
>  #include <libbluray/bluray.h>
>  #include <libbluray/meta_data.h>
> +#include <libbluray/overlay.h>

Is this include needed for this patch 

> @@ -75,7 +81,8 @@ struct demux_sys_t
>   * Local prototypes
>   *****************************************************************************/
>  static int     blurayControl(demux_t *, int, va_list);
> -static int     blurayDemux  (demux_t *);
> +static int     blurayDemux(demux_t *);
> +static int     blurayDemuxMenu(demux_t *);
Forward declaration.

> @@ -197,7 +216,7 @@ static int blurayOpen( vlc_object_t *object )
>      }
>  
>      p_demux->pf_control = blurayControl;
> -    p_demux->pf_demux   = blurayDemux;
> +    p_demux->pf_demux   = b_menu ? blurayDemuxMenu : blurayDemux;
>  
>      return VLC_SUCCESS;
>  
> @@ -484,17 +503,50 @@ static int blurayDemux(demux_t *p_demux)
>          return -1;
>      }
>  
> +    //Handle events first.
>      blurayHandleEvents( p_demux );
> +
>      int nread = bd_read(p_sys->bluray, p_block->p_buffer,
>                          NB_TS_PACKETS * BD_TS_PACKET_SIZE);
> +
>      if (nread < 0) {
>          block_Release(p_block);
>          return nread;
>      }
> -
Some cosmetics.

> +static int blurayDemuxMenu( demux_t *p_demux )
> +{
> +    demux_sys_t *p_sys = p_demux->p_sys;
> +    BD_EVENT    e;
> +
> +    block_t     *p_block = block_New(p_demux, NB_TS_PACKETS * (int64_t)BD_TS_PACKET_SIZE);
> +    int         nread = bd_read_ext( p_sys->bluray, p_block->p_buffer,
> +                                      NB_TS_PACKETS * BD_TS_PACKET_SIZE, &e );
> +    /*
> +     * bd_read_ext will return 0 if there is some events to process.
> +     * We need to handle them before doing anything else
> +     */
> +    if ( nread == 0 )
> +    {
> +        if ( e.event == BD_EVENT_NONE )
> +            msg_Info( p_demux, "We reached the end of a title" );
> +        else
> +            blurayHandleEvent( p_demux, &e );
> +        block_Release(p_block);
> +        return 1;
> +    }
> +    if (nread < 0)
> +    {
> +        block_Release(p_block);
> +        return nread;
> +    }
> +    p_block->i_buffer = nread;
>  
> +    stream_DemuxSend( p_sys->p_parser, p_block );
>      return 1;
>  }
Is it worth it to create another demux function ?



> Subject: [PATCH 04/11] bluray: Adding an overlay handling skeleton.
> 
> ---
>  modules/access/bluray.c |   15 +++++++++++++--
>  1 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/modules/access/bluray.c b/modules/access/bluray.c
> index 9733725..c6a2243 100644
> --- a/modules/access/bluray.c
> +++ b/modules/access/bluray.c
> @@ -87,8 +87,10 @@ static int     blurayDemuxMenu(demux_t *);
>  static int     blurayInitTitles(demux_t *p_demux );
>  static int     bluraySetTitle(demux_t *p_demux, int i_title);
>  
> -static void     blurayHandleEvent( demux_t *p_demux, const BD_EVENT *e );
> -static void     blurayHandleEvents( demux_t *p_demux );
> +static void    blurayHandleEvent( demux_t *p_demux, const BD_EVENT *e );
> +static void    blurayHandleEvents( demux_t *p_demux );
> +
Cosmetics.

> +static void    blurayOverlayProc( void *ptr, const BD_OVERLAY * const overlay );
Forward declaration.

>  #define FROM_TICKS(a) (a*CLOCK_FREQ / INT64_C(90000))
>  #define TO_TICKS(a)   (a*INT64_C(90000)/CLOCK_FREQ)
> @@ -192,6 +194,8 @@ static int blurayOpen( vlc_object_t *object )
>           * Therefore, We don't have to select any title.
>           */
>          bd_play( p_sys->bluray );
> +        /* Registering overlay event handler */
> +        bd_register_overlay_proc( p_sys->bluray, p_demux, blurayOverlayProc );
>      }
>      else
>      {
> @@ -250,6 +254,13 @@ static void blurayClose( vlc_object_t *object )
>      free(p_sys);
>  }
>  
> +static void blurayOverlayProc( void *ptr, const BD_OVERLAY *const overlay )
> +{
> +    demux_t     *p_demux = (demux_t*)ptr;
> +    demux_sys_t *p_sys = p_demux->p_sys;
> +
> +    //FIXME
> +}
I think that this patch should be merged with some following ones (I
haven't checked which one would make the most senses).


> Subject: [PATCH 05/11] bluray: Nasty hack to avoid crashing when changing
>  title.
> 
> ---
>  modules/access/bluray.c |   17 +++++++++++++++--
>  1 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/modules/access/bluray.c b/modules/access/bluray.c
> index c6a2243..6d8e1bd 100644
> --- a/modules/access/bluray.c
> +++ b/modules/access/bluray.c
> @@ -300,8 +300,22 @@ static int blurayInitTitles(demux_t *p_demux )
>      return VLC_SUCCESS;
>  }
>  
> +static void blurayResetParser( demux_t *p_demux )
> +{
> +    demux_sys_t *p_sys = p_demux->p_sys;
> +    if ( p_sys->p_parser )
> +        stream_Delete( p_sys->p_parser );
> +    p_sys->p_parser = stream_DemuxNew(p_demux, "ts", p_demux->out);
> +    if (!p_sys->p_parser) {
> +        msg_Err(p_demux, "Failed to create TS demuxer");
> +    }
> +}
> +
>  static void blurayUpdateTitle( demux_t *p_demux, int i_title )
>  {
> +    blurayResetParser( p_demux );
> +    if ( i_title >= p_demux->p_sys->i_title )
> +        return ;
>      /* read title info and init some values */
>      p_demux->info.i_title = i_title;
>      p_demux->info.i_seekpoint = 0;
> @@ -473,8 +487,7 @@ static void     blurayHandleEvent( demux_t *p_demux, const BD_EVENT *e )
>      switch( e->event )
>      {
>          case BD_EVENT_TITLE:
> -            if ( e->param < p_sys->i_title )
> -                blurayUpdateTitle( p_demux, e->param );
> +            blurayUpdateTitle( p_demux, e->param );
>              break ;
>          case BD_EVENT_PLAYITEM:
>              break ;
Why is this patch needed? The comment make it look like an workaround
instead of a proper fix.


> Subject: [PATCH 06/11] bluray: Adding support for overlay.
> @@ -4,6 +4,7 @@
>   * Copyright © 2010-2011 VideoLAN, VLC authors and libbluray AUTHORS
>   *
>   * Authors: Jean-Baptiste Kempf <jb at videolan.org>
> + *          Hugo Beauzée-Luyssen <hugo at videolan.org>
It doesn't belong to this patch.

> @@ -32,6 +33,8 @@
>  #include <vlc_demux.h>                      /* demux_t */
>  #include <vlc_input.h>                      /* Seekpoints, chapters */
>  #include <vlc_dialog.h>                     /* BD+/AACS warnings */
> +#include <vlc_picture.h>
> +#include <vlc_vout.h>
>  
>  #include <libbluray/bluray.h>
>  #include <libbluray/meta_data.h>
> @@ -63,6 +66,8 @@ vlc_module_begin ()
>      set_callbacks( blurayOpen, blurayClose )
>  vlc_module_end ()
>  
> +//libbluray's overlay.h defines 2 types of overlay.
> +#define MAX_OVERLAY 2
 Shouldn't it then based on a define of libbluray's overlay.h?

> @@ -73,6 +78,13 @@ struct demux_sys_t
>      unsigned int    i_longest_title;
>      input_title_t **pp_title;
>  
> +    /* Overlay stuff */
> +    subpicture_t    *p_pic[MAX_OVERLAY];
> +    int             current_overlay; // -1 if no current overlay;
> +
> +    input_thread_t  *p_input;
> +    vout_thread_t   *p_vout;
> +
>      /* TS stream */
>      stream_t       *p_parser;
>  };
> @@ -119,7 +131,8 @@ static int blurayOpen( vlc_object_t *object )
>      if (unlikely(!p_sys)) {
>          return VLC_ENOMEM;
>      }
> -    p_sys->p_parser = NULL;
> +    memset( p_sys, 0, sizeof( *p_sys ) );
 Use calloc instead of mallocing p_sys then.

> +    p_sys->current_overlay = -1;
>  
>      /* init demux info fields */
>      p_demux->info.i_update    = 0;
> @@ -240,6 +256,18 @@ static void blurayClose( vlc_object_t *object )
>      demux_t *p_demux = (demux_t*)object;
>      demux_sys_t *p_sys = p_demux->p_sys;
>  
> +    /*
> +     * Close libbluray first. This will close all the overlays before we release p_vout
> +     *
> +     * bd_close( NULL ) can crash
> +     */
> +    assert(p_sys->bluray);
> +    bd_close(p_sys->bluray);
> +
> +    if ( p_sys->p_vout != NULL )
> +        vlc_object_release( p_sys->p_vout );
> +    if ( p_sys->p_input != NULL )
> +        vlc_object_release( p_sys->p_input );
>      if (p_sys->p_parser)
>          stream_Delete(p_sys->p_parser);
>  
> @@ -248,18 +276,147 @@ static void blurayClose( vlc_object_t *object )
>          vlc_input_title_Delete(p_sys->pp_title[i]);
>      TAB_CLEAN( p_sys->i_title, p_sys->pp_title );
>  
> -    /* bd_close( NULL ) can crash */
> -    assert(p_sys->bluray);
> -    bd_close(p_sys->bluray);
>      free(p_sys);
>  }
>  
> +static void blurayDeleteOverlaySubPic( demux_t *p_demux, int i_plane )
> +{
> +    demux_sys_t     *p_sys = p_demux->p_sys;
> +
> +    if ( p_sys->p_pic[i_plane] == NULL )
> +        return ;
It's a cosmetic remarks, but it might be better to change i_plane into
i_layer or something like that (unless the libluray does use plane of
course). For (sub)picture, plane usually means the pixel plane.

> +    if ( p_sys->p_vout )
> +        vout_FlushSubpictureChannel( p_sys->p_vout, p_sys->p_pic[i_plane]->i_channel );
> +    else
> +        subpicture_Delete( p_sys->p_pic[i_plane] );
> +    p_sys->p_pic[i_plane] = NULL;
> +}
 The subpicture_Delete() being called only in one place looks suspicious.

> +static void blurayCloseOverlay( demux_t *p_demux )
> +{
> +    demux_sys_t     *p_sys = p_demux->p_sys;
> +
> +    for ( int i = 0; i < MAX_OVERLAY; ++i )
> +        blurayDeleteOverlaySubPic( p_demux, i );
> +    p_demux->p_sys->current_overlay = -1;
> +    if ( p_sys->p_vout != NULL )
> +    {
> +        vlc_object_release( p_sys->p_vout );
> +        p_sys->p_vout = NULL;
> +    }
> +}
> +
> +static void blurayDisplayOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
> +{
> +    demux_sys_t     *p_sys = p_demux->p_sys;
> +    /*
> +     * Mark the overlay as available, but don't display it right now.
> +     * the blurayDemuxMenu will send it to vout, as it may be unavailable when
> +     * the overlay is computed
> +     */
> +    p_sys->current_overlay = ov->plane;
> +}
> +
> +static void blurayInitOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
> +{
> +    demux_sys_t     *p_sys = p_demux->p_sys;
> +
> +    if ( p_sys->p_pic[ov->plane] != NULL )
> +        blurayDeleteOverlaySubPic( p_demux, ov->plane );
> +
> +    p_sys->p_pic[ov->plane] = subpicture_New( NULL );
> +    p_sys->p_pic[ov->plane]->i_original_picture_width = ov->w;
> +    p_sys->p_pic[ov->plane]->i_original_picture_height = ov->h;
> +    p_sys->p_pic[ov->plane]->b_ephemer = true;
> +    p_sys->p_pic[ov->plane]->b_absolute = true;
> +}
> +
> +static void blurayClearOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
> +{
> +    demux_sys_t     *p_sys = p_demux->p_sys;
> +
> +    //FIXME
> +}
> +
> +static void blurayDrawOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
> +{
> +    demux_sys_t     *p_sys = p_demux->p_sys;
> +
> +    if ( ov->img )
 You can reverse the test, it will avoid a full level of indentention.
> +    {
> +        video_format_t          fmt;
> +        video_format_Init( &fmt, 0 );
> +        video_format_Setup( &fmt, VLC_CODEC_YUVP, ov->w, ov->h, 1, 1 );
> +
> +        subpicture_region_t     *region = subpicture_region_New( &fmt );
> +        region->i_x = ov->x;
> +        region->i_y = ov->y;
> +
> +        const BD_PG_RLE_ELEM    *img = ov->img;
> +        for ( int y = 0; y < ov->h; y++ )
> +        {
> +            for ( int x = 0; x < ov->w; )
> +            {
> +                memset( region->p_picture->p[0].p_pixels +
> +                        y * region->p_picture->p[0].i_pitch + x,
> +                        img->color, img->len );
> +                x += img->len;
> +                img++;
> +            }
> +        }
> +        if ( ov->palette )
> +        {
> +            region->fmt.p_palette->i_entries = 256;
> +            for ( int i = 0; i < 256; ++i )
> +            {
> +                region->fmt.p_palette->palette[i][0] = ov->palette[i].Y;
> +                region->fmt.p_palette->palette[i][1] = ov->palette[i].Cb;
> +                region->fmt.p_palette->palette[i][2] = ov->palette[i].Cr;
> +                region->fmt.p_palette->palette[i][3] = ov->palette[i].T;
> +            }
> +        }
> +        //Append the new region at the end of our current regions:
> +        subpicture_region_t *p_reg = p_sys->p_pic[ov->plane]->p_region;
> +        if ( p_reg == NULL )
> +        {
> +            p_sys->p_pic[ov->plane]->p_region = region;
> +            return ;
> +        }
> +        while ( p_reg && p_reg->p_next )
> +            p_reg = p_reg->p_next;
> +        p_reg->p_next = region;
> +    }
> +}
> +
>  static void blurayOverlayProc( void *ptr, const BD_OVERLAY *const overlay )
>  {
>      demux_t     *p_demux = (demux_t*)ptr;
> -    demux_sys_t *p_sys = p_demux->p_sys;
>  
> -    //FIXME
> +    if ( overlay == NULL )
> +    {
> +        msg_Info( p_demux, "Closing overlay." );
> +        blurayCloseOverlay( p_demux );
> +        return ;
> +    }
> +    switch ( overlay->cmd )
> +    {
> +        case BD_OVERLAY_INIT:
> +            blurayInitOverlay( p_demux, overlay );
> +            break ;
> +        case BD_OVERLAY_CLEAR:
> +            blurayClearOverlay( p_demux, overlay );
> +            break ;
> +        case BD_OVERLAY_FLUSH:
> +            msg_Info( p_demux, "Displaying overlay" );
> +            blurayDisplayOverlay( p_demux, overlay );
> +            break ;
> +        case BD_OVERLAY_DRAW:
> +            blurayDrawOverlay( p_demux, overlay );
> +            break ;
> +        default:
> +            msg_Warn( p_demux, "Unknown BD overlay command: %u", overlay->cmd );
> +            break ;
> +    }
>  }
>  
>  static int blurayInitTitles(demux_t *p_demux )
> @@ -557,11 +714,9 @@ static int blurayDemuxMenu( demux_t *p_demux )
>       */
>      if ( nread == 0 )
>      {
> -        if ( e.event == BD_EVENT_NONE )
> -            msg_Info( p_demux, "We reached the end of a title" );
> -        else
> +        if ( e.event != BD_EVENT_NONE )
>              blurayHandleEvent( p_demux, &e );
> -        block_Release(p_block);
> +        block_Release( p_block );
>          return 1;
>      }
>      if (nread < 0)
> @@ -571,6 +726,20 @@ static int blurayDemuxMenu( demux_t *p_demux )
>      }
>      p_block->i_buffer = nread;
>  
> +    if ( p_sys->current_overlay != -1 )
> +    {
> +        if ( p_sys->p_vout == NULL )
> +            p_sys->p_vout = input_GetVout( p_sys->p_input );
> +        if ( p_sys->p_vout != NULL )
> +        {
> +            p_sys->p_pic[p_sys->current_overlay]->i_start =
> +                    p_sys->p_pic[p_sys->current_overlay]->i_stop = mdate();
> +            p_sys->p_pic[p_sys->current_overlay]->i_channel =
> +                    vout_RegisterSubpictureChannel( p_sys->p_vout );
> +            vout_PutSubpicture( p_sys->p_vout, p_sys->p_pic[p_sys->current_overlay] );
That's not valid, a picture sent to the vout must not be accessed later.
You must merged the part that properly does that from the next patch into
this one.


> Subject: [PATCH 07/11] bluray: Handle mouse event.
> 
> ---
>  modules/access/bluray.c |   69 +++++++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 61 insertions(+), 8 deletions(-)
> 
> diff --git a/modules/access/bluray.c b/modules/access/bluray.c
> index 15f62d3..1f78975 100644
> --- a/modules/access/bluray.c
> +++ b/modules/access/bluray.c
> @@ -37,6 +37,7 @@
>  #include <vlc_vout.h>
>  
>  #include <libbluray/bluray.h>
> +#include <libbluray/keys.h>
>  #include <libbluray/meta_data.h>
>  #include <libbluray/overlay.h>
>  
> @@ -104,6 +105,9 @@ static void    blurayHandleEvents( demux_t *p_demux );
>  
>  static void    blurayOverlayProc( void *ptr, const BD_OVERLAY * const overlay );
>  
> +static int     onMouseEvent( vlc_object_t *p_vout, const char *psz_var, vlc_value_t old,
> +                                vlc_value_t val, void *p_data );
> +
Forward declaration.

> +/*****************************************************************************
> + * libbluray overlay handling:
> + *****************************************************************************/
> +
Does not belong to this patch (should be merged with the right patch).
>  static void blurayDeleteOverlaySubPic( demux_t *p_demux, int i_plane )
>  {
>      demux_sys_t     *p_sys = p_demux->p_sys;
> @@ -301,12 +309,14 @@ static void blurayCloseOverlay( demux_t *p_demux )
>      p_demux->p_sys->current_overlay = -1;
>      if ( p_sys->p_vout != NULL )
>      {
> +        var_DelCallback( p_sys->p_vout, "mouse-moved", &onMouseEvent, p_demux );
> +        var_DelCallback( p_sys->p_vout, "mouse-clicked", &onMouseEvent, p_demux );
>          vlc_object_release( p_sys->p_vout );
>          p_sys->p_vout = NULL;
>      }
>  }
>  
> -static void blurayDisplayOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
> +static void blurayActivateOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
>  {
>      demux_sys_t     *p_sys = p_demux->p_sys;
>      /*
> @@ -408,7 +418,7 @@ static void blurayOverlayProc( void *ptr, const BD_OVERLAY *const overlay )
>              break ;
>          case BD_OVERLAY_FLUSH:
>              msg_Info( p_demux, "Displaying overlay" );
> -            blurayDisplayOverlay( p_demux, overlay );
> +            blurayActivateOverlay( p_demux, overlay );
>              break ;
>          case BD_OVERLAY_DRAW:
>              blurayDrawOverlay( p_demux, overlay );
> @@ -419,6 +429,54 @@ static void blurayOverlayProc( void *ptr, const BD_OVERLAY *const overlay )
>      }
>  }
>  
> +/*****************************************************************************
> + * overlay display and events:
> + *****************************************************************************/
> +
> +static int  onMouseEvent( vlc_object_t *p_vout, const char *psz_var, vlc_value_t old,
> +                          vlc_value_t val, void *p_data )
> +{
> +    demux_t     *p_demux = (demux_t*)p_data;
> +    demux_sys_t *p_sys = p_demux->p_sys;
> +    VLC_UNUSED( old );
> +    VLC_UNUSED( p_vout );
> +
> +    if ( psz_var[6] == 'm' ) //Mouse moved
> +        bd_mouse_select( p_sys->bluray, mdate(), val.coords.x, val.coords.y );
> +    else if ( psz_var[6] == 'c' )
> +    {
> +        int64_t     now = mdate();
It could be moved a bit higher to be reused.
> +        bd_mouse_select( p_sys->bluray, now, val.coords.x, val.coords.y );
> +        bd_user_input( p_sys->bluray, now, BD_VK_MOUSE_ACTIVATE );
> +    }
> +    else
> +    {
> +        msg_Warn( p_demux, "Unknown mouse event: %s", psz_var );
> +        return VLC_EGENERIC;
 You could assert instead as it cannot happens.
> +    }
> +    return VLC_SUCCESS;
> +}
> +
> +static void blurayDisplayOverlay( demux_t *p_demux )
> +{
> +    demux_sys_t *p_sys = p_demux->p_sys;
> +
> +    //Sending subpicture to vout:
> +    p_sys->p_pic[p_sys->current_overlay]->i_start =
> +            p_sys->p_pic[p_sys->current_overlay]->i_stop = mdate();
> +    p_sys->p_pic[p_sys->current_overlay]->i_channel =
> +            vout_RegisterSubpictureChannel( p_sys->p_vout );
> +    vout_PutSubpicture( p_sys->p_vout, p_sys->p_pic[p_sys->current_overlay] );
 Still not valid.
> +    p_sys->current_overlay = -1;
> +    //Activating vout events handling:
> +    var_AddCallback( p_sys->p_vout, "mouse-moved", &onMouseEvent, p_demux );
> +    var_AddCallback( p_sys->p_vout, "mouse-clicked", &onMouseEvent, p_demux );
I am unsure, but I don't think you should do it everytime (unless you do unregister
the same amounts of time). It would be cleaner to attach them when you get the vout
variable (and detach them when you release it).

> +/*****************************************************************************
> + * Titles management:
> + *****************************************************************************/
> +
 It does not belong to this patch.

>  static int blurayInitTitles(demux_t *p_demux )
>  {
>      demux_sys_t *p_sys = p_demux->p_sys;
> @@ -732,12 +790,7 @@ static int blurayDemuxMenu( demux_t *p_demux )
>              p_sys->p_vout = input_GetVout( p_sys->p_input );
>          if ( p_sys->p_vout != NULL )
>          {
> -            p_sys->p_pic[p_sys->current_overlay]->i_start =
> -                    p_sys->p_pic[p_sys->current_overlay]->i_stop = mdate();
> -            p_sys->p_pic[p_sys->current_overlay]->i_channel =
> -                    vout_RegisterSubpictureChannel( p_sys->p_vout );
> -            vout_PutSubpicture( p_sys->p_vout, p_sys->p_pic[p_sys->current_overlay] );
> -            p_sys->current_overlay = -1;
> +            blurayDisplayOverlay( p_demux );
It would be better to have introduce the blurayDisplayOverlay() function earlier.
>          }
>      }
>      stream_DemuxSend( p_sys->p_parser, p_block );

I will review the other patches once the remarks about the ones in this
mail has been taken care of (it will be easier for every one I think).

Regards,

-- 
fenrir



More information about the vlc-devel mailing list