[vlc-devel] [PATCH] decoder: document vout/aout locking

[Steve Lhomme]

I agree with adding more documentation. But I think it's incorrect, see 
my patch about this particular comment line. I found examples of 
p_aout/p_vout used from different threads at the same time.

p_aout can even be set outside of the "OutputThread".

IMO it's dangerous to document variables as lock only on write. They 
*do* need locks when called from DecoderThread (unless there's some 
synchronization between the two+ threads). It would be better to 
document when the lock is *not* needed (in the case of p_aout I don't 
even know if it's even possible).

On 2019-09-03 11:11, Thomas Guillem wrote:
> ---
>   src/input/decoder.c | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/src/input/decoder.c b/src/input/decoder.c
> index 782dc0a422..8a1227a2d2 100644
> --- a/src/input/decoder.c
> +++ b/src/input/decoder.c
> @@ -101,9 +101,20 @@ struct decoder_owner
>       vlc_cond_t  wait_acknowledge;
>       vlc_cond_t  wait_fifo; /* TODO: merge with wait_acknowledge */
>   
> -    /* -- These variables need locking on write(only) -- */
> +    /* -- These variables need locking on write(only) --
> +     * 3 threads can read/write these output variables, the DecoderThread, the
> +     * input thread, and the OutputThread. The OutputThread is either the
> +     * DecoderThread for synchronous modules or any threads for asynchronous
> +     * modules. The asynchronous module is responsible for serializing/locking
> +     * every output calls, cf. decoder_UpdateVideoFormat() and
> +     * decoder_NewPicture() notes. The OutputThread is the owner of these
> +     * variables, it should hold the lock when writing them but doesn't have to
> +     * hold it when using them. The DecoderThread should always hold the lock
> +     * when reading/using aout/vouts. The input thread can read these variables
> +     * in order to stop outputs, when both OutputThread and DecoderThread are
> +     * stopped (from DecoderDelete()).
> +     */
>       audio_output_t *p_aout;
> -
>       vout_thread_t   *p_vout;
>   
>       /* -- Theses variables need locking on read *and* write -- */
> -- 
> 2.20.1
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>