[x265] [PATCH] dqp: add param for maxCuDQPDepth

[Steve Borho]

On 03/12, gopu at multicorewareinc.com wrote:
> # HG changeset patch
> # User Gopu Govindaswamy <gopu at multicorewareinc.com>
> # Date 1426140129 -19800
> #      Thu Mar 12 11:32:09 2015 +0530
> # Node ID 1105c6db84119001ae0ed30849b00d4aca17018a
> # Parent  b931c50d55011a1ddc08f0a230b9632fcb4674d7
> dqp: add param for maxCuDQPDepth
> 
> currently the maxCuDQPDepth is default set to 0, make mazCuDQPDepth as
> configurable

typo

> 
> diff -r b931c50d5501 -r 1105c6db8411 source/common/param.cpp
> --- a/source/common/param.cpp	Wed Mar 11 21:58:02 2015 -0500
> +++ b/source/common/param.cpp	Thu Mar 12 11:32:09 2015 +0530
> @@ -210,6 +210,7 @@
>      param->rc.zones = NULL;
>      param->rc.bEnableSlowFirstPass = 0;
>      param->rc.bStrictCbr = 0;
> +    param->rc.maxCuDQPDepth = 0;
>  
>      /* Video Usability Information (VUI) */
>      param->vui.aspectRatioIdc = 0;
> @@ -838,6 +839,7 @@
>      OPT2("pools", "numa-pools") p->numaPools = strdup(value);
>      OPT("lambda-file") p->rc.lambdaFileName = strdup(value);
>      OPT("analysis-file") p->analysisFileName = strdup(value);
> +    OPT("maxdqp-depth") p->rc.maxCuDQPDepth = atoi(value);

is the max necessary in the param name?  In other places we use max it
is max- something, so this either needs to be max-dqp-depth or just
dqp-depth.

>      else
>          return X265_PARAM_BAD_NAME;
>  #undef OPT
> diff -r b931c50d5501 -r 1105c6db8411 source/encoder/encoder.cpp
> --- a/source/encoder/encoder.cpp	Wed Mar 11 21:58:02 2015 -0500
> +++ b/source/encoder/encoder.cpp	Thu Mar 12 11:32:09 2015 +0530
> @@ -1548,12 +1548,12 @@
>      if (!m_param->bLossless && (m_param->rc.aqMode || bIsVbv))
>      {
>          pps->bUseDQP = true;
> -        pps->maxCuDQPDepth = 0; /* TODO: make configurable? */
> +        pps->maxCuDQPDepth = m_param->rc.maxCuDQPDepth;
>      }
>      else
>      {
>          pps->bUseDQP = false;
> -        pps->maxCuDQPDepth = 0;
> +        pps->maxCuDQPDepth = m_param->rc.maxCuDQPDepth;

I think the point of the if() expression is to set it zero in the else
clause. If you're forcing it to be zero in the logic below, you might as
well remove 'pps->maxCuDQPDepth =' from this expression and set it
unconditionally.  As far as I know, maxCuDQPDepth will not be signalled
if bUseDQP is false, thus the encoder will (should) ignore the param.

>      }
>  
>      pps->chromaQpOffset[0] = m_param->cbQpOffset;
> @@ -1773,6 +1773,17 @@
>          p->analysisMode = X265_ANALYSIS_OFF;
>          x265_log(p, X265_LOG_WARNING, "Analysis save and load mode not supported for distributed mode analysis\n");
>      }
> +    bool bIsVbv = m_param->rc.vbvBufferSize > 0 && m_param->rc.vbvMaxBitrate > 0;
> +    if (!m_param->bLossless && (m_param->rc.aqMode || bIsVbv))
> +    {
> +        if (p->rc.maxCuDQPDepth > (NUM_CU_DEPTH - 1))
> +        {
> +            p->rc.maxCuDQPDepth = 0;
> +            x265_log(p, X265_LOG_WARNING, "The maxCUDQPDepth should be less than maxCUDepth setting maxCUDQPDepth = %d \n", 0);
> +        }
> +    }
> +    else
> +        p->rc.maxCuDQPDepth = 0;
>  }
>  
>  void Encoder::allocAnalysis(x265_analysis_data* analysis)
> diff -r b931c50d5501 -r 1105c6db8411 source/x265.h
> --- a/source/x265.h	Wed Mar 11 21:58:02 2015 -0500
> +++ b/source/x265.h	Thu Mar 12 11:32:09 2015 +0530
> @@ -977,6 +977,9 @@
>          /* Enable stricter conditions to check bitrate deviations in CBR mode. May compromise 
>           * quality to maintain bitrate adherence */
>          int bStrictCbr;
> +
> +        /* Max depth of a minimum CuDQP for sub-LCU-level delta QP */
> +        int maxCuDQPDepth;
>      } rc;
>  
>      /*== Video Usability Information ==*/
> diff -r b931c50d5501 -r 1105c6db8411 source/x265cli.h
> --- a/source/x265cli.h	Wed Mar 11 21:58:02 2015 -0500
> +++ b/source/x265cli.h	Thu Mar 12 11:32:09 2015 +0530
> @@ -202,6 +202,7 @@
>      { "strict-cbr",           no_argument, NULL, 0 },
>      { "temporal-layers",      no_argument, NULL, 0 },
>      { "no-temporal-layers",   no_argument, NULL, 0 },
> +    { "maxdqp-depth",   required_argument, NULL, 0 },
>      { 0, 0, 0, 0 },
>      { 0, 0, 0, 0 },
>      { 0, 0, 0, 0 },

Needs restructured text documentation, including some analysis on what
happens when the param is changed from the default.

-- 
Steve Borho