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

[Thomas Guillem]

If you are OK, I have to do the same patch for iomx-dr.

On Tue, Oct 28, 2014, at 17:01, Thomas Guillem wrote:
> 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
>