[vlc-devel] [PATCH 3/3] omxil: move input handling into DecodeVideoInput

[Martin Storsjö]

On Thu, 10 Jul 2014, Thomas Guillem wrote:

> ---
> modules/codec/omxil/omxil.c |  135 ++++++++++++++++++++++++-------------------
> 1 file changed, 74 insertions(+), 61 deletions(-)
>
> diff --git a/modules/codec/omxil/omxil.c b/modules/codec/omxil/omxil.c
> index 99e6d2a..36430bf 100644
> --- a/modules/codec/omxil/omxil.c
> +++ b/modules/codec/omxil/omxil.c
> @@ -1268,72 +1268,24 @@ error:
>     return -1;
> }
>
> -/*****************************************************************************
> - * DecodeVideo: Called to decode one frame
> - *****************************************************************************/
> -static picture_t *DecodeVideo( decoder_t *p_dec, block_t **pp_block )
> +static int DecodeVideoInput( decoder_t *p_dec, OmxPort *p_port, block_t **pp_block,
> +                             bool *p_reconfig )
> {
>     decoder_sys_t *p_sys = p_dec->p_sys;
> -    picture_t *p_pic = NULL;
> -    OMX_ERRORTYPE omx_error;
> -    unsigned int i;
> -
>     OMX_BUFFERHEADERTYPE *p_header;
> -    block_t *p_block;
>     unsigned int i_input_used = 0;
>     struct H264ConvertState convert_state = { 0, 0 };
> +    block_t *p_block = *pp_block;
>
> -    if( !pp_block || !*pp_block )
> -        return NULL;
> -
> -    p_block = *pp_block;
> -
> -    /* Check for errors from codec */
> -    if(p_sys->b_error)
> -    {
> -        msg_Dbg(p_dec, "error during decoding");
> -        block_Release( p_block );
> -        return 0;
> -    }
> -
> -    if( p_block->i_flags & (BLOCK_FLAG_DISCONTINUITY|BLOCK_FLAG_CORRUPTED) )
> -    {
> -        block_Release( p_block );
> -        if(!p_sys->in.b_flushed)
> -        {
> -            msg_Dbg(p_dec, "flushing");
> -            OMX_SendCommand( p_sys->omx_handle, OMX_CommandFlush,
> -                             p_sys->in.definition.nPortIndex, 0 );
> -        }
> -        p_sys->in.b_flushed = true;
> -        return NULL;
> -    }
> -
> -    /* Use the aspect ratio provided by the input (ie read from packetizer).
> -     * In case the we get aspect ratio info from the decoder (as in the
> -     * broadcom OMX implementation on RPi), don't let the packetizer values
> -     * override what the decoder says, if anything - otherwise always update
> -     * even if it already is set (since it can change within a stream). */
> -    if((p_dec->fmt_in.video.i_sar_num != 0 && p_dec->fmt_in.video.i_sar_den != 0) &&
> -       (p_dec->fmt_out.video.i_sar_num == 0 || p_dec->fmt_out.video.i_sar_den == 0 ||
> -             !p_sys->b_aspect_ratio_handled))
> -    {
> -        p_dec->fmt_out.video.i_sar_num = p_dec->fmt_in.video.i_sar_num;
> -        p_dec->fmt_out.video.i_sar_den = p_dec->fmt_in.video.i_sar_den;
> -    }
> -
> -    /* Take care of decoded frames first */
> -    if( DecodeVideoOutput( p_dec, &p_sys->out, &p_pic ) != 0 )
> -        goto error;
> -
> -more_input:
>     /* Send the input buffer to the component */
> -    OMX_FIFO_GET_TIMEOUT(&p_sys->in.fifo, p_header, 200000);
> +    OMX_FIFO_GET_TIMEOUT(&p_port->fifo, p_header, 200000);
>
>     if (p_header && p_header->nFlags & SENTINEL_FLAG) {
>         free(p_header);
> -        goto reconfig;
> +        *p_reconfig = true;
> +        return 0;
>     }
> +    *p_reconfig = false;
>
>     if(p_header)
>     {
> @@ -1348,7 +1300,7 @@ more_input:
>
>         /* In direct mode we pass the input pointer as is.
>          * Otherwise we memcopy the data */
> -        if(p_sys->in.b_direct)
> +        if(p_port->b_direct)
>         {
>             p_header->pOutputPortPrivate = p_header->pBuffer;
>             p_header->pBuffer = p_block->p_buffer;
> @@ -1383,19 +1335,80 @@ more_input:
>         msg_Dbg( p_dec, "EmptyThisBuffer %p, %p, %i, %"PRId64, p_header, p_header->pBuffer,
>                  (int)p_header->nFilledLen, FromOmxTicks(p_header->nTimeStamp) );
> #endif
> -        OMX_EmptyThisBuffer(p_sys->omx_handle, p_header);
> -        p_sys->in.b_flushed = false;
> +        OMX_EmptyThisBuffer(p_port->omx_handle, p_header);
> +        p_port->b_flushed = false;
>         if (decode_more)
> -            goto more_input;
> +            return DecodeVideoInput( p_dec, p_port, pp_block, p_reconfig );

Handling splitting one input packet over multiple buffers doesn't work 
when you do it as a recursion. Note the i_input_used local variable - if 
you do it as a recursion you never actually use this.

This is necessary if you decode really high bitrate content - you might be 
able to simulate that condition by artifically lowering the size of the 
input buffers as well.

The patch otherwise looks quite nice so it would probably be worthwhile 
merging if you'd take another look at this aspect.

// Martin