[vlc-devel] [Patches] bluray menu
Hugo Beauzée-Luyssen
beauze.h at gmail.com
Sun Feb 26 18:13:32 CET 2012
On Sat, Feb 18, 2012 at 1:24 AM, Hugo Beauzée-Luyssen
<beauze.h at gmail.com> wrote:
> 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,
>
>
Hello,
Ping! :)
Another question though, should this be backported to 2.0? (When and
if it get applied)
Regards,
--
Hugo Beauzée-Luyssen
More information about the vlc-devel
mailing list