GNOME Bugzilla – Bug 749601
win32: glimagesink always popup a new window
Last modified: 2015-06-19 20:41:27 UTC
On windows, using glimagesink, if I call set_window_handle() in READY state (before it creates its internal window) then when going in PLAYING state it will popup a new window.
Created attachment 303624 [details] [review] gl: win32: set the parent window when creating internal window When _set_window_handle() was called in READY state, it wasn't set to the internal window created later.
Note that to work it also need to revert commit a12ca13750a15300ab3c718ebde2984dc3d587b3, see bug #745090 comment #14
Just tested, and this patch is also needed in the case I set the window from "prepare-window-handle" message.
Created attachment 303696 [details] [review] gl: win32: set the parent window when creating internal window When _set_window_handle() was called in READY state, it wasn't set to the internal window created later.
Created attachment 303697 [details] [review] gl: win32: use a GMainContext to dispatch win32 messages gst_gl_window_win32_send_message_async() could be called before the internal window is created so we cannot use PostMessage there. x11 and wayland backends both create a custom GSource for this, so there is no reason to not do that for win32.
Created attachment 303698 [details] [review] gl: win32: set the parent window when creating internal window When _set_window_handle() was called in READY state, it wasn't set to the internal window created later.
Created attachment 303714 [details] [review] gl: win32: use a GMainContext to dispatch win32 messages gst_gl_window_win32_send_message_async() could be called before the internal window is created so we cannot use PostMessage there. x11 and wayland backends both create a custom GSource for this, so there is no reason to not do that for win32.
Created attachment 303715 [details] [review] gl: win32: set the parent window when creating internal window When _set_window_handle() was called in READY state, it wasn't set to the internal window created later.
Created attachment 303716 [details] [review] gl: win32: Fix leaked GstGLContext
Created attachment 303717 [details] [review] gl: win32: fix crash when finalizing GstGLContext gst_gl_context_finalize() is calling gst_gl_window_win32_quit() which was posting a message. But then window_proc takes window's context and get a NULL. Now that we've got a GMainLoop we can do like other backends and simply call g_main_loop_quit(). This also remove duplicated code to release the parent window and potential crash there because parent_proc could be NULL if we never created the internal window. That could happen for example if setting state to READY then setting a window_handle, and go back to NULL state.
Review of attachment 303714 [details] [review]: Looks good
Review of attachment 303715 [details] [review]: All looks fine except for. CC libgstgl_win32_la-gstglwindow_win32.lo gstglwindow_win32.c: In function ‘gst_gl_window_win32_set_window_handle’: gstglwindow_win32.c:340:8: warning: unused variable ‘parent_id’ [-Wunused-variable] HWND parent_id;
Review of attachment 303716 [details] [review]: Looks good.
Review of attachment 303717 [details] [review]: Looks good.
Created attachment 303757 [details] [review] gl: win32: fix crash when finalizing GstGLContext gst_gl_context_finalize() is calling gst_gl_window_win32_quit() which was posting a message. But then window_proc takes window's context and get a NULL. Now that we've got a GMainLoop we can do like other backends and simply call g_main_loop_quit(). This also remove duplicated code to release the parent window and potential crash there because parent_proc could be NULL if we never created the internal window. That could happen for example if setting state to READY then setting a window_handle, and go back to NULL state.
(In reply to Matthew Waters from comment #12) > CC libgstgl_win32_la-gstglwindow_win32.lo > gstglwindow_win32.c: In function ‘gst_gl_window_win32_set_window_handle’: > gstglwindow_win32.c:340:8: warning: unused variable ‘parent_id’ > [-Wunused-variable] > HWND parent_id; That warning actually comes from attachment #303717 [details], now updated.
Attachment 303714 [details] pushed as e24bc34 - gl: win32: use a GMainContext to dispatch win32 messages Attachment 303715 [details] pushed as 0acc18c - gl: win32: set the parent window when creating internal window Attachment 303716 [details] pushed as e1a8278 - gl: win32: Fix leaked GstGLContext Attachment 303757 [details] pushed as 247697c - gl: win32: fix crash when finalizing GstGLContext
Created attachment 303779 [details] [review] gl: win32: remove unused code
Created attachment 303780 [details] [review] gl: win32: do not call SetParent in release_parent_win_id() When called from gst_gl_window_win32_close(), internal window could not exist, and if it does it's going to be destroyed just after that anyway. Also it causes window_proc() to be called and crash because it gets a NULL context. When called from gst_gl_window_win32_set_window_handle() we are going to set another parent anyway, and it's probably better to reparent directly instead of passing by a NULL parent which could cause the internal window to popup briefly.
Reopening since I found another crash in related code.
Review of attachment 303779 [details] [review]: Looks good.
Review of attachment 303780 [details] [review]: Looks Good.
Attachment 303779 [details] pushed as ae77afe - gl: win32: remove unused code Attachment 303780 [details] pushed as 1a98fac - gl: win32: do not call SetParent in release_parent_win_id()