After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 766520 - gl: win32: race when handling window messages while creating the internal window
gl: win32: race when handling window messages while creating the internal window
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
unspecified
Other Linux
: Normal normal
: 1.8.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-05-16 15:21 UTC by Xavier Claessens
Modified: 2016-05-20 15:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gl: win32: Don't still parent forcus when creating internal window (1.18 KB, patch)
2016-05-16 19:27 UTC, Xavier Claessens
none Details | Review
gl: win32: Don't steal parent forcus when creating internal window (1.18 KB, patch)
2016-05-16 19:27 UTC, Xavier Claessens
none Details | Review
gl: win32: Don't steal parent focus when creating internal window (1.25 KB, patch)
2016-05-17 13:00 UTC, Xavier Claessens
committed Details | Review

Description Xavier Claessens 2016-05-16 15:21:01 UTC
I wrote a .NET app that embed glimagesink in my main Form. On the form I connect key press events but the callback is never called. But when I set GST_DEBUG=glwindow:6 then I get all my key press events... so there is a race somewhere in the initialization.

gst_gl_window_win32_create_window() will call set_parent_win_id() at the end. But there is gap between the moment the internal window has been created and set_parent_win_id() has been called. During that period, any message from the internal window will call window_proc with window_win32->parent_win_id==0, and thus some messages are not forwarded to parent.

I'm not sure what messages we don't fwd at initialization that prevent future event from being dispatched correctly, though. I don't know win32 API enough.

I'm wondering if set_parent_win_id() should be called from WM_CREATE case, or something like that.
Comment 1 Xavier Claessens 2016-05-16 16:19:01 UTC
Ok, seems problem comes when gst_gl_window_win32_set_window_handle() is called after gst_gl_window_win32_create_window().
Comment 2 Xavier Claessens 2016-05-16 18:19:14 UTC
Digging more the issue, it seems to be a active/focus window issue. If I understand correctly, when creating internal window with no parent it receives WM_ACTIVATE and WM_SETFOCUS, but since it has no parent yet, those msg are not forwarded to parent window, and thus parent window loses its focus. When parenting the internal window, it won't transfer focus to parent magically, and since an internal window cannot have focus, nobody has focus.

Switching to another window with alt-tab then getting back to gstreamer's window give focus back.
Comment 3 Xavier Claessens 2016-05-16 19:27:29 UTC
Created attachment 328009 [details] [review]
gl: win32: Don't still parent forcus when creating internal window
Comment 4 Xavier Claessens 2016-05-16 19:27:58 UTC
Created attachment 328010 [details] [review]
gl: win32: Don't steal parent forcus when creating internal window
Comment 5 Julien Isorce 2016-05-17 08:37:21 UTC
Review of attachment 328010 [details] [review]:

lgtm (just a typo in the title: forcus->focus), also if I am not mistaken it looks like a fix for your previous fix https://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/gst-libs/gst/gl/win32/gstglwindow_win32.c?id=0acc18c60f6f962cc6553f6047fdb64891bab544 :)
Please add that commit hash in the commit message.

Xavier, could you please check that your patch does not break this example: https://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/tests/examples/gl/gtk/switchvideooverlay . According to the diff it should not make things worse but worth a try. Thx!
Comment 6 Xavier Claessens 2016-05-17 13:00:38 UTC
Created attachment 328074 [details] [review]
gl: win32: Don't steal parent focus when creating internal window

This fix regression introduced by 0acc18c60f6f962cc6553f6047fdb64891bab544.
Comment 7 Xavier Claessens 2016-05-17 17:23:26 UTC
(In reply to Julien Isorce from comment #5)
> Xavier, could you please check that your patch does not break this example:
> https://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/tests/examples/
> gl/gtk/switchvideooverlay . According to the diff it should not make things
> worse but worth a try. Thx!

How do you build that example for windows? Tried to cross build gtk+3 in cerbero but libepoxy won't build.

configure.ac:36: error: must install xorg-macros 1.8 or later before running autoconf/autogen.
  Hint: either install from source, git://anongit.freedesktop.org/xorg/util/macros or,
  depending on you distribution, try package 'xutils-dev' or 'xorg-x11-util-macros'
Comment 8 Matthew Waters (ystreet00) 2016-05-17 19:57:55 UTC
Review of attachment 328074 [details] [review]:

Ack
Comment 9 Xavier Claessens 2016-05-18 13:26:53 UTC
Note that I don't have commit access, so if you're fine with the patch please merge it yourself. Thanks.
Comment 10 Matthew Waters (ystreet00) 2016-05-20 15:06:45 UTC
commit 83bb89f4eccdfe9019251e4b70b42b95b4fe08fa
Author: Xavier Claessens <xavier.claessens@collabora.com>
Date:   Mon May 16 15:26:53 2016 -0400

    gl: win32: Don't steal parent focus when creating internal window
    
    This fix regression introduced by 0acc18c60f6f962cc6553f6047fdb64891bab544.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=766520