[vlc-devel] [PATCH] decoder: remove misleading comment

[Thomas Guillem]

On Tue, Sep 3, 2019, at 14:07, Thomas Guillem wrote:
> 
> 
> On Tue, Sep 3, 2019, at 14:05, Steve Lhomme wrote:
> > On 2019-09-03 13:43, Thomas Guillem wrote:
> > > 
> > > On Tue, Sep 3, 2019, at 13:22, Steve Lhomme wrote:
> > >> The p_aout and p_vout need locking for reading as well (it wouldn't make much
> > >> sense to only lock when writing).
> > >>
> > >> For example p_aout can be modified in the DecoderThread during a ReloadDecoder.
> > > 
> > > ReloadDecoder() will first unload the audio decoder before writing the aout so we are safe here. I could document it.
> > 
> > It reads p_owner->p_aout and do aout_DecDelete( p_aout ) on it in the 
> > DecoderThread. It is the same kind of code done in the decoder thread 
> > calling aout_update_format(). It is safe only because the 
> > reading/resetting/setting is done under lock in both threads.
> 
> No, it is safe before the module is unloaded, so no threads can call 
> aout_update_format().

s/before/because/

> 
> > 
> > IMO that contradicts the "These variables need locking on write(only)" doc.
> > 
> > >>
> > >> p_vout cannot be modified outside of the vout_update_format/spu_new_buffer but
> > >> it is also access in the DecoderThread, for example during DecoderProcessFlush()
> > >> or the OutputChangeXXX functions.
> > > 
> > > That is what my comment say no ?
> > 
> > Yes
> > 
> > >>
> > >> For functions only called by the decoder, it might be OK not to lock as the
> > >> p_out/p_aout should only change during vout_update_format/aout_update_format
> > >> which the decoder should call before doing other calls from various threads.
> > >> ---
> > >>   src/input/decoder.c | 3 +--
> > >>   1 file changed, 1 insertion(+), 2 deletions(-)
> > >>
> > >> diff --git a/src/input/decoder.c b/src/input/decoder.c
> > >> index 65e3dd6f6c..7011c99226 100644
> > >> --- a/src/input/decoder.c
> > >> +++ b/src/input/decoder.c
> > >> @@ -101,12 +101,11 @@ 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) -- */
> > >> +    /* -- Theses variables need locking on read *and* write -- */
> > >>       audio_output_t *p_aout;
> > >>   
> > >>       vout_thread_t   *p_vout;
> > >>   
> > >> -    /* -- Theses variables need locking on read *and* write -- */
> > >>       /* Preroll */
> > >>       vlc_tick_t i_preroll_end;
> > >>   
> > >> -- 
> > >> 2.17.1
> > >>
> > >> _______________________________________________
> > >> vlc-devel mailing list
> > >> To unsubscribe or modify your subscription options:
> > >> https://mailman.videolan.org/listinfo/vlc-devel
> > > _______________________________________________
> > > vlc-devel mailing list
> > > To unsubscribe or modify your subscription options:
> > > https://mailman.videolan.org/listinfo/vlc-devel
> > > 
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel