[vlc-devel] [PATCH] VLM add inputs events

[Laurent Aimar]

Hi,

I will first comment only on the vlm core part.
Could you resend your patch splitted in 2 part: libvlc and vlm ? (As the vlm
part is independant).

On Wed, Jul 01, 2009, Sébastien Escudier wrote:
> diff --git a/include/vlc_vlm.h b/include/vlc_vlm.h
> index 0f16f4d..a17090b 100644
> --- a/include/vlc_vlm.h
> +++ b/include/vlc_vlm.h
> @@ -102,6 +102,12 @@ enum vlm_event_type_e
>      /* */
>      VLM_EVENT_MEDIA_INSTANCE_STARTED    = 0x200,
>      VLM_EVENT_MEDIA_INSTANCE_STOPPED,
> +    VLM_EVENT_MEDIA_INSTANCE_INIT,
> +    VLM_EVENT_MEDIA_INSTANCE_PLAYING,
> +    VLM_EVENT_MEDIA_INSTANCE_OPENING,
> +    VLM_EVENT_MEDIA_INSTANCE_PAUSE,
> +    VLM_EVENT_MEDIA_INSTANCE_END,
> +    VLM_EVENT_MEDIA_INSTANCE_ERROR,
 It would be simpler to have a unique event like VLM_EVENT_MEDIA_INSTANCE_STATE
(or a better name) and simply reuse input_state_e (at least at the vlm core
level).

>  };
>  
>  typedef struct
> diff --git a/src/input/vlm.c b/src/input/vlm.c
> index c7cf269..e49be6b 100644
> --- a/src/input/vlm.c
> +++ b/src/input/vlm.c
> @@ -69,6 +69,42 @@ static void vlm_Destructor( vlm_t *p_vlm );
>  static void* Manage( void * );
>  static int vlm_MediaVodControl( void *, vod_media_t *, const char *, int, va_list );
>  
> +static int InputEvent  ( vlc_object_t *p_this, char const *psz_cmd,
> +                         vlc_value_t oldval, vlc_value_t newval,
> +                         void *p_data )
> +{
> +    input_thread_t *p_input = (input_thread_t *)p_this;
> +    vlm_t *p_vlm = libvlc_priv( p_input->p_libvlc )->p_vlm;
> +    vlm_media_sys_t *p_media = (vlm_media_sys_t*)p_data;
 Useless cast.
> +    const int i_unknownId = -1;
 Could you use the right id at this point ? I think p_media->cfg.id is valid
at this stage, no ? About the event, it might also be better to send the
instance name as well (vlm_media_instance_t::psz_name).
 Also, avoid camel case.

> +    if( newval.i_int == INPUT_EVENT_STATE )
> +    {
> +        switch ( var_GetInteger( p_input, "state" ) )
> +        {
> +            case INIT_S:
> +                vlm_SendEventMediaInstanceInit( p_vlm, i_unknownId, p_media->cfg.psz_name );
> +                break;
> +            case OPENING_S:
> +                vlm_SendEventMediaInstanceOpening( p_vlm, i_unknownId, p_media->cfg.psz_name );
> +                break;
> +            case PLAYING_S:
> +                vlm_SendEventMediaInstancePlaying( p_vlm, i_unknownId, p_media->cfg.psz_name );
> +                break;
> +            case PAUSE_S:
> +                vlm_SendEventMediaInstancePause( p_vlm, i_unknownId, p_media->cfg.psz_name );
> +                break;
> +            case END_S:
> +                vlm_SendEventMediaInstanceEnd( p_vlm, i_unknownId, p_media->cfg.psz_name );
> +                break;
> +            case ERROR_S:
> +                vlm_SendEventMediaInstanceError( p_vlm, i_unknownId, p_media->cfg.psz_name );
> +                break;
 If a unique VLM_EVENT_MEDIA_INSTANCE_STATE is used, it can be greatly
simplified (same for vlm event).

Regards,

-- 
fenrir