[vlc-devel] [PATCH v3 00/18] Get rid of the control command API

[Steve Lhomme]

On 2020-08-19 17:27, Alexandre Janniaux wrote:
> Hi,
> 
> On Wed, Aug 19, 2020 at 04:58:56PM +0200, Rémi Denis-Courmont wrote:
>> Hi,
>>
>> I don't know where this assertion that mouse events should be processed in a different thread comes from. It sounds saner to me to process them in the UI loop (like UIs typically do) than in the render thread.

This is what I went with first. I agree that's the logical thing to do.

> I agree, that is what is done in almost all interface using
> LibVLC without the mouse handling too (VLC for Android, VLC
> for iOS, etc).
> 
> It's the filter / other mouse event client responsability to
> manage the events correctly to not have race when making the
> final output, and in the very worse case make the UI wait for
> the event completion instead of letting it spam more event
> than can be handled, though blocking the UI thread is probably
> a mistake in most cases.

Except that's a totally new requirement on filters. All of the filters 
handling mouse events have to be refactored to be thread safe. The mouse 
events were always done in the same thread that does the filtering.

Still, I looked at the implementations because they probably don't do a 
lot with the mouse. I found 2 filters that do big loops on pixels when 
processing the mouse:

https://code.videolan.org/videolan/vlc/-/blob/master/modules/video_filter/freeze.c#L248
https://code.videolan.org/videolan/vlc/-/blob/master/modules/video_filter/puzzle.c#L702

IMO these loops, that were never done in the UI thread, should not be 
done in the UI thread.

>>
>> Le 18 août 2020 16:58:27 GMT+02:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
>>> The filter commands can be replaced with states of the latest filter
>>> request.
>>>
>>> The mouse events still need to be processed in sequence and most likely
>>> outside
>>> of the UI thread where they are generated. They are handled in a
>>> similar way
>>> as it was done in the control API but with tighter integration with the
>>> vout
>>> state.
>>>
>>> The terminate command can be internally with a boolean and trigger the
>>> wait
>>> condition on mouse events.
>>>
>>> The mouse event callback of filters is also simplified for more
>>> readability.
>>>
>>> Steve Lhomme (18):
>>>   filters: initialize the output mouse state to the new state
>>>   filters: remove the duplicate new mouse state
>>>   video_output: only reset the mouse_event if it was set on display
>>>     release
>>>   video_output: don't include control.h in include shared across the
>>>     core
>>>   video_output: use filter lock to change filters in the vout thread
>>>   video_output: use filter lock to change deinterlacing in the vout
>>>     thread
>>>   video_output: remove always true boolean from ThreadChangeFilters
>>>   video_output: rework the deadline parameter return
>>> video_output: use the rendering delay when we were given a deadline to
>>>     wait for
>>>   video_output: use INT64_MAX for the invalid control pop deadline
>>>   vout/control: keep the lock instead of setting a held flag
>>>   vout/control: move the lock outside of the control API
>>>   vout/control: move the condition on pictures/control outside of the
>>>     control API
>>>   video_output: use an internal bool to detect when we must exit the
>>>     vout thread
>>>   video_output: move the mouse events handling outside of the control
>>>     pop
>>>   video_output: do not wait for new mouse commands if we have a picture
>>>     ready
>>>   video_output: remove unused control API
>>>   video_output: code cleaning
>>>
>>> include/vlc_filter.h                          |   3 +-
>>> modules/spu/logo.c                            |   7 +-
>>> modules/video_chroma/chain.c                  |   5 +-
>>> modules/video_filter/ci_filters.m             |   6 +-
>>> .../video_filter/deinterlace/deinterlace.c    |   6 +-
>>> modules/video_filter/freeze.c                 |   7 +-
>>> modules/video_filter/magnify.c                |   9 +-
>>> modules/video_filter/puzzle.c                 |   8 +-
>>> modules/video_filter/puzzle.h                 |   3 +-
>>> modules/video_filter/transform.c              |   3 +-
>>> src/Makefile.am                               |   2 -
>>> src/misc/filter_chain.c                       |   4 +-
>>> src/video_output/control.c                    | 174 -------------
>>> src/video_output/control.h                    |  82 ------
>>> src/video_output/video_output.c               | 233 +++++++++++-------
>>> src/video_output/vout_internal.h              |   1 -
>>> 16 files changed, 171 insertions(+), 382 deletions(-)
>>> delete mode 100644 src/video_output/control.c
>>> delete mode 100644 src/video_output/control.h
>>>
>>> --
>>> 2.26.2
>>>
>>> _______________________________________________
>>> vlc-devel mailing list
>>> To unsubscribe or modify your subscription options:
>>> https://mailman.videolan.org/listinfo/vlc-devel
>>
>> --
>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
> 
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>