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

[Thomas Guillem]

On Wed, Oct 29, 2014, at 18:43, Martin Storsjö wrote:
> 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.

Ah, you're right, I should not have moved it.

> 
> 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 :-)

Thanks you for the quick review.

I tested it on some devices, and I saw just one regression that need a
hack or a core fix (I'am on it). When you open a video with subtitles,
spu_new_buffer will fails and hangs for 3-4 sec because of a NULL vout
since we didn't called decoder_NewPicture yet (that do a RequestVout).

Don't push these patches now, I'll test it more tomorrow.

> 
> // Martin
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel