[vlc-devel] [PATCH 02/12] chromecast: use a local sout_stream_id_sys_t containing the es_format_t

[Rémi Denis-Courmont]

Le 2016-04-25 17:46, Steve Lhomme a écrit :
> ---
>  modules/stream_out/chromecast/cast.cpp | 65
> ++++++++++++++++++++++++++++++++--
>  1 file changed, 63 insertions(+), 2 deletions(-)
>
> diff --git a/modules/stream_out/chromecast/cast.cpp
> b/modules/stream_out/chromecast/cast.cpp
> index d05ca1c..ef7f428 100644
> --- a/modules/stream_out/chromecast/cast.cpp
> +++ b/modules/stream_out/chromecast/cast.cpp
> @@ -44,16 +44,23 @@ struct sout_stream_sys_t
>          , b_has_video(has_video)
>      {
>          assert(p_intf != NULL);
> +        vlc_mutex_init( &es_lock );
>      }
>
>      ~sout_stream_sys_t()
>      {
>          sout_StreamChainDelete(p_out, p_out);
> +        vlc_mutex_destroy( &es_lock );
>      }
>
>      sout_stream_t * const p_out;
>      intf_sys_t * const p_intf;
>      const bool b_has_video;
> +
> +    sout_stream_id_sys_t *GetSubId( sout_stream_t*, 
> sout_stream_id_sys_t* );
> +
> +    vlc_mutex_t                        es_lock;
> +    std::vector<sout_stream_id_sys_t*> streams;

As a general rule, VLC module instances (i.e. VLC objects, basically) 
do not need to be reentrant - all callbacks must be serialized by the 
owner. So that mutex does not seem to make much sense.

(There are exceptions, of course, such as object variable callbacks.)

>  };
>
>  #define SOUT_CFG_PREFIX "sout-chromecast-"
> @@ -100,6 +107,12 @@ vlc_module_begin ()
>  vlc_module_end ()
>
>
> +struct sout_stream_id_sys_t
> +{
> +    es_format_t           fmt;
> +    sout_stream_id_sys_t  *p_sub_id;
> +};
> +
>
> 
> /*****************************************************************************
>   * Sout callbacks
>
> 
> *****************************************************************************/
> @@ -112,7 +125,17 @@ static sout_stream_id_sys_t *Add(sout_stream_t
> *p_stream, const es_format_t *p_f
>          if (p_fmt->i_cat != AUDIO_ES)
>              return NULL;
>      }
> -    return sout_StreamIdAdd(p_sys->p_out, p_fmt);
> +
> +    sout_stream_id_sys_t *p_sys_id = (sout_stream_id_sys_t *)malloc(
> sizeof(sout_stream_id_sys_t) );
> +    if (p_sys_id != NULL)
> +    {
> +        es_format_Copy( &p_sys_id->fmt, p_fmt );
> +        p_sys_id->p_sub_id = NULL;
> +
> +        vlc_mutex_locker locker( &p_sys->es_lock );
> +        p_sys->streams.push_back( p_sys_id );
> +    }
> +    return p_sys_id;
>  }
>
>
> @@ -120,15 +143,49 @@ static void Del(sout_stream_t *p_stream,
> sout_stream_id_sys_t *id)
>  {
>      sout_stream_sys_t *p_sys = p_stream->p_sys;
>
> -    sout_StreamIdDel(p_sys->p_out, id);
> +    vlc_mutex_locker locker( &p_sys->es_lock );
> +    for (size_t i=0; i<p_sys->streams.size(); i++)
> +    {
> +        if ( p_sys->streams[i] == id )
> +        {
> +            if ( p_sys->streams[i]->p_sub_id != NULL )
> +                sout_StreamIdDel( p_sys->p_out,
> p_sys->streams[i]->p_sub_id );
> +
> +            es_format_Clean( &p_sys->streams[i]->fmt );
> +            free( p_sys->streams[i] );
> +            p_sys->streams.erase( p_sys->streams.begin() +  i );
> +            break;
> +        }
> +    }
>  }
>
> +sout_stream_id_sys_t *sout_stream_sys_t::GetSubId( sout_stream_t 
> *p_stream,
> +
> sout_stream_id_sys_t *id )
> +{
> +    size_t i;
> +
> +    assert( p_stream->p_sys == this );
> +
> +    vlc_mutex_locker locker( &es_lock );
> +    for (i = 0; i < streams.size(); ++i)
> +    {
> +        if ( id == (sout_stream_id_sys_t*) streams[i] )
> +            return streams[i]->p_sub_id;
> +    }
> +
> +    msg_Err( p_stream, "unknown stream ID" );
> +    return NULL;
> +}

So, either this method is intrinsically serialized with the plugin 
callbacks, and the lock is not necessary. Or it is not, and this won't 
work anyway because <id> would be potentially invalid.

>  static int Send(sout_stream_t *p_stream, sout_stream_id_sys_t *id,
>                  block_t *p_buffer)
>  {
>      sout_stream_sys_t *p_sys = p_stream->p_sys;
>
> +    id = p_sys->GetSubId( p_stream, id );
> +    if ( id == NULL )
> +        return VLC_EGENERIC;
> +
>      return sout_StreamIdSend(p_sys->p_out, id, p_buffer);
>  }
>
> @@ -136,6 +193,10 @@ static void Flush( sout_stream_t *p_stream,
> sout_stream_id_sys_t *id )
>  {
>      sout_stream_sys_t *p_sys = p_stream->p_sys;
>
> +    id = p_sys->GetSubId( p_stream, id );
> +    if ( id == NULL )
> +        return;
> +
>      sout_StreamFlush( p_sys->p_out, id );
>  }

-- 
Rémi Denis-Courmont
http://www.remlab.net/