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

Rémi Denis-Courmont remi at remlab.net
Thu Aug 20 18:11:20 CEST 2020


Hi,

Badly designed or implemented plugins are what they are. Just as big loops in the UI thread are bad for interactivity, big loops in the render thread are bad for timeliness. And besides those egregiously broken plugins you have, e.g., rotate and transform who actually translate the mouse event, and should obviously run synchronously - not at the next render.

Le 20 août 2020 10:02:30 GMT+02:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
>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
>> 
>_______________________________________________
>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é.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20200820/057cb21a/attachment.html>


More information about the vlc-devel mailing list