[vlc-devel] [Patches] bluray menu

Laurent Aimar fenrir at elivagar.org
Tue Feb 28 21:34:47 CET 2012


Hi,

On Sat, Feb 18, 2012 at 01:24:50AM +0100, Hugo Beauzée-Luyssen wrote:
> >>> 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
> >>> @@ -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.
> >>
> > Most of the time, Open() function is the first defined function, in
> > this case, a forward declaration is mandatory.
> > If you wish I can move open/close at the bottom of the file, I'll let
> > you decide.
 I think that if it allows to avoid a bunch of forward declaration,
then it is worth it.

> >>> +static int blurayDemuxMenu( demux_t *p_demux )
> >>> +{
> >>>  }
> >> Is it worth it to create another demux function ?
> >>
> > I find it more easy to read, but I can merge both. But I think there
> > is no need to test if we are using menu for each block. However, this
> > is mostly a matter of tastes.
> > Again, I'll let you chose.
 IMHO the main question is if it is ease maintenance or not.


Overall, for all the patches, it would be great to respect the current coding
style of bluray.c 

> Subject: [PATCH 01/12] bluray: Adding some event handling basics.
> 
> ---
>  modules/access/bluray.c |   59 +++++++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 52 insertions(+), 7 deletions(-)
> 
> diff --git a/modules/access/bluray.c b/modules/access/bluray.c
> index 2f9ef6f..f8692bd 100644
> --- a/modules/access/bluray.c
> +++ b/modules/access/bluray.c
> @@ -193,6 +193,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 */
> @@ -285,6 +290,13 @@ static int blurayInitTitles(demux_t *p_demux )
>      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
> @@ -306,11 +318,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;
>  }
> @@ -381,7 +389,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:
> @@ -400,7 +408,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:
> @@ -447,6 +456,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)
> @@ -460,6 +504,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) {
> -- 
> 1.7.9.1
Ok.


> Subject: [PATCH 02/12] 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 f8692bd..2995773 100644
> --- a/modules/access/bluray.c
> +++ b/modules/access/bluray.c
> @@ -421,7 +421,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;
>  
> -- 
> 1.7.9.1
Ok.


> Subject: [PATCH 03/12] bluray: Preparing menu handling.
> 
> ---
>  modules/access/bluray.c |   72 +++++++++++++++++++++++++++++++++++++++-------
>  1 files changed, 61 insertions(+), 11 deletions(-)
> 
> diff --git a/modules/access/bluray.c b/modules/access/bluray.c
> index 2995773..acac745 100644
> --- a/modules/access/bluray.c
> +++ b/modules/access/bluray.c
> @@ -44,6 +44,10 @@
>   * Module descriptor
>   *****************************************************************************/
>  
> +#define BD_MENU_TEXT        N_( "Bluray menus" )
> +#define BD_MENU_LONGTEXT    N_( "Use bluray menus. If disabled, "\
> +                                "the movie will start directly" )
> +
>  /* Callbacks */
>  static int  blurayOpen ( vlc_object_t * );
>  static void blurayClose( vlc_object_t * );
> @@ -55,6 +59,7 @@ vlc_module_begin ()
>      set_category( CAT_INPUT )
>      set_subcategory( SUBCAT_INPUT_ACCESS )
>      set_capability( "access_demux", 200)
> +    add_bool( "bluray-menu", false, BD_MENU_TEXT, BD_MENU_LONGTEXT, false )
>  
>      add_shortcut( "bluray", "file" )
>  
> @@ -79,7 +84,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 *);
>  
>  static int     blurayInitTitles(demux_t *p_demux );
>  static int     bluraySetTitle(demux_t *p_demux, int i_title);
> @@ -198,17 +204,29 @@ static int blurayOpen( vlc_object_t *object )
>       */
>      bd_get_event( p_sys->bluray, NULL );
>  
> -    /* get title request */
> -    if ((pos_title = strrchr(bd_path, ':'))) {
> -        /* found character ':' for title information */
> -        *(pos_title++) = '\0';
> -        i_title = atoi(pos_title);
> +    bool b_menu = var_InheritBool( p_demux, "bluray-menu" );
> +    if ( b_menu )
> +    {
> +        /*
> +         * libbluray will start playback from "First-Title" title
> +         * Therefore, We don't have to select any title.
> +         */
> +        bd_play( p_sys->bluray );
>      }
> +    else
> +    {
> +        /* get title request */
> +        if ((pos_title = strrchr(bd_path, ':'))) {
> +            /* found character ':' for title information */
> +            *(pos_title++) = '\0';
> +            i_title = atoi(pos_title);
 It's not related to this patch in itself but I don't think this part
is needed. The core input code does already parse the MRL to extract
the title/chapter part (see MRLSections in src/input/input.c).

> +        }
>  
> -    /* set start title number */
> -    if (bluraySetTitle(p_demux, i_title) != VLC_SUCCESS) {
> -        msg_Err( p_demux, "Could not set the title %d", i_title );
> -        goto error;
> +        /* set start title number */
> +        if (bluraySetTitle(p_demux, i_title) != VLC_SUCCESS) {
> +            msg_Err( p_demux, "Could not set the title %d", i_title );
> +            goto error;
> +        }
>      }
>  
>      p_sys->p_parser   = stream_DemuxNew(p_demux, "ts", p_demux->out);
> @@ -218,7 +236,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;
>  
> @@ -504,6 +522,7 @@ 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);
> @@ -515,6 +534,37 @@ static int blurayDemux(demux_t *p_demux)
>      p_block->i_buffer = nread;
>  
>      stream_DemuxSend( p_sys->p_parser, p_block );
> +    return 1;
> +}
> +
> +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;
>  }
> -- 
> 1.7.9.1

Except the answer to the question about blurayDemuxMenu(), OK.


> Subject: [PATCH 04/12] bluray: Nasty hack to avoid crashing when changing
>  title.
> 
> ---
>  modules/access/bluray.c |   23 +++++++++++++++++++++--
>  1 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/modules/access/bluray.c b/modules/access/bluray.c
> index acac745..199f83f 100644
> --- a/modules/access/bluray.c
> +++ b/modules/access/bluray.c
> @@ -308,8 +308,28 @@ static int blurayInitTitles(demux_t *p_demux )
>      return VLC_SUCCESS;
>  }
>  
> +static void blurayResetParser( demux_t *p_demux )
> +{
> +    /*
> +     * This is a hack and will have to be removed.
> +     * The parser should be flushed, and not destroy/created each time
> +     * we are changing title.
> +     */
> +    demux_sys_t *p_sys = p_demux->p_sys;
> +    if ( p_sys->p_parser == NULL )
> +        return ;
> +    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;
> @@ -481,8 +501,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 ;
> -- 
> 1.7.9.1
OK.

> Subject: [PATCH 05/12] bluray: Adding support for overlay.
> 
> ---
>  modules/access/bluray.c |  436 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 427 insertions(+), 9 deletions(-)
> 
> diff --git a/modules/access/bluray.c b/modules/access/bluray.c
> index 199f83f..ca5b110 100644
> --- a/modules/access/bluray.c
> +++ b/modules/access/bluray.c
> @@ -36,9 +36,11 @@
>  #include <vlc_demux.h>                      /* demux_t */
>  #include <vlc_input.h>                      /* Seekpoints, chapters */
>  #include <vlc_dialog.h>                     /* BD+/AACS warnings */
> +#include <vlc_vout.h>                       /* vout_PutSubpicture / subpicture_t */
>  
>  #include <libbluray/bluray.h>
>  #include <libbluray/meta_data.h>
> +#include <libbluray/overlay.h>
>  
>  /*****************************************************************************
>   * Module descriptor
> @@ -66,6 +68,16 @@ vlc_module_begin ()
>      set_callbacks( blurayOpen, blurayClose )
>  vlc_module_end ()
>  
> +/* libbluray's overlay.h defines 2 types of overlay (bd_overlay_plane_e). */
> +#define MAX_OVERLAY 2
> +
> +typedef enum    OverlayStatus
> +{
> +    Closed = 0,
> +    ToDisplay,  //Used to mark the overlay to be displayed the first time.
> +    Displayed,
> +    Outdated    //used to update the overlay after it has been sent to the vout
> +}               OverlayStatus;
>  
>  struct demux_sys_t
>  {
> @@ -76,10 +88,26 @@ struct demux_sys_t
>      unsigned int    i_longest_title;
>      input_title_t **pp_title;
>  
> +    /* Overlay stuff */
> +    vlc_mutex_t             overlay_lock;
> +    subpicture_t            *p_pic[MAX_OVERLAY];
> +    OverlayStatus           overlay_status[MAX_OVERLAY];
> +    subpicture_region_t     *p_regions[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;
>  };
>  
> +struct subpicture_updater_sys_t
> +{
> +    demux_sys_t *p_demux_sys;
> +    int         i_plane;
> +};
> +
>  /*****************************************************************************
>   * Local prototypes
>   *****************************************************************************/
> @@ -90,6 +118,8 @@ static int     blurayDemuxMenu(demux_t *);
>  static int     blurayInitTitles(demux_t *p_demux );
>  static int     bluraySetTitle(demux_t *p_demux, int i_title);
>  
> +static void    blurayOverlayProc( void *ptr, const BD_OVERLAY * const overlay );
> +
>  #define FROM_TICKS(a) (a*CLOCK_FREQ / INT64_C(90000))
>  #define TO_TICKS(a)   (a*INT64_C(90000)/CLOCK_FREQ)
>  #define CUR_LENGTH    p_sys->pp_title[p_demux->info.i_title]->i_length
> @@ -113,11 +143,12 @@ static int blurayOpen( vlc_object_t *object )
>      }
>  
>      /* */
> -    p_demux->p_sys = p_sys = malloc(sizeof(*p_sys));
> +    p_demux->p_sys = p_sys = calloc( 1, sizeof(*p_sys) );
>      if (unlikely(!p_sys)) {
>          return VLC_ENOMEM;
>      }
> -    p_sys->p_parser = NULL;
> +    vlc_mutex_init( &p_sys->overlay_lock );
> +    p_sys->current_overlay = -1;
>  
>      /* init demux info fields */
>      p_demux->info.i_update    = 0;
> @@ -207,11 +238,16 @@ static int blurayOpen( vlc_object_t *object )
>      bool b_menu = var_InheritBool( p_demux, "bluray-menu" );
>      if ( b_menu )
>      {
> +        p_sys->p_input = demux_GetParentInput( p_demux );
> +        if ( unlikely( p_sys->p_input == NULL ) )
> +            goto error;
>          /*
>           * libbluray will start playback from "First-Title" title
>           * 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
>      {
> @@ -256,20 +292,392 @@ 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);
> -
> +    vlc_mutex_destroy( &p_sys->overlay_lock );
>      /* Titles */
>      for (unsigned int i = 0; i < p_sys->i_title; i++)
>          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);
>  }
>  
> +/*****************************************************************************
> + * subpicture_updater_t functions:
> + *****************************************************************************/
> +static int      subpictureUpdaterValidate( subpicture_t *p_subpic,
> +                                           bool b_fmt_src, const video_format_t *p_fmt_src,
> +                                           bool b_fmt_dst, const video_format_t *p_fmt_dst,
> +                                           mtime_t i_ts )
> +{
> +    VLC_UNUSED( b_fmt_src ); VLC_UNUSED( b_fmt_dst );
> +    VLC_UNUSED( p_fmt_src ); VLC_UNUSED( p_fmt_dst );
> +    VLC_UNUSED( i_ts );
> +    subpicture_updater_sys_t    *p_upd_sys = p_subpic->updater.p_sys;
> +    demux_sys_t                 *p_sys = p_upd_sys->p_demux_sys;
> +
> +    vlc_mutex_lock( &p_sys->overlay_lock );
> +    int res = p_sys->overlay_status[p_upd_sys->i_plane] == Outdated;
> +    vlc_mutex_unlock( &p_sys->overlay_lock );
> +    return res;
> +}
> +
> +/* This should probably be moved to subpictures.c afterward */
> +static subpicture_region_t*     subpicture_region_Clone( subpicture_region_t *p_region_src )
> +{
> +    if ( p_region_src == NULL )
> +        return NULL;
> +    subpicture_region_t *p_region_dst = subpicture_region_New( &p_region_src->fmt );
> +    if ( unlikely( p_region_dst == NULL ) )
> +        return NULL;
> +    p_region_dst->i_x = p_region_src->i_x;
> +    p_region_dst->i_y = p_region_src->i_y;
> +    p_region_dst->i_align = p_region_src->i_align;
> +    p_region_dst->i_alpha = p_region_src->i_alpha;
> +
> +    p_region_dst->psz_text = p_region_src->psz_text ? strdup( p_region_src->psz_text ) : NULL;
> +    p_region_dst->psz_html = p_region_src->psz_html ? strdup( p_region_src->psz_html ) : NULL;
> +    if ( p_region_src->p_style != NULL )
> +    {
> +        p_region_dst->p_style = malloc( sizeof( *p_region_dst->p_style ) );
> +        p_region_dst->p_style = text_style_Copy( p_region_dst->p_style,
> +                                                 p_region_src->p_style );
text_style_Duplicate() is simpler.
> +    }
> +
> +    //Palette is already copied by subpicture_region_New, we just have to duplicate p_pixels
> +    for ( int i = 0; i < p_region_src->p_picture->i_planes; i++ )
> +        memcpy( p_region_dst->p_picture->p[i].p_pixels,
> +                p_region_src->p_picture->p[i].p_pixels,
> +                p_region_src->p_picture->p[i].i_lines * p_region_src->p_picture->p[i].i_pitch );
picture_CopyPixels() is simpler and will implement it properly.
> +    return p_region_dst;
> +}
> +
> +static void     subpictureUpdaterUpdate( subpicture_t *p_subpic,
> +                              const video_format_t *p_fmt_src,
> +                              const video_format_t *p_fmt_dst,
> +                              mtime_t i_ts )
> +{
> +    VLC_UNUSED( p_fmt_src ); VLC_UNUSED( p_fmt_dst ); VLC_UNUSED( i_ts );
> +    subpicture_updater_sys_t    *p_upd_sys = p_subpic->updater.p_sys;
> +    demux_sys_t                 *p_sys = p_upd_sys->p_demux_sys;
> +
> +    /*
> +     * When this function is called, all p_subpic regions are gone.
> +     * We need to duplicate our regions (stored internaly) to this subpic.
> +     */
> +    vlc_mutex_lock( &p_sys->overlay_lock );
 As is, you cannot acces p_sys from this callback, the subpicture may live
longer than the demux. You need some refcouting of the part you share
between the subpicture and the demux (you should merge the 8 patch with
this one).

> +    p_subpic->p_region = subpicture_region_Clone( p_sys->p_regions[p_upd_sys->i_plane] );
Are you sure that at this point p_sys->p_regions[] is never NULL? (It seems
to me that it can sometime be when the subpicture is Outdated, in
blurayClearOverlay).
> +    if ( p_subpic->p_region == NULL )
> +    {
> +        vlc_mutex_unlock( &p_sys->overlay_lock );
> +        return ;
> +    }
> +    subpicture_region_t *p_src = p_sys->p_regions[p_upd_sys->i_plane]->p_next;
> +    subpicture_region_t *p_dst = p_subpic->p_region;
> +    while ( p_src != NULL )
> +    {
> +        p_dst->p_next = subpicture_region_Clone( p_src );
> +        if ( p_dst->p_next == NULL )
> +            break ;
> +        p_dst = p_dst->p_next;
> +        p_src = p_src->p_next;
> +    }
> +    p_dst->p_next = NULL;
 You can easily merge the first iteration in the loop, it will be simpler.

> +    p_sys->overlay_status[p_upd_sys->i_plane] = Displayed;
> +    vlc_mutex_unlock( &p_sys->overlay_lock );
> +}
> +
> +static void     subpictureUpdaterDestroy( subpicture_t *p_subpic )
> +{
> +    free( p_subpic->updater.p_sys );
> +}
> +
> +/*****************************************************************************
> + * libbluray overlay handling:
> + *****************************************************************************/
> +static void blurayDeleteOverlaySubPic( demux_t *p_demux, int i_layer )
> +{
> +    demux_sys_t     *p_sys = p_demux->p_sys;
> +
> +    vlc_mutex_lock( &p_sys->overlay_lock );
> +    p_sys->overlay_status[i_layer] = Closed;
> +    if ( p_sys->p_pic[i_layer] == NULL )
> +    {
> +        vlc_mutex_unlock( &p_sys->overlay_lock );
> +        return ;
> +    }
> +    //If we have a vout, let it cleanup by itself.
> +    if ( p_sys->p_vout )
> +        vout_FlushSubpictureChannel( p_sys->p_vout, p_sys->p_pic[i_layer]->i_channel );
> +    else
> +    {
> +        /*
> +         * Else, the subpicture hasn't been sent yet, clean it ourselves.
> +         * In that case, the vout doesn't hold the picture, we can use it safely
> +         */
> +        subpicture_Delete( p_sys->p_pic[i_layer] );
> +    }
> +    p_sys->p_pic[i_layer] = NULL;
> +    vlc_mutex_unlock( &p_sys->overlay_lock );
> +}
> +
> +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;
> +    }
> +}
> +
> +/*
> + * Mark the overlay as "ToDisplay".
> + * This will not send the overlay to the vout instantly, as the vout
> + * may not be acquired (not acquirable) yet
> + * If is has already been acquire, the overlay has already been sent to it,
> + * therefore, we only flag the overlay as "Outdated"
> + */
> +static void blurayActivateOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
> +{
> +    demux_sys_t     *p_sys = p_demux->p_sys;
> +
> +    /*
> +     * If the overlay is already displayed, mark the picture as outdated.
> +     * We must NOT use vout_PutSubpicture if a picture is already displayed.
> +     */
> +    vlc_mutex_lock( &p_sys->overlay_lock );
> +    if ( ( p_sys->overlay_status[ov->plane] == Displayed ||
> +           p_sys->overlay_status[ov->plane] == Outdated )
> +         && p_sys->p_vout )
> +    {
> +        p_sys->overlay_status[ov->plane] = Outdated;
> +        vlc_mutex_unlock( &p_sys->overlay_lock );
> +        return ;
> +    }
> +    /*
> +     * 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;
> +    p_sys->overlay_status[ov->plane] = ToDisplay;
> +    vlc_mutex_unlock( &p_sys->overlay_lock );
> +}
> +
> +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 );
> +
> +    subpicture_updater_sys_t    *p_upd_sys = malloc( sizeof( *p_upd_sys ) );
> +    if ( unlikely( p_upd_sys == NULL ) )
> +        return ;
> +    p_upd_sys->p_demux_sys = p_sys;
> +    p_upd_sys->i_plane = ov->plane;
> +    subpicture_updater_t    updater =
> +    {
> +        .pf_validate = subpictureUpdaterValidate,
> +        .pf_update = subpictureUpdaterUpdate,
> +        .pf_destroy = subpictureUpdaterDestroy,
> +        .p_sys = p_upd_sys,
> +    };
> +    p_sys->p_pic[ov->plane] = subpicture_New( &updater );
> +    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;
 Why do you create the subpicture at this point? And why do you keep them
in the demuxer private context?

> +}
> +
> +/**
> + * Destroy every regions in the subpictures.
> + * This is done in two steps:
> + * - Wiping our private regions list
> + * - Flagging the overlay as outdated, so the changes are replicated from
> + *   the subpicture_updater_t::pf_update
> + * This doesn't destroy the subpicture, as the overlay may be used again by libbluray.
> + */
> +static void blurayClearOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
> +{
> +    demux_sys_t     *p_sys = p_demux->p_sys;
> +
> +    vlc_mutex_lock( &p_sys->overlay_lock );
> +    subpicture_region_ChainDelete( p_sys->p_regions[ov->plane] );
> +    p_sys->p_regions[ov->plane] = NULL;
> +    p_sys->overlay_status[ov->plane] = Outdated;
> +    vlc_mutex_unlock( &p_sys->overlay_lock );
> +}
> +
> +/*
> + * This will draw to the overlay by adding a region to our region list
> + * This will have to be copied to the subpicture used to render the overlay.
> + */
> +static void blurayDrawOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
> +{
> +    demux_sys_t     *p_sys = p_demux->p_sys;
> +
> +    /*
> +     * Compute a subpicture_region_t.
> +     * It will be copied and sent to the vout later.
> +     */
> +    if ( ov->img == NULL )
> +        return ;
> +    video_format_t          fmt;
> +    video_format_Init( &fmt, 0 );
> +    video_format_Setup( &fmt, VLC_CODEC_YUVP, ov->w, ov->h, 1, 1 );
Is the BD_OVERLAY picture size always expressed with an aspect ratio of 1:1?
> +    vlc_mutex_lock( &p_sys->overlay_lock );
> +
> +    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:
> +     * If there's no region yet, don't bother checking for update...
> +     */
> +    subpicture_region_t *p_reg = p_sys->p_regions[ov->plane];
> +    if ( p_reg == NULL )
> +    {
> +        p_sys->p_regions[ov->plane] = region;
> +        vlc_mutex_unlock( &p_sys->overlay_lock );
> +        return ;
> +    }
> +    /*
> +     * While adding the region, check if any region matches the one we're drawing.
> +     * if so, delete the old region, and add the new one at the back of the list.
> +     */
> +    subpicture_region_t *p_prev = NULL;
> +    while ( p_reg )
> +    {
> +        if ( p_reg->i_x == region->i_x && p_reg->i_y == region->i_y &&
> +             p_reg->fmt.i_width == region->fmt.i_width &&
> +             p_reg->fmt.i_height == region->fmt.i_height )
> +        {
> +            if ( p_prev == NULL )
> +            {
> +                p_sys->p_regions[ov->plane] = p_reg->p_next;
> +                subpicture_region_Delete( p_reg );
> +                p_reg = p_sys->p_regions[ov->plane];
> +            }
> +            else
> +            {
> +                p_prev->p_next = p_reg->p_next;
> +                subpicture_region_Delete( p_reg );
> +                p_reg = p_prev->p_next;
> +            }
> +        }
> +        else
> +        {
> +            p_prev = p_reg;
> +            p_reg = p_reg->p_next;
> +        }
> +    }
> +    /*
> +     * If there's only one region which has been updated,
> +     * both prev and reg will be NULL
> +     */
> +    if ( p_prev == NULL )
> +        p_sys->p_regions[ov->plane]->p_next = region;
> +    else
> +        p_prev->p_next = region;
 I think you could simplify a lot the code by checking first if there is
already a region to update and if so simply reusing it, and if not, then
creating a new region that you append (or even prepend if the order don't
matter).

> +    vlc_mutex_unlock( &p_sys->overlay_lock );
> +    /*
> +     * /!\ The region is now stored in our internal list, but not in the subpicture /!\
> +     */
> +}
> +
> +static void blurayOverlayProc( void *ptr, const BD_OVERLAY *const overlay )
> +{
> +    demux_t     *p_demux = (demux_t*)ptr;
> +
> +    if ( overlay == NULL )
> +    {
> +        msg_Info( p_demux, "Closing overlay." );
> +        blurayCloseOverlay( p_demux );
> +        return ;
> +    }
> +    switch ( overlay->cmd )
> +    {
> +        case BD_OVERLAY_INIT:
> +            msg_Info( p_demux, "Initializing overlay" );
> +            blurayInitOverlay( p_demux, overlay );
> +            break ;
> +        case BD_OVERLAY_CLEAR:
> +            blurayClearOverlay( p_demux, overlay );
> +            break ;
> +        case BD_OVERLAY_FLUSH:
> +            blurayActivateOverlay( 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 void bluraySendOverlayToVout( demux_t *p_demux )
> +{
> +    demux_sys_t *p_sys = p_demux->p_sys;
> +
> +    assert( p_sys->current_overlay >= 0 &&
> +            p_sys->p_pic[p_sys->current_overlay] != 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 );
 The channel registration should be done only once, I think it can be moved
where you get the vout pointer.
> +    /*
> +     * After this point, the picture should not be accessed from the demux thread,
> +     * as it's hold by the vout thread.
> +     */
> +    vout_PutSubpicture( p_sys->p_vout, p_sys->p_pic[p_sys->current_overlay] );
> +    /* Mark the picture as Outdated, as it contains no region for now. */
> +    p_sys->overlay_status[p_sys->current_overlay] = Outdated;
> +}
>  
>  static int blurayInitTitles(demux_t *p_demux )
>  {
> @@ -570,9 +978,7 @@ 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);
>          return 1;
> @@ -584,6 +990,18 @@ static int blurayDemuxMenu( demux_t *p_demux )
>      }
>      p_block->i_buffer = nread;
>  
> +    vlc_mutex_lock( &p_sys->overlay_lock );
> +    if ( p_sys->current_overlay != -1 &&
> +         p_sys->overlay_status[p_sys->current_overlay] == ToDisplay )
> +    {
> +        vlc_mutex_unlock( &p_sys->overlay_lock );
> +        if ( p_sys->p_vout == NULL )
> +            p_sys->p_vout = input_GetVout( p_sys->p_input );
> +        if ( p_sys->p_vout != NULL )
> +            bluraySendOverlayToVout( p_demux );
> +    }
> +    else
> +        vlc_mutex_unlock( &p_sys->overlay_lock );
>      stream_DemuxSend( p_sys->p_parser, p_block );
>      return 1;
>  }
> -- 
> 1.7.9.1

> Subject: [PATCH 06/12] bluray: Handle mouse event.
> 
> ---
>  modules/access/bluray.c |   34 ++++++++++++++++++++++++++++++++++
>  1 files changed, 34 insertions(+), 0 deletions(-)
> 
> diff --git a/modules/access/bluray.c b/modules/access/bluray.c
> index ca5b110..be0bb03 100644
> --- a/modules/access/bluray.c
> +++ b/modules/access/bluray.c
> @@ -39,6 +39,7 @@
>  #include <vlc_vout.h>                       /* vout_PutSubpicture / subpicture_t */
>  
>  #include <libbluray/bluray.h>
> +#include <libbluray/keys.h>
>  #include <libbluray/meta_data.h>
>  #include <libbluray/overlay.h>
>  
> @@ -406,6 +407,32 @@ static void     subpictureUpdaterDestroy( subpicture_t *p_subpic )
>  }
>  
>  /*****************************************************************************
> + * User input 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;
> +    int64_t     now = mdate();
> +    VLC_UNUSED( old );
> +    VLC_UNUSED( p_vout );
> +
> +    if ( psz_var[6] == 'm' ) //Mouse moved
> +        bd_mouse_select( p_sys->bluray, now, val.coords.x, val.coords.y );
> +    else if ( psz_var[6] == 'c' )
> +    {
> +        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
> +    {
> +        assert(0);
> +    }
> +    return VLC_SUCCESS;
> +}
> +
> +/*****************************************************************************
>   * libbluray overlay handling:
>   *****************************************************************************/
>  static void blurayDeleteOverlaySubPic( demux_t *p_demux, int i_layer )
> @@ -443,6 +470,8 @@ 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 );
 It seems that there is two place where the vout is released, but you remove
the callbacks only once. One of them should be fixed.

>          p_sys->p_vout = NULL;
>      }
> @@ -996,7 +1025,12 @@ static int blurayDemuxMenu( demux_t *p_demux )
>      {
>          vlc_mutex_unlock( &p_sys->overlay_lock );
>          if ( p_sys->p_vout == NULL )
> +        {
>              p_sys->p_vout = input_GetVout( p_sys->p_input );
> +            /* 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 );
> +        }
>          if ( p_sys->p_vout != NULL )
>              bluraySendOverlayToVout( p_demux );
>      }
> -- 
> 1.7.9.1

> Subject: [PATCH 07/12] bluray: Don't crash when providing an incorrect path.
> 
> ---
>  modules/access/bluray.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/modules/access/bluray.c b/modules/access/bluray.c
> index be0bb03..89b1f24 100644
> --- a/modules/access/bluray.c
> +++ b/modules/access/bluray.c
> @@ -192,6 +192,11 @@ static int blurayOpen( vlc_object_t *object )
>  
>      /* Warning the user about AACS/BD+ */
>      const BLURAY_DISC_INFO *disc_info = bd_get_disc_info(p_sys->bluray);
> +    if ( disc_info->bluray_detected == 0 )
> +    {
> +        error_msg = "Path doesn't appear to be a bluray";
> +        goto error;
> +    }
>      msg_Info(p_demux, "First play: %i, Top menu: %i\n"
>                        "HDMV Titles: %i, BD-J Titles: %i, Other: %i",
>               disc_info->first_play_supported, disc_info->top_menu_supported,
> -- 
> 1.7.9.1
OK.


> Subject: [PATCH 08/12] bluray: Use refcounting to avoid accessing invalid
>  fields from vout.
> 
> ---
>  modules/access/bluray.c |  234 +++++++++++++++++++++++++---------------------
>  1 files changed, 127 insertions(+), 107 deletions(-)
> 
> diff --git a/modules/access/bluray.c b/modules/access/bluray.c
> index 89b1f24..bfc581f 100644
> --- a/modules/access/bluray.c
> +++ b/modules/access/bluray.c
> @@ -80,33 +80,39 @@ typedef enum    OverlayStatus
>      Outdated    //used to update the overlay after it has been sent to the vout
>  }               OverlayStatus;
>  
> -struct demux_sys_t
> +typedef struct bluray_overlay_t
>  {
> -    BLURAY         *bluray;
> +    VLC_GC_MEMBERS
> +
> +    vlc_mutex_t             lock;
> +    subpicture_t            *p_pic;
> +    OverlayStatus           status;
> +    subpicture_region_t     *p_regions;
> +}   bluray_overlay_t;
> +
> +struct  demux_sys_t
> +{
> +    BLURAY              *bluray;
>  
>      /* Titles */
> -    unsigned int    i_title;
> -    unsigned int    i_longest_title;
> -    input_title_t **pp_title;
> +    unsigned int        i_title;
> +    unsigned int        i_longest_title;
> +    input_title_t       **pp_title;
>  
>      /* Overlay stuff */
> -    vlc_mutex_t             overlay_lock;
> -    subpicture_t            *p_pic[MAX_OVERLAY];
> -    OverlayStatus           overlay_status[MAX_OVERLAY];
> -    subpicture_region_t     *p_regions[MAX_OVERLAY];
> -    int             current_overlay; // -1 if no current overlay;
> +    bluray_overlay_t    *p_overlays[MAX_OVERLAY];
> +    int                 current_overlay; // -1 if no current overlay;
>  
> -    input_thread_t  *p_input;
> -    vout_thread_t   *p_vout;
> +    input_thread_t      *p_input;
> +    vout_thread_t       *p_vout;
>  
>      /* TS stream */
> -    stream_t       *p_parser;
> +    stream_t            *p_parser;
>  };
>  
>  struct subpicture_updater_sys_t
>  {
> -    demux_sys_t *p_demux_sys;
> -    int         i_plane;
> +    bluray_overlay_t    *p_overlay;
>  };
>  
>  /*****************************************************************************
> @@ -148,7 +154,6 @@ static int blurayOpen( vlc_object_t *object )
>      if (unlikely(!p_sys)) {
>          return VLC_ENOMEM;
>      }
> -    vlc_mutex_init( &p_sys->overlay_lock );
>      p_sys->current_overlay = -1;
>  
>      /* init demux info fields */
> @@ -312,7 +317,6 @@ static void blurayClose( vlc_object_t *object )
>          vlc_object_release( p_sys->p_input );
>      if (p_sys->p_parser)
>          stream_Delete(p_sys->p_parser);
> -    vlc_mutex_destroy( &p_sys->overlay_lock );
>      /* Titles */
>      for (unsigned int i = 0; i < p_sys->i_title; i++)
>          vlc_input_title_Delete(p_sys->pp_title[i]);
> @@ -333,11 +337,11 @@ static int      subpictureUpdaterValidate( subpicture_t *p_subpic,
>      VLC_UNUSED( p_fmt_src ); VLC_UNUSED( p_fmt_dst );
>      VLC_UNUSED( i_ts );
>      subpicture_updater_sys_t    *p_upd_sys = p_subpic->updater.p_sys;
> -    demux_sys_t                 *p_sys = p_upd_sys->p_demux_sys;
> +    bluray_overlay_t            *p_overlay = p_upd_sys->p_overlay;
>  
> -    vlc_mutex_lock( &p_sys->overlay_lock );
> -    int res = p_sys->overlay_status[p_upd_sys->i_plane] == Outdated;
> -    vlc_mutex_unlock( &p_sys->overlay_lock );
> +    vlc_mutex_lock( &p_overlay->lock );
> +    int res = p_overlay->status == Outdated;
> +    vlc_mutex_unlock( &p_overlay->lock );
>      return res;
>  }
>  
> @@ -378,20 +382,22 @@ static void     subpictureUpdaterUpdate( subpicture_t *p_subpic,
>  {
>      VLC_UNUSED( p_fmt_src ); VLC_UNUSED( p_fmt_dst ); VLC_UNUSED( i_ts );
>      subpicture_updater_sys_t    *p_upd_sys = p_subpic->updater.p_sys;
> -    demux_sys_t                 *p_sys = p_upd_sys->p_demux_sys;
> +    bluray_overlay_t            *p_overlay = p_upd_sys->p_overlay;
>  
>      /*
>       * When this function is called, all p_subpic regions are gone.
>       * We need to duplicate our regions (stored internaly) to this subpic.
>       */
> -    vlc_mutex_lock( &p_sys->overlay_lock );
> -    p_subpic->p_region = subpicture_region_Clone( p_sys->p_regions[p_upd_sys->i_plane] );
> +    vlc_mutex_lock( &p_overlay->lock );
> +    if ( p_overlay->p_regions == NULL )
> +        return ;
> +    p_subpic->p_region = subpicture_region_Clone( p_overlay->p_regions );
>      if ( p_subpic->p_region == NULL )
>      {
> -        vlc_mutex_unlock( &p_sys->overlay_lock );
> +        vlc_mutex_unlock( &p_overlay->lock );
>          return ;
>      }
> -    subpicture_region_t *p_src = p_sys->p_regions[p_upd_sys->i_plane]->p_next;
> +    subpicture_region_t *p_src = p_overlay->p_regions->p_next;
>      subpicture_region_t *p_dst = p_subpic->p_region;
>      while ( p_src != NULL )
>      {
> @@ -402,13 +408,13 @@ static void     subpictureUpdaterUpdate( subpicture_t *p_subpic,
>          p_src = p_src->p_next;
>      }
>      p_dst->p_next = NULL;
> -    p_sys->overlay_status[p_upd_sys->i_plane] = Displayed;
> -    vlc_mutex_unlock( &p_sys->overlay_lock );
> +    p_overlay->status = Displayed;
> +    vlc_mutex_unlock( &p_overlay->lock );
>  }
>  
>  static void     subpictureUpdaterDestroy( subpicture_t *p_subpic )
>  {
> -    free( p_subpic->updater.p_sys );
> +    vlc_gc_decref( p_subpic->updater.p_sys->p_overlay );
>  }
>  
>  /*****************************************************************************
> @@ -437,44 +443,39 @@ static int  onMouseEvent( vlc_object_t *p_vout, const char *psz_var, vlc_value_t
>      return VLC_SUCCESS;
>  }
>  
> -/*****************************************************************************
> - * libbluray overlay handling:
> - *****************************************************************************/
> -static void blurayDeleteOverlaySubPic( demux_t *p_demux, int i_layer )
> +static void     blurayCleanOverayStruct( gc_object_t *p_gc )
>  {
> -    demux_sys_t     *p_sys = p_demux->p_sys;
> +    bluray_overlay_t *p_overlay = vlc_priv( p_gc, bluray_overlay_t );
>  
> -    vlc_mutex_lock( &p_sys->overlay_lock );
> -    p_sys->overlay_status[i_layer] = Closed;
> -    if ( p_sys->p_pic[i_layer] == NULL )
> -    {
> -        vlc_mutex_unlock( &p_sys->overlay_lock );
> -        return ;
> -    }
> -    //If we have a vout, let it cleanup by itself.
> -    if ( p_sys->p_vout )
> -        vout_FlushSubpictureChannel( p_sys->p_vout, p_sys->p_pic[i_layer]->i_channel );
> -    else
> -    {
> -        /*
> -         * Else, the subpicture hasn't been sent yet, clean it ourselves.
> -         * In that case, the vout doesn't hold the picture, we can use it safely
> -         */
> -        subpicture_Delete( p_sys->p_pic[i_layer] );
> -    }
> -    p_sys->p_pic[i_layer] = NULL;
> -    vlc_mutex_unlock( &p_sys->overlay_lock );
> +    /*
> +     * This will be called when destroying the picture.
> +     * Don't delete it again from here!
> +     */
> +    vlc_mutex_destroy( &p_overlay->lock );
> +    subpicture_region_Delete( p_overlay->p_regions );
> +    free( p_overlay );
>  }
>  
> -static void blurayCloseOverlay( demux_t *p_demux )
> +/*****************************************************************************
> + * libbluray overlay callbacks:
> + *****************************************************************************/
> +static void blurayCloseAllOverlays( 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 )
>      {
> +        for ( int i = 0; i < 0; i++ )
> +        {
> +            if ( p_sys->p_overlays[i] != NULL )
> +            {
> +                vout_FlushSubpictureChannel( p_sys->p_vout,
> +                                             p_sys->p_overlays[i]->p_pic->i_channel );
> +                vlc_gc_decref( p_sys->p_overlays[i] );
> +                p_sys->p_overlays[i] = 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 );
> @@ -497,13 +498,13 @@ static void blurayActivateOverlay( demux_t *p_demux, const BD_OVERLAY* const ov
>       * If the overlay is already displayed, mark the picture as outdated.
>       * We must NOT use vout_PutSubpicture if a picture is already displayed.
>       */
> -    vlc_mutex_lock( &p_sys->overlay_lock );
> -    if ( ( p_sys->overlay_status[ov->plane] == Displayed ||
> -           p_sys->overlay_status[ov->plane] == Outdated )
> +    vlc_mutex_lock( &p_sys->p_overlays[ov->plane]->lock );
> +    if ( ( p_sys->p_overlays[ov->plane]->status == Displayed ||
> +           p_sys->p_overlays[ov->plane]->status == Outdated )
>           && p_sys->p_vout )
>      {
> -        p_sys->overlay_status[ov->plane] = Outdated;
> -        vlc_mutex_unlock( &p_sys->overlay_lock );
> +        p_sys->p_overlays[ov->plane]->status = Outdated;
> +        vlc_mutex_unlock( &p_sys->p_overlays[ov->plane]->lock );
>          return ;
>      }
>      /*
> @@ -512,22 +513,31 @@ static void blurayActivateOverlay( demux_t *p_demux, const BD_OVERLAY* const ov
>       * the overlay is computed
>       */
>      p_sys->current_overlay = ov->plane;
> -    p_sys->overlay_status[ov->plane] = ToDisplay;
> -    vlc_mutex_unlock( &p_sys->overlay_lock );
> +    p_sys->p_overlays[ov->plane]->status = ToDisplay;
> +    vlc_mutex_unlock( &p_sys->p_overlays[ov->plane]->lock );
>  }
>  
>  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 );
> +    assert( p_sys->p_overlays[ov->plane] == NULL );
>  
> +    p_sys->p_overlays[ov->plane] = calloc( 1, sizeof( **p_sys->p_overlays ) );
> +    if ( unlikely( p_sys->p_overlays[ov->plane] == NULL ) )
> +        return ;
>      subpicture_updater_sys_t    *p_upd_sys = malloc( sizeof( *p_upd_sys ) );
>      if ( unlikely( p_upd_sys == NULL ) )
> +    {
> +        free( p_sys->p_overlays[ov->plane] );
> +        p_sys->p_overlays[ov->plane] = NULL;
>          return ;
> -    p_upd_sys->p_demux_sys = p_sys;
> -    p_upd_sys->i_plane = ov->plane;
> +    }
> +    vlc_gc_init( p_sys->p_overlays[ov->plane], blurayCleanOverayStruct );
> +    /* Incrementing refcounter: vout + demux */
> +    vlc_gc_incref( p_sys->p_overlays[ov->plane] );
> +
> +    p_upd_sys->p_overlay = p_sys->p_overlays[ov->plane];
>      subpicture_updater_t    updater =
>      {
>          .pf_validate = subpictureUpdaterValidate,
> @@ -535,11 +545,11 @@ static void blurayInitOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
>          .pf_destroy = subpictureUpdaterDestroy,
>          .p_sys = p_upd_sys,
>      };
> -    p_sys->p_pic[ov->plane] = subpicture_New( &updater );
> -    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;
> +    p_sys->p_overlays[ov->plane]->p_pic = subpicture_New( &updater );
> +    p_sys->p_overlays[ov->plane]->p_pic->i_original_picture_width = ov->w;
> +    p_sys->p_overlays[ov->plane]->p_pic->i_original_picture_height = ov->h;
> +    p_sys->p_overlays[ov->plane]->p_pic->b_ephemer = true;
> +    p_sys->p_overlays[ov->plane]->p_pic->b_absolute = true;
>  }
>  
>  /**
> @@ -554,11 +564,12 @@ static void blurayClearOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
>  {
>      demux_sys_t     *p_sys = p_demux->p_sys;
>  
> -    vlc_mutex_lock( &p_sys->overlay_lock );
> -    subpicture_region_ChainDelete( p_sys->p_regions[ov->plane] );
> -    p_sys->p_regions[ov->plane] = NULL;
> -    p_sys->overlay_status[ov->plane] = Outdated;
> -    vlc_mutex_unlock( &p_sys->overlay_lock );
> +    vlc_mutex_lock( &p_sys->p_overlays[ov->plane]->lock );
> +
> +    subpicture_region_ChainDelete( p_sys->p_overlays[ov->plane]->p_regions );
> +    p_sys->p_overlays[ov->plane]->p_regions = NULL;
> +    p_sys->p_overlays[ov->plane]->status = Outdated;
> +    vlc_mutex_unlock( &p_sys->p_overlays[ov->plane]->lock );
>  }
>  
>  /*
> @@ -578,7 +589,8 @@ static void blurayDrawOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
>      video_format_t          fmt;
>      video_format_Init( &fmt, 0 );
>      video_format_Setup( &fmt, VLC_CODEC_YUVP, ov->w, ov->h, 1, 1 );
> -    vlc_mutex_lock( &p_sys->overlay_lock );
> +
> +    vlc_mutex_lock( &p_sys->p_overlays[ov->plane]->lock );
>  
>      subpicture_region_t     *region = subpicture_region_New( &fmt );
>      region->i_x = ov->x;
> @@ -611,11 +623,11 @@ static void blurayDrawOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
>       * Append the new region at the end of our current regions:
>       * If there's no region yet, don't bother checking for update...
>       */
> -    subpicture_region_t *p_reg = p_sys->p_regions[ov->plane];
> +    subpicture_region_t *p_reg = p_sys->p_overlays[ov->plane]->p_regions;
>      if ( p_reg == NULL )
>      {
> -        p_sys->p_regions[ov->plane] = region;
> -        vlc_mutex_unlock( &p_sys->overlay_lock );
> +        p_sys->p_overlays[ov->plane]->p_regions = region;
> +        vlc_mutex_unlock( &p_sys->p_overlays[ov->plane]->lock );
>          return ;
>      }
>      /*
> @@ -631,9 +643,9 @@ static void blurayDrawOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
>          {
>              if ( p_prev == NULL )
>              {
> -                p_sys->p_regions[ov->plane] = p_reg->p_next;
> +                p_sys->p_overlays[ov->plane]->p_regions = p_reg->p_next;
>                  subpicture_region_Delete( p_reg );
> -                p_reg = p_sys->p_regions[ov->plane];
> +                p_reg = p_sys->p_overlays[ov->plane]->p_regions;
>              }
>              else
>              {
> @@ -653,10 +665,10 @@ static void blurayDrawOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
>       * both prev and reg will be NULL
>       */
>      if ( p_prev == NULL )
> -        p_sys->p_regions[ov->plane]->p_next = region;
> +        p_sys->p_overlays[ov->plane]->p_regions->p_next = region;
>      else
>          p_prev->p_next = region;
> -    vlc_mutex_unlock( &p_sys->overlay_lock );
> +    vlc_mutex_unlock( &p_sys->p_overlays[ov->plane]->lock );
>      /*
>       * /!\ The region is now stored in our internal list, but not in the subpicture /!\
>       */
> @@ -669,7 +681,7 @@ static void blurayOverlayProc( void *ptr, const BD_OVERLAY *const overlay )
>      if ( overlay == NULL )
>      {
>          msg_Info( p_demux, "Closing overlay." );
> -        blurayCloseOverlay( p_demux );
> +        blurayCloseAllOverlays( p_demux );
>          return ;
>      }
>      switch ( overlay->cmd )
> @@ -698,19 +710,25 @@ static void bluraySendOverlayToVout( demux_t *p_demux )
>      demux_sys_t *p_sys = p_demux->p_sys;
>  
>      assert( p_sys->current_overlay >= 0 &&
> -            p_sys->p_pic[p_sys->current_overlay] != NULL );
> +            p_sys->p_overlays[p_sys->current_overlay] != NULL &&
> +            p_sys->p_overlays[p_sys->current_overlay]->p_pic != 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 =
> +    p_sys->p_overlays[p_sys->current_overlay]->p_pic->i_start =
> +            p_sys->p_overlays[p_sys->current_overlay]->p_pic->i_stop = mdate();
> +    p_sys->p_overlays[p_sys->current_overlay]->p_pic->i_channel =
>              vout_RegisterSubpictureChannel( p_sys->p_vout );
>      /*
>       * After this point, the picture should not be accessed from the demux thread,
>       * as it's hold by the vout thread.
> +     * This must be done only once per subpicture, ie. only once between each
> +     * blurayInitOverlay & blurayCloseOverlay call.
> +     */
> +    vout_PutSubpicture( p_sys->p_vout, p_sys->p_overlays[p_sys->current_overlay]->p_pic );
> +    /*
> +     * Mark the picture as Outdated, as it contains no region for now.
> +     * This will make the subpicture_updater_t call pf_update
>       */
> -    vout_PutSubpicture( p_sys->p_vout, p_sys->p_pic[p_sys->current_overlay] );
> -    /* Mark the picture as Outdated, as it contains no region for now. */
> -    p_sys->overlay_status[p_sys->current_overlay] = Outdated;
> +    p_sys->p_overlays[p_sys->current_overlay]->status = Outdated;
>  }
>  
>  static int blurayInitTitles(demux_t *p_demux )
> @@ -1024,23 +1042,25 @@ static int blurayDemuxMenu( demux_t *p_demux )
>      }
>      p_block->i_buffer = nread;
>  
> -    vlc_mutex_lock( &p_sys->overlay_lock );
> -    if ( p_sys->current_overlay != -1 &&
> -         p_sys->overlay_status[p_sys->current_overlay] == ToDisplay )
> +    if ( p_sys->current_overlay != -1 )
>      {
> -        vlc_mutex_unlock( &p_sys->overlay_lock );
> -        if ( p_sys->p_vout == NULL )
> +        vlc_mutex_lock( &p_sys->p_overlays[p_sys->current_overlay]->lock );
> +        if ( p_sys->p_overlays[p_sys->current_overlay]->status == ToDisplay )
>          {
> -            p_sys->p_vout = input_GetVout( p_sys->p_input );
> -            /* 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 );
> +            vlc_mutex_unlock( &p_sys->p_overlays[p_sys->current_overlay]->lock );
> +            if ( p_sys->p_vout == NULL )
> +            {
> +                p_sys->p_vout = input_GetVout( p_sys->p_input );
> +                /* 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 );
> +            }
> +            if ( p_sys->p_vout != NULL )
> +                bluraySendOverlayToVout( p_demux );
>          }
> -        if ( p_sys->p_vout != NULL )
> -            bluraySendOverlayToVout( p_demux );
> +        else
> +            vlc_mutex_unlock( &p_sys->p_overlays[p_sys->current_overlay]->lock );
>      }
> -    else
> -        vlc_mutex_unlock( &p_sys->overlay_lock );
>      stream_DemuxSend( p_sys->p_parser, p_block );
>      return 1;
>  }
> -- 
> 1.7.9.1
 You should merge this patch with one of the previous one, it will reduce
the number of changes, and will avoid to commit incorrect changes.

Regards,

-- 
fenrir




More information about the vlc-devel mailing list