[vlc-devel] [PATCH v2 01/11] core: move all actions handling in actions.c

[Rémi Denis-Courmont]

Le torstaina 10. elokuuta 2017, 17.35.59 EEST Hugo Beauzée-Luyssen a écrit :
> > > diff --git a/include/vlc_actions.h b/include/vlc_actions.h
> > > index 404c1662a8..0995d3db5c 100644
> > > --- a/include/vlc_actions.h
> > > +++ b/include/vlc_actions.h
> > > @@ -241,6 +241,7 @@ typedef enum vlc_action_id {
> > > 
> > >      ACTIONID_VIEWPOINT_FOV_OUT,
> > >      ACTIONID_VIEWPOINT_ROLL_CLOCK,
> > >      ACTIONID_VIEWPOINT_ROLL_ANTICLOCK,
> > > 
> > > +    ACTIONID_VIEWPOINT_UPDATE, /* Arg 1: vlc_viewpoint_t* */
> > 
> > The whole point (as you said yourself) of actions is to factor the
> > calling
> > code. Indeed, it enables callers to map a key code or a string to an
> > action ID
> > without knowing/caring what it actually means.
> > 
> > What exactly is the point of variadic actions then?
> 
> To specify a context. It is also safe to pass none, and a default one
> will be inferred

I am referring to VIEWPOINT_UPDATE needing a viewpoint structure. Actions are 
mapped from key codes. So there may be a context (vout window, aout or input) 
from which the key press was captured. But I don´t see how any other parameter 
can be generated in general, and a viewpoint structure in particular.

> (also granted, this is done in next patch, but I'll gladly merge the 2)
> Since the context differs from action to action, I'm not sure how to do
> this without a variadic function, or N functions with different
> prototype per action category. A variadic function also eases the user
> from not providing a context when the default one is enough.

You can´t have a default with variadic parameters. You need to know if there 
will be a first variadic parameter, and if so, of what type, from the fixed 
parameters. And none of the three fixed parameters allow that.

> > >      /* Combo Actions */
> > >      ACTIONID_COMBO_VOL_FOV_UP,
> > >      ACTIONID_COMBO_VOL_FOV_DOWN,
> > > 
> > > @@ -272,4 +273,18 @@ VLC_API const char* const*
> > > 
> > >  vlc_actions_get_key_names(vlc_object_t *p_obj);
> > >  #define vlc_actions_get_key_names(x)
> > > 
> > > vlc_actions_get_key_names(VLC_OBJECT(x))
> > > 
> > > +/**
> > > + * Execute an action
> > > + *
> > > + * \param a valid vlc object
> > > + * \param i_action action to execute
> > > + * \param b_notify notify a new state via the OSD of the current vout
> > > + * \return VLC_SUCCESS or a VLC error
> > > + */
> > > +VLC_API int
> > > +vlc_actions_do(vlc_object_t *p_obj, vlc_action_id_t i_action,
> > > +               bool b_notify, ...);
> > > +#define vlc_actions_do(a,b,c,...) vlc_actions_do(VLC_OBJECT(a),b,c, \
> > > +    ##__VA_ARGS__)
> > 
> > As far as I am aware, this macro violates ISO C.
> 
> I suppose you are referring to the fact that the macro is one multiple
> lines?
> I'm genuinely interested in the source for this, as this is not
> how I understand §6.10.3.5

Previous paragraph:

(...) "there shall be *more* arguments in the invocation than there are 
parameters in the macro definition (excluding the ...)."
(emphasis mine)

-- 
雷米‧德尼-库尔蒙
https://www.remlab.net/