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

Steve Lhomme robux4 at ycbcr.xyz
Thu Aug 20 20:29:48 CEST 2020


On 2020-08-20 18:11, Rémi Denis-Courmont wrote:
> 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.

Synchronously ? I have seen nothing in the UI thread, the one that will 
display the rendered result, that says a frame is allowed to be 
displayed ASAP. It will wait until the next loop, in that thread. 
Putting that in the UI thread is just pretending the rendering will be 
more responsive.

> 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é.
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
> 


More information about the vlc-devel mailing list