[vlc-devel] [PATCH 5/5] core: first cleaning of input_thread_t variables/event

[Thomas Guillem]

On Tue, Jul 10, 2018, at 18:31, Rémi Denis-Courmont wrote:
> Le tiistaina 10. heinäkuuta 2018, 16.43.09 EEST Thomas Guillem a écrit :
> > Currently, the input_thread_t is controllable by either input_Control,
> > specific functions, by variables or by the 3 previous solutions. Some
> > functionalities are only controllable by variables (chapter for example).
> > 
> > The goal of this commit is to remove variables usage when it's not
> > necessary. This commit doesn't remove variables that should be used to pass
> > users settings (cf.  input_ConfigVarInit).
> > 
> > The "intf-event" callback is replaced by the new callback
> > input_thread_events_cb that pass a new event: struct vlc_input_event.
> 
> I would go even further and replace it with a structure of callbacks, one per 
> event type, if only to maximize type safety.

No strong opinions about it. It's more code, but lot of clients will only need to listen to only one event, so they can implement only one function (see preparser/VLM/fingerprinter example).

> 
> > There
> > can be only one listener: the creator of the input_thread_t. In the future,
> > the new vlc input manager will receive these events and forward them to all
> > listeners.
> 
> OK.

> 
> > In the meantime, I added input_LegacyVarInit,
> > input_LegacyVarStop, and input_LegacyEvents, 3 helpers functions that
> > reproduce the legacy variable behavior (transform new vlc_input_event to
> > old intf-event events). These 3 functions are meant to be removed for 4.0
> > release (when vlc input manager is added).
> > 
> > For now, the playlist and the media_player still use the legacy variables.
> > VLM, modules, and the preparser use the new event handling.
> > 
> > FIXME: there are some functionalities that are only controlable by
> > variables, like chapter selection, I need then to add a public API or a
> > public INPUT_CONTROL for thoses.
> 
> I think APIs are better than controls, also for the sake of type safety.
> 
> The internal input controls from input_control_e are (currently) needed for 
> message passing to the input thread. But the external controls from 
> input_query_e do not seem to have any real motivation.

I have a question for the distant future where the input manager/player is controlled via IPC for the sandbox:
Will it make easier to have only one control function ? Is there a generic way to serialize va_args ? I guess no since you need the event type in order to know the size and the number of args.

BTW, I put some thought on input_thread_t vs input_manager_t:

I want to remove all getters from the input_thread_t.
The only way to know the current state (pos/time/length...) is to listen to all callbacks from the beginning. This is necessarily the case since you must add a callback when you create the object.

Then the input manager (or the playlist with the legacy var foo) will save the input_thread_t internal state (by listening to the events). Therefore, any clients (qt or http intf for example) that query an input will be able to get the initial state via this input manager.
I thought about triggering events in order to give the initial state when a client is first registering to events (via vlc_input_manager_AddListener).

> -- 
> レミ・デニ-クールモン
> http://www.remlab.net/
> 
> 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel