[vlc-devel] [PATCH] decoder: fix NULL dereference when video format is updating
Thomas Guillem
thomas at gllm.fr
Tue Apr 28 10:49:05 CEST 2015
On Mon, Apr 27, 2015, at 19:46, Rémi Denis-Courmont wrote:
> Le lundi 27 avril 2015, 19:14:44 Thomas Guillem a écrit :
> > On Mon, Apr 27, 2015, at 18:25, Rémi Denis-Courmont wrote:
> > > Le lundi 27 avril 2015, 18:08:44 Thomas Guillem a écrit :
> > > > p_owner->p_vout can be set to NULL by decoder_UpdateVideoFormat that can
> > > > be
> > > > called from a different thread than the decoder one (see avcodec).
> > >
> > > If the current reads p_owner->p_vout (at line 861 and also in the calling
> > > function), then p_owner->p_vout must be read-only.
> > >
> > > So this is a bug in avcodec, or a limitation of the decoder core
> > > depending on
> > > your point of view. Either way, the patch does not seem "sufficient" to
> > > me.
> >
> > OK, it's not sufficient because p_vout should be written only by the
> > DecoderThread (for now).
>
> I guess it can be written by another thread when:
> - the decoder thread is inside pf_decode_video(), thus ensuring that the
> decoder thread will not access the vout, and
> - the owner lock is held, thus ensuring that the decoder owner (the input
> thread or whatever) will not access the vout, nor will any other threaded
> decoder plugin thread (if it exists).
>
> But at any other times, the decoder thread can indeed read the value
> without
> lock, so vout pointer must be read-only.
Could we add a rwlock to protect p_vout and p_aout ?
>
> > If it's not the case anymore, we should add
> > locks everytime we read p_vout from this thread. Am I right ?
>
> Yes.
>
> And also somehow ensure that pictures from a previous vout (before last
> format
> update) is not queued to the current vout. It could be argued that
> threaded
> decoder plugins should be responsible for this though.
Could we add a i_id in vout_thread_t, that is incremented from
vout_Request ?
This i_id will be set to a p_pic->i_vout_id from vout_GetPicture.
>
> --
> Rémi Denis-Courmont
> http://www.remlab.net/
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list