[vlc-devel] [PATCH 3/3] dec: lock p_vout/p_aout when read from the DecoderThread

[Thomas Guillem]

So, I need to keep the lock while calling vout/aout Change
Rate/Pause/Flush. Right?
On Thu, May 17, 2018, at 14:49, Rémi Denis-Courmont wrote:
> No. Holding vout/aout gives access to variables and interface
> controls, nothing else.> 
> Le 17 mai 2018 15:36:55 GMT+03:00, Thomas Guillem <thomas at gllm.fr>
> a écrit :>> Since these variables can be written from any thread from
>> format_update()
>>>> callbacks.
>> 
>> ---
>> 
>>  src/input/decoder.c | 59 +++++++++++++++++++++++++++++++++++++--------
>>>>  1 file changed, 49 insertions(+), 10 deletions(-)
>> 
>> 
>> 
>> diff --git a/src/input/decoder.c b/src/input/decoder.c
>> 
>> index 1fdc4fb7ed..e8e4c109ef 100644
>> 
>> --- a/src/input/decoder.c
>> 
>> +++ b/src/input/decoder.c
>> 
>> @@ -103,10 +103,11 @@ struct decoder_owner
>> 
>>      vlc_cond_t  wait_fifo; /* TODO: merge with wait_acknowledge */
>>>>      vlc_cond_t  wait_timed;
>> 
>>  
>> 
>> -    /* -- These variables need locking on write(only) -- */
>> 
>> +    /* These variables are written from the format_update()
>>      callbacks threads.
>>>> +     * They need locking when they are read from the DecoderThread
>>       (use
>>>> +     * dec_get_aout()/dec_get_vout() helpers). */
>> 
>>      audio_output_t *p_aout;
>> 
>> -
>> 
>> -    vout_thread_t   *p_vout;
>> 
>> +    vout_thread_t  *p_vout;
>> 
>>  
>> 
>>      /* -- Theses variables need locking on read *and* write -- */
>>>>      /* Preroll */
>> 
>> @@ -158,6 +159,24 @@ static inline struct decoder_owner
>> *dec_get_owner( decoder_t *p_dec )
>>>>      return container_of( p_dec, struct decoder_owner, dec );
>> 
>>  }
>> 
>>  
>> 
>> +static audio_output_t *dec_get_aout( decoder_t *p_dec )
>> 
>> +{
>> 
>> +    struct decoder_owner *p_owner = dec_get_owner( p_dec );
>> 
>> +    vlc_mutex_lock( &p_owner->lock );
>> 
>> +    audio_output_t *p_aout = p_owner->p_aout ? vlc_object_hold( p_owner-
>>      >p_aout ) : NULL;
>>>> +    vlc_mutex_unlock( &p_owner->lock );
>> 
>> +    return p_aout;
>> 
>> +}
>> 
>> +
>> 
>> +static vout_thread_t *dec_get_vout( decoder_t *p_dec )
>> 
>> +{
>> 
>> +    struct decoder_owner *p_owner = dec_get_owner( p_dec );
>> 
>> +    vlc_mutex_lock( &p_owner->lock );
>> 
>> +    vout_thread_t *p_vout = p_owner->p_vout ? vlc_object_hold( p_owner-
>>      >p_vout ) : NULL;
>>>> +    vlc_mutex_unlock( &p_owner->lock );
>> 
>> +    return p_vout;
>> 
>> +}
>> 
>> +
>> 
>>  /**
>> 
>>   * Load a decoder module
>> 
>>   */
>> 
>> @@ -238,15 +257,17 @@ static int ReloadDecoder( decoder_t *p_dec,
>> bool b_packetizer,
>>>>      if( reload == RELOAD_DECODER_AOUT )
>> 
>>      {
>> 
>>          assert( p_owner->fmt.i_cat == AUDIO_ES );
>> 
>> -        audio_output_t *p_aout = p_owner->p_aout;
>> 
>>  
>> 
>>          vlc_mutex_lock( &p_owner->lock );
>> 
>> +        audio_output_t *p_aout = p_owner->p_aout ? vlc_object_hold(
>>          p_owner->p_aout ) : NULL;
>>>>          p_owner->p_aout = NULL;
>> 
>>          vlc_mutex_unlock( &p_owner->lock );
>> 
>> +
>> 
>>          if( p_aout )
>> 
>>          {
>> 
>>              aout_DecDelete( p_aout );
>> 
>>              input_resource_PutAout( p_owner->p_resource, p_aout );
>>>> +            vlc_object_release( p_aout );
>> 
>>          }
>> 
>>      }
>> 
>>  
>> 
>> @@ -1538,13 +1559,25 @@ static void *DecoderThread( void *p_data )
>>>>              switch( p_dec->fmt_out.i_cat )
>> 
>>              {
>> 
>>                  case VIDEO_ES:
>> 
>> -                    if( p_owner->p_vout != NULL )
>> 
>> -                        vout_ChangePause( p_owner->p_vout, paused,
>>                          date );
>>>> +                {
>> 
>> +                    vout_thread_t *p_vout = dec_get_vout( p_dec );
>>>> +                    if( p_vout != NULL )
>> 
>> +                    {
>> 
>> +                        vout_ChangePause( p_vout, paused, date );
>>>> +                        vlc_object_release( p_vout );
>> 
>> +                    }
>> 
>>                      break;
>> 
>> +                }
>> 
>>                  case AUDIO_ES:
>> 
>> -                    if( p_owner->p_aout != NULL )
>> 
>> -                        aout_DecChangePause( p_owner->p_aout,
>>                          paused, date );
>>>> +                {
>> 
>> +                    audio_output_t *p_aout = dec_get_aout( p_dec );
>>>> +                    if( p_aout != NULL )
>> 
>> +                    {
>> 
>> +                        aout_DecChangePause( p_aout, paused, date );
>>>> +                        vlc_object_release( p_aout );
>> 
>> +                    }
>> 
>>                      break;
>> 
>> +                }
>> 
>>                  case SPU_ES:
>> 
>>                      break;
>> 
>>                  default:
>> 
>> @@ -1569,9 +1602,15 @@ static void *DecoderThread( void *p_data )
>>>>                  case VIDEO_ES:
>> 
>>                      break;
>> 
>>                  case AUDIO_ES:
>> 
>> -                    if( p_owner->p_aout != NULL )
>> 
>> -                        aout_DecChangeRate( p_owner->p_aout, rate );
>>>> +                {
>> 
>> +                    audio_output_t *p_aout = dec_get_aout( p_dec );
>>>> +                    if( p_aout != NULL )
>> 
>> +                    {
>> 
>> +                        aout_DecChangeRate( p_aout, rate );
>> 
>> +                        vlc_object_release( p_aout );
>> 
>> +                    }
>> 
>>                      break;
>> 
>> +                }
>> 
>>                  case SPU_ES:
>> 
>>                      break;
>> 
>>                  default:
>> 
> 
> --
>  Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
>  excuser ma brièveté.> _________________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180517/04a7c078/attachment.html>