[x265] [PATCH] tenccfg : move loop filter configuration params from tenccfg to x265_param

Steve Borho steve at borho.org
Mon Oct 28 17:58:21 CET 2013


On Mon, Oct 28, 2013 at 4:36 AM, Gopu Govindaswamy <
gopu at multicorewareinc.com> wrote:

> # HG changeset patch
> # User Gopu Govindaswamy <gopu at multicorewareinc.com>
> # Date 1382952951 -19800
> # Node ID 26475c9ced90e338399e9a93d886824aad55dc83
> # Parent  7916afc6c9c4835e0db6abe4a87f9b7c3579deec
> tenccfg : move loop filter configuration params from tenccfg to x265_param
>

no space before the colon


> diff -r 7916afc6c9c4 -r 26475c9ced90 source/Lib/TLibEncoder/TEncCfg.h
> --- a/source/Lib/TLibEncoder/TEncCfg.h  Mon Oct 28 11:59:47 2013 +0530
> +++ b/source/Lib/TLibEncoder/TEncCfg.h  Mon Oct 28 15:05:51 2013 +0530
> @@ -82,11 +82,6 @@
>      UInt      m_quadtreeTULog2MaxSize;
>      UInt      m_quadtreeTULog2MinSize;
>
> -    //====== Loop/Deblock Filter ========
> -    bool      m_loopFilterOffsetInPPS;
> -    int       m_loopFilterBetaOffsetDiv2;
> -    int       m_loopFilterTcOffsetDiv2;
> -    int       m_maxNumOffsetsPerPic;
>
>      //====== Lossless ========
>      bool      m_useLossless;
> @@ -183,11 +178,11 @@
>      UInt getQuadtreeTULog2MinSize() const { return
> m_quadtreeTULog2MinSize; }
>
>      //==== Loop/Deblock Filter ========
> -    bool getLoopFilterOffsetInPPS() { return m_loopFilterOffsetInPPS; }
> +//    bool getLoopFilterOffsetInPPS() { return m_loopFilterOffsetInPPS; }
>
> -    int getLoopFilterBetaOffset() { return m_loopFilterBetaOffsetDiv2; }
> +//    int getLoopFilterBetaOffset() { return m_loopFilterBetaOffsetDiv2; }
>
> -    int getLoopFilterTcOffset() { return m_loopFilterTcOffsetDiv2; }
> +//    int getLoopFilterTcOffset() { return m_loopFilterTcOffsetDiv2; }
>

we should never comment out lines, these should just be removed

     //==== Quality control ========
>      int getMaxCuDQPDepth() { return m_maxCuDQPDepth; }
> @@ -208,7 +203,7 @@
>
>      UInt getPCMLog2MinSize() { return m_pcmLog2MinSize; }
>
> -    int   getMaxNumOffsetsPerPic() { return m_maxNumOffsetsPerPic; }
> + //   int   getMaxNumOffsetsPerPic() { return m_maxNumOffsetsPerPic; }
>
>      bool  getLFCrossTileBoundaryFlag() { return
> m_loopFilterAcrossTilesEnabledFlag; }
>
> diff -r 7916afc6c9c4 -r 26475c9ced90 source/encoder/encoder.cpp
> --- a/source/encoder/encoder.cpp        Mon Oct 28 11:59:47 2013 +0530
> +++ b/source/encoder/encoder.cpp        Mon Oct 28 15:05:51 2013 +0530
> @@ -847,7 +847,7 @@
>      pps->setOutputFlagPresentFlag(false);
>      pps->setSignHideFlag(param.bEnableSignHiding);
>      pps->setDeblockingFilterControlPresentFlag(!param.bEnableLoopFilter);
> -    pps->setDeblockingFilterOverrideEnabledFlag(!m_loopFilterOffsetInPPS);
> +
>  pps->setDeblockingFilterOverrideEnabledFlag(!param.loopFilterOffsetInPPS);
>      pps->setPicDisableDeblockingFilterFlag(!param.bEnableLoopFilter);
>      pps->setLog2ParallelMergeLevelMinus2(m_log2ParallelMergeLevelMinus2);
>      pps->setCabacInitPresentFlag(param.frameNumThreads > 1 ? 0 :
> CABAC_INIT_PRESENT_FLAG);
> @@ -1057,9 +1057,9 @@
>
>      //====== Enforce these hard coded settings before initializeGOP() to
>      //       avoid a valgrind warning
> -    m_loopFilterOffsetInPPS = 0;
> -    m_loopFilterBetaOffsetDiv2 = 0;
> -    m_loopFilterTcOffsetDiv2 = 0;
> +    param.loopFilterOffsetInPPS = 0;
> +    param.loopFilterBetaOffsetDiv2 = 0;
> +    param.loopFilterTcOffsetDiv2 = 0;
>

 The fact that these fields are all hard-coded in the encoder configure
function means we do not allow them to be user-specified.  So adding them
to the public x265_param structure is fairly misleading.  We need a
different method for dealing with HM parameters that we do not allow to be
user-configurable.  Perhaps the values should just be hard-coded and the
variables removed.  But those decisions must be made on a case-by-case
basis after reviewing each of the fields involved.


>      m_loopFilterAcrossTilesEnabledFlag = 1;
>

This is a bad default, since we do not use tiles (I realize this is only
patch context, not a line you changed)


>      //====== HM Settings not exposed for configuration ======
> @@ -1077,7 +1077,7 @@
>
>      m_vps = vps;
>      m_maxCuDQPDepth = 0;
> -    m_maxNumOffsetsPerPic = 2048;
> +    param.maxNumOffsetsPerPic = 2048;
>      m_log2ParallelMergeLevelMinus2 = 0;
>
>      //========= set default display window
> ==================================
> diff -r 7916afc6c9c4 -r 26475c9ced90 source/encoder/frameencoder.cpp
> --- a/source/encoder/frameencoder.cpp   Mon Oct 28 11:59:47 2013 +0530
> +++ b/source/encoder/frameencoder.cpp   Mon Oct 28 15:05:51 2013 +0530
> @@ -268,16 +268,16 @@
>
>      if (slice->getPPS()->getDeblockingFilterControlPresentFlag())
>      {
> -
>  slice->getPPS()->setDeblockingFilterOverrideEnabledFlag(!m_cfg->getLoopFilterOffsetInPPS());
> -
>  slice->setDeblockingFilterOverrideFlag(!m_cfg->getLoopFilterOffsetInPPS());
> +
>  slice->getPPS()->setDeblockingFilterOverrideEnabledFlag(!m_cfg->param.loopFilterOffsetInPPS);
> +
>  slice->setDeblockingFilterOverrideFlag(!m_cfg->param.loopFilterOffsetInPPS);
>
>  slice->getPPS()->setPicDisableDeblockingFilterFlag(!m_cfg->param.bEnableLoopFilter);
>
>  slice->setDeblockingFilterDisable(!m_cfg->param.bEnableLoopFilter);
>          if (!slice->getDeblockingFilterDisable())
>          {
> -
>  slice->getPPS()->setDeblockingFilterBetaOffsetDiv2(m_cfg->getLoopFilterBetaOffset());
> -
>  slice->getPPS()->setDeblockingFilterTcOffsetDiv2(m_cfg->getLoopFilterTcOffset());
> -
>  slice->setDeblockingFilterBetaOffsetDiv2(m_cfg->getLoopFilterBetaOffset());
> -
>  slice->setDeblockingFilterTcOffsetDiv2(m_cfg->getLoopFilterTcOffset());
> +
>  slice->getPPS()->setDeblockingFilterBetaOffsetDiv2(m_cfg->param.loopFilterBetaOffsetDiv2);
> +
>  slice->getPPS()->setDeblockingFilterTcOffsetDiv2(m_cfg->param.loopFilterTcOffsetDiv2);
> +
>  slice->setDeblockingFilterBetaOffsetDiv2(m_cfg->param.loopFilterBetaOffsetDiv2);
> +
>  slice->setDeblockingFilterTcOffsetDiv2(m_cfg->param.loopFilterTcOffsetDiv2);
>          }
>      }
>      else
> diff -r 7916afc6c9c4 -r 26475c9ced90 source/encoder/framefilter.cpp
> --- a/source/encoder/framefilter.cpp    Mon Oct 28 11:59:47 2013 +0530
> +++ b/source/encoder/framefilter.cpp    Mon Oct 28 15:05:51 2013 +0530
> @@ -73,7 +73,7 @@
>      {
>          m_sao.setSaoLcuBoundary(top->param.saoLcuBoundary);
>
>  m_sao.setSaoLcuBasedOptimization(top->param.saoLcuBasedOptimization);
> -        m_sao.setMaxNumOffsetsPerPic(top->getMaxNumOffsetsPerPic());
> +        m_sao.setMaxNumOffsetsPerPic(top->param.maxNumOffsetsPerPic);
>          m_sao.create(top->param.sourceWidth, top->param.sourceHeight,
> g_maxCUWidth, g_maxCUHeight);
>          m_sao.createEncBuffer();
>      }
> diff -r 7916afc6c9c4 -r 26475c9ced90 source/x265.h
> --- a/source/x265.h     Mon Oct 28 11:59:47 2013 +0530
> +++ b/source/x265.h     Mon Oct 28 15:05:51 2013 +0530
> @@ -333,6 +333,13 @@
>          int       aqMode;                      ///< Adaptive QP (AQ)
>          double    aqStrength;
>      } rc;
> +
> +    //====== Loop/Deblock Filter ========
> +    bool      loopFilterOffsetInPPS;
>

1 - this header must be C only, so bool types cannot be used.
2 - parameters that have boolean semantics should start with a b prefix


> +    int       loopFilterBetaOffsetDiv2;
> +    int       loopFilterTcOffsetDiv2;
> +    int       maxNumOffsetsPerPic;
> +
>

we don't generally have trailing white-space at the end of structures


>  } x265_param;
>
>  /***
> _______________________________________________
> x265-devel mailing list
> x265-devel at videolan.org
> https://mailman.videolan.org/listinfo/x265-devel
>



-- 
Steve Borho
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/x265-devel/attachments/20131028/b5fbfb78/attachment.html>


More information about the x265-devel mailing list