[vlc-devel] [PATCH 4/4] avcodec: fix deadlock when closing with mt

[Rémi Denis-Courmont]

Le 2015-11-18 13:05, Thomas Guillem a écrit :
>> > @@ -536,7 +538,11 @@ static picture_t *DecodeVideo( decoder_t 
>> *p_dec,
>> > block_t **pp_block )
>> >              /* NOTE: data is good only the timeline changed so do
>> > not flush decoder */
>> >              post_mt( p_sys );
>> >              if( p_block->i_flags & BLOCK_FLAG_DISCONTINUITY )
>> > +            {
>> > +                atomic_store( &p_sys->flushing_buffers, true );
>>
>> This assumes that libavcodec won't need new buffers while flushing. 
>> I
>> don't know if that is currently the case, and in any case, that 
>> seems
>> like an implementation detail of libavcodec (threaded) decoders that 
>> we
>> cannot rely on.
>
> Before DecoderIsFlushing was removed, we relied on it. Indeed,
> lavc_GetFrame returned -1 when Decoder was flushing (via
> decoder_NewPicture that returned NULL).

I don't think so.  Leaving aside that DecoderIsFlushing() was racy and 
kludgy, it was only triggered on actual decoder flush, not on any type 
of discontinuity. Besides, it was only (ab)used before the actual flush 
- while we knew the decoder is processing pre-flush pictures. This patch 
however forces picture allocation to fail during avcodec flush proper.

This could break if the decoder reallocates pictures right at the end 
of flush. Queuing decoded pictures during flush and retaining existing 
reference pictures throughout flush is invalid. But as far as I can 
tell, allocating new picture buffers is perfectly valid and legitimate 
during flush (as opposed to close).

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