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 > + 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. > + > + 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