[vlc-devel] [Patches] bluray menu
Hugo Beauzée-Luyssen
beauze.h at gmail.com
Sat Feb 18 01:24:50 CET 2012
On Sat, Feb 18, 2012 at 12:46 AM, Hugo Beauzée-Luyssen
<beauze.h at gmail.com> wrote:
> Hello,
>
> Answers are inline, patches are attached.
>
> On Tue, Feb 14, 2012 at 9:11 PM, Laurent Aimar <fenrir at elivagar.org> wrote:
>> Hi,
>>
>> On Tue, Jan 24, 2012 at 12:30:05AM +0100, Hugo Beauzée-Luyssen wrote:
>>> Here are some new versions for bluray menu patches.
>>
>> Sorry for the delay.
>>
>>> Subject: [PATCH 01/11] bluray: Adding some event handling basics.
>>>
>>> ---
>>> modules/access/bluray.c | 63 +++++++++++++++++++++++++++++++++++++++++-----
>>> 1 files changed, 56 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/modules/access/bluray.c b/modules/access/bluray.c
>>> index 7bfc3f0..443d884 100644
>>> --- a/modules/access/bluray.c
>>> +++ b/modules/access/bluray.c
>>> @@ -80,6 +80,9 @@ static int blurayDemux (demux_t *);
>>> static int blurayInitTitles(demux_t *p_demux );
>>> static int bluraySetTitle(demux_t *p_demux, int i_title);
>>>
>>> +static void blurayHandleEvent( demux_t *p_demux, const BD_EVENT *e );
>>> +static void blurayHandleEvents( demux_t *p_demux );
>>
>> IMHO, It will be better in the long term to avoid forward declaration.
>>
>>> @@ -169,6 +172,11 @@ static int blurayOpen( vlc_object_t *object )
>>> goto error;
>>> }
>>>
>>> + /*
>>> + * Initialize the event queue, so we can receive then in blurayDemux(Menu) later.
>>> + */
>>> + bd_get_event( p_sys->bluray, NULL );
>>> +
>>> /* get title request */
>>> if ((pos_title = strrchr(bd_path, ':'))) {
>>> /* found character ':' for title information */
>>> @@ -258,9 +266,17 @@ static int blurayInitTitles(demux_t *p_demux )
>>> TAB_APPEND( p_sys->i_title, p_sys->pp_title, t );
>>> bd_free_title_info(title_info);
>>> }
>>> +
>> Cosmetic.
> Fixed
>
>>> return VLC_SUCCESS;
>>> }
>>>
>>> +static void blurayUpdateTitle( demux_t *p_demux, int i_title )
>>> +{
>>> + /* read title info and init some values */
>>> + p_demux->info.i_title = i_title;
>>> + p_demux->info.i_seekpoint = 0;
>>> + p_demux->info.i_update |= INPUT_UPDATE_TITLE | INPUT_UPDATE_SEEKPOINT;
>>> +}
>>>
>>> /*****************************************************************************
>>> * bluraySetTitle: select new BD title
>>> @@ -282,11 +298,7 @@ static int bluraySetTitle(demux_t *p_demux, int i_title)
>>> msg_Err(p_demux, "cannot select bd title '%d'", p_demux->info.i_title);
>>> return VLC_EGENERIC;
>>> }
>>> -
>>> - /* read title info and init some values */
>>> - p_demux->info.i_title = i_title;
>>> - p_demux->info.i_seekpoint = 0;
>>> - p_demux->info.i_update |= INPUT_UPDATE_TITLE | INPUT_UPDATE_SEEKPOINT;
>>> + blurayUpdateTitle( p_demux, i_title );
>>>
>>> return VLC_SUCCESS;
>>> }
>>> @@ -357,7 +369,7 @@ static int blurayControl(demux_t *p_demux, int query, va_list args)
>>> case DEMUX_GET_LENGTH:
>>> {
>>> int64_t *pi_length = (int64_t*)va_arg(args, int64_t *);
>>> - *pi_length = CUR_LENGTH;
>>> + *pi_length = p_demux->info.i_title < p_sys->i_title ? CUR_LENGTH : 0;
>>> return VLC_SUCCESS;
>>> }
>>> case DEMUX_SET_TIME:
>>> @@ -376,7 +388,8 @@ static int blurayControl(demux_t *p_demux, int query, va_list args)
>>> case DEMUX_GET_POSITION:
>>> {
>>> double *pf_position = (double*)va_arg( args, double * );
>>> - *pf_position = (double)FROM_TICKS(bd_tell_time(p_sys->bluray))/CUR_LENGTH;
>>> + *pf_position = p_demux->info.i_title < p_sys->i_title ?
>>> + (double)FROM_TICKS(bd_tell_time(p_sys->bluray))/CUR_LENGTH : 0.0;
>>> return VLC_SUCCESS;
>>> }
>>> case DEMUX_SET_POSITION:
>>> @@ -423,6 +436,41 @@ static int blurayControl(demux_t *p_demux, int query, va_list args)
>>> return VLC_SUCCESS;
>>> }
>>>
>>> +static void blurayHandleEvent( demux_t *p_demux, const BD_EVENT *e )
>>> +{
>>> + demux_sys_t *p_sys = p_demux->p_sys;
>>> +
>>> + switch( e->event )
>>> + {
>>> + case BD_EVENT_TITLE:
>>> + if ( e->param < p_sys->i_title )
>>> + blurayUpdateTitle( p_demux, e->param );
>>> + break ;
>>> + case BD_EVENT_PLAYITEM:
>>> + break ;
>>> + case BD_EVENT_AUDIO_STREAM:
>>> + break ;
>>> + case BD_EVENT_CHAPTER:
>>> + p_demux->info.i_update |= INPUT_UPDATE_SEEKPOINT;
>>> + p_demux->info.i_seekpoint = 0;
>>> + break ;
>>> + case BD_EVENT_ANGLE:
>>> + case BD_EVENT_IG_STREAM:
>>> + default:
>>> + msg_Warn( p_demux, "event: %d param: %d", e->event, e->param );
>>> + break ;
>>> + }
>>> +}
>>> +
>>> +static void blurayHandleEvents( demux_t *p_demux )
>>> +{
>>> + BD_EVENT e;
>>> +
>>> + while ( bd_get_event( p_demux->p_sys->bluray, &e ) )
>>> + {
>>> + blurayHandleEvent( p_demux, &e );
>>> + }
>>> +}
>>>
>>> #define BD_TS_PACKET_SIZE (192)
>>> #define NB_TS_PACKETS (200)
>>> @@ -436,6 +484,7 @@ static int blurayDemux(demux_t *p_demux)
>>> return -1;
>>> }
>>>
>>> + blurayHandleEvents( p_demux );
>>> int nread = bd_read(p_sys->bluray, p_block->p_buffer,
>>> NB_TS_PACKETS * BD_TS_PACKET_SIZE);
>>> if (nread < 0) {
>>
>> Otherwise OK.
>>
>>
>>> Subject: [PATCH 02/11] bluray: Fixing a warning
>>>
>>> ---
>>> modules/access/bluray.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/modules/access/bluray.c b/modules/access/bluray.c
>>> index 443d884..ea1bca8 100644
>>> --- a/modules/access/bluray.c
>>> +++ b/modules/access/bluray.c
>>> @@ -401,7 +401,7 @@ static int blurayControl(demux_t *p_demux, int query, va_list args)
>>>
>>> case DEMUX_GET_META:
>>> {
>>> - struct meta_dl *meta = bd_get_meta(p_sys->bluray);
>>> + const struct meta_dl *meta = bd_get_meta(p_sys->bluray);
>>> if(!meta)
>>> return VLC_EGENERIC;
>>>
>> OK.
>>
>>
>>> Subject: [PATCH 03/11] bluray: Preparing menu handling.
>>>
>>> ---
>>> modules/access/bluray.c | 76 +++++++++++++++++++++++++++++++++++++++-------
>>> 1 files changed, 64 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/modules/access/bluray.c b/modules/access/bluray.c
>>> index ea1bca8..9733725 100644
>>> --- a/modules/access/bluray.c
>>> +++ b/modules/access/bluray.c
>>> @@ -35,11 +35,16 @@
>>>
>>> #include <libbluray/bluray.h>
>>> #include <libbluray/meta_data.h>
>>> +#include <libbluray/overlay.h>
>>
>> Is this include needed for this patch
>>
> No, it has been moved to the appropriate patch.
>
>>> @@ -75,7 +81,8 @@ struct demux_sys_t
>>> * Local prototypes
>>> *****************************************************************************/
>>> static int blurayControl(demux_t *, int, va_list);
>>> -static int blurayDemux (demux_t *);
>>> +static int blurayDemux(demux_t *);
>>> +static int blurayDemuxMenu(demux_t *);
>> Forward declaration.
>>
> 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.
>
>>> @@ -197,7 +216,7 @@ static int blurayOpen( vlc_object_t *object )
>>> }
>>>
>>> p_demux->pf_control = blurayControl;
>>> - p_demux->pf_demux = blurayDemux;
>>> + p_demux->pf_demux = b_menu ? blurayDemuxMenu : blurayDemux;
>>>
>>> return VLC_SUCCESS;
>>>
>>> @@ -484,17 +503,50 @@ static int blurayDemux(demux_t *p_demux)
>>> return -1;
>>> }
>>>
>>> + //Handle events first.
>>> blurayHandleEvents( p_demux );
>>> +
>>> int nread = bd_read(p_sys->bluray, p_block->p_buffer,
>>> NB_TS_PACKETS * BD_TS_PACKET_SIZE);
>>> +
>>> if (nread < 0) {
>>> block_Release(p_block);
>>> return nread;
>>> }
>>> -
>> Some cosmetics.
> Moved to another patch (not sent here, we'll handle cosmetic later)
>
>>
>>> +static int blurayDemuxMenu( demux_t *p_demux )
>>> +{
>>> + demux_sys_t *p_sys = p_demux->p_sys;
>>> + BD_EVENT e;
>>> +
>>> + block_t *p_block = block_New(p_demux, NB_TS_PACKETS * (int64_t)BD_TS_PACKET_SIZE);
>>> + int nread = bd_read_ext( p_sys->bluray, p_block->p_buffer,
>>> + NB_TS_PACKETS * BD_TS_PACKET_SIZE, &e );
>>> + /*
>>> + * bd_read_ext will return 0 if there is some events to process.
>>> + * We need to handle them before doing anything else
>>> + */
>>> + if ( nread == 0 )
>>> + {
>>> + if ( e.event == BD_EVENT_NONE )
>>> + msg_Info( p_demux, "We reached the end of a title" );
>>> + else
>>> + blurayHandleEvent( p_demux, &e );
>>> + block_Release(p_block);
>>> + return 1;
>>> + }
>>> + if (nread < 0)
>>> + {
>>> + block_Release(p_block);
>>> + return nread;
>>> + }
>>> + p_block->i_buffer = nread;
>>>
>>> + stream_DemuxSend( p_sys->p_parser, p_block );
>>> return 1;
>>> }
>> Is it worth it to create another demux function ?
>>
> 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.
>
>>
>>
>>> Subject: [PATCH 04/11] bluray: Adding an overlay handling skeleton.
>>>
>>> ---
>>> modules/access/bluray.c | 15 +++++++++++++--
>>> 1 files changed, 13 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/modules/access/bluray.c b/modules/access/bluray.c
>>> index 9733725..c6a2243 100644
>>> --- a/modules/access/bluray.c
>>> +++ b/modules/access/bluray.c
>>> @@ -87,8 +87,10 @@ static int blurayDemuxMenu(demux_t *);
>>> static int blurayInitTitles(demux_t *p_demux );
>>> static int bluraySetTitle(demux_t *p_demux, int i_title);
>>>
>>> -static void blurayHandleEvent( demux_t *p_demux, const BD_EVENT *e );
>>> -static void blurayHandleEvents( demux_t *p_demux );
>>> +static void blurayHandleEvent( demux_t *p_demux, const BD_EVENT *e );
>>> +static void blurayHandleEvents( demux_t *p_demux );
>>> +
>> Cosmetics.
> These forward declarations have been removed.
>
>>
>>> +static void blurayOverlayProc( void *ptr, const BD_OVERLAY * const overlay );
>> Forward declaration.
> Removed.
>
>>
>>> #define FROM_TICKS(a) (a*CLOCK_FREQ / INT64_C(90000))
>>> #define TO_TICKS(a) (a*INT64_C(90000)/CLOCK_FREQ)
>>> @@ -192,6 +194,8 @@ static int blurayOpen( vlc_object_t *object )
>>> * Therefore, We don't have to select any title.
>>> */
>>> bd_play( p_sys->bluray );
>>> + /* Registering overlay event handler */
>>> + bd_register_overlay_proc( p_sys->bluray, p_demux, blurayOverlayProc );
>>> }
>>> else
>>> {
>>> @@ -250,6 +254,13 @@ static void blurayClose( vlc_object_t *object )
>>> free(p_sys);
>>> }
>>>
>>> +static void blurayOverlayProc( void *ptr, const BD_OVERLAY *const overlay )
>>> +{
>>> + demux_t *p_demux = (demux_t*)ptr;
>>> + demux_sys_t *p_sys = p_demux->p_sys;
>>> +
>>> + //FIXME
>>> +}
>> I think that this patch should be merged with some following ones (I
>> haven't checked which one would make the most senses).
>>
> I've merged this one, the "Handle overlay" and "Handle overlay
> updating" (formerly 4, 6 and 8) in one patch.
> Patch 8 wasn't reviewed in this batch, but you did refer to it as the
> correct way of handling subpicture, so I guess that was the best way.
>
>
>>
>>> Subject: [PATCH 05/11] bluray: Nasty hack to avoid crashing when changing
>>> title.
>>>
>>> ---
>>> modules/access/bluray.c | 17 +++++++++++++++--
>>> 1 files changed, 15 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/modules/access/bluray.c b/modules/access/bluray.c
>>> index c6a2243..6d8e1bd 100644
>>> --- a/modules/access/bluray.c
>>> +++ b/modules/access/bluray.c
>>> @@ -300,8 +300,22 @@ static int blurayInitTitles(demux_t *p_demux )
>>> return VLC_SUCCESS;
>>> }
>>>
>>> +static void blurayResetParser( demux_t *p_demux )
>>> +{
>>> + demux_sys_t *p_sys = p_demux->p_sys;
>>> + if ( p_sys->p_parser )
>>> + stream_Delete( p_sys->p_parser );
>>> + p_sys->p_parser = stream_DemuxNew(p_demux, "ts", p_demux->out);
>>> + if (!p_sys->p_parser) {
>>> + msg_Err(p_demux, "Failed to create TS demuxer");
>>> + }
>>> +}
>>> +
>>> static void blurayUpdateTitle( demux_t *p_demux, int i_title )
>>> {
>>> + blurayResetParser( p_demux );
>>> + if ( i_title >= p_demux->p_sys->i_title )
>>> + return ;
>>> /* read title info and init some values */
>>> p_demux->info.i_title = i_title;
>>> p_demux->info.i_seekpoint = 0;
>>> @@ -473,8 +487,7 @@ static void blurayHandleEvent( demux_t *p_demux, const BD_EVENT *e )
>>> switch( e->event )
>>> {
>>> case BD_EVENT_TITLE:
>>> - if ( e->param < p_sys->i_title )
>>> - blurayUpdateTitle( p_demux, e->param );
>>> + blurayUpdateTitle( p_demux, e->param );
>>> break ;
>>> case BD_EVENT_PLAYITEM:
>>> break ;
>> Why is this patch needed? The comment make it look like an workaround
>> instead of a proper fix.
>>
> It is, the aim was to have something working, because titles will
> always change when moving from FIRST_TITLE to the actual title.
> I focused on the bluray part, but will have a look at this later.
> This hack is also used in old bluray access_demux. ( bd.c:633)
> I've add some comments in the code and will work on a clean way of
> flushing the stream_demux asap.
>
>>
>>> Subject: [PATCH 06/11] bluray: Adding support for overlay.
>>> @@ -4,6 +4,7 @@
>>> * Copyright © 2010-2011 VideoLAN, VLC authors and libbluray AUTHORS
>>> *
>>> * Authors: Jean-Baptiste Kempf <jb at videolan.org>
>>> + * Hugo Beauzée-Luyssen <hugo at videolan.org>
>> It doesn't belong to this patch.
>>
> Removed.
>
>>> @@ -32,6 +33,8 @@
>>> #include <vlc_demux.h> /* demux_t */
>>> #include <vlc_input.h> /* Seekpoints, chapters */
>>> #include <vlc_dialog.h> /* BD+/AACS warnings */
>>> +#include <vlc_picture.h>
>>> +#include <vlc_vout.h>
>>>
>>> #include <libbluray/bluray.h>
>>> #include <libbluray/meta_data.h>
>>> @@ -63,6 +66,8 @@ vlc_module_begin ()
>>> set_callbacks( blurayOpen, blurayClose )
>>> vlc_module_end ()
>>>
>>> +//libbluray's overlay.h defines 2 types of overlay.
>>> +#define MAX_OVERLAY 2
>> Shouldn't it then based on a define of libbluray's overlay.h?
>>
> Actualy there is no define for this. This could be patched in
> libbluray afterward.
>
>>> @@ -73,6 +78,13 @@ struct demux_sys_t
>>> unsigned int i_longest_title;
>>> input_title_t **pp_title;
>>>
>>> + /* Overlay stuff */
>>> + subpicture_t *p_pic[MAX_OVERLAY];
>>> + int current_overlay; // -1 if no current overlay;
>>> +
>>> + input_thread_t *p_input;
>>> + vout_thread_t *p_vout;
>>> +
>>> /* TS stream */
>>> stream_t *p_parser;
>>> };
>>> @@ -119,7 +131,8 @@ static int blurayOpen( vlc_object_t *object )
>>> if (unlikely(!p_sys)) {
>>> return VLC_ENOMEM;
>>> }
>>> - p_sys->p_parser = NULL;
>>> + memset( p_sys, 0, sizeof( *p_sys ) );
>> Use calloc instead of mallocing p_sys then.
>>
> Done.
>
>>> + p_sys->current_overlay = -1;
>>>
>>> /* init demux info fields */
>>> p_demux->info.i_update = 0;
>>> @@ -240,6 +256,18 @@ static void blurayClose( vlc_object_t *object )
>>> demux_t *p_demux = (demux_t*)object;
>>> demux_sys_t *p_sys = p_demux->p_sys;
>>>
>>> + /*
>>> + * Close libbluray first. This will close all the overlays before we release p_vout
>>> + *
>>> + * bd_close( NULL ) can crash
>>> + */
>>> + assert(p_sys->bluray);
>>> + bd_close(p_sys->bluray);
>>> +
>>> + if ( p_sys->p_vout != NULL )
>>> + vlc_object_release( p_sys->p_vout );
>>> + if ( p_sys->p_input != NULL )
>>> + vlc_object_release( p_sys->p_input );
>>> if (p_sys->p_parser)
>>> stream_Delete(p_sys->p_parser);
>>>
>>> @@ -248,18 +276,147 @@ static void blurayClose( vlc_object_t *object )
>>> vlc_input_title_Delete(p_sys->pp_title[i]);
>>> TAB_CLEAN( p_sys->i_title, p_sys->pp_title );
>>>
>>> - /* bd_close( NULL ) can crash */
>>> - assert(p_sys->bluray);
>>> - bd_close(p_sys->bluray);
>>> free(p_sys);
>>> }
>>>
>>> +static void blurayDeleteOverlaySubPic( demux_t *p_demux, int i_plane )
>>> +{
>>> + demux_sys_t *p_sys = p_demux->p_sys;
>>> +
>>> + if ( p_sys->p_pic[i_plane] == NULL )
>>> + return ;
>> It's a cosmetic remarks, but it might be better to change i_plane into
>> i_layer or something like that (unless the libluray does use plane of
>> course). For (sub)picture, plane usually means the pixel plane.
>>
> Done.
>
>>> + if ( p_sys->p_vout )
>>> + vout_FlushSubpictureChannel( p_sys->p_vout, p_sys->p_pic[i_plane]->i_channel );
>>> + else
>>> + subpicture_Delete( p_sys->p_pic[i_plane] );
>>> + p_sys->p_pic[i_plane] = NULL;
>>> +}
>> The subpicture_Delete() being called only in one place looks suspicious.
>>
> subpicture_New is only called once when the BD_OVERLAY_INIT command is
> sent to the overlay callback.
> If I'm not mistaking this command is only sent the first time, and
> after the callback has been called with a NULL BD_OVERLAY param, which
> closes all the existing overlay, thus calling subpicture_Delete.
>
>>> +static void blurayCloseOverlay( demux_t *p_demux )
>>> +{
>>> + demux_sys_t *p_sys = p_demux->p_sys;
>>> +
>>> + for ( int i = 0; i < MAX_OVERLAY; ++i )
>>> + blurayDeleteOverlaySubPic( p_demux, i );
>>> + p_demux->p_sys->current_overlay = -1;
>>> + if ( p_sys->p_vout != NULL )
>>> + {
>>> + vlc_object_release( p_sys->p_vout );
>>> + p_sys->p_vout = NULL;
>>> + }
>>> +}
>>> +
>>> +static void blurayDisplayOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
>>> +{
>>> + demux_sys_t *p_sys = p_demux->p_sys;
>>> + /*
>>> + * Mark the overlay as available, but don't display it right now.
>>> + * the blurayDemuxMenu will send it to vout, as it may be unavailable when
>>> + * the overlay is computed
>>> + */
>>> + p_sys->current_overlay = ov->plane;
>>> +}
>>> +
>>> +static void blurayInitOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
>>> +{
>>> + demux_sys_t *p_sys = p_demux->p_sys;
>>> +
>>> + if ( p_sys->p_pic[ov->plane] != NULL )
>>> + blurayDeleteOverlaySubPic( p_demux, ov->plane );
>>> +
>>> + p_sys->p_pic[ov->plane] = subpicture_New( NULL );
>>> + p_sys->p_pic[ov->plane]->i_original_picture_width = ov->w;
>>> + p_sys->p_pic[ov->plane]->i_original_picture_height = ov->h;
>>> + p_sys->p_pic[ov->plane]->b_ephemer = true;
>>> + p_sys->p_pic[ov->plane]->b_absolute = true;
>>> +}
>>> +
>>> +static void blurayClearOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
>>> +{
>>> + demux_sys_t *p_sys = p_demux->p_sys;
>>> +
>>> + //FIXME
>>> +}
>>> +
>>> +static void blurayDrawOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
>>> +{
>>> + demux_sys_t *p_sys = p_demux->p_sys;
>>> +
>>> + if ( ov->img )
>> You can reverse the test, it will avoid a full level of indentention.
> Done.
>
>>> + {
>>> + video_format_t fmt;
>>> + video_format_Init( &fmt, 0 );
>>> + video_format_Setup( &fmt, VLC_CODEC_YUVP, ov->w, ov->h, 1, 1 );
>>> +
>>> + subpicture_region_t *region = subpicture_region_New( &fmt );
>>> + region->i_x = ov->x;
>>> + region->i_y = ov->y;
>>> +
>>> + const BD_PG_RLE_ELEM *img = ov->img;
>>> + for ( int y = 0; y < ov->h; y++ )
>>> + {
>>> + for ( int x = 0; x < ov->w; )
>>> + {
>>> + memset( region->p_picture->p[0].p_pixels +
>>> + y * region->p_picture->p[0].i_pitch + x,
>>> + img->color, img->len );
>>> + x += img->len;
>>> + img++;
>>> + }
>>> + }
>>> + if ( ov->palette )
>>> + {
>>> + region->fmt.p_palette->i_entries = 256;
>>> + for ( int i = 0; i < 256; ++i )
>>> + {
>>> + region->fmt.p_palette->palette[i][0] = ov->palette[i].Y;
>>> + region->fmt.p_palette->palette[i][1] = ov->palette[i].Cb;
>>> + region->fmt.p_palette->palette[i][2] = ov->palette[i].Cr;
>>> + region->fmt.p_palette->palette[i][3] = ov->palette[i].T;
>>> + }
>>> + }
>>> + //Append the new region at the end of our current regions:
>>> + subpicture_region_t *p_reg = p_sys->p_pic[ov->plane]->p_region;
>>> + if ( p_reg == NULL )
>>> + {
>>> + p_sys->p_pic[ov->plane]->p_region = region;
>>> + return ;
>>> + }
>>> + while ( p_reg && p_reg->p_next )
>>> + p_reg = p_reg->p_next;
>>> + p_reg->p_next = region;
>>> + }
>>> +}
>>> +
>>> static void blurayOverlayProc( void *ptr, const BD_OVERLAY *const overlay )
>>> {
>>> demux_t *p_demux = (demux_t*)ptr;
>>> - demux_sys_t *p_sys = p_demux->p_sys;
>>>
>>> - //FIXME
>>> + if ( overlay == NULL )
>>> + {
>>> + msg_Info( p_demux, "Closing overlay." );
>>> + blurayCloseOverlay( p_demux );
>>> + return ;
>>> + }
>>> + switch ( overlay->cmd )
>>> + {
>>> + case BD_OVERLAY_INIT:
>>> + blurayInitOverlay( p_demux, overlay );
>>> + break ;
>>> + case BD_OVERLAY_CLEAR:
>>> + blurayClearOverlay( p_demux, overlay );
>>> + break ;
>>> + case BD_OVERLAY_FLUSH:
>>> + msg_Info( p_demux, "Displaying overlay" );
>>> + blurayDisplayOverlay( p_demux, overlay );
>>> + break ;
>>> + case BD_OVERLAY_DRAW:
>>> + blurayDrawOverlay( p_demux, overlay );
>>> + break ;
>>> + default:
>>> + msg_Warn( p_demux, "Unknown BD overlay command: %u", overlay->cmd );
>>> + break ;
>>> + }
>>> }
>>>
>>> static int blurayInitTitles(demux_t *p_demux )
>>> @@ -557,11 +714,9 @@ static int blurayDemuxMenu( demux_t *p_demux )
>>> */
>>> if ( nread == 0 )
>>> {
>>> - if ( e.event == BD_EVENT_NONE )
>>> - msg_Info( p_demux, "We reached the end of a title" );
>>> - else
>>> + if ( e.event != BD_EVENT_NONE )
>>> blurayHandleEvent( p_demux, &e );
>>> - block_Release(p_block);
>>> + block_Release( p_block );
>>> return 1;
>>> }
>>> if (nread < 0)
>>> @@ -571,6 +726,20 @@ static int blurayDemuxMenu( demux_t *p_demux )
>>> }
>>> p_block->i_buffer = nread;
>>>
>>> + if ( p_sys->current_overlay != -1 )
>>> + {
>>> + if ( p_sys->p_vout == NULL )
>>> + p_sys->p_vout = input_GetVout( p_sys->p_input );
>>> + if ( p_sys->p_vout != NULL )
>>> + {
>>> + p_sys->p_pic[p_sys->current_overlay]->i_start =
>>> + p_sys->p_pic[p_sys->current_overlay]->i_stop = mdate();
>>> + p_sys->p_pic[p_sys->current_overlay]->i_channel =
>>> + vout_RegisterSubpictureChannel( p_sys->p_vout );
>>> + vout_PutSubpicture( p_sys->p_vout, p_sys->p_pic[p_sys->current_overlay] );
>> That's not valid, a picture sent to the vout must not be accessed later.
>> You must merged the part that properly does that from the next patch into
>> this one.
>>
> Patches has been merged.
>
>>
>>> Subject: [PATCH 07/11] bluray: Handle mouse event.
>>>
>>> ---
>>> modules/access/bluray.c | 69 +++++++++++++++++++++++++++++++++++++++++-----
>>> 1 files changed, 61 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/modules/access/bluray.c b/modules/access/bluray.c
>>> index 15f62d3..1f78975 100644
>>> --- a/modules/access/bluray.c
>>> +++ b/modules/access/bluray.c
>>> @@ -37,6 +37,7 @@
>>> #include <vlc_vout.h>
>>>
>>> #include <libbluray/bluray.h>
>>> +#include <libbluray/keys.h>
>>> #include <libbluray/meta_data.h>
>>> #include <libbluray/overlay.h>
>>>
>>> @@ -104,6 +105,9 @@ static void blurayHandleEvents( demux_t *p_demux );
>>>
>>> static void blurayOverlayProc( void *ptr, const BD_OVERLAY * const overlay );
>>>
>>> +static int onMouseEvent( vlc_object_t *p_vout, const char *psz_var, vlc_value_t old,
>>> + vlc_value_t val, void *p_data );
>>> +
>> Forward declaration.
> Removed.
>
>>
>>> +/*****************************************************************************
>>> + * libbluray overlay handling:
>>> + *****************************************************************************/
>>> +
>> Does not belong to this patch (should be merged with the right patch).
>>> static void blurayDeleteOverlaySubPic( demux_t *p_demux, int i_plane )
>>> {
>>> demux_sys_t *p_sys = p_demux->p_sys;
>>> @@ -301,12 +309,14 @@ static void blurayCloseOverlay( demux_t *p_demux )
>>> p_demux->p_sys->current_overlay = -1;
>>> if ( p_sys->p_vout != NULL )
>>> {
>>> + var_DelCallback( p_sys->p_vout, "mouse-moved", &onMouseEvent, p_demux );
>>> + var_DelCallback( p_sys->p_vout, "mouse-clicked", &onMouseEvent, p_demux );
>>> vlc_object_release( p_sys->p_vout );
>>> p_sys->p_vout = NULL;
>>> }
>>> }
>>>
>>> -static void blurayDisplayOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
>>> +static void blurayActivateOverlay( demux_t *p_demux, const BD_OVERLAY* const ov )
>>> {
>>> demux_sys_t *p_sys = p_demux->p_sys;
>>> /*
>>> @@ -408,7 +418,7 @@ static void blurayOverlayProc( void *ptr, const BD_OVERLAY *const overlay )
>>> break ;
>>> case BD_OVERLAY_FLUSH:
>>> msg_Info( p_demux, "Displaying overlay" );
>>> - blurayDisplayOverlay( p_demux, overlay );
>>> + blurayActivateOverlay( p_demux, overlay );
>>> break ;
>>> case BD_OVERLAY_DRAW:
>>> blurayDrawOverlay( p_demux, overlay );
>>> @@ -419,6 +429,54 @@ static void blurayOverlayProc( void *ptr, const BD_OVERLAY *const overlay )
>>> }
>>> }
>>>
>>> +/*****************************************************************************
>>> + * overlay display and events:
>>> + *****************************************************************************/
>>> +
>>> +static int onMouseEvent( vlc_object_t *p_vout, const char *psz_var, vlc_value_t old,
>>> + vlc_value_t val, void *p_data )
>>> +{
>>> + demux_t *p_demux = (demux_t*)p_data;
>>> + demux_sys_t *p_sys = p_demux->p_sys;
>>> + VLC_UNUSED( old );
>>> + VLC_UNUSED( p_vout );
>>> +
>>> + if ( psz_var[6] == 'm' ) //Mouse moved
>>> + bd_mouse_select( p_sys->bluray, mdate(), val.coords.x, val.coords.y );
>>> + else if ( psz_var[6] == 'c' )
>>> + {
>>> + int64_t now = mdate();
>> It could be moved a bit higher to be reused.
> Done.
>
>>> + bd_mouse_select( p_sys->bluray, now, val.coords.x, val.coords.y );
>>> + bd_user_input( p_sys->bluray, now, BD_VK_MOUSE_ACTIVATE );
>>> + }
>>> + else
>>> + {
>>> + msg_Warn( p_demux, "Unknown mouse event: %s", psz_var );
>>> + return VLC_EGENERIC;
>> You could assert instead as it cannot happens.
> Done.
>
>>> + }
>>> + return VLC_SUCCESS;
>>> +}
>>> +
>>> +static void blurayDisplayOverlay( demux_t *p_demux )
>>> +{
>>> + demux_sys_t *p_sys = p_demux->p_sys;
>>> +
>>> + //Sending subpicture to vout:
>>> + p_sys->p_pic[p_sys->current_overlay]->i_start =
>>> + p_sys->p_pic[p_sys->current_overlay]->i_stop = mdate();
>>> + p_sys->p_pic[p_sys->current_overlay]->i_channel =
>>> + vout_RegisterSubpictureChannel( p_sys->p_vout );
>>> + vout_PutSubpicture( p_sys->p_vout, p_sys->p_pic[p_sys->current_overlay] );
>> Still not valid.
> Should be fixed by merging the two overlay patches.
>
>>> + p_sys->current_overlay = -1;
>>> + //Activating vout events handling:
>>> + var_AddCallback( p_sys->p_vout, "mouse-moved", &onMouseEvent, p_demux );
>>> + var_AddCallback( p_sys->p_vout, "mouse-clicked", &onMouseEvent, p_demux );
>> I am unsure, but I don't think you should do it everytime (unless you do unregister
>> the same amounts of time). It would be cleaner to attach them when you get the vout
>> variable (and detach them when you release it).
>>
> Indeed. Fixed.
>
>>> +/*****************************************************************************
>>> + * Titles management:
>>> + *****************************************************************************/
>>> +
>> It does not belong to this patch.
>>
>>> static int blurayInitTitles(demux_t *p_demux )
>>> {
>>> demux_sys_t *p_sys = p_demux->p_sys;
>>> @@ -732,12 +790,7 @@ static int blurayDemuxMenu( demux_t *p_demux )
>>> p_sys->p_vout = input_GetVout( p_sys->p_input );
>>> if ( p_sys->p_vout != NULL )
>>> {
>>> - p_sys->p_pic[p_sys->current_overlay]->i_start =
>>> - p_sys->p_pic[p_sys->current_overlay]->i_stop = mdate();
>>> - p_sys->p_pic[p_sys->current_overlay]->i_channel =
>>> - vout_RegisterSubpictureChannel( p_sys->p_vout );
>>> - vout_PutSubpicture( p_sys->p_vout, p_sys->p_pic[p_sys->current_overlay] );
>>> - p_sys->current_overlay = -1;
>>> + blurayDisplayOverlay( p_demux );
>> It would be better to have introduce the blurayDisplayOverlay() function earlier.
>>> }
>>> }
>>> stream_DemuxSend( p_sys->p_parser, p_block );
>>
>> I will review the other patches once the remarks about the ones in this
>> mail has been taken care of (it will be easier for every one I think).
>>
>> Regards,
>>
>
Let's pretend I didn't send a patch with an unmerged conflict in it, shall we?
Also, these patches have been rebased against master, as bluray.c
changed since last time.
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/20120218/63cf1a9d/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-bluray-Fixing-a-warning.patch
Type: application/octet-stream
Size: 808 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120218/63cf1a9d/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-bluray-Preparing-menu-handling.patch
Type: application/octet-stream
Size: 4641 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120218/63cf1a9d/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: 1838 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120218/63cf1a9d/attachment-0003.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-bluray-Adding-support-for-overlay.patch
Type: application/octet-stream
Size: 19210 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120218/63cf1a9d/attachment-0004.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-bluray-Handle-mouse-event.patch
Type: application/octet-stream
Size: 2975 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120218/63cf1a9d/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: 1041 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120218/63cf1a9d/attachment-0006.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0008-bluray-Use-refcounting-to-avoid-accessing-invalid-fi.patch
Type: application/octet-stream
Size: 17885 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20120218/63cf1a9d/attachment-0007.obj>
More information about the vlc-devel
mailing list