[vlc-devel] [PATCH 0/1] dshow: Fix assertion when stopping

Rémi Denis-Courmont remi at remlab.net
Wed May 31 18:26:56 CEST 2017


Le keskiviikkona 31. toukokuuta 2017, 9.52.26 EEST Hugo Beauzée-Luyssen a 
écrit :
> > In general, a plugin _cannot_ assume that its callbacks are in a single
> > thread.
> 
> That seems like even more of a reason to go away from
> COINIT_APARTMENTTHREADED.

Yes, of course. I didn´t mean to challenge that aspect.

> If we can't guarantee the thread from which
> things will be called, we either use MTA and deal with synchronisation
> if needed, or marshall all of our calls to COM functions. The later
> doesn't sound appealing to me.

TBH, I am not familiar with marshalling.

> > Isn´t FindDevices() potentially called from the GUI thread?
> > Isn´t the GUI thread entered in an STA?
> 
> Only the Qt GUI calls CoInitializeEx (as far as interfaces are
> concerned, obviously)

Yes but isn´t the Qt Win32 port also entering an apartment internally, and isn
´t that unfortunately an STA?

> meaning the other interfaces would eventually enter an
> MTA, should they invoke FindDevices.

FindDevices() has to release whichever apartment that it succesfully entered. 
Otherwise we would have no way to prevent leakage.

> They would also leave the MTA after fetching the devices.
> 
> > > As such, it is safe to create & release those objects from another
> > > thread without synchronisation.
> > 
> > As far as I know, it would be safe if and only if the given thread were
> > in the
> > MTA when it creates, uses or destroys any of the objects, *and* if the
> > MTA has
> > at least one outstanding reference throughout the lifetime of each
> > object.
> 
> That is the case here. All objects are locally created (within
> FindCaptureDevice and AppendAudioEnabledVDevs), and guaranteed to be
> destroyed when leaving those function, which is before we call
> CoUninitialize

The enumeration callbacks indeed seem fine. But what about the access 
callbacks? What happens if Open() and Read(), or Open() and Close() are 
serialized yet called in different threads?

(...)
> Given that you say it would be safe if (...), and that this condition is
> met, I'm not sure why this patch is wrong. What would be the correct
> solution in your opinion?

AFAICT, either the DirectShow code has a thread keeping the MTA alive for the 
lifetime of the access/access_demux instance, or we ensure that another thread 
takes care of the same (e.g. in Win32 specific core code).

MMdevice has a background thread taking care of that, so the problem might 
only be apparent if you disable audio. The SAPI plugin is also buggy in this 
respect, IIRC.

-- 
雷米‧德尼-库尔蒙
https://www.remlab.net/



More information about the vlc-devel mailing list