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