On Mon, Jan 13, 2020 at 1:02 PM Kavitha Sampath <
kavi...@multicorewareinc.com> wrote:

> Patch looks good overall. Minor suggestions to consider
>
> On Fri, Jan 10, 2020 at 5:22 PM Aruna Matheswaran <
> ar...@multicorewareinc.com> wrote:
>
>> # HG changeset patch
>> # User Aruna Matheswaran <ar...@multicorewareinc.com>
>> # Date 1578656801 -19800
>> #      Fri Jan 10 17:16:41 2020 +0530
>> # Node ID 5fdecb3d4af75a9c1a788b6af8577c3bf507697e
>> # Parent  a28fd843b302128cff31172fe140aac19f93ce80
>> analysis-save/load: Validate conformance window offsets
>>
>> diff -r a28fd843b302 -r 5fdecb3d4af7 source/CMakeLists.txt
>> --- a/source/CMakeLists.txt     Fri Jan 10 17:01:08 2020 +0530
>> +++ b/source/CMakeLists.txt     Fri Jan 10 17:16:41 2020 +0530
>> @@ -29,7 +29,7 @@
>>  option(STATIC_LINK_CRT "Statically link C runtime for release builds"
>> OFF)
>>  mark_as_advanced(FPROFILE_USE FPROFILE_GENERATE NATIVE_BUILD)
>>  # X265_BUILD must be incremented each time the public API is changed
>> -set(X265_BUILD 187)
>> +set(X265_BUILD 188)
>>  configure_file("${PROJECT_SOURCE_DIR}/x265.def.in"
>>                 "${PROJECT_BINARY_DIR}/x265.def")
>>  configure_file("${PROJECT_SOURCE_DIR}/x265_config.h.in"
>> diff -r a28fd843b302 -r 5fdecb3d4af7 source/common/param.cpp
>> --- a/source/common/param.cpp   Fri Jan 10 17:01:08 2020 +0530
>> +++ b/source/common/param.cpp   Fri Jan 10 17:16:41 2020 +0530
>> @@ -313,6 +313,10 @@
>>      param->log2MaxPocLsb = 8;
>>      param->maxSlices = 1;
>>
>> +    /*Conformance window*/
>> +    param->confWinRightOffset = 0;
>> +    param->confWinBottomOffset = 0;
>> +
>>      param->bEmitVUITimingInfo   = 1;
>>      param->bEmitVUIHRDInfo      = 1;
>>      param->bOptQpPPS            = 0;
>> @@ -1783,6 +1787,8 @@
>>          param->bSingleSeiNal = 0;
>>          x265_log(param, X265_LOG_WARNING, "None of the SEI messages are
>> enabled. Disabling Single SEI NAL\n");
>>      }
>> +    CHECK(param->confWinRightOffset < 0, "Conformance Window Right
>> Offset must be 0 or greater");
>> +    CHECK(param->confWinBottomOffset < 0, "Conformance Window Bottom
>> Offset must be 0 or greater");
>>      return check_failed;
>>  }
>>
>> @@ -2197,6 +2203,7 @@
>>      BOOL(p->bEnableSceneCutAwareQp, "scenecut-aware-qp");
>>      if (p->bEnableSceneCutAwareQp)
>>          s += sprintf(s, " scenecut-window=%d max-qp-delta=%d",
>> p->scenecutWindow, p->maxQpDelta);
>> +    s += sprintf(s, "conformance-window-offsets right=%d bottom=%d",
>> p->confWinRightOffset, p->confWinBottomOffset);
>>  #undef BOOL
>>      return buf;
>>  }
>> @@ -2547,6 +2554,8 @@
>>      dst->maxQpDelta = src->maxQpDelta;
>>      dst->bField = src->bField;
>>
>> +    dst->confWinRightOffset = src->confWinRightOffset;
>> +    dst->confWinBottomOffset = src->confWinBottomOffset;
>>  #ifdef SVT_HEVC
>>      memcpy(dst->svtHevcParam, src->svtHevcParam,
>> sizeof(EB_H265_ENC_CONFIGURATION));
>>  #endif
>> diff -r a28fd843b302 -r 5fdecb3d4af7 source/encoder/api.cpp
>> --- a/source/encoder/api.cpp    Fri Jan 10 17:01:08 2020 +0530
>> +++ b/source/encoder/api.cpp    Fri Jan 10 17:16:41 2020 +0530
>> @@ -176,6 +176,8 @@
>>
>>      // may change params for auto-detect, etc
>>      encoder->configure(param);
>> +    if (encoder->m_aborted)
>> +        goto fail;
>>      // may change rate control and CPB params
>>      if (!enforceLevel(*param, encoder->m_vps))
>>          goto fail;
>> diff -r a28fd843b302 -r 5fdecb3d4af7 source/encoder/encoder.cpp
>> --- a/source/encoder/encoder.cpp        Fri Jan 10 17:01:08 2020 +0530
>> +++ b/source/encoder/encoder.cpp        Fri Jan 10 17:16:41 2020 +0530
>> @@ -81,6 +81,7 @@
>>  #define MVTHRESHOLD (10*10)
>>  #define PU_2Nx2N 1
>>  #define MAX_CHROMA_QP_OFFSET 12
>> +#define CONF_OFFSET_BYTES (2 * sizeof(int))
>>  static const char* defaultAnalysisFileName = "x265_analysis.dat";
>>
>>  using namespace X265_NS;
>> @@ -474,15 +475,6 @@
>>              m_aborted = true;
>>          }
>>      }
>> -    if (m_param->analysisLoad && m_param->bUseAnalysisFile)
>> -    {
>> -        m_analysisFileIn = x265_fopen(m_param->analysisLoad, "rb");
>> -        if (!m_analysisFileIn)
>> -        {
>> -            x265_log_file(NULL, X265_LOG_ERROR, "Analysis load: failed
>> to open file %s\n", m_param->analysisLoad);
>> -            m_aborted = true;
>> -        }
>> -    }
>>
>>      if (m_param->analysisMultiPassRefine ||
>> m_param->analysisMultiPassDistortion)
>>      {
>> @@ -1751,11 +1743,11 @@
>>          if (m_param->analysisLoad)
>>          {
>>              /* reads analysis data for the frame and allocates memory
>> based on slicetype */
>> -            static int paramBytes = 0;
>> +            static int paramBytes = CONF_OFFSET_BYTES;
>>              if (!inFrame->m_poc && m_param->bAnalysisType != HEVC_INFO)
>>              {
>> -                x265_analysis_data analysisData = inputPic->analysisData;
>> -                paramBytes = validateAnalysisData(&analysisData, 0);
>> +                x265_analysis_validate saveParam =
>> inputPic->analysisData.saveParam;
>> +                paramBytes += validateAnalysisData(&saveParam, 0);
>>                  if (paramBytes == -1)
>>                  {
>>                      m_aborted = true;
>> @@ -3775,20 +3767,66 @@
>>      m_conformanceWindow.topOffset = 0;
>>      m_conformanceWindow.bottomOffset = 0;
>>      m_conformanceWindow.leftOffset = 0;
>> +
>> +    uint32_t padsize = 0;
>> +    if (m_param->analysisLoad && m_param->bUseAnalysisFile)
>> +    {
>>
> [KS] Warn user on possible encoder crash if param's conformance window
> offset is set along with file-based analysis load
>
[AM] Okay, will add the warning.

> +        m_analysisFileIn = x265_fopen(m_param->analysisLoad, "rb");
>> +        if (!m_analysisFileIn)
>> +        {
>> +            x265_log_file(NULL, X265_LOG_ERROR, "Analysis load: failed
>> to open file %s\n", m_param->analysisLoad);
>> +            m_aborted = true;
>> +        }
>> +        else
>> +        {
>> +            if (fread(&m_conformanceWindow.rightOffset, sizeof(int), 1,
>> m_analysisFileIn) != 1)
>> +            {
>> +                x265_log(NULL, X265_LOG_ERROR, "Error reading analysis
>> data. Conformance window right offset missing\n");
>> +                m_aborted = true;
>> +            }
>> +            else if (m_conformanceWindow.rightOffset)
>> +            {
>> +                padsize = m_conformanceWindow.rightOffset * 2;
>> +                p->sourceWidth += padsize;
>> +                m_conformanceWindow.bEnabled = true;
>> +                m_conformanceWindow.rightOffset = padsize;
>> +            }
>> +
>> +            if (fread(&m_conformanceWindow.bottomOffset, sizeof(int), 1,
>> m_analysisFileIn) != 1)
>> +            {
>> +                x265_log(NULL, X265_LOG_ERROR, "Error reading analysis
>> data. Conformance window bottom offset missing\n");
>> +                m_aborted = true;
>> +            }
>> +            else if (m_conformanceWindow.bottomOffset)
>> +            {
>> +                padsize = m_conformanceWindow.bottomOffset * 2;
>> +                p->sourceHeight += padsize;
>> +                m_conformanceWindow.bEnabled = true;
>> +                m_conformanceWindow.bottomOffset = padsize;
>> +            }
>> +        }
>> +    }
>> +
>>      /* set pad size if width is not multiple of the minimum CU size */
>> -    if (p->scaleFactor == 2 && ((p->sourceWidth / 2) & (p->minCUSize -
>> 1)) && p->analysisLoad)
>> -    {
>> -        uint32_t rem = (p->sourceWidth / 2) & (p->minCUSize - 1);
>> -        uint32_t padsize = p->minCUSize - rem;
>> -        p->sourceWidth += padsize * 2;
>> -
>> -        m_conformanceWindow.bEnabled = true;
>> -        m_conformanceWindow.rightOffset = padsize * 2;
>> -    }
>> -    else if(p->sourceWidth & (p->minCUSize - 1))
>> +    if (p->confWinRightOffset)
>> +    {
>> +        if ((p->sourceWidth + p->confWinRightOffset) & (p->minCUSize -
>> 1))
>> +        {
>> +            x265_log(p, X265_LOG_ERROR, "Incompatible conformance window
>> right offset."
>> +                                          " This when added to the
>> source width should be a multiple of minCUSize\n");
>> +            m_aborted = true;
>> +        }
>> +        else
>> +        {
>> +            p->sourceWidth += p->confWinRightOffset;
>> +            m_conformanceWindow.bEnabled = true;
>> +            m_conformanceWindow.rightOffset = p->confWinRightOffset;
>> +        }
>> +    }
>> +    else if (p->sourceWidth & (p->minCUSize - 1))
>>      {
>>          uint32_t rem = p->sourceWidth & (p->minCUSize - 1);
>> -        uint32_t padsize = p->minCUSize - rem;
>> +        padsize = p->minCUSize - rem;
>>          p->sourceWidth += padsize;
>>
>>          m_conformanceWindow.bEnabled = true;
>> @@ -3989,18 +4027,25 @@
>>          }
>>      }
>>      /* set pad size if height is not multiple of the minimum CU size */
>> -    if (p->scaleFactor == 2 && ((p->sourceHeight / 2) & (p->minCUSize -
>> 1)) && p->analysisLoad)
>> -    {
>> -        uint32_t rem = (p->sourceHeight / 2) & (p->minCUSize - 1);
>> -        uint32_t padsize = p->minCUSize - rem;
>> -        p->sourceHeight += padsize * 2;
>> -        m_conformanceWindow.bEnabled = true;
>> -        m_conformanceWindow.bottomOffset = padsize * 2;
>> +    if (p->confWinBottomOffset)
>> +    {
>> +        if ((p->sourceHeight + p->confWinBottomOffset) & (p->minCUSize -
>> 1))
>> +        {
>> +            x265_log(p, X265_LOG_ERROR, "Incompatible conformance window
>> bottom offset."
>> +                " This when added to the source height should be a
>> multiple of minCUSize\n");
>> +            m_aborted = true;
>> +        }
>> +        else
>> +        {
>> +            p->sourceHeight += p->confWinBottomOffset;
>> +            m_conformanceWindow.bEnabled = true;
>> +            m_conformanceWindow.bottomOffset = p->confWinBottomOffset;
>> +        }
>>      }
>>      else if(p->sourceHeight & (p->minCUSize - 1))
>>      {
>>          uint32_t rem = p->sourceHeight & (p->minCUSize - 1);
>> -        uint32_t padsize = p->minCUSize - rem;
>> +        padsize = p->minCUSize - rem;
>>          p->sourceHeight += padsize;
>>          m_conformanceWindow.bEnabled = true;
>>          m_conformanceWindow.bottomOffset = padsize;
>> @@ -4835,7 +4880,7 @@
>>  }
>>
>>
>> -int Encoder::validateAnalysisData(x265_analysis_data* analysis, int
>> writeFlag)
>> +int Encoder::validateAnalysisData(x265_analysis_validate* saveParam, int
>> writeFlag)
>>  {
>>  #define X265_PARAM_VALIDATE(analysisParam, size, bytes, param, errorMsg)\
>>      if(!writeFlag)\
>> @@ -4876,11 +4921,16 @@
>>      }\
>>      count++;
>>
>> -    x265_analysis_validate *saveParam = &analysis->saveParam;
>>      FILE*     fileOffset = NULL;
>>      int       readValue = 0;
>>      int       count = 0;
>>
>> +    if (m_param->bUseAnalysisFile && writeFlag)
>> +    {
>> +        X265_PARAM_VALIDATE(saveParam->rightOffset, sizeof(int), 1,
>> &m_conformanceWindow.rightOffset, right-offset);
>> +        X265_PARAM_VALIDATE(saveParam->bottomOffset, sizeof(int), 1,
>> &m_conformanceWindow.bottomOffset, bottom-offset);
>> +    }
>> +
>>      X265_PARAM_VALIDATE(saveParam->intraRefresh, sizeof(int), 1,
>> &m_param->bIntraRefresh, intra-refresh);
>>      X265_PARAM_VALIDATE(saveParam->maxNumReferences, sizeof(int), 1,
>> &m_param->maxNumReferences, ref);
>>      X265_PARAM_VALIDATE(saveParam->keyframeMax, sizeof(int), 1,
>> &m_param->keyframeMax, keyint);
>> @@ -5235,7 +5285,7 @@
>>
>>      if (!analysis->poc)
>>      {
>> -        if (validateAnalysisData(analysis, 1) == -1)
>> +        if (validateAnalysisData(&analysis->saveParam, 1) == -1)
>>          {
>>              m_aborted = true;
>>              return;
>> diff -r a28fd843b302 -r 5fdecb3d4af7 source/encoder/encoder.h
>> --- a/source/encoder/encoder.h  Fri Jan 10 17:01:08 2020 +0530
>> +++ b/source/encoder/encoder.h  Fri Jan 10 17:16:41 2020 +0530
>> @@ -358,7 +358,7 @@
>>
>>      void finishFrameStats(Frame* pic, FrameEncoder *curEncoder,
>> x265_frame_stats* frameStats, int inPoc);
>>
>> -    int validateAnalysisData(x265_analysis_data* analysis, int
>> readWriteFlag);
>> +    int validateAnalysisData(x265_analysis_validate* param, int
>> readWriteFlag);
>>
>>      void readUserSeiFile(x265_sei_payload& seiMsg, int poc);
>>
>> diff -r a28fd843b302 -r 5fdecb3d4af7 source/x265.h
>> --- a/source/x265.h     Fri Jan 10 17:01:08 2020 +0530
>> +++ b/source/x265.h     Fri Jan 10 17:16:41 2020 +0530
>> @@ -132,6 +132,8 @@
>>      int     chunkEnd;
>>      int     cuTree;
>>      int     ctuDistortionRefine;
>> +    int     rightOffset;
>> +    int     bottomOffset;
>>  }x265_analysis_validate;
>>
>>  /* Stores intra analysis data for a single frame. This struct needs
>> better packing */
>> @@ -1877,6 +1879,28 @@
>>      * analysis information reused in analysis-load. Higher the refine
>> level higher
>>      * the information reused. Default is 5 */
>>      int       analysisLoadReuseLevel;
>> +
>> +    /* Conformance window right offset specifies the padding offset to
>> the
>> +    * right side of the internal copy of the input pictures in the
>> library.
>> +    * The decoded picture will be cropped based on conformance window
>> right offset
>> +    * signaled in the SPS before output. Default is 0.
>> +    * Recommended to set this during non-file based analysis-load to
>> inform
>> +    * it about the conformace window right offset to be added to match
>> the
>> +    * number of CUs across the width for which analysis info is available
>> +    * from the corresponding analysis-save. */
>> +
>> +    int       confWinRightOffset;
>> +
>> +    /* Conformance window bottom offset specifies the padding offset to
>> the
>> +    * bottom side of the internal copy of the input pictures in the
>> library.
>> +    * The decoded picture will be cropped based on conformance window
>> bottom offset
>> +    * signaled in the SPS before output. Default is 0.
>> +    * Recommended to set this during non-file based analysis-load to
>> inform
>> +    * it about the conformace window bottom offset to be added to match
>> the
>> +    * number of CUs across the height for which analysis info is
>> available
>> +    * from the corresponding analysis-save. */
>>
> [KS] Shorter lines will be more readable.
>
[AM] Okay, will re-phrase the comment.

> +
>> +    int      confWinBottomOffset;
>>  } x265_param;
>>
>>  /* x265_param_alloc:
>> _______________________________________________
>> x265-devel mailing list
>> x265-devel@videolan.org
>> https://mailman.videolan.org/listinfo/x265-devel
>>
>
>
> --
> Regards,
> Kavitha
> _______________________________________________
> x265-devel mailing list
> x265-devel@videolan.org
> https://mailman.videolan.org/listinfo/x265-devel
>


-- 
Regards,
*Aruna Matheswaran,*
Video Codec Engineer,
Media & AI analytics BU,
_______________________________________________
x265-devel mailing list
x265-devel@videolan.org
https://mailman.videolan.org/listinfo/x265-devel

Reply via email to