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

[Marvin Scholz]

On 27 Aug 2020, at 10:23, Romain Vimont wrote:

> 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".

IMO this is bad for readability.
While we have no rules on code style, I think it should
help readability and not make things harder to read just
for sake of consistency.

> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel