GNOME Bugzilla – Bug 618921
[dshowvideosink] Replace CoIntialize with CoInitializeEx for bettrer integration with GStreamer threads
Last modified: 2010-08-14 13:13:21 UTC
Created attachment 161257 [details] [review] 0001-dshowvideosink-use-CoInitializeEx-for-multithread-su.patch Current implementation uses CoInitialize and doesn't integrates well with GStreamer's thread. An example is setting the element to NULL and back to PLAY. It will stop on ::start() trying to create a new instance of a direct show grap: "Can't create an instance of the dshow graph manager (error=8000ffff)" Using CoInitializeEx with COINIT_MULTITHREADED fixes this issue and allows to implement preroll using #579926.
The way COM is handled in the sink is a total mess; this just changes it without really fixing anything. Specifically: COM _must_ be uninitialised from the same thread it is initialised. The current sink doesn't do that, and just unintialises it in finalize, which could be called from anywhere. I don't have a good solution for this - but my workaround in songbird was to make _all_ of the COM stuff in this sink (and other elements) get done from a dedicated thread, which initialises COM on creation, and uninits it when it shuts down the thread. Everything that currently uses COM is changed to wait for that thread to become available and call the COM code. It's a bit messy, and probably decreases performance significantly, but reliably works. I'm not super-excited by patches like this that just change things around a bit without fixing this underlying flaw.
Although it doesn't fix the COM mess, at least it fixes a particular use case. The current implementation doesn't allow any transition. Just try to set the sink from PLAYING to NULL and back to PLAY: it won't be able to create a new instance of the graph. I prefer to have a sink that can do transitions changing 2 lines of code rather than having a sink that doesn't. Trying to solve the COM mess, a possible solution would be to create a thread on which we call CoInitializeEx to create a MTA. Subsequent calls to CoInitializeEx from other threads will enter this MTA and can safely call CoUnitialize since all of them use the same MTA, one unique per process (only the refcount will change and the MTA won't be freed since the first thread hasn't called yet CoUnitialize). In finalize the thread used to initialize the MTA will be the last one to leave the apartement and it can be unitialized properly.
Created attachment 161395 [details] [review] 0001-dshowvideosink-initialize-and-deininitialize-COM-pro.patch This patch tries to solve the COM mess. It initializes COM in a MTA on a separate thread which will be the last one to leave the MTA, and therefore COM will be uninitialized properly. I though a CoInitializeEx/CoUninitialize pair call would be needed on each thread calling a COM object, but it works as it is now. Anyway CoInitializeEx/CoUninitialize pairs can be added safely because the last thread to leave the MTA will the one the created it.
Created attachment 161974 [details] [review] 0001-dshowvideosink-initialize-and-deininitialize-COM-pro.patch Fix typo in previous patch causing a deadlock (g_mutext_unlock instead of g_mutex_lock)
Was squashed in: http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=a51d318759b07eca60811e3d47d8e81b6d12cb8c