[vlc-devel] [RFC PATCH] input: decoder: optimize dpb_size of H264

[Filip Roséen]

Hi Zhao,

On 2017-05-19 16:31, Zhao Zhili wrote:

> ---
>  src/input/decoder.c | 120
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 120 insertions(+)
> diff --git a/src/input/decoder.c b/src/input/decoder.c
> index 400464d..2283503 100644
> --- a/src/input/decoder.c
> +++ b/src/input/decoder.c
> @@ -370,6 +370,80 @@ static int aout_update_format( decoder_t *p_dec )
>      return 0;
>  }
> 
> +/* copy and paste from modules/packetizer/h264_nal.c
> + */
> +static bool h264_get_profile_level(const es_format_t *p_fmt, int
> *pi_profile,
> +                            int *pi_level, uint8_t *pi_nal_length_size)
> +{
> +    uint8_t *p = (uint8_t*)p_fmt->p_extra;
> +    if(p_fmt->i_extra < 8)
> +        return false;
> +
> +    /* Check the profile / level */
> +    if (p[0] == 1 && p_fmt->i_extra >= 12)
> +    {
> +        if (pi_nal_length_size)
> +            *pi_nal_length_size = 1 + (p[4]&0x03);
> +        p += 8;
> +    }
> +    else if(!p[0] && !p[1]) /* FIXME: WTH is setting AnnexB data here ? */
> +    {
> +        if (!p[2] && p[3] == 1)
> +            p += 4;
> +        else if (p[2] == 1)
> +            p += 3;
> +        else
> +            return false;
> +    }
> +    else return false;
> +
> +    if ( ((*p++)&0x1f) != 7) return false;
> +
> +    if (pi_profile)
> +        *pi_profile = p[0];
> +
> +    if (pi_level)
> +        *pi_level = p[2];
> +
> +    return true;
> +}
> +
> +static inline int h264_get_max_dpb_mbs(int level)
> +{
> +    int max_dpb_mbs = -1;
> +    switch (level)
> +    {
> +    case 30:
> +        max_dpb_mbs = 8100;
> +        break;
> +    case 31:
> +        max_dpb_mbs = 18000;
> +        break;
> +    case 32:
> +        max_dpb_mbs = 20480;
> +        break;
> +    case 40:
> +    case 41:
> +        max_dpb_mbs = 32768;
> +        break;
> +    case 42:
> +        max_dpb_mbs = 34816;
> +        break;
> +    case 50:
> +        max_dpb_mbs = 110400;
> +        break;
> +    case 51:
> +    case 52:
> +        max_dpb_mbs = 184320;
> +        break;
> +    default:
> +        max_dpb_mbs = -1;
> +        break;
> +    }
> +
> +    return max_dpb_mbs;
> +}
> +
>  static int vout_update_format( decoder_t *p_dec )
>  {
>      decoder_owner_sys_t *p_owner = p_dec->p_owner;
> @@ -462,8 +536,54 @@ static int vout_update_format( decoder_t *p_dec )
>          unsigned dpb_size;
>          switch( p_dec->fmt_in.i_codec )
>          {
> +        // TODO: optimize dpb_size as H264
>          case VLC_CODEC_HEVC:
> +            dpb_size = 18;
> +            break;
>          case VLC_CODEC_H264:
> +            {
> +#define H264_MAX_DPB_SIZE   18

This define looks *out-of-place*, either declare it as a `const` variable
within the function, or put the define somewhere else.

> +                dpb_size = H264_MAX_DPB_SIZE;
> +
> +                int profile = -1;
> +                int level = -1;
> +
> +                profile = p_dec->fmt_in.i_profile;
> +                level = p_dec->fmt_in.i_level;
> +                if( profile == -1 || level == -1 )
> +                {
> +                    if( h264_get_profile_level( &p_dec->fmt_in, &profile,
> &level, NULL ) == false )
> +                    {
> +                        msg_Warn( p_dec, "get profile and level failed" );
> +                        break;
> +                    }
> +                }
> +                msg_Dbg( p_dec, "H264 profile %d, level %d", profile,
> level );
> +
> +#define PROFILE_H264_MAIN                 77
> +#define PROFILE_H264_HIGH                 100
> +                if( profile != PROFILE_H264_MAIN && profile !=
> PROFILE_H264_HIGH )

These defines are only used once, and as with the previous one; they
look very *out-of-place*.

> +                    break;
> +
> +                int max_dpb_mbs = h264_get_max_dpb_mbs( level );
> +                if( max_dpb_mbs <= 0 )
> +                    break;
> +
> +                int pic_width_in_mbs = fmt.i_visible_width / 16;
> +                int frame_height_in_mbs = fmt.i_visible_height / 16;
> +                if( pic_width_in_mbs <= 0 || frame_height_in_mbs <= 0 )
> +                    break;
> +
> +                dpb_size = max_dpb_mbs / (pic_width_in_mbs *
> frame_height_in_mbs);

The above will suffer from a *division-by-zero* if
`fmt.i_visible_height` is less than `16`.

> +                if( dpb_size < 5 )
> +                    dpb_size = 5;
> +                else if( dpb_size > H264_MAX_DPB_SIZE )
> +                    dpb_size = H264_MAX_DPB_SIZE;
> +
> +                msg_Dbg( p_dec, "width %d, height %d, dpb_size %d",
> +                        fmt.i_visible_width, fmt.i_visible_height,

`video_format_t.i_visible_{width,height}` are `unsigned`, so the
format-string is wrong.

> dpb_size );
> +            }
> +            break;
>          case VLC_CODEC_DIRAC: /* FIXME valid ? */
>              dpb_size = 18;
>              break;
> -- 
> 2.7.4

Best Regards,\
Filip
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170519/e62cfd95/attachment.html>