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

[Thomas Guillem]

fix grammar:

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)

On Wed, Oct 22, 2014, at 16:58, Thomas Guillem wrote:
> Currently, when using direct rendering, iomx and mediacodec decoder can
> wait
> indefinitely for an output frame when video is paused. In order to don't
> block
> everything, these decoders return an invalid_picture after a certain
> time. That
> way, in src/input/decoder.c, DecoderIsExitRequested will be checked and
> decoder
> will be exited.
> 
> The problem with that behavior is that DecoderDecodeVideo in
> src/input/decoder.c will terminate only if video is unpaused (and an
> output
> buffer become available) 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(-)
> 
> diff --git a/modules/codec/omxil/android_mediacodec.c
> b/modules/codec/omxil/android_mediacodec.c
> index 44184c6..0f291b7 100644
> --- a/modules/codec/omxil/android_mediacodec.c
> +++ b/modules/codec/omxil/android_mediacodec.c
> @@ -962,25 +962,8 @@ static picture_t *DecodeVideo(decoder_t *p_dec,
> block_t **pp_block)
>                 vout therefore we implement a timeout for polling in
>                 order to avoid being indefinitely stalled in this loop.
>                 */
>              if (p_sys->direct_rendering && attempts ==
>              max_polling_attempts) {
> -                picture_t *invalid_picture = decoder_NewPicture(p_dec);
> -                if (invalid_picture) {
> -                    invalid_picture->date = VLC_TS_INVALID;
> -                    picture_sys_t *p_picsys = invalid_picture->p_sys;
> -                    p_picsys->pf_display_callback = NULL;
> -                    p_picsys->pf_unlock_callback = NULL;
> -                    p_picsys->p_dec = NULL;
> -                    p_picsys->i_index = -1;
> -                    p_picsys->b_valid = false;
> -                }
> -                else {
> -                    /* 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;
> -                }
>                  jni_detach_thread();
> -                return invalid_picture;
> +                return NULL;
>              }
>              continue;
>          }
> diff --git a/modules/codec/omxil/omxil.c b/modules/codec/omxil/omxil.c
> index a9b496a..302065d 100644
> --- a/modules/codec/omxil/omxil.c
> +++ b/modules/codec/omxil/omxil.c
> @@ -1627,27 +1627,8 @@ static picture_t *DecodeVideo( decoder_t *p_dec,
> block_t **pp_block )
>             vout therefore we implement a timeout for polling in
>             order to avoid being indefinitely stalled in this loop, if
>             playback is paused. */
> -        if( p_sys->out.p_hwbuf && attempts == max_polling_attempts ) {
> -#ifdef USE_IOMX
> -            picture_t *invalid_picture = decoder_NewPicture(p_dec);
> -            if (invalid_picture) {
> -                invalid_picture->date = VLC_TS_INVALID;
> -                picture_sys_t *p_picsys = invalid_picture->p_sys;
> -                p_picsys->pf_display_callback = NULL;
> -                p_picsys->pf_unlock_callback = NULL;
> -                p_picsys->p_dec = NULL;
> -                p_picsys->i_index = -1;
> -                p_picsys->b_valid = false;
> -            } else {
> -                /* 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;
> -            }
> -            return invalid_picture;
> -#endif
> -        }
> +        if( p_sys->out.p_hwbuf && attempts == max_polling_attempts )
> +            return NULL;
>      }
>  
>      return p_pic;
> -- 
> 2.1.0
>