[vlc-devel] [RFC 1/2] executor: introduce new executor API

[Romain Vimont]

On Thu, Aug 27, 2020 at 07:09:00AM +0200, Steve Lhomme wrote:
> On 2020-08-26 19:19, Romain Vimont wrote:
> > Introduce a new API to execute "runnables". The execution can be
> > automatically interrupted on timeout or via an explicit cancelation
> > request.
> > ---
> >   include/vlc_executor.h | 194 +++++++++++++++
> >   src/Makefile.am        |   1 +
> >   src/libvlccore.sym     |   4 +
> >   src/misc/executor.c    | 545 +++++++++++++++++++++++++++++++++++++++++
> >   4 files changed, 744 insertions(+)
> >   create mode 100644 include/vlc_executor.h
> >   create mode 100644 src/misc/executor.c
> > 
> > diff --git a/include/vlc_executor.h b/include/vlc_executor.h
> > new file mode 100644
> > index 0000000000..4dd8b93628
> > --- /dev/null
> > +++ b/include/vlc_executor.h
> > @@ -0,0 +1,194 @@
> > +/*****************************************************************************
> > + * vlc_executor.h
> > + *****************************************************************************
> > + * Copyright (C) 2020 Videolabs, VLC authors and VideoLAN
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU Lesser General Public License as published by
> > + * the Free Software Foundation; either version 2.1 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public License
> > + * along with this program; if not, write to the Free Software Foundation,
> > + * Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
> > + *****************************************************************************/
> > +
> > +#ifndef VLC_EXECUTOR_H
> > +#define VLC_EXECUTOR_H
> > +
> > +#include <vlc_common.h>
> > +#include <vlc_tick.h>
> > +
> > +# ifdef __cplusplus
> > +extern "C" {
> > +# endif
> > +
> > +/** Executor type (opaque) */
> > +typedef struct vlc_executor vlc_executor_t;
> > +
> > +/**
> > + * A Runnable is intended to be run from another thread by an executor.
> > + */
> > +struct vlc_runnable {
> > +
> > +    /**
> > +     * This function is to be executed by a vlc_executor_t.
> > +     *
> > +     * It must implement the actions (arbitrarily long) to execute from an
> > +     * executor thread, synchronously. As soon as run() returns, the execution
> > +     * of this runnable is complete.
> > +     *
> > +     * After the runnable is submitted to an executor via
> > +     * vlc_executor_Submit(), the run() function is executed at most once (zero
> > +     * if the execution is canceled before it was started).
> > +     *
> > +     * It may not be NULL.
> > +     *
> > +     * \param userdata the userdata provided to vlc_executor_Submit()
> > +     */
> > +    void
> 
> Why
> 
> > +    (*run)(void *userdata);
> 
> Do you
> use two lines
> for this ?
> 
> When grepping for code I always stumble such unreadable lines. I have to
> open the files for each of them to read the actual prototype.

We adopted this "code style" with Thomas on the playlist/player.

The reason is that the content before the function name may be very
long, and we always align the arguments when there is not enough space:

static inline const struct vlc_something_t *vlc_something_GetSomething(struct vlc_something_t *something,
                                                                       vlc_tick_t tick,
                                                                       const char *yet_another_arg);

I prefer:

static inline const struct vlc_something_t *
vlc_something_GetSomething(struct vlc_something_t *something,
                           vlc_tick_t tick,
                           const char *yet_another_arg);

(both if the column width limit is 80 or 100 chars)

For consistency, I do the same even for a simple "void".