[vlc-devel] [Patches] bluray menu
Hugo Beauzée-Luyssen
beauze.h at gmail.com
Sun Mar 4 20:52:57 CET 2012
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-bluray-Adding-some-event-handling-basics.patch
Type: application/octet-stream
Size: 4224 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120304/6fc82919/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-bluray-Fixing-a-warning.patch
Type: application/octet-stream
Size: 805 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120304/6fc82919/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-bluray-Preparing-menu-handling.patch
Type: application/octet-stream
Size: 4746 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120304/6fc82919/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-bluray-Nasty-hack-to-avoid-crashing-when-changing-ti.patch
Type: application/octet-stream
Size: 1837 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120304/6fc82919/attachment-0003.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-bluray-Adding-support-for-overlay.patch
Type: application/octet-stream
Size: 19433 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120304/6fc82919/attachment-0004.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-bluray-Handle-mouse-event.patch
Type: application/octet-stream
Size: 3914 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120304/6fc82919/attachment-0005.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-bluray-Don-t-crash-when-providing-an-incorrect-path.patch
Type: application/octet-stream
Size: 1024 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120304/6fc82919/attachment-0006.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0008-bluray-Store-the-meta-informations-in-p_sys.patch
Type: application/octet-stream
Size: 1614 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120304/6fc82919/attachment-0007.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0009-bluray-Removing-hacks-for-FIRST_TITLE-TOP_MENU-title.patch
Type: application/octet-stream
Size: 3201 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120304/6fc82919/attachment-0008.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0010-bluray-Cosmetics.patch
Type: application/octet-stream
Size: 38327 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120304/6fc82919/attachment-0009.obj>
More information about the vlc-devel
mailing list