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

[Rémi Denis-Courmont]

Le tiistaina 30. toukokuuta 2017, 18.53.06 EEST Hugo Beauzée-Luyssen a écrit :
> This patch aims at fixing #16935
> The basic reason for this assertion to trigger is that the access thread
> enters a single thread appartment.

VLC does not have an "access thread". The access_demux will, effectively, 
always run in the input thread - provided that no threaded demuxing filters is 
involved. The access however may well be created in the input thread and used 
in a demux or stream filter thread later.

In general, a plugin _cannot_ assume that its callbacks are in a single 
thread.

> When we stop the playback, the audio
> output will be flushed, but mmdevice tries to enter a multi thread
> appartment, which is bound to fail, triggering the assertion.
> 
> As things stand, I don't see a reason for dshow to use a STA. All the
> interfaces I checked don't mention such a requirement, and the only
> interaction from another thread is FindDevices, which only creates new COM
> objects.

Isn´t FindDevices() potentially called from the GUI thread?
Isn´t the GUI thread entered in an STA?

> 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.

CommonOpen() meets those conditions. But I don´t see either of those 
conditions ensured in Demux, Read and both Control functions. Admittedly, that 
is a preexisting bug.

> That being said, this is my current personal understanding, and I'd gladly
> stand corrected if I missed something.

Neither do I know if DirectShow is supposed to work in an MTA. But in any 
case, the current code and this patch both look incorrect to me.

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