GNOME Bugzilla – Bug 691419
osxvideosink: doesn't close internal window in case of window-id assignment
Last modified: 2013-07-11 16:51:34 UTC
osxvideosink doesn't close its internal window if it's already created when another window-id is assigned using GstXOverlay interface.
The problem is related to the bug described in bug report #691421
Can you provide a patch for this?
Not yet, I tried to fix it but it's still not finished.
Have you had a chance to cook up a patch yet?
No, not really. Hopefully I'll give it another try today and tomorrow, perhaps, would be able to fix it this time.
Created attachment 248567 [details] [review] osxvideosink: close the internal window correctly Make sure to close our internal window properly
Thanks, Andoni. Hope to try your patch tomorrow, too.
Created attachment 248568 [details] [review] osxvideosink: close the internal window correctly Fixes a small issue with the autorelease pool
I tried to apply the patch to my modified version, it didn't work out of the box (maybe, my fault in applying) but I also added this closing code to *_set_window_handle() function and modify it a little bit so that it finally started working correctly. I hope to prepare a patch this week (I've done some considerable changes so it needed to be sorted out a little bit).
Actually this patch only guaranties that the window is closed on a state change transition and might require something more only when adding the window handle.
The window handle set by set_window_handle() should only be considered for each switch from READY->PAUSED. Changing the window handle in PAUSED/PLAYING should not have any effect until the next time the sink goes through READY. Is that the problem here?
Created attachment 248708 [details] [review] osxvideosink: close the internal window when setting a new window handle Fix the case in which we have an internal window and we change the window handle in playing.
Created attachment 248709 [details] [review] osxvideosink: close the internal window when setting a new window handle Fix indentation
Review of attachment 248709 [details] [review]: ::: sys/osxvideo/osxvideosink.m @@ +644,3 @@ + osxvideosink->osxwindow->closed = TRUE; + [osxvideosink->osxwindow->win close]; + [osxvideosink->osxwindow->win release]; You do realize that this can be called from any thread? Is this a problem? What happens if at the same time the sink wants to render something, shouldn't there be some locking?
Review of attachment 248709 [details] [review]: Right, this should be delegated to the main thread like it's done in the other operations done in this function. No locking would be needed in that case since rendering would than happen in the same thread. Thanks for the review :)
Review of attachment 248709 [details] [review]: In fact, some locking is still needed there.
Created attachment 248730 [details] [review] 0001-osxvideosink-close-the-internal-window-correctly.patch
Created attachment 248731 [details] [review] 0002-osxvideosink-default-to-the-main-in-case-we-are-not-.patch
Created attachment 248732 [details] [review] 0003-osxvideosink-delegate-the-window-handle-setup-to-the.patch
Thanks, Andoni and Sebastian. I've tested my code with changing window-id during PLAYING state, it works, but I don't have any locks, too, so we will wait for Andoni's implementation be merged. One thing that, I think, is important in mine, is that it seems better to create GstGLView outside Cocoa's GstOSXVideoSinkWindow as it is used both by external and internal windows and it's usually a pain to look after all these copies generated in osxvideosink.m and in cocoawindow.m.
commit d57ef52cadd51643dfc812779dac444bf3d9eaae Author: Andoni Morales Alastruey <ylatuya@gmail.com> Date: Tue Jul 9 15:34:04 2013 +0200 osxvideosink: defer the window handle setup to the main thread commit 34a5b93637c40b782ab7a768c31e84ed1737981c Author: Andoni Morales Alastruey <ylatuya@gmail.com> Date: Tue Jul 9 15:33:18 2013 +0200 osxvideosink: default to the main in case we are not setup yet commit 0e321b87d4df4f86176cb71ff87f68a8a802f015 Author: Andoni Morales Alastruey <ylatuya@gmail.com> Date: Sun Jul 7 22:16:05 2013 +0200 osxvideosink: close the internal window correctly