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

[Thomas Guillem]

OK, that should do it. "i_input_used" is passed recursively.

But I think I just spotted a bug (not introduced by my patch, I hope ?).
What happens if OMX_FIFO_GET_TIMEOUT does timeout the next time when
i_input_used is != 0 ?
I think DecodeVideo will return and will be called again but with the same
p_block and with i_input_used = 0.
I think we should use something like i_last_input_used in OmxPort struct,
in order to save the p_block offset across several calls of DecodeVideo.

I you agree, I'll propose an other patch.


On Thu, Jul 10, 2014 at 11:53 PM, Thomas Guillem <thomas.guillem at gmail.com>
wrote:

> From: Thomas Guillem <guillem at archos.com>
>
> ---
>  modules/codec/omxil/omxil.c | 137
> ++++++++++++++++++++++++--------------------
>  1 file changed, 75 insertions(+), 62 deletions(-)
>
> diff --git a/modules/codec/omxil/omxil.c b/modules/codec/omxil/omxil.c
> index 99e6d2a..f67b526 100644
> --- a/modules/codec/omxil/omxil.c
> +++ b/modules/codec/omxil/omxil.c
> @@ -1268,72 +1268,23 @@ 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,
> +                             unsigned int i_input_used, 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 +1299,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 +1334,81 @@ 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,
> i_input_used,
> +                                     p_reconfig );
>          else
>              *pp_block = NULL; /* Avoid being fed the same packet again */
>      }
>
> +    return 0;
> +}
> +
>
> +/*****************************************************************************
> + * DecodeVideo: Called to decode one frame
> +
> *****************************************************************************/
> +static picture_t *DecodeVideo( decoder_t *p_dec, block_t **pp_block )
> +{
> +    decoder_sys_t *p_sys = p_dec->p_sys;
> +    picture_t *p_pic = NULL;
> +    OMX_ERRORTYPE omx_error;
> +    unsigned int i;
> +    bool b_reconfig;
> +    block_t *p_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;
> +
> +    if( DecodeVideoInput( p_dec, &p_sys->in, pp_block, 0, &b_reconfig )
> != 0 )
> +        goto error;
> +
>      /* If we don't have a p_pic from the first try. Try again */
> -    if( !p_pic && DecodeVideoOutput( p_dec, &p_sys->out, &p_pic ) != 0 )
> +    if( !b_reconfig && !p_pic &&
> +        DecodeVideoOutput( p_dec, &p_sys->out, &p_pic ) != 0 )
>          goto error;
>
> -reconfig:
>      /* Handle the PortSettingsChanged events */
>      for(i = 0; i < p_sys->ports; i++)
>      {
> --
> 2.0.0.rc2
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20140711/44a883ed/attachment.html>