[vlc-devel] [PATCH]: fix a CoInitializeEx() issue on Windows

[Steve Lhomme]

Hi,

On 2019-07-12 16:46, erwan.tulou at gmail.com wrote:
> Yes, sorry
> 
> On 12/07/2019 16:46, Hugo Beauzée-Luyssen wrote:
>> I think you're missing the mailing list in CC :)
>>
>> On Fri, Jul 12, 2019, at 4:42 PM, erwan.tulou at gmail.com wrote:
>>> On 12/07/2019 15:34, Hugo Beauzée-Luyssen wrote:
>>>>> Anyway it seems correct. The CoInitializeEx() calls in our Qt code 
>>>>> uses
>>>>> COINIT_APARTMENTTHREADED.
>>>>>
>>>> AFAIU, a thread doesn't belong to anything (with regard to the COM 
>>>> object concurrency mode) before the first call to CoInitializeEx. 
>>>> After that call, the thread is bound to that mode.
>>>>
>>>> If this is to be called from the UI thread, then it needs to match 
>>>> what's done in the UI, or the UI changed the other way around, but I 
>>>> think Qt mandates APARTMENTTHREADED (which would match this remark 
>>>> from MSDN: «The multi-threaded apartment is intended for use by 
>>>> non-GUI threads. Threads in multi-threaded apartments should not 
>>>> perform UI actions.»)
>>>>
>>>> However it seems CommonChangeThumbnailClip might still be called 
>>>> from the vout thread, and I fear that changing the object 
>>>> concurrency mode for the vout thread won't play well with SAPI, 
>>>> which is using MTA. I didn't find any requirement for SAPI to use 
>>>> COINIT_MULTITHREADED, so I suppose SAPI should be changed as well 
>>>> (unless I missed such a requirement, in which case I'm really not 
>>>> sure what to do here)
>>> Yes, CommonChangeThumbnailCli() is called twice in a row from two
>>> different threads, one with no previous concurrency mode setup and the
>>> GUI thread that demands APARTMENTTHREADED. I guess this function is
>>> called multiple times because it was placed in CommonPlacePicture()
>>> which is a more general multi purpose function, which is a bit
>>> unfortunate. How and where the function is called needs to be reworked.
>>> The function just needs
>>>
>>> to know that the video size/position has changed and update the tasklist
>>> thumbernail accordingly. This is GUI stuff and therefore the GUI thread
>>> doing the job directly via vout_window_ReportResize() seems a good
>>> thing. Another thread also doing the job seems a side effect which was
>>> not intended and should be regarded as a bug.

It could be handled by the window rather that the vout thread. After all 
it tells the OS the thumbnail area to use in the parent HWND of the vout 
HWND, that is the window one. So if it's resized in the window it's 
known there. We could also disable it when the window is enabled/disabled.

The downside is that the code would have to be used in each GUI and the 
standalone window. Either by copying code or sharing it between modules.