[vlc-devel] [Patches] bluray menu
Hugo Beauzée-Luyssen
beauze.h at gmail.com
Mon Mar 5 19:03:22 CET 2012
On Sun, Mar 4, 2012 at 8:52 PM, Hugo Beauzée-Luyssen <beauze.h at gmail.com> wrote:
> Hi,
>
> On Tue, Feb 28, 2012 at 9:34 PM, Laurent Aimar <fenrir at elivagar.org> wrote:
>> 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.
>>
>
> Since I basically need to move Open and Close at the bottom of the
> file, It has been added to a cosmetic patch.
>
>>> >>> +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.
>>
> They have been merged.
>
>>
>> Overall, for all the patches, it would be great to respect the current coding
>> style of bluray.c
>>
> This will also be in a coscmetic patch. However, I tried to use K&R in
> my last edit. I must confess I'm not at ease with this style yet.
>
>>> 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).
>>
> I'll patch it afterward, once this first patchset is done.
>
>>> + }
>>>
>>> - /* 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.
>>
> As mentioned earlier, they have been merged.
>
>>
>>> 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).
>>
> Done.
>
>>> + 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).
> A check is present in patch 8, so this should be ensured by merging
> patches 5 and 8.
>
>>> + 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.
>
> Done
>>
>>> + 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?
>>
> That was what felt logical to me for the BD_OVERLAY_INIT event.
> Although, it's not in the demux private context anymore, but in the
> bluray_overlay_t.
>
>>> +}
>>> +
>>> +/**
>>> + * 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).
>>
> Indeed! done.
>
>>> + 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.
>>
> Fixed. Although, this requires another forward declaration.
> There is an extra patch that:
> - formats the file as K&R
> - moves the open/close function at the end
> - removes every forward declaration
>
>>> 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.
>>
> Done.
>
>> Regards,
>>
>> --
>> fenrir
>>
>
> Best regards,
>
> --
> Hugo Beauzée-Luyssen
Woops, please disregard patch 0009, I thought I removed it locally...
--
Hugo Beauzée-Luyssen
More information about the vlc-devel
mailing list