[vlc-devel] [PATCHv2 05/10] input: always use helpers for es_out privcontrols

Thomas Guillem thomas at gllm.fr
Fri Feb 28 07:59:41 CET 2020



On Thu, Feb 27, 2020, at 21:44, Remi Denis-Courmont wrote:
> Le 2020-02-27 17:19, Thomas Guillem a écrit :
> > Mainly to avoid confusion between private and public controls.
> 
> I'm not against the patch as such, but I don't really get how it reduces 
> the risk of confusion. What reduces the risk of confusion is that 
> they're not defined in include/, i.e. the previous patches.

These 2 patches are not perfect yet.
There is no protection against using a public query from the private control and vice-versa.

I wanted to add enums in control and privcontrol parameters to add more protection but it's not that easy:
 - c++ doesn't like undefined typedef enum (for the private ones)
 - some module add private control internally (and we don't want to extend the public enum just for them)

I will try to improve it again, but later, when I pushed everything (I have 43 patches in my branch).

> 
> > ---
> >  src/input/es_out.h | 42 ++++++++++++++++++++++++++++++++++++++++++
> >  src/input/input.c  | 31 +++++++++++++------------------
> >  2 files changed, 55 insertions(+), 18 deletions(-)
> > 
> > diff --git a/src/input/es_out.h b/src/input/es_out.h
> > index 421f0d3f40e..7fab0fb8d43 100644
> > --- a/src/input/es_out.h
> > +++ b/src/input/es_out.h
> > @@ -129,6 +129,37 @@ static inline vlc_tick_t es_out_GetWakeup(
> > es_out_t *p_out )
> >      assert( !i_ret );
> >      return i_wu;
> >  }
> > +static inline int es_out_SetEsList( es_out_t *p_out,
> > +                                    enum es_format_category_e cat,
> > +                                    vlc_es_id_t **ids )
> > +{
> > +    return es_out_PrivControl( p_out, ES_OUT_PRIV_SET_ES_LIST, cat, 
> > ids );
> > +}
> > +static inline int es_out_SetAutoSelect( es_out_t *p_out,
> > +                                        enum es_format_category_e
> > cat, bool enabled )
> > +{
> > +    return es_out_PrivControl( p_out, ES_OUT_PRIV_SET_AUTOSELECT,
> > cat, enabled );
> > +}
> > +static inline int es_out_SetEsById( es_out_t *p_out, int id, bool 
> > forced )
> > +{
> > +    return es_out_PrivControl( p_out, ES_OUT_PRIV_SET_ES_BY_ID, id, 
> > forced );
> > +}
> > +static inline int es_out_RestartEsById( es_out_t *p_out, int id )
> > +{
> > +    return es_out_PrivControl( p_out, ES_OUT_PRIV_RESTART_ES_BY_ID, id 
> >  );
> > +}
> > +static inline int es_out_SetEsDefaultById( es_out_t *p_out, int id )
> > +{
> > +    return es_out_PrivControl( p_out, 
> > ES_OUT_PRIV_SET_ES_DEFAULT_BY_ID, id );
> > +}
> > +static inline int es_out_StopAllEs( es_out_t *p_out, void **context )
> > +{
> > +    return es_out_PrivControl( p_out, ES_OUT_PRIV_STOP_ALL_ES, context 
> > );
> > +}
> > +static inline int es_out_StartAllEs( es_out_t *p_out, void *context )
> > +{
> > +    return es_out_PrivControl( p_out, ES_OUT_PRIV_START_ALL_ES, 
> > context );
> > +}
> >  static inline bool es_out_GetBuffering( es_out_t *p_out )
> >  {
> >      bool b;
> > @@ -198,6 +229,17 @@ static inline void es_out_Eos( es_out_t *p_out )
> >      int i_ret = es_out_PrivControl( p_out, ES_OUT_PRIV_SET_EOS );
> >      assert( !i_ret );
> >  }
> > +static inline int es_out_SetVbiPage( es_out_t *p_out, vlc_es_id_t *id,
> > +                                     unsigned page )
> > +{
> > +    return es_out_PrivControl( p_out, ES_OUT_PRIV_SET_VBI_PAGE, id, 
> > page );
> > +}
> > +static inline int es_out_SetVbiTransparency( es_out_t *p_out, 
> > vlc_es_id_t *id,
> > +                                             bool enabled )
> > +{
> > +    return es_out_PrivControl( p_out, 
> > ES_OUT_PRIV_SET_VBI_TRANSPARENCY, id,
> > +                               enabled );
> > +}
> > 
> >  es_out_t  *input_EsOutNew( input_thread_t *, float rate );
> >  es_out_t  *input_EsOutTimeshiftNew( input_thread_t *, es_out_t *,
> > float i_rate );
> > diff --git a/src/input/input.c b/src/input/input.c
> > index 36b243aa75f..26b93ad8eb5 100644
> > --- a/src/input/input.c
> > +++ b/src/input/input.c
> > @@ -2014,9 +2014,8 @@ static bool Control( input_thread_t *p_input,
> >              break;
> >          case INPUT_CONTROL_SET_ES_LIST:
> >          {
> > -            if( es_out_PrivControl( 
> > input_priv(p_input)->p_es_out_display,
> > -                                    ES_OUT_PRIV_SET_ES_LIST, 
> > param.list.cat,
> > -                                    param.list.ids ) == VLC_SUCCESS )
> > +            if( es_out_SetEsList( 
> > input_priv(p_input)->p_es_out_display,
> > +                                  param.list.cat, param.list.ids ) ==
> > VLC_SUCCESS )
> >              {
> >                  if( param.list.ids[0] != NULL && param.list.ids[1] == 
> > NULL )
> >                      demux_Control(
> > input_priv(p_input)->master->p_demux, DEMUX_SET_ES,
> > @@ -2238,8 +2237,7 @@ static bool Control( input_thread_t *p_input,
> >                  break;
> > 
> >              void *context;
> > -            if( es_out_PrivControl( priv->p_es_out_display,
> > -                                    ES_OUT_PRIV_STOP_ALL_ES, &context
> > ) != VLC_SUCCESS )
> > +            if( es_out_StopAllEs( priv->p_es_out_display, &context )
> > != VLC_SUCCESS )
> >                  break;
> > 
> >              if ( p_priv->p_renderer )
> > @@ -2261,23 +2259,22 @@ static bool Control( input_thread_t *p_input,
> > 
> > vlc_renderer_item_demux_filter( p_item ) );
> >                  }
> >              }
> > -            es_out_PrivControl( priv->p_es_out_display,
> > ES_OUT_PRIV_START_ALL_ES,
> > -                                context );
> > +            es_out_StartAllEs( priv->p_es_out_display, context );
> >  #endif
> >              break;
> >          }
> >          case INPUT_CONTROL_SET_VBI_PAGE:
> > -            es_out_PrivControl( priv->p_es_out_display,
> > ES_OUT_PRIV_SET_VBI_PAGE,
> > -                                param.vbi_page.id, param.vbi_page.page 
> > );
> > +            es_out_SetVbiPage( priv->p_es_out_display, 
> > param.vbi_page.id,
> > +                               param.vbi_page.page );
> >              break;
> >          case INPUT_CONTROL_SET_VBI_TRANSPARENCY:
> > -            es_out_PrivControl( priv->p_es_out_display,
> > ES_OUT_PRIV_SET_VBI_TRANSPARENCY,
> > -                                param.vbi_transparency.id,
> > -                                param.vbi_transparency.enabled );
> > +            es_out_SetVbiTransparency( priv->p_es_out_display,
> > +                                       param.vbi_transparency.id,
> > +                                       param.vbi_transparency.enabled 
> > );
> >              break;
> >          case INPUT_CONTROL_SET_ES_AUTOSELECT:
> > -            es_out_PrivControl( priv->p_es_out_display,
> > ES_OUT_PRIV_SET_AUTOSELECT,
> > -                                param.es_autoselect.cat,
> > param.es_autoselect.enabled );
> > +            es_out_SetAutoSelect( priv->p_es_out_display,
> > param.es_autoselect.cat,
> > +                                  param.es_autoselect.enabled );
> >              break;
> > 
> >          case INPUT_CONTROL_NAV_ACTIVATE:
> > @@ -3306,10 +3303,8 @@ static int input_SlaveSourceAdd( input_thread_t 
> > *p_input,
> > 
> >      assert( priv->i_last_es_id != -1 );
> > 
> > -    es_out_PrivControl( priv->p_es_out_display,
> > ES_OUT_PRIV_SET_ES_DEFAULT_BY_ID,
> > -                        priv->i_last_es_id );
> > -    es_out_PrivControl( priv->p_es_out_display, 
> > ES_OUT_PRIV_SET_ES_BY_ID,
> > -                        priv->i_last_es_id, false );
> > +    es_out_SetEsDefaultById( priv->p_es_out_display, 
> > priv->i_last_es_id );
> > +    es_out_SetEsById( priv->p_es_out_display, priv->i_last_es_id, 
> > false );
> > 
> >      return VLC_SUCCESS;
> >  }
> 
> -- 
> Rémi Denis-Courmont
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel


More information about the vlc-devel mailing list