[vlc-devel] [RFC PATCH] mediacodec: fix a deadlock when seeking on pause

[Thomas Guillem]

MediaCodec with direct rendering can be stuck waiting for an input buffer when
video is paused. Indeed, input buffers availability depends on output buffers.
When the video is paused, output buffers are not released or displayed anymore,
therefore, no more input buffers are available.

A previous hack was made in order to fix a deadlock when mediacodec was stuck
and when exit was requested. Mediacodec returned an invalid picture after a
small delay, then DecoderIsExitRequested in DecoderDecodeVideo was checked and
this decoder could be exited without blocking.

This previous hack didn't fix a deadlock when a paused video was flushed, due
to a seek. Indeed, when stuck, mediacodec is feed with the same block.
Therefore it won't receive a new block containing the FLUSH flags and it will
be stuck returning an invalid picture.

One (ugly) way to fix this deadlock is, when you return an invalid_picture, to
put the block in an internal FIFO and tell the core decoder that the block is
processed. This way, new blocks will be given on next Decode calls and the
Decoder will be able to handle a flush block. If the decoder is stuck when
paused and resumed again, the Decode function will first process the block that
come from the internal FIFO. There will be no discontinuity at all.

(fixes #12397)
---
 modules/codec/omxil/android_mediacodec.c | 40 +++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/modules/codec/omxil/android_mediacodec.c b/modules/codec/omxil/android_mediacodec.c
index 6d204d7..0673887 100644
--- a/modules/codec/omxil/android_mediacodec.c
+++ b/modules/codec/omxil/android_mediacodec.c
@@ -173,6 +173,7 @@ struct decoder_sys_t
     picture_t** inflight_picture; /**< stores the inflight picture for each output buffer or NULL */
 
     timestamp_fifo_t *timestamp_fifo;
+    block_fifo_t *p_extra_input_fifo;
 };
 
 enum Types
@@ -536,6 +537,9 @@ static int OpenDecoder(vlc_object_t *p_this)
         (*env)->ExceptionClear(env);
         goto error;
     }
+    p_sys->p_extra_input_fifo = block_FifoNew();
+    if (!p_sys->p_extra_input_fifo)
+        goto error;
     p_sys->started = true;
 
     p_sys->input_buffers = (*env)->CallObjectMethod(env, p_sys->codec, p_sys->get_input_buffers);
@@ -606,6 +610,9 @@ static void CloseDecoder(vlc_object_t *p_this)
         (*env)->DeleteGlobalRef(env, p_sys->buffer_info);
     jni_detach_thread();
 
+    if (p_sys->p_extra_input_fifo)
+        block_FifoRelease(p_sys->p_extra_input_fifo);
+
     free(p_sys->name);
     ArchitectureSpecificCopyHooksDestroy(p_sys->pixel_format, &p_sys->architecture_specific_data);
     free(p_sys->inflight_picture);
@@ -909,8 +916,10 @@ static picture_t *DecodeVideo(decoder_t *p_dec, block_t **pp_block)
 
     jni_attach_thread(&env, THREAD_NAME);
 
+    /* First check if the block in arguments has any flush flags */
     if (p_block->i_flags & (BLOCK_FLAG_DISCONTINUITY|BLOCK_FLAG_CORRUPTED)) {
         block_Release(p_block);
+        block_FifoEmpty(p_sys->p_extra_input_fifo);
         timestamp_FifoEmpty(p_sys->timestamp_fifo);
         if (p_sys->decoded) {
             /* Invalidate all pictures that are currently in flight
@@ -931,6 +940,12 @@ static picture_t *DecodeVideo(decoder_t *p_dec, block_t **pp_block)
         return NULL;
     }
 
+    if (block_FifoCount(p_sys->p_extra_input_fifo) > 0) {
+        /* Don't dequeue the first block, but just peek it. The dequeue is done
+         * at the end if the block has been processed */
+        p_block = block_FifoShow(p_sys->p_extra_input_fifo);
+    }
+
     /* Use the aspect ratio provided by the input (ie read from packetizer).
      * Don't check the current value of the aspect ratio in fmt_out, since we
      * want to allow changes in it to propagate. */
@@ -972,6 +987,14 @@ static picture_t *DecodeVideo(decoder_t *p_dec, block_t **pp_block)
             if (p_sys->direct_rendering && attempts == max_polling_attempts) {
                 picture_t *invalid_picture = decoder_NewPicture(p_dec);
                 if (invalid_picture) {
+                    /* HACK: Assign NULL to *pp_block in order to get a new
+                     * pp_block for the next call and put this block in an
+                     * internal fifo in order to not lost it. That way, the
+                     * decoder won't be stuck in case of seek since it will
+                     * receive a new block with the flush flag. */
+                    block_FifoPut(p_sys->p_extra_input_fifo, *pp_block);
+                    *pp_block = NULL;
+
                     invalid_picture->date = VLC_TS_INVALID;
                     picture_sys_t *p_picsys = invalid_picture->p_sys;
                     p_picsys->pf_display_callback = NULL;
@@ -984,8 +1007,10 @@ static picture_t *DecodeVideo(decoder_t *p_dec, block_t **pp_block)
                     /* If we cannot return a picture we must free the
                        block since the decoder will proceed with the
                        next block. */
-                    block_Release(p_block);
-                    *pp_block = NULL;
+                    if (p_block == *pp_block) {
+                        block_Release(p_block);
+                        *pp_block = NULL;
+                    }
                 }
                 jni_detach_thread();
                 return invalid_picture;
@@ -1032,7 +1057,16 @@ static picture_t *DecodeVideo(decoder_t *p_dec, block_t **pp_block)
         GetOutput(p_dec, env, &p_pic, 0);
     jni_detach_thread();
 
-    block_Release(p_block);
+    if (p_block == *pp_block) {
+        /* Normal way: the block comes from arguments */
+        block_Release(p_block);
+    } else {
+        /* Hack way: the block comes from the internal fifo */
+        /* Dequeue the first block since it has been processed */
+        p_block = block_FifoGet(p_sys->p_extra_input_fifo);
+        /* The pp_block in arguments was not processed, so put it in the FIFO */
+        block_FifoPut(p_sys->p_extra_input_fifo, *pp_block);
+    }
     *pp_block = NULL;
 
     return p_pic;
-- 
2.1.0