[vlc-devel] [PATCH] dshow: Use MTA and stop leaking thread context out of the module

[Hugo Beauzée-Luyssen]

On Thu, Jun 1, 2017, at 06:49 PM, Rémi Denis-Courmont wrote:
> Le torstaina 1. kesäkuuta 2017, 18.43.19 EEST Hugo Beauzée-Luyssen a
> écrit :
> > Fix #16935
> > ---
> >  modules/access/dshow/dshow.cpp | 44
> > +++++++++++++++++++++++++++++++----------- 1 file changed, 33
> > insertions(+), 11 deletions(-)
> > 
> > diff --git a/modules/access/dshow/dshow.cpp b/modules/access/dshow/dshow.cpp
> > index d42ec0871e..817584f86d 100644
> > --- a/modules/access/dshow/dshow.cpp
> > +++ b/modules/access/dshow/dshow.cpp
> > @@ -35,6 +35,7 @@
> >  #include <list>
> >  #include <string>
> >  #include <assert.h>
> > +#include <stdexcept>
> > 
> >  #define VLC_MODULE_LICENSE VLC_LICENSE_GPL_2_PLUS
> >  #include <vlc_common.h>
> > @@ -299,6 +300,19 @@ vlc_module_begin ()
> > 
> >  vlc_module_end ()
> > 
> > +struct ComContext
> > +{
> > +    ComContext( int mode )
> > +    {
> > +        if( !SUCCEEDED( CoInitializeEx( NULL, mode ) ) )
> 
> Nit but why not if(FAILED()) ?

Because... reasons? Will changed locally, it makes more sense indeed.

> 
> > +            throw std::runtime_error( "CoInitializeEx failed" );
> > +    }
> > +    ~ComContext()
> > +    {
> > +        CoUninitialize();
> > +    }
> > +

Trailing whitespace removed locally.

> > +};
> > 
> >  /**************************************************************************
> > *** * DirectShow elementary stream descriptor
> > @@ -392,10 +406,6 @@ static int CommonOpen( vlc_object_t *p_this,
> > access_sys_t *p_sys, bool b_use_audio = true;
> >      bool b_use_video = true;
> > 
> > -    /* Initialize OLE/COM */
> > -    if( FAILED(CoInitializeEx( NULL, COINIT_APARTMENTTHREADED )) )
> > -        vlc_assert_unreachable();
> > -
> >      var_Create( p_this,  CFG_PREFIX "config", VLC_VAR_BOOL |
> > VLC_VAR_DOINHERIT ); var_Create( p_this,  CFG_PREFIX "tuner", VLC_VAR_BOOL
> > | VLC_VAR_DOINHERIT ); psz_val = var_CreateGetString( p_this, CFG_PREFIX
> > "vdev" );
> > @@ -665,6 +675,8 @@ static int DemuxOpen( vlc_object_t *p_this )
> >          return VLC_ENOMEM;
> >      p_demux->p_sys = (demux_sys_t *)p_sys;
> > 
> > +    ComContext ctx( COINIT_MULTITHREADED );
> > +
> >      if( CommonOpen( p_this, p_sys, true ) != VLC_SUCCESS )
> >      {
> >          CommonClose( p_this, p_sys );
> > @@ -770,6 +782,8 @@ static int AccessOpen( vlc_object_t *p_this )
> >      if( !p_sys )
> >          return VLC_ENOMEM;
> > 
> > +    ComContext ctx( COINIT_MULTITHREADED );
> > +
> >      if( CommonOpen( p_this, p_sys, false ) != VLC_SUCCESS )
> >      {
> >          CommonClose( p_this, p_sys );
> > @@ -804,9 +818,6 @@ static void CommonClose( vlc_object_t *p_this,
> > access_sys_t *p_sys )
> > 
> >      DeleteDirectShowGraph( p_this, p_sys );
> > 
> > -    /* Uninitialize OLE/COM */
> > -    CoUninitialize();
> > -
> >      vlc_delete_all( p_sys->pp_streams );
> > 
> >      vlc_mutex_destroy( &p_sys->lock );
> > @@ -823,6 +834,8 @@ static void AccessClose( vlc_object_t *p_this )
> >      access_t     *p_access = (access_t *)p_this;
> >      access_sys_t *p_sys    = (access_sys_t *)p_access->p_sys;
> > 
> > +    ComContext ctx( COINIT_MULTITHREADED );
> > +
> >      /* Stop capturing stuff */
> >      p_sys->p_control->Stop();
> > 
> > @@ -837,6 +850,8 @@ static void DemuxClose( vlc_object_t *p_this )
> >      demux_t      *p_demux = (demux_t *)p_this;
> >      access_sys_t *p_sys   = (access_sys_t *)p_demux->p_sys;
> > 
> > +    ComContext ctx( COINIT_MULTITHREADED );
> > +
> >      /* Stop capturing stuff */
> >      p_sys->p_control->Stop();
> > 
> > @@ -1738,6 +1753,8 @@ static size_t EnumDeviceCaps( vlc_object_t *p_this,
> > IBaseFilter *p_filter,
> > ***************************************************************************
> > **/ static block_t *ReadCompressed( access_t *p_access, bool *eof )
> >  {
> > +    ComContext ctx( COINIT_MULTITHREADED );
> > +
> >      access_sys_t   *p_sys = (access_sys_t *)p_access->p_sys;
> >      /* There must be only 1 elementary stream to produce a valid stream
> >       * of MPEG or DV data */
> > @@ -1783,6 +1800,8 @@ out:
> >  
> > ***************************************************************************
> > */ static int Demux( demux_t *p_demux )
> >  {
> > +    ComContext ctx( COINIT_MULTITHREADED );
> > +
> >      access_sys_t *p_sys = (access_sys_t *)p_demux->p_sys;
> >      int i_found_samples;
> > 
> > @@ -2017,11 +2036,12 @@ static int FindDevices( vlc_object_t *p_this, const
> > char *psz_name, {
> >      /* Find list of devices */
> >      std::list<std::string> list_devices;
> > -    if( SUCCEEDED(CoInitializeEx( NULL, COINIT_MULTITHREADED ))
> > -     || SUCCEEDED(CoInitializeEx( NULL, COINIT_APARTMENTTHREADED )) )
> > +    try
> >      {
> >          bool b_audio = !strcmp( psz_name, CFG_PREFIX "adev" );
> > 
> > +        ComContext ctx( COINIT_MULTITHREADED );
> > +
> >          FindCaptureDevice( p_this, NULL, &list_devices, b_audio );
> > 
> >          if( b_audio )
> > @@ -2031,8 +2051,10 @@ static int FindDevices( vlc_object_t *p_this, const
> > char *psz_name, if( !list_vdevs.empty() )
> >                  AppendAudioEnabledVDevs( p_this, list_devices, list_vdevs
> > ); }
> > -
> > -        CoUninitialize();
> > +    }
> > +    catch (const std::runtime_error& ex)
> > +    {
> > +        msg_Err( p_this, "Failed fetch devices: %s", ex.what() );
> >      }
> > 
> >      unsigned count = 2 + list_devices.size(), i = 2;
> 
> 
> -- 
> 雷米‧德尼-库尔蒙
> https://www.remlab.net/
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel


-- 
  Hugo Beauzée-Luyssen
  hugo at beauzee.fr