[vlc-devel] [PATCH] android direct rendering: fix deadlock when seeking on pause.

[Felix Abecassis]

2014-10-22 10:41 GMT-07:00 Martin Storsjö <martin at martin.st>:
> On Wed, 22 Oct 2014, Thomas Guillem wrote:
>
>> Currently, when video is paused and when direct rendering is activated,
>> iomx
>> and mediacodec decoders can wait indefinitely for an output buffer. In
>> order to
>> not block DecoderThread (see src/input/decoder.c), these decoders return
>> an
>> invalid_picture after a certain time. This way, DecoderIsExitRequested
>> will be
>> checked and decoder will be exited.
>>
>> The problem with this behavior is that DecoderDecodeVideo  will terminate
>> only
>> if video is unpaused or is exited.
>>
>> Then, in case of seek, DecoderDecodeVideo won't be feed with an other
>> p_block
>> containing the FLUSH flags and a deadlock will happen.
>>
>> One way to fix it is to return NULL instead of an invalid_picture.
>> DecoderIsExitRequested won't be checked but DecoderDecodeVideo will
>> return, and
>> a new p_block with FLUSH or EOS flags will be feed in case of seek or
>> exit.
>>
>> (fixes #12397)
>> ---
>> modules/codec/omxil/android_mediacodec.c | 19 +------------------
>> modules/codec/omxil/omxil.c              | 23 ++---------------------
>> 2 files changed, 3 insertions(+), 39 deletions(-)
>
>
> As said on IRC, I don't remember directly why we did this with an invalid
> picture instead of just NULL (I guess we would have tried/evaluated NULL
> before trying this), and I think it was Felix who came up with it, so he's
> probably the best to answer if there's some important catch we're missing.
>

Hello,

Concerning the MediaCodec code: if you return NULL here, the input
block will be lost and it will leak memory each time this happens.
When pausing the video, many input blocks are still fed to the
decoder, and this code path is executed many times since the HW
decoder cannot make any progress. Thus, many blocks will be dropped
and the video will glitch when resuming because of the skipped frames.

By using invalid pictures, we are periodically giving the core the
opportunity to recover if the decoder is bogus, but the decode
function is called with the same input block afterwards.

Regards,

-- 
Félix Abecassis
http://felix.abecassis.me