We ran into this at songbird too (though with other elements: dshowdecwrapper and another plugin that isn't in gst git yet).

The problem is actually a bit more complex than this - and I'm really not sure how to fix it reliably. Any suggestions would be VERY welcome.

According to COM rules, you need to initialise COM on each thread that uses COM, not just on the main thread. The COM initialisation refcount is then maintained by COM for each... and here I couldn't figure out for certain what the rules are... but I think for each apartment (i.e. for each thread where COM is initialised with COINIT_MULTITHREADED, plus globally for _all_ threads that initialise COM with COINIT_APARTMENTTHREADED).

Unfortunately, we have no way to ensure that we call CoInitialize() and CoUninitialize() from the same thread. e.g. in dshowaudiosrc, CoInitialize() happens in the object init method, and CoUninitialize() happens in the dispose method (or it could be in the finalize method as you suggest). There's no way to ensure that these two methods get called from the same thread - and indeed, in gstreamer application, they're frequently NOT called from the same thread!

I think in practice this is mostly ok for dshowaudiosrc (the source is typically created directly by the application, and then destroyed by the application, from the main thread). It's not so good for other plugins though, like dshowdecwrapper - typically, that will be created (from decodebin/etc) in a streaming thread, but destroyed on the main thread. Even if we properly use the return values from CoInitialize() then, we'll incorrectly call CoUnitialize() on the main thread's apartment (which is, depending on the app, either the single threaded apartment, or a multi threaded apartment), instead of the streaming thread's apartment (typically a multi threaded apartment).

Additionally, we should technically be calling CoInitialize/CoUninitialize from _every_ thread that uses COM objects - right now that doesn't seem to be causing real problems, but at least in theory it can.

The only idea I've had to deal with this at all is to:
 - Initialize (and possibly uninitialize) COM somewhere in core gstreamer for the main thread. Or rely on the app to do that.
 - For each task thread gstreamer creates, initialize COM at startup, and uninitialize at shutdown of that thread. 

This is intrusive though - it seems like there should be a way to use COM safely from a plugin without having to do this.

All that said: what you're suggesting is a step in the right direction at least, so even if you don't have any good ideas on how to fix the deeper problems, could you write/attach a patch to fix the problems you described?

Hi,

I have experienced the same problem that's why I am doing something like you suggested for a long time:

in gst_dshowvideosrc_init:

A
--------------------------------
hres = CoInitializeEx (NULL, COINIT_MULTITHREADED); 
if (FAILED(hres))
{
  if (hres == RPC_E_CHANGED_MODE)
  {
    src->is_com_holder = FALSE;
    GST_INFO("COM ALREADY INITIALIZED 0x%x", hres);
  }
  else
    GST_WARNING("FAILED TO INITIALIZED COM 0x%x", hres);
}
else
  GST_INFO("COM INITIALIZED");
--------------------------------


and in gst_dshowvideosrc_dispose:

B
--------------------------------
if (src->is_com_holder)
{
  CoUninitialize ();
  GST_INFO("COM UNINITIALIZED");
}
else
  GST_INFO("NO COM HOLDER");
--------------------------------

But I have not commited it yet.

It solved the issue in my case but as Michael said we cannot ensure that the _dispose function is called in the same thread as _init when src->is_com_holder is TRUE.

What I understand about COM and CoInit and CoUninit is that the thread that calls CoInitializeEx(NULL, COINIT_MULTITHREADED) for the first time must be the one that calls CoUninitialize () (and when we are sure that other threads does not attempt to use COM again)

So what is the solution I am suggesting:

We should call A in gst_init and B in gst_deinit (which is automatically called when the program terminates, am I right on this point ?)
Then we should also just call CoInitializeEx (NULL, COINIT_MULTITHREADED)
in _init function of a GstElment (dshowvideosrc for example) without checking the return value. And finally never call CoUninitialize in _dispose.

It means that I am not sure that this is necessary to call CoUninitialize in all threads that are using COM. I think only the first one which have succeeded to call CoInitializeEx, have to call CoUninitialize.

Julien

Julien:

Some issues with your suggestions:

1. gst_deinit is not normally called at all. It's mostly for cleanup in unit tests etc, so you can check for leaks in globally-allocated data structures.

2. Calling CoInitializeEx(0, COINIT_MULTITHREADED) in the element's init function doesn't help anything. Often this will be on the main thread (in which case this isn't needed), and if/when it isn't, there's no reason to expect anything else would be called from the same thread.

3. Not calling CoUninitialize() is probably likely to leak memory - if you have a multithreaded apartment, and then you destroy the thread without having called CoUninitialize(), then the COM stuff for that apartment is probably leaked. I haven't checked this closely enough to be certain, though.

4. You're not quite right about when you need to call CoUninitialize(). The actual rule, as far as I can tell, is:
   - If you call CoInitialize() successfully, you must call CoUninitialize() from the same thread.
But a clearer way of thinking about it is:
   - For each successful call to CoInitialize() within a given apartment, there must be a corresponding call to CoUninitialize() from within the same apartment.
The 'apartments' here are: 1) The single threaded apartment, if any. 2) One apartment for each multithreaded apartment (generally corresponding to one per non-main thread)

Maybe looking at wine project sources would help use, at least about about our doubts:

ole32 dll:

http://source.winehq.org/git/wine.git/?a=tree;f=dlls/ole32;h=22c22835cddb43763de477f64525c353140d1a3c;hb=e4106b6273c612334319ddcf1c54863a3dc1bf33

CoInitializeEx and CoUninitialize are implemented in Cocompobj.c:

http://source.winehq.org/git/wine.git/?a=blob;f=dlls/ole32/compobj.c;h=b2013cfbfa3ba737e3851f7f9b9c48644f7e2285;hb=e4106b6273c612334319ddcf1c54863a3dc1bf33

Note that their implementation may be a little bit different than on Windows.

Maybe I can suggest using the using the stream status messages to keep track of the various threads. And possibly have a special GstTaskPool on windows that does the CoInitialize() stuff for you. Or just have a sync handler in the app that calls it when threads are started/stopped.

Any progress on this?

Maybe the work from http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/sys/dshowdecwrapper?h=0.10&id=efd840bbb7e398d89ee9d4230e1f9c6250deee6b should be imported

(In reply to comment #7)
> Maybe the work from
> http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/sys/dshowdecwrapper?h=0.10&id=efd840bbb7e398d89ee9d4230e1f9c6250deee6b
> should be imported

What do you mean? It's in the 0.10 branch and master

Julien?


Or maybe we could just initialize COM in gst_init() on Windows and deinitialize it in gst_deinit()? ;)

(In reply to comment #8)
> (In reply to comment #7)
> > Maybe the work from
> > http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/sys/dshowdecwrapper?h=0.10&id=efd840bbb7e398d89ee9d4230e1f9c6250deee6b
> > should be imported
> 
> What do you mean? It's in the 0.10 branch and master

What do you mean ? It's done ni dshowdec but not in dshowsrc.

Ah I see. The problem I see here is that it only fixes it between a fixed set of plugins, not if anybody else initialized/deininitialized COM too...

What I understand is that init/deinit COM has to be called in any thread before to use dshow api in any other thread. I mean you can init/deinit COM in thread A in order to use dshow api in thread B. (in case of COINIT_MULTITHREADED sure)

And init COM can be called in several thread. But for each com initialized thread then a deinit has to be call in those threads.

So I do not see any pb with the Julien Moutte 's solution here https://bugzilla.gnome.org/show_bug.cgi?id=592415, in the case an application that uses dshowdec/src also can call init/denit COM. There will be no impact on those calls from dshowdec/src I think.

If nobody reported a pb with dshowdec since this change, then I think we should just apply the same solution to dshowsrc :)

Sorry I pointed the wrong number: https://bugzilla.gnome.org/show_bug.cgi?id=625190

Maybe just nobody is using it enough? Anyway, please go ahead :)

I think this change http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/sys/dshowdecwrapper?h=0.10&id=efd840bbb7e398d89ee9d4230e1f9c6250deee6b still applies on gst 1.x and needs to be included in all our dshow elements.

The Right Thing To Do (c) would be to serialize all COM calls in a dedicated thread IMO. efd840bbb7e398d89ee9d4230e1f9c6250deee6b seems to only serialize CoInitialize/Uninitialize calls, which is probably wrong.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/12.