<div dir="ltr"><div>Patch looks good overall. Minor suggestions to consider</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jan 10, 2020 at 5:22 PM Aruna Matheswaran <<a href="mailto:aruna@multicorewareinc.com">aruna@multicorewareinc.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"># HG changeset patch<br>
# User Aruna Matheswaran <<a href="mailto:aruna@multicorewareinc.com" target="_blank">aruna@multicorewareinc.com</a>><br>
# Date 1578656801 -19800<br>
# Fri Jan 10 17:16:41 2020 +0530<br>
# Node ID 5fdecb3d4af75a9c1a788b6af8577c3bf507697e<br>
# Parent a28fd843b302128cff31172fe140aac19f93ce80<br>
analysis-save/load: Validate conformance window offsets<br>
<br>
diff -r a28fd843b302 -r 5fdecb3d4af7 source/CMakeLists.txt<br>
--- a/source/CMakeLists.txt Fri Jan 10 17:01:08 2020 +0530<br>
+++ b/source/CMakeLists.txt Fri Jan 10 17:16:41 2020 +0530<br>
@@ -29,7 +29,7 @@<br>
option(STATIC_LINK_CRT "Statically link C runtime for release builds" OFF)<br>
mark_as_advanced(FPROFILE_USE FPROFILE_GENERATE NATIVE_BUILD)<br>
# X265_BUILD must be incremented each time the public API is changed<br>
-set(X265_BUILD 187)<br>
+set(X265_BUILD 188)<br>
configure_file("${PROJECT_SOURCE_DIR}/<a href="http://x265.def.in" rel="noreferrer" target="_blank">x265.def.in</a>"<br>
"${PROJECT_BINARY_DIR}/x265.def")<br>
configure_file("${PROJECT_SOURCE_DIR}/<a href="http://x265_config.h.in" rel="noreferrer" target="_blank">x265_config.h.in</a>"<br>
diff -r a28fd843b302 -r 5fdecb3d4af7 source/common/param.cpp<br>
--- a/source/common/param.cpp Fri Jan 10 17:01:08 2020 +0530<br>
+++ b/source/common/param.cpp Fri Jan 10 17:16:41 2020 +0530<br>
@@ -313,6 +313,10 @@<br>
param->log2MaxPocLsb = 8;<br>
param->maxSlices = 1;<br>
<br>
+ /*Conformance window*/<br>
+ param->confWinRightOffset = 0;<br>
+ param->confWinBottomOffset = 0;<br>
+<br>
param->bEmitVUITimingInfo = 1;<br>
param->bEmitVUIHRDInfo = 1;<br>
param->bOptQpPPS = 0;<br>
@@ -1783,6 +1787,8 @@<br>
param->bSingleSeiNal = 0;<br>
x265_log(param, X265_LOG_WARNING, "None of the SEI messages are enabled. Disabling Single SEI NAL\n");<br>
}<br>
+ CHECK(param->confWinRightOffset < 0, "Conformance Window Right Offset must be 0 or greater");<br>
+ CHECK(param->confWinBottomOffset < 0, "Conformance Window Bottom Offset must be 0 or greater");<br>
return check_failed;<br>
}<br>
<br>
@@ -2197,6 +2203,7 @@<br>
BOOL(p->bEnableSceneCutAwareQp, "scenecut-aware-qp");<br>
if (p->bEnableSceneCutAwareQp)<br>
s += sprintf(s, " scenecut-window=%d max-qp-delta=%d", p->scenecutWindow, p->maxQpDelta);<br>
+ s += sprintf(s, "conformance-window-offsets right=%d bottom=%d", p->confWinRightOffset, p->confWinBottomOffset);<br>
#undef BOOL<br>
return buf;<br>
}<br>
@@ -2547,6 +2554,8 @@<br>
dst->maxQpDelta = src->maxQpDelta;<br>
dst->bField = src->bField;<br>
<br>
+ dst->confWinRightOffset = src->confWinRightOffset;<br>
+ dst->confWinBottomOffset = src->confWinBottomOffset;<br>
#ifdef SVT_HEVC<br>
memcpy(dst->svtHevcParam, src->svtHevcParam, sizeof(EB_H265_ENC_CONFIGURATION));<br>
#endif<br>
diff -r a28fd843b302 -r 5fdecb3d4af7 source/encoder/api.cpp<br>
--- a/source/encoder/api.cpp Fri Jan 10 17:01:08 2020 +0530<br>
+++ b/source/encoder/api.cpp Fri Jan 10 17:16:41 2020 +0530<br>
@@ -176,6 +176,8 @@<br>
<br>
// may change params for auto-detect, etc<br>
encoder->configure(param);<br>
+ if (encoder->m_aborted)<br>
+ goto fail;<br>
// may change rate control and CPB params<br>
if (!enforceLevel(*param, encoder->m_vps))<br>
goto fail;<br>
diff -r a28fd843b302 -r 5fdecb3d4af7 source/encoder/encoder.cpp<br>
--- a/source/encoder/encoder.cpp Fri Jan 10 17:01:08 2020 +0530<br>
+++ b/source/encoder/encoder.cpp Fri Jan 10 17:16:41 2020 +0530<br>
@@ -81,6 +81,7 @@<br>
#define MVTHRESHOLD (10*10)<br>
#define PU_2Nx2N 1<br>
#define MAX_CHROMA_QP_OFFSET 12<br>
+#define CONF_OFFSET_BYTES (2 * sizeof(int))<br>
static const char* defaultAnalysisFileName = "x265_analysis.dat";<br>
<br>
using namespace X265_NS;<br>
@@ -474,15 +475,6 @@<br>
m_aborted = true;<br>
}<br>
}<br>
- if (m_param->analysisLoad && m_param->bUseAnalysisFile)<br>
- {<br>
- m_analysisFileIn = x265_fopen(m_param->analysisLoad, "rb");<br>
- if (!m_analysisFileIn)<br>
- {<br>
- x265_log_file(NULL, X265_LOG_ERROR, "Analysis load: failed to open file %s\n", m_param->analysisLoad);<br>
- m_aborted = true;<br>
- }<br>
- }<br>
<br>
if (m_param->analysisMultiPassRefine || m_param->analysisMultiPassDistortion)<br>
{<br>
@@ -1751,11 +1743,11 @@<br>
if (m_param->analysisLoad)<br>
{<br>
/* reads analysis data for the frame and allocates memory based on slicetype */<br>
- static int paramBytes = 0;<br>
+ static int paramBytes = CONF_OFFSET_BYTES;<br>
if (!inFrame->m_poc && m_param->bAnalysisType != HEVC_INFO)<br>
{<br>
- x265_analysis_data analysisData = inputPic->analysisData;<br>
- paramBytes = validateAnalysisData(&analysisData, 0);<br>
+ x265_analysis_validate saveParam = inputPic->analysisData.saveParam;<br>
+ paramBytes += validateAnalysisData(&saveParam, 0);<br>
if (paramBytes == -1)<br>
{<br>
m_aborted = true;<br>
@@ -3775,20 +3767,66 @@<br>
m_conformanceWindow.topOffset = 0;<br>
m_conformanceWindow.bottomOffset = 0;<br>
m_conformanceWindow.leftOffset = 0;<br>
+<br>
+ uint32_t padsize = 0;<br>
+ if (m_param->analysisLoad && m_param->bUseAnalysisFile)<br>
+ {<br></blockquote><div>[KS] Warn user on possible encoder crash if param's conformance window offset is set along with file-based analysis load</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ m_analysisFileIn = x265_fopen(m_param->analysisLoad, "rb");<br>
+ if (!m_analysisFileIn)<br>
+ {<br>
+ x265_log_file(NULL, X265_LOG_ERROR, "Analysis load: failed to open file %s\n", m_param->analysisLoad);<br>
+ m_aborted = true;<br>
+ }<br>
+ else<br>
+ {<br>
+ if (fread(&m_conformanceWindow.rightOffset, sizeof(int), 1, m_analysisFileIn) != 1)<br>
+ {<br>
+ x265_log(NULL, X265_LOG_ERROR, "Error reading analysis data. Conformance window right offset missing\n");<br>
+ m_aborted = true;<br>
+ }<br>
+ else if (m_conformanceWindow.rightOffset)<br>
+ {<br>
+ padsize = m_conformanceWindow.rightOffset * 2;<br>
+ p->sourceWidth += padsize;<br>
+ m_conformanceWindow.bEnabled = true;<br>
+ m_conformanceWindow.rightOffset = padsize;<br>
+ }<br>
+<br>
+ if (fread(&m_conformanceWindow.bottomOffset, sizeof(int), 1, m_analysisFileIn) != 1)<br>
+ {<br>
+ x265_log(NULL, X265_LOG_ERROR, "Error reading analysis data. Conformance window bottom offset missing\n");<br>
+ m_aborted = true;<br>
+ }<br>
+ else if (m_conformanceWindow.bottomOffset)<br>
+ {<br>
+ padsize = m_conformanceWindow.bottomOffset * 2;<br>
+ p->sourceHeight += padsize;<br>
+ m_conformanceWindow.bEnabled = true;<br>
+ m_conformanceWindow.bottomOffset = padsize;<br>
+ }<br>
+ }<br>
+ }<br>
+<br>
/* set pad size if width is not multiple of the minimum CU size */<br>
- if (p->scaleFactor == 2 && ((p->sourceWidth / 2) & (p->minCUSize - 1)) && p->analysisLoad)<br>
- {<br>
- uint32_t rem = (p->sourceWidth / 2) & (p->minCUSize - 1);<br>
- uint32_t padsize = p->minCUSize - rem;<br>
- p->sourceWidth += padsize * 2;<br>
-<br>
- m_conformanceWindow.bEnabled = true;<br>
- m_conformanceWindow.rightOffset = padsize * 2;<br>
- }<br>
- else if(p->sourceWidth & (p->minCUSize - 1))<br>
+ if (p->confWinRightOffset)<br>
+ {<br>
+ if ((p->sourceWidth + p->confWinRightOffset) & (p->minCUSize - 1))<br>
+ {<br>
+ x265_log(p, X265_LOG_ERROR, "Incompatible conformance window right offset."<br>
+ " This when added to the source width should be a multiple of minCUSize\n");<br>
+ m_aborted = true;<br>
+ }<br>
+ else<br>
+ {<br>
+ p->sourceWidth += p->confWinRightOffset;<br>
+ m_conformanceWindow.bEnabled = true;<br>
+ m_conformanceWindow.rightOffset = p->confWinRightOffset;<br>
+ }<br>
+ }<br>
+ else if (p->sourceWidth & (p->minCUSize - 1))<br>
{<br>
uint32_t rem = p->sourceWidth & (p->minCUSize - 1);<br>
- uint32_t padsize = p->minCUSize - rem;<br>
+ padsize = p->minCUSize - rem;<br>
p->sourceWidth += padsize;<br>
<br>
m_conformanceWindow.bEnabled = true;<br>
@@ -3989,18 +4027,25 @@<br>
}<br>
}<br>
/* set pad size if height is not multiple of the minimum CU size */<br>
- if (p->scaleFactor == 2 && ((p->sourceHeight / 2) & (p->minCUSize - 1)) && p->analysisLoad)<br>
- {<br>
- uint32_t rem = (p->sourceHeight / 2) & (p->minCUSize - 1);<br>
- uint32_t padsize = p->minCUSize - rem;<br>
- p->sourceHeight += padsize * 2;<br>
- m_conformanceWindow.bEnabled = true;<br>
- m_conformanceWindow.bottomOffset = padsize * 2;<br>
+ if (p->confWinBottomOffset)<br>
+ {<br>
+ if ((p->sourceHeight + p->confWinBottomOffset) & (p->minCUSize - 1))<br>
+ {<br>
+ x265_log(p, X265_LOG_ERROR, "Incompatible conformance window bottom offset."<br>
+ " This when added to the source height should be a multiple of minCUSize\n");<br>
+ m_aborted = true;<br>
+ }<br>
+ else<br>
+ {<br>
+ p->sourceHeight += p->confWinBottomOffset;<br>
+ m_conformanceWindow.bEnabled = true;<br>
+ m_conformanceWindow.bottomOffset = p->confWinBottomOffset;<br>
+ }<br>
}<br>
else if(p->sourceHeight & (p->minCUSize - 1))<br>
{<br>
uint32_t rem = p->sourceHeight & (p->minCUSize - 1);<br>
- uint32_t padsize = p->minCUSize - rem;<br>
+ padsize = p->minCUSize - rem;<br>
p->sourceHeight += padsize;<br>
m_conformanceWindow.bEnabled = true;<br>
m_conformanceWindow.bottomOffset = padsize;<br>
@@ -4835,7 +4880,7 @@<br>
}<br>
<br>
<br>
-int Encoder::validateAnalysisData(x265_analysis_data* analysis, int writeFlag)<br>
+int Encoder::validateAnalysisData(x265_analysis_validate* saveParam, int writeFlag)<br>
{<br>
#define X265_PARAM_VALIDATE(analysisParam, size, bytes, param, errorMsg)\<br>
if(!writeFlag)\<br>
@@ -4876,11 +4921,16 @@<br>
}\<br>
count++;<br>
<br>
- x265_analysis_validate *saveParam = &analysis->saveParam;<br>
FILE* fileOffset = NULL;<br>
int readValue = 0;<br>
int count = 0;<br>
<br>
+ if (m_param->bUseAnalysisFile && writeFlag)<br>
+ {<br>
+ X265_PARAM_VALIDATE(saveParam->rightOffset, sizeof(int), 1, &m_conformanceWindow.rightOffset, right-offset);<br>
+ X265_PARAM_VALIDATE(saveParam->bottomOffset, sizeof(int), 1, &m_conformanceWindow.bottomOffset, bottom-offset);<br>
+ }<br>
+<br>
X265_PARAM_VALIDATE(saveParam->intraRefresh, sizeof(int), 1, &m_param->bIntraRefresh, intra-refresh);<br>
X265_PARAM_VALIDATE(saveParam->maxNumReferences, sizeof(int), 1, &m_param->maxNumReferences, ref);<br>
X265_PARAM_VALIDATE(saveParam->keyframeMax, sizeof(int), 1, &m_param->keyframeMax, keyint);<br>
@@ -5235,7 +5285,7 @@<br>
<br>
if (!analysis->poc)<br>
{<br>
- if (validateAnalysisData(analysis, 1) == -1)<br>
+ if (validateAnalysisData(&analysis->saveParam, 1) == -1)<br>
{<br>
m_aborted = true;<br>
return;<br>
diff -r a28fd843b302 -r 5fdecb3d4af7 source/encoder/encoder.h<br>
--- a/source/encoder/encoder.h Fri Jan 10 17:01:08 2020 +0530<br>
+++ b/source/encoder/encoder.h Fri Jan 10 17:16:41 2020 +0530<br>
@@ -358,7 +358,7 @@<br>
<br>
void finishFrameStats(Frame* pic, FrameEncoder *curEncoder, x265_frame_stats* frameStats, int inPoc);<br>
<br>
- int validateAnalysisData(x265_analysis_data* analysis, int readWriteFlag);<br>
+ int validateAnalysisData(x265_analysis_validate* param, int readWriteFlag);<br>
<br>
void readUserSeiFile(x265_sei_payload& seiMsg, int poc);<br>
<br>
diff -r a28fd843b302 -r 5fdecb3d4af7 source/x265.h<br>
--- a/source/x265.h Fri Jan 10 17:01:08 2020 +0530<br>
+++ b/source/x265.h Fri Jan 10 17:16:41 2020 +0530<br>
@@ -132,6 +132,8 @@<br>
int chunkEnd;<br>
int cuTree;<br>
int ctuDistortionRefine;<br>
+ int rightOffset;<br>
+ int bottomOffset;<br>
}x265_analysis_validate;<br>
<br>
/* Stores intra analysis data for a single frame. This struct needs better packing */<br>
@@ -1877,6 +1879,28 @@<br>
* analysis information reused in analysis-load. Higher the refine level higher<br>
* the information reused. Default is 5 */<br>
int analysisLoadReuseLevel;<br>
+<br>
+ /* Conformance window right offset specifies the padding offset to the<br>
+ * right side of the internal copy of the input pictures in the library.<br>
+ * The decoded picture will be cropped based on conformance window right offset<br>
+ * signaled in the SPS before output. Default is 0.<br>
+ * Recommended to set this during non-file based analysis-load to inform<br>
+ * it about the conformace window right offset to be added to match the <br>
+ * number of CUs across the width for which analysis info is available<br>
+ * from the corresponding analysis-save. */<br>
+<br>
+ int confWinRightOffset;<br>
+<br>
+ /* Conformance window bottom offset specifies the padding offset to the<br>
+ * bottom side of the internal copy of the input pictures in the library.<br>
+ * The decoded picture will be cropped based on conformance window bottom offset<br>
+ * signaled in the SPS before output. Default is 0. <br>
+ * Recommended to set this during non-file based analysis-load to inform<br>
+ * it about the conformace window bottom offset to be added to match the<br>
+ * number of CUs across the height for which analysis info is available<br>
+ * from the corresponding analysis-save. */<br></blockquote><div>[KS] Shorter lines will be more readable. </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+ int confWinBottomOffset;<br>
} x265_param;<br>
<br>
/* x265_param_alloc:<br>
_______________________________________________<br>
x265-devel mailing list<br>
<a href="mailto:x265-devel@videolan.org" target="_blank">x265-devel@videolan.org</a><br>
<a href="https://mailman.videolan.org/listinfo/x265-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/x265-devel</a><br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><span style="color:rgb(0,0,0)">Regards,<br>Kavitha</span></div></div></div></div></div>