[vlc-devel] [PATCH 02/17] video_output: translate the mouse coordinates to display coordinates early

"zhilizhao(赵志立)" quinkblack at foxmail.com
Mon Nov 2 12:46:58 CET 2020



> On Nov 2, 2020, at 6:15 PM, Alexandre Janniaux <ajanni at videolabs.io> wrote:
> 
> Hi,
> 
> On Mon, Nov 02, 2020 at 10:35:09AM +0100, Steve Lhomme wrote:
>> It seems the issue is that code in macosx.m
>> 
>>    @synchronized(self) {
>>        _hasPendingReshape = NO;
>>    }
>> 
>> It might use an atomic bool for this variable. Using less locks is always a
>> good idea when possible. I don't know if/how Objective-C can handle atomics.
>> 
>> On the other hand the original patch is not mandatory and doesn't improve
>> anything much. So if necessary I'm OK with reverting it.
> 
> I read the issue as «you cannot get the display lock for mouse update, since
> display can currently report the mouse state, thus already under display lock».
> 
> Is that different from reality?
> 
> Using the work from Marvin, we'll be able to remove the mouse API from display
> on MacOSX, so finally solve this issue.

So it’s OK to just revert? The master branch is totally unusable now.

> 
> Regards,
> --
> Alexandre Janniaux
> Videolabs
> 
>> On 2020-10-31 17:49, Zhao Zhili wrote:
>>> Hi Steve,
>>> 
>>>> On Sep 14, 2020, at 10:03 PM, Steve Lhomme <robux4 at ycbcr.xyz> wrote:
>>>> 
>>>> We can do this in the UI thread, the processing is light.
>>>> Locking the display_lock doesn't have a big impact on lag as it's only taking
>>>> time when reloading a display module, which is very rare.
>>>> ---
>>>> src/video_output/video_output.c | 21 ++++++++++++---------
>>>> 1 file changed, 12 insertions(+), 9 deletions(-)
>>>> 
>>>> diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
>>>> index cbc66b9dbc0..ed1e3bfcb07 100644
>>>> --- a/src/video_output/video_output.c
>>>> +++ b/src/video_output/video_output.c
>>>> @@ -389,7 +389,14 @@ 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);
>>>> -    cmd.mouse = *mouse;
>>>> +
>>>> +    /* Translate window coordinates to video coordinates */
>>>> +    vlc_mutex_lock(&sys->display_lock);
>>> 
>>> This change leads to a deadlock on macOS.
>>> 
>>> * thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGSTOP
>>>   * frame #0: 0x00007fff6dcdd882 libsystem_kernel.dylib`__psynch_cvwait + 10
>>>     frame #1: 0x00007fff6dd9e425 libsystem_pthread.dylib`_pthread_cond_wait + 698
>>>     frame #2: 0x000000010023d27c libvlccore.dylib`vlc_atomic_wait(addr=0x000000010ef31ff0, value=2) at wait.c:84:9
>>>     frame #3: 0x0000000100226d4e libvlccore.dylib`vlc_mutex_lock(mtx=0x000000010ef31ff0) at threads.c:175:9
>>>     frame #4: 0x00000001001eb968 libvlccore.dylib`vout_MouseState(vout=0x000000010ef31d50, mouse=0x000000010ed8e4a8) at video_output.c:389:5
>>>     frame #5: 0x00000001002005f4 libvlccore.dylib`vout_display_window_MouseEvent(window=0x000000010ed8e370, ev=0x00007ffeefbfdd80) at window.c:321:5
>>>     frame #6: 0x0000000107fc11b9 libvout_macosx_plugin.dylib`vout_window_SendMouseEvent(window=0x000000010ed8e370, mouse=0x00007ffeefbfdd80) at vlc_vout_window.h:595:9
>>>     frame #7: 0x0000000107fc1239 libvout_macosx_plugin.dylib`vout_window_ReportMouseMoved(window=0x000000010ed8e370, x=2038, y=943) at vlc_vout_window.h:614:5
>>>     frame #8: 0x0000000107fc05b8 libvout_macosx_plugin.dylib`vout_display_SendMouseMovedDisplayCoordinates(vd=0x0000000110911340, m_x=2038, m_y=943) at vlc_vout_display.h:467:5
>>>     frame #9: 0x0000000107fc0507 libvout_macosx_plugin.dylib`-[VLCOpenGLVideoView mouseMoved:](self=0x0000000110911da0, _cmd="mouseMoved:", o_event=0x0000000100676520) at macosx.m:778:17
>>> 
>>> 
>>>   thread #42
>>>     frame #0: 0x00007fff6dcdc55e libsystem_kernel.dylib`__ulock_wait + 10
>>>     frame #1: 0x00007fff6dd909c3 libsystem_platform.dylib`_os_unfair_lock_lock_slow + 160
>>>     frame #2: 0x00007fff6c9e8dc7 libobjc.A.dylib`objc_sync_enter + 27
>>>     frame #3: 0x0000000107fbf4e4 libvout_macosx_plugin.dylib`-[VLCOpenGLVideoView setVoutFlushing:](self=0x0000000110911da0, _cmd="setVoutFlushing:", flushing=YES) at macosx.m:550:5
>>>     frame #4: 0x0000000107fc0cf0 libvout_macosx_plugin.dylib`PictureDisplay(vd=0x0000000110911340, pic=0x000000010ae1d640) at macosx.m:331:5
>>>     frame #5: 0x00000001001f2b09 libvlccore.dylib`vout_display_Display(vd=0x0000000110911340, picture=0x000000010ae1d640) at vlc_vout_display.h:431:9
>>>     frame #6: 0x00000001001f2127 libvlccore.dylib`ThreadDisplayRenderPicture(vout=0x000000010ef31d50, render_now=false) at video_output.c:1446:5
>>>     frame #7: 0x00000001001edf69 libvlccore.dylib`ThreadDisplayPicture(vout=0x000000010ef31d50, deadline=0x00007000061d6fa0) at video_output.c:1572:15
>>>     frame #8: 0x00000001001f0409 libvlccore.dylib`Thread(object=0x000000010ef31d50) at video_output.c:1914:16
>>>     frame #9: 0x00007fff6dd9e109 libsystem_pthread.dylib`_pthread_start + 148
>>>     frame #10: 0x00007fff6dd99b8b libsystem_pthread.dylib`thread_start + 15
>>> 
>>> 
>>> 
>>>> +    if (sys->display)
>>>> +        vout_display_TranslateMouseState(sys->display, &cmd.mouse, mouse);
>>>> +    else
>>>> +        cmd.mouse = *mouse;
>>>> +    vlc_mutex_unlock(&sys->display_lock);
>>>> 
>>>>     vout_control_Push(&sys->control, &cmd);
>>>> }
>>>> @@ -1678,17 +1685,13 @@ 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 vid_mouse, tmp1, tmp2, *m;
>>>> +    vlc_mouse_t tmp1, tmp2;
>>>> +    const vlc_mouse_t *m;
>>>>     vout_thread_t *vout = &p_vout->obj;
>>>>     vout_thread_sys_t *sys = p_vout;
>>>> 
>>>> -    /* 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;
>>>> +    /* pass mouse coordinates in the filter chains. */
>>>> +    m = win_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
>>> 
>>> _______________________________________________
>>> 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 <https://mailman.videolan.org/listinfo/vlc-devel>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20201102/02cf567b/attachment.html>


More information about the vlc-devel mailing list