[vlc-devel] [PATCH] Revert "video_output: translate the mouse coordinates to display coordinates early"
Alexandre Janniaux
ajanni at videolabs.io
Tue Feb 9 15:42:21 UTC 2021
Hi,
I jump back on this discussion because I actually have other issues
from this patch (not the revert) and this is the last one talking about
this.
I have another problem alike this one, target Android (so another platform
with the main thread doing most WSI work).
SurfaceTexture can have listener to signal that a new frame is available.
This is useless in single buffer mode because there is already a
synchronization mechanism (through releaseTexImage) but mandatory for
multiple buffer support with asynchronous provider. In particular,
mediacodec is an asynchronous decoder, so without this event we get the
bug in ticket #25361.
The documentation mentions that the callback can be called from an arbitrary
thread but for some (probably stupid usability) reasons, this event is
triggered from the main thread.
To implement #25361, I wait in the interop from the event to be emitted and
then use the texture. It cannot be done before because only the last updated
buffer of the SurfaceTexture can be used, so the producer cannot do it itself
if the texture is to be reused, and the consumer being the display will
always be locked behind display_lock.
I also cannot do that asynchronously because the display modules cannot
really be asynchronous, because of the chrono metric and the lack of feedback
to start preparing the next frame.
So when I touch the screen, I get stuck with the following stacktrace:
Thread 1 "main" received signal SIGINT, Interrupt.
0x00000076126023ac in syscall () from /home/alexandre/workspace/videolabs/vlc-android/libvlc/.gdb/obj/local/arm64-v8a/system/lib64/libc.so
(gdb) bt
#0 0x00000076126023ac in syscall () from /home/alexandre/workspace/videolabs/vlc-android/libvlc/.gdb/obj/local/arm64-v8a/system/lib64/libc.so
#1 0x000000757046f700 in vlc_mutex_lock (mtx=0x75845db298) at ../../src/misc/threads.c:172
#2 0x000000757044dd60 in vout_MouseState (vout=0x75845db000, mouse=0x75748191c8) at ../../src/video_output/video_output.c:369
#3 0x00000075703e724c in vout_window_SendMouseEvent (window=0x7574635a80, mouse=0x7fc581c228) at ../../include/vlc_vout_window.h:595
#4 vout_window_ReportMouseMoved (window=0x7574635a80, x=<optimized out>, y=<optimized out>) at ../../include/vlc_vout_window.h:614
#5 OnNewMouseCoords (wnd=0x7574635a80, coords=0x7fc581c278) at ../../modules/video_output/android/window.c:70
#6 0x00000075703e7468 in AndroidNativeWindow_onMouseEvent (env=<optimized out>, clazz=<optimized out>, handle=2, action=0, button=0, x=-1, y=-1) at ../../modules/video_output/android/utils.c:1428
#7 0x000000758e1dffe4 in ?? ()
Backtrace stopped: previous frame identical to this frame (corrupt stack?)
and display / opengl / interop is stuck waiting on the semaphore that is
triggered from the main thread.
I guess that this deadlock can be solved by adding yet another lock in
the display code, saving the state of the last frame drawn to be able to
map the event matching with the current frame, but I'm unsure whether we
want this kind of complexity, and though I haven't tested, resizing will
also make this code break.
Here, the situation is a bit more peculiar than the display sending mouse
event situation, because the components are more clearly separated, the
event is coming from the vout window component, and the main point of state
sharing leading to this is the access to the main thread.
I can workaround this deadlock starting with API 21 using the following API
public void setOnFrameAvailableListener (SurfaceTexture.OnFrameAvailableListener listener,
Handler handler)
Or starting with API 26 using the AHardwareBuffer interface, but both are
much higher level than the API level we support. I can also just disable
synchronization and have glitches for phones below API 21.
Is there another solution to this threading mess on those platforms?
Regards,
--
Alexandre Janniaux
Videolabs
On Tue, Nov 03, 2020 at 12:56:58PM +0100, Steve Lhomme wrote:
> This reverts commit 5b2129eda58a4a19d37a9fb33e64af3777555233.
>
> This causes deadlocks with display modules where the display and windowing is
> tied. The display_lock and the internal UI/vout thread locking potentially
> called in different orders.
> ---
> src/video_output/video_output.c | 21 +++++++++------------
> 1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
> index 57ee54e6e8b..2e1d7ffc8eb 100644
> --- a/src/video_output/video_output.c
> +++ b/src/video_output/video_output.c
> @@ -384,14 +384,7 @@ void vout_MouseState(vout_thread_t *vout, const vlc_mouse_t *mouse)
> assert(mouse);
> vout_control_cmd_t cmd;
> vout_control_cmd_Init(&cmd, VOUT_CONTROL_MOUSE_STATE);
> -
> - /* Translate window coordinates to video coordinates */
> - vlc_mutex_lock(&sys->display_lock);
> - if (sys->display)
> - vout_display_TranslateMouseState(sys->display, &cmd.mouse, mouse);
> - else
> - cmd.mouse = *mouse;
> - vlc_mutex_unlock(&sys->display_lock);
> + cmd.mouse = *mouse;
>
> vout_control_Push(&sys->control, &cmd);
> }
> @@ -1708,13 +1701,17 @@ void vout_ChangeSpuRate(vout_thread_t *vout, size_t channel_id, float rate)
> static void ThreadProcessMouseState(vout_thread_sys_t *p_vout,
> const vlc_mouse_t *win_mouse)
> {
> - vlc_mouse_t tmp1, tmp2;
> - const vlc_mouse_t *m;
> + vlc_mouse_t vid_mouse, tmp1, tmp2, *m;
> vout_thread_t *vout = &p_vout->obj;
> vout_thread_sys_t *sys = p_vout;
>
> - /* pass mouse coordinates in the filter chains. */
> - m = win_mouse;
> + /* Translate window coordinates to video coordinates */
> + vlc_mutex_lock(&sys->display_lock);
> + vout_display_TranslateMouseState(sys->display, &vid_mouse, win_mouse);
> + vlc_mutex_unlock(&sys->display_lock);
> +
> + /* Then pass up the filter chains. */
> + m = &vid_mouse;
> vlc_mutex_lock(&sys->filter.lock);
> if (sys->filter.chain_static && sys->filter.chain_interactive) {
> if (!filter_chain_MouseFilter(sys->filter.chain_interactive,
> --
> 2.26.2
>
> _______________________________________________
> 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