[vlc-devel] [PATCH 4/4] mediacodec: process input buffers in a separate thread

[Martin Storsjö]

On Wed, 29 Oct 2014, Thomas Guillem wrote:

> DecodeVideo will now send input blocks to a FIFO that will be processed by a
> separate thread. This function will return instantaneously, either a valid
> pic or NULL if there was no output buffers available. "invalid_picture hack" is
> not needed anymore.
>
> Fix a deadlock when a slow video was flushed, due to a seek.
> (fixes #12397)
> ---
> modules/codec/omxil/android_mediacodec.c | 268 ++++++++++++++++++++++++-------
> 1 file changed, 208 insertions(+), 60 deletions(-)
>
> diff --git a/modules/codec/omxil/android_mediacodec.c b/modules/codec/omxil/android_mediacodec.c
> index e352266..de8943a 100644
> --- a/modules/codec/omxil/android_mediacodec.c
> +++ b/modules/codec/omxil/android_mediacodec.c
> @@ -161,7 +161,6 @@ struct decoder_sys_t
>
>     bool allocated;
>     bool started;
> -    bool decoded;
>     bool error_state;
>     bool error_event_sent;
>
> @@ -173,6 +172,17 @@ struct decoder_sys_t
>     picture_t** inflight_picture; /**< stores the inflight picture for each output buffer or NULL */
>
>     timestamp_fifo_t *timestamp_fifo;
> +
> +    vlc_mutex_t   input_mutex;
> +    vlc_cond_t    input_cond;
> +    vlc_thread_t  input_thread;
> +    block_fifo_t *p_input_fifo;
> +    enum {
> +        INPUT_THREAD_STATE_RUNNING,
> +        INPUT_THREAD_STATE_FLUSH,
> +        INPUT_THREAD_STATE_EXIT,
> +        INPUT_THREAD_STATE_ERROR,
> +    } input_state;
> };
>
> enum Types
> @@ -259,6 +269,13 @@ static void CloseDecoder(vlc_object_t *);
>
> static picture_t *DecodeVideo(decoder_t *, block_t **);
>
> +static void *InputThread(void *data);
> +static int InputThreadInit(decoder_t *p_dec);
> +static void InputThreadDestroy(decoder_t *p_dec);
> +static void InputThreadFlush(decoder_t *p_dec);
> +static void InputThreadWake(decoder_t *p_dec);
> +static bool InputThreadGetErrorState(decoder_t *p_dec);
> +
> static void InvalidateAllPictures(decoder_t *);
>
> /*****************************************************************************
> @@ -536,7 +553,6 @@ static int OpenDecoder(vlc_object_t *p_this)
>         (*env)->ExceptionClear(env);
>         goto error;
>     }
> -    p_sys->started = true;
>
>     p_sys->input_buffers = (*env)->CallObjectMethod(env, p_sys->codec, p_sys->get_input_buffers);
>     p_sys->output_buffers = (*env)->CallObjectMethod(env, p_sys->codec, p_sys->get_output_buffers);
> @@ -552,6 +568,11 @@ static int OpenDecoder(vlc_object_t *p_this)
>
>     jni_detach_thread();
>
> +    if (InputThreadInit(p_dec) != 0)
> +        goto error;
> +
> +    p_sys->started = true;
> +

If there's an error between the place where you removed started = true 
above and here, we won't call Stop() when cleaning up. Not sure if it 
matters much but I'm not quite sure why you moved it either.

Other than that I guess it looks good if you've tested it and thought it 
through... I've mostly got the same concerns as in the previous version 
this morning, but as you said it didn't buffer the whole file, only a few 
packets, I guess it isn't all that bad.

So no major objections from me, but please test it quite thoroughly :-)

// Martin