[vlc-devel] [RFC v2 2/2] preparser: use the new executor API

[Alexandre Janniaux]

Hi,

On Wed, Sep 02, 2020 at 11:06:55AM +0200, Romain Vimont wrote:
> On Wed, Sep 02, 2020 at 08:28:14AM +0200, Steve Lhomme wrote:
> > On 2020-09-01 18:13, Romain Vimont wrote:

> > Did you mean VLC_TICK_INVALID ?
>
> I keep the meaning exposed to the user:
>  - -1: use the default timeout (so not possible here)
>  - 0: infinite timeout
>  - >0: use the given timeout

This is independent of the merge of this patchset, but maybe
additional define would help the reading here.

> > >   void input_preparser_Deactivate( input_preparser_t* preparser )
> > >   {
> > >       atomic_store( &preparser->deactivated, true );
> > > -    background_worker_Cancel( preparser->worker, NULL );
> > > +    input_preparser_Cancel(preparser, NULL);
> > >   }
> > >   void input_preparser_Delete( input_preparser_t *preparser )
> > >   {
> > > -    background_worker_Delete( preparser->worker );
> > > +    vlc_executor_Delete(preparser->executor);
> >
> > It seems submitted_tasks is not emptied before release.
>
> _Cancel() is called in input_preparser_Deactivate(), which IMO must be
> called (in practice it is) before input_preparser_Delete() in the
> current preparser API (otherwise, it would crash).
>
> (I would like not to touch the preparser API.)

I agree too, same reason as everywhere, we should not write this like
defensive programming and expect all the tasks to be achieved before
the preparser's destruction.

Regards,
--
Alexandre Janniaux
Videolabs