[x265] [PATCH REVIEW] cli: add cli option max-tu-size and support for it

Steve Borho steve at borho.org
Mon Feb 9 18:51:34 CET 2015


On 02/09, santhoshini at multicorewareinc.com wrote:
> # HG changeset patch
> # User Santhoshini Sekar<santhoshini at multicorewareinc.com>
> # Date 1423470883 -19800
> #      Mon Feb 09 14:04:43 2015 +0530
> # Node ID 68e8c1d2c0a16d7569f356536a16b49f1cd990c3
> # Parent  b6f36b277234d7f402d67986dab969e7bc655646
> cli: add cli option max-tu-size and support for it
> 
> diff -r b6f36b277234 -r 68e8c1d2c0a1 source/common/common.h
> --- a/source/common/common.h	Fri Feb 06 21:57:45 2015 -0600
> +++ b/source/common/common.h	Mon Feb 09 14:04:43 2015 +0530
> @@ -252,6 +252,7 @@
>  #define MAX_LOG2_CU_SIZE        6                           // log2(maxCUSize)
>  #define MIN_CU_SIZE             (1 << MIN_LOG2_CU_SIZE)     // minimum allowable size of CU
>  #define MAX_CU_SIZE             (1 << MAX_LOG2_CU_SIZE)     // maximum allowable size of CU
> +#define MAX_LOG2_TU_SIZE        5                           // log2(maxCUSize)
>  
>  #define LOG2_UNIT_SIZE          2                           // log2(unitSize)
>  #define UNIT_SIZE               (1 << LOG2_UNIT_SIZE)       // unit size of CU partition
> diff -r b6f36b277234 -r 68e8c1d2c0a1 source/common/constants.cpp
> --- a/source/common/constants.cpp	Fri Feb 06 21:57:45 2015 -0600
> +++ b/source/common/constants.cpp	Mon Feb 09 14:04:43 2015 +0530
> @@ -125,6 +125,7 @@
>  uint32_t g_maxCUDepth    = NUM_CU_DEPTH - 1;
>  uint32_t g_zscanToRaster[MAX_NUM_PARTITIONS] = { 0, };
>  uint32_t g_rasterToZscan[MAX_NUM_PARTITIONS] = { 0, };
> +uint32_t g_maxLog2TUSize = MAX_LOG2_TU_SIZE;

I don't see the point of adding a global variable - these are
problematic since all encoders in the same process share these so they
would all need to use the same value.  The only variable we allow that
with today is the max CU size, only because it is used in so many
different places in so many different forms (max size, max depth, min
depth, etc). We have to check to make sure every encode in the same
process uses the same value so if you add a new global variable that is
set based on a user param, you must add this validation.

It would be much better if this global var didn't exist, and the code
used the param or SPS structure instead.

>  const uint8_t g_zscanToPelX[MAX_NUM_PARTITIONS] =
>  {
> diff -r b6f36b277234 -r 68e8c1d2c0a1 source/common/constants.h
> --- a/source/common/constants.h	Fri Feb 06 21:57:45 2015 -0600
> +++ b/source/common/constants.h	Mon Feb 09 14:04:43 2015 +0530
> @@ -56,6 +56,7 @@
>  extern uint32_t g_maxCUSize;
>  extern uint32_t g_maxCUDepth;
>  extern uint32_t g_maxFullDepth;
> +extern uint32_t g_maxLog2TUSize;
>  
>  extern const int16_t g_t4[4][4];
>  extern const int16_t g_t8[8][8];
> diff -r b6f36b277234 -r 68e8c1d2c0a1 source/common/param.cpp
> --- a/source/common/param.cpp	Fri Feb 06 21:57:45 2015 -0600
> +++ b/source/common/param.cpp	Mon Feb 09 14:04:43 2015 +0530
> @@ -129,6 +129,7 @@
>      param->maxCUSize = 64;
>      param->tuQTMaxInterDepth = 1;
>      param->tuQTMaxIntraDepth = 1;
> +    param->maxTUSize = 32;
>  
>      /* Coding Structure */
>      param->keyframeMin = 0;
> @@ -572,6 +573,7 @@
>      OPT("ctu") p->maxCUSize = (uint32_t)atoi(value);
>      OPT("tu-intra-depth") p->tuQTMaxIntraDepth = (uint32_t)atoi(value);
>      OPT("tu-inter-depth") p->tuQTMaxInterDepth = (uint32_t)atoi(value);
> +    OPT("max-tu-size") p->maxTUSize = (uint32_t)atoi(value);
>      OPT("subme") p->subpelRefine = atoi(value);
>      OPT("merange") p->searchRange = atoi(value);
>      OPT("rect") p->bEnableRectInter = atobool(value);
> @@ -1012,7 +1014,8 @@
>            "QuadtreeTUMaxDepthIntra must be greater 0 and less than 5");
>      CHECK(maxLog2CUSize < tuQTMinLog2Size + param->tuQTMaxIntraDepth - 1,
>            "QuadtreeTUMaxDepthInter must be less than or equal to the difference between log2(maxCUSize) and QuadtreeTULog2MinSize plus 1");
> -
> +    CHECK(param->maxTUSize != 32 && param->maxTUSize != 16 && param->maxTUSize != 8 && param->maxTUSize != 4,
> +          "max TU size must be 4, 8, 16, or 32");
>      CHECK(param->maxNumMergeCand < 1, "MaxNumMergeCand must be 1 or greater.");
>      CHECK(param->maxNumMergeCand > 5, "MaxNumMergeCand must be 5 or smaller.");
>  
> @@ -1166,10 +1169,12 @@
>      else
>      {
>          uint32_t maxLog2CUSize = (uint32_t)g_log2Size[param->maxCUSize];
> +        uint32_t maxLog2TUSize = (uint32_t)g_log2Size[param->maxTUSize];
>  
>          // set max CU width & height
>          g_maxCUSize     = param->maxCUSize;
>          g_maxLog2CUSize = maxLog2CUSize;
> +        g_maxLog2TUSize = maxLog2TUSize;
>  
>          // compute actual CU depth with respect to config depth and max transform size
>          g_maxCUDepth   = maxLog2CUSize - MIN_LOG2_CU_SIZE;
> @@ -1194,8 +1199,8 @@
>      if (param->interlaceMode)
>          x265_log(param, X265_LOG_INFO, "Interlaced field inputs             : %s\n", x265_interlace_names[param->interlaceMode]);
>  
> -    x265_log(param, X265_LOG_INFO, "CTU size / RQT depth inter / intra  : %d / %d / %d\n",
> -             param->maxCUSize, param->tuQTMaxInterDepth, param->tuQTMaxIntraDepth);
> +    x265_log(param, X265_LOG_INFO, "CTU size / TU size / RQT depth inter / intra  : %d / %d / %d / %d\n",
> +        param->maxCUSize, param->maxTUSize, param->tuQTMaxInterDepth, param->tuQTMaxIntraDepth);
>  
>      x265_log(param, X265_LOG_INFO, "ME / range / subpel / merge         : %s / %d / %d / %d\n",
>               x265_motion_est_names[param->searchMethod], param->searchRange, param->subpelRefine, param->maxNumMergeCand);
> @@ -1291,6 +1296,7 @@
>      s += sprintf(s, " bitdepth=%d", p->internalBitDepth);
>      BOOL(p->bEnableWavefront, "wpp");
>      s += sprintf(s, " ctu=%d", p->maxCUSize);
> +    s += sprintf(s, " max-tu-size=%d", p->maxTUSize);
>      s += sprintf(s, " tu-intra-depth=%d", p->tuQTMaxIntraDepth);
>      s += sprintf(s, " tu-inter-depth=%d", p->tuQTMaxInterDepth);
>      s += sprintf(s, " me=%d", p->searchMethod);
> diff -r b6f36b277234 -r 68e8c1d2c0a1 source/common/slice.h
> --- a/source/common/slice.h	Fri Feb 06 21:57:45 2015 -0600
> +++ b/source/common/slice.h	Mon Feb 09 14:04:43 2015 +0530
> @@ -226,6 +226,8 @@
>      uint32_t quadtreeTUMaxDepthInter; // use param
>      uint32_t quadtreeTUMaxDepthIntra; // use param
>  
> +    uint32_t quadtreeTUMaxSize;
> +
>      bool     bUseSAO; // use param
>      bool     bUseAMP; // use param
>      uint32_t maxAMPDepth;
> diff -r b6f36b277234 -r 68e8c1d2c0a1 source/encoder/encoder.cpp
> --- a/source/encoder/encoder.cpp	Fri Feb 06 21:57:45 2015 -0600
> +++ b/source/encoder/encoder.cpp	Mon Feb 09 14:04:43 2015 +0530
> @@ -1439,10 +1439,11 @@
>      sps->log2MinCodingBlockSize = g_maxLog2CUSize - g_maxCUDepth;
>      sps->log2DiffMaxMinCodingBlockSize = g_maxCUDepth;
>  
> -    sps->quadtreeTULog2MaxSize = X265_MIN(g_maxLog2CUSize, 5);
> +    sps->quadtreeTULog2MaxSize = X265_MIN(g_maxLog2CUSize, g_maxLog2TUSize);

This needs to be checked above, so the user can't specify a TU size
larger than the max CU size (and use m_param->maxTUSize here)

>      sps->quadtreeTULog2MinSize = 2;
>      sps->quadtreeTUMaxDepthInter = m_param->tuQTMaxInterDepth;
>      sps->quadtreeTUMaxDepthIntra = m_param->tuQTMaxIntraDepth;

The TU quad tree max depths also might need adjustments if max CU size
is 64 and max TU size is 16. I'm not sure what the rule is for these.

> +    sps->quadtreeTUMaxSize = m_param->maxTUSize;

Do we need quadtreeTULog2MaxSize and quadtreeTUMaxSize?

>      sps->bUseSAO = m_param->bEnableSAO;
>  
> diff -r b6f36b277234 -r 68e8c1d2c0a1 source/x265.h
> --- a/source/x265.h	Fri Feb 06 21:57:45 2015 -0600
> +++ b/source/x265.h	Mon Feb 09 14:04:43 2015 +0530
> @@ -507,6 +507,11 @@
>       * compressed by the DCT transforms, at the expense of much more compute */
>      uint32_t  tuQTMaxIntraDepth;
>  
> +    /* Maxiumum TU width and height in pixels.  The size must be 32, 16, 8 or 4.
> +     * The larger the size the more efficiently the residual can be
> +     * compressed by the DCT transforms, at the expense of huge computation */
> +    uint32_t  maxTUSize;
> +
>      /*== GOP Structure and Lokoahead ==*/
>  
>      /* Enable open GOP - meaning I slices are not necessariy IDR and thus frames
> diff -r b6f36b277234 -r 68e8c1d2c0a1 source/x265cli.h
> --- a/source/x265cli.h	Fri Feb 06 21:57:45 2015 -0600
> +++ b/source/x265cli.h	Mon Feb 09 14:04:43 2015 +0530
> @@ -71,6 +71,7 @@
>      { "no-wpp",               no_argument, NULL, 0 },
>      { "wpp",                  no_argument, NULL, 0 },
>      { "ctu",            required_argument, NULL, 's' },
> +    { "max-tu-size",    required_argument, NULL, 's' },
>      { "tu-intra-depth", required_argument, NULL, 0 },
>      { "tu-inter-depth", required_argument, NULL, 0 },
>      { "me",             required_argument, NULL, 0 },
> @@ -264,6 +265,7 @@
>      H0("                                 psnr, ssim, grain, zerolatency, fastdecode\n");
>      H0("\nQuad-Tree size and depth:\n");
>      H0("-s/--ctu <64|32|16>              Maximum CU size (WxH). Default %d\n", param->maxCUSize);
> +    H0("-s/--max-tu-size <32|16|8|4>     Maximum TU size (WxH). Default %d\n", param->maxTUSize);
>      H0("   --tu-intra-depth <integer>    Max TU recursive depth for intra CUs. Default %d\n", param->tuQTMaxIntraDepth);
>      H0("   --tu-inter-depth <integer>    Max TU recursive depth for inter CUs. Default %d\n", param->tuQTMaxInterDepth);
>      H0("\nAnalysis:\n");
> _______________________________________________
> x265-devel mailing list
> x265-devel at videolan.org
> https://mailman.videolan.org/listinfo/x265-devel

-- 
Steve Borho


More information about the x265-devel mailing list