<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jun 9, 2020 at 8:35 PM Praveen Kumar Karadugattu <<a href="mailto:praveenkumar@multicorewareinc.com">praveenkumar@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"><div dir="ltr"><div dir="ltr"><div>From 0bc864dbc48624902e5a8314d9ec49ce19a84146 Mon Sep 17 00:00:00 2001<br>From: Praveen Kumar Karadugattu <<a href="mailto:praveenkumar@multicorewareinc.com" target="_blank">praveenkumar@multicorewareinc.com</a>><br>Date: Tue, 9 Jun 2020 20:27:48 +0530<br>Subject: [PATCH] Fixed the --hist-scenecut feature to consider the max<br> variation in chroma histograms along with luma edge histograms. Also fixed<br> the formula for normalizing both the SAD values from 0.0 to 1.0. This would<br> alleviate the false positive scene-cuts observed with this feature.</div></div></div></blockquote><div><br></div><div> [KS] re phrase with the following syntax</div><div> 1. Fix: Issue description followed by algorithm changes and optimizations. </div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div>---<br> doc/reST/cli.rst | 8 ++---<br> source/common/param.cpp | 4 +--<br> source/encoder/encoder.cpp | 75 ++++++++++++++++++++++------------------------<br> source/encoder/encoder.h | 6 ++--<br> source/x265cli.cpp | 4 +--<br> 5 files changed, 46 insertions(+), 51 deletions(-)<br> mode change 100644 => 100755 doc/reST/cli.rst<br> mode change 100644 => 100755 source/common/param.cpp<br> mode change 100644 => 100755 source/encoder/encoder.cpp<br> mode change 100644 => 100755 source/encoder/encoder.h<br> mode change 100644 => 100755 source/x265cli.cpp</div><div>diff --git a/doc/reST/cli.rst b/doc/reST/cli.rst<br>old mode 100644<br>new mode 100755<br>index eceec40..c9e288e<br>--- a/doc/reST/cli.rst<br>+++ b/doc/reST/cli.rst<br>@@ -1462,13 +1462,13 @@ Slice decision options<br> .. option:: --hist-scenecut, --no-hist-scenecut<br> <br> Indicates that scenecuts need to be detected using luma edge and chroma histograms.<br>- :option: `--hist-scenecut` enables scenecut detection using the histograms and disables the default scene cut algorithm.<br>- :option: `--no-hist-scenecut` disables histogram based scenecut algorithm.<br>+ :option:`--hist-scenecut` enables scenecut detection using the histograms and disables the default scene cut algorithm.<br>+ :option:`--no-hist-scenecut` disables histogram based scenecut algorithm.<br> <br>-.. option:: --hist-threshold <0.0..2.0><br>+.. option:: --hist-threshold <0.0..1.0><br> <br> This value represents the threshold for normalized SAD of edge histograms used in scenecut detection.<br>- This requires :option: `--hist-scenecut` to be enabled. For example, a value of 0.2 indicates that a frame with normalized SAD value <br>+ This requires :option:`--hist-scenecut` to be enabled. For example, a value of 0.2 indicates that a frame with normalized SAD value <br> greater than 0.2 against the previous frame as scenecut. <br> Default 0.01.<br> <br>diff --git a/source/common/param.cpp b/source/common/param.cpp<br>old mode 100644<br>new mode 100755<br>index fb7244e..925f0c4<br>--- a/source/common/param.cpp<br>+++ b/source/common/param.cpp<br>@@ -1688,8 +1688,8 @@ int x265_check_params(x265_param* param)<br> "scenecutThreshold must be greater than 0");<br> CHECK(param->scenecutBias < 0 || 100 < param->scenecutBias,<br> "scenecut-bias must be between 0 and 100");<br>- CHECK(param->edgeTransitionThreshold < 0.0 || 2.0 < param->edgeTransitionThreshold,<br>- "hist-threshold must be between 0.0 and 2.0");<br>+ CHECK(param->edgeTransitionThreshold < 0.0 || 1.0 < param->edgeTransitionThreshold,<br>+ "hist-threshold must be between 0.0 and 1.0");<br> CHECK(param->radl < 0 || param->radl > param->bframes,<br> "radl must be between 0 and bframes");<br> CHECK(param->rdPenalty < 0 || param->rdPenalty > 2,<br>diff --git a/source/encoder/encoder.cpp b/source/encoder/encoder.cpp<br>old mode 100644<br>new mode 100755<br>index 752e5b2..f6bc540<br>--- a/source/encoder/encoder.cpp<br>+++ b/source/encoder/encoder.cpp<br>@@ -222,12 +222,9 @@ void Encoder::create()<br> uint32_t pixelbytes = m_param->internalBitDepth > 8 ? 2 : 1;<br> m_edgePic = X265_MALLOC(pixel, m_planeSizes[0] * pixelbytes);<br> m_edgeHistThreshold = m_param->edgeTransitionThreshold;<br>- m_chromaHistThreshold = m_edgeHistThreshold * 10.0;<br>- m_chromaHistThreshold = x265_min(m_chromaHistThreshold, MAX_SCENECUT_THRESHOLD);<br>- m_scaledEdgeThreshold = m_edgeHistThreshold * SCENECUT_STRENGTH_FACTOR;<br>- m_scaledEdgeThreshold = x265_min(m_scaledEdgeThreshold, MAX_SCENECUT_THRESHOLD);<br>- m_scaledChromaThreshold = m_chromaHistThreshold * SCENECUT_STRENGTH_FACTOR;<br>- m_scaledChromaThreshold = x265_min(m_scaledChromaThreshold, MAX_SCENECUT_THRESHOLD);<br>+ m_chromaHistThreshold = x265_min(m_edgeHistThreshold * 10.0, MAX_SCENECUT_THRESHOLD);<br>+ m_scaledEdgeThreshold = x265_min(m_edgeHistThreshold * SCENECUT_STRENGTH_FACTOR, MAX_SCENECUT_THRESHOLD);<br>+ m_scaledChromaThreshold = x265_min(m_chromaHistThreshold * SCENECUT_STRENGTH_FACTOR, MAX_SCENECUT_THRESHOLD);<br> if (m_param->sourceBitDepth != m_param->internalBitDepth)<br> {<br> int size = m_param->sourceWidth * m_param->sourceHeight;<br>@@ -1450,13 +1447,14 @@ bool Encoder::computeHistograms(x265_picture *pic)<br> memset(edgeHist, 0, EDGE_BINS * sizeof(int32_t));<br> for (uint32_t i = 0; i < m_planeSizes[0]; i++)<br> {<br>- if (!m_edgePic[i])<br>- edgeHist[0]++;<br>+ if (m_edgePic[i])<br>+ edgeHist[1]++;<br> else<br>- edgeHist[1]++;<br>+ edgeHist[0]++;<br> }<br>+<br> /* Y Histogram Calculation */<br>- int32_t* yHist = m_curYUVHist[0];<br>+ int32_t *yHist = m_curYUVHist[0];<br> memset(yHist, 0, HISTOGRAM_BINS * sizeof(int32_t));<br> for (uint32_t i = 0; i < m_planeSizes[0]; i++)<br> {<br>@@ -1468,7 +1466,7 @@ bool Encoder::computeHistograms(x265_picture *pic)<br> {<br> /* U Histogram Calculation */<br> int32_t *uHist = m_curYUVHist[1];<br>- memset(uHist, 0, HISTOGRAM_BINS * sizeof(int32_t));<br></div></div></div></blockquote><div> [KS] PLease extend this optimization fix to rest of the file and give that as separate patch or update review for analysis save /load patch scd by <a class="gmail_plusreply" id="plusReplyChip-0" href="mailto:soundariya@multicorewareinc.com" tabindex="-1">@Soundariya Ranin Venkatesh</a> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div>+ memset(uHist, 0, sizeof(m_curYUVHist[1]));<br> for (uint32_t i = 0; i < m_planeSizes[1]; i++)<br> {<br> pixelVal = planeU[i];<br>@@ -1478,52 +1476,56 @@ bool Encoder::computeHistograms(x265_picture *pic)<br> /* V Histogram Calculation */<br> pixelVal = 0;<br> int32_t *vHist = m_curYUVHist[2];<br>- memset(vHist, 0, HISTOGRAM_BINS * sizeof(int32_t));<br>+ memset(vHist, 0, sizeof(m_curYUVHist[2]));<br> for (uint32_t i = 0; i < m_planeSizes[2]; i++)<br> {<br> pixelVal = planeV[i];<br> vHist[pixelVal]++;<br> }<br>- for (int i = 0; i < HISTOGRAM_BINS; i++)<br>- {<br>- m_curMaxUVHist[i] = x265_max(uHist[i], vHist[i]);<br>- }<br> }<br> return true;<br> }<br> <br>-void Encoder::computeHistogramSAD(double *maxUVNormalizedSad, double *edgeNormalizedSad, int curPoc)<br>+void Encoder::computeHistogramSAD(double *normalizedMaxUVSad, double *normalizedEdgeSad, int curPoc)<br> {<br> <br> if (curPoc == 0)<br> { /* first frame is scenecut by default no sad computation for the same. */<br>- *maxUVNormalizedSad = 0.0;<br>- *edgeNormalizedSad = 0.0;<br>+ *normalizedMaxUVSad = 0.0;<br>+ *normalizedEdgeSad = 0.0;<br> }<br> else<br> {<br>- /* compute sum of absolute difference of normalized histogram bins for maxUV and edge histograms. */<br>- int32_t edgefreqDiff = 0;<br>- int32_t maxUVfreqDiff = 0;<br>- double edgeProbabilityDiff = 0;<br>+ /* compute sum of absolute differences of histogram bins of chroma and luma edge response between the current and prev pictures. */<br>+ int32_t edgeHistSad = 0;<br>+ int32_t uHistSad = 0;<br>+ int32_t vHistSad = 0;<br>+ double normalizedUSad = 0.0;<br>+ double normalizedVSad = 0.0;<br> <br> for (int j = 0; j < HISTOGRAM_BINS; j++)<br> {<br> if (j < 2)<br> {<br>- edgefreqDiff = abs(m_curEdgeHist[j] - m_prevEdgeHist[j]);<br>- edgeProbabilityDiff = (double) edgefreqDiff / m_planeSizes[0];<br>- *edgeNormalizedSad += edgeProbabilityDiff;<br>+ edgeHistSad += abs(m_curEdgeHist[j] - m_prevEdgeHist[j]);<br> }<br>- maxUVfreqDiff = abs(m_curMaxUVHist[j] - m_prevMaxUVHist[j]);<br>- *maxUVNormalizedSad += (double)maxUVfreqDiff / m_planeSizes[2];<br>+ uHistSad += abs(m_curYUVHist[1][j] - m_prevYUVHist[1][j]);<br>+ vHistSad += abs(m_curYUVHist[2][j] - m_prevYUVHist[2][j]);<br> }<br>+ *normalizedEdgeSad = normalizeRange(edgeHistSad, 0, 2 * m_planeSizes[0], 0.0, 1.0);<br>+ normalizedUSad = normalizeRange(uHistSad, 0, 2 * m_planeSizes[1], 0.0, 1.0);<br>+ normalizedVSad = normalizeRange(vHistSad, 0, 2 * m_planeSizes[2], 0.0, 1.0);<br>+ *normalizedMaxUVSad = x265_max(normalizedUSad, normalizedVSad);<br> }<br> <br> /* store histograms of previous frame for reference */<br>- size_t bufsize = HISTOGRAM_BINS * sizeof(int32_t);<br>- memcpy(m_prevMaxUVHist, m_curMaxUVHist, bufsize);<br>- memcpy(m_prevEdgeHist, m_curEdgeHist, 2 * sizeof(int32_t));<br>+ memcpy(m_prevEdgeHist, m_curEdgeHist, sizeof(m_curEdgeHist));<br>+ memcpy(m_prevYUVHist, m_curYUVHist, sizeof(m_curYUVHist));<br>+}<br>+<br>+double Encoder::normalizeRange(int32_t value, int32_t minValue, int32_t maxValue, double rangeStart, double rangeEnd)<br>+{<br>+ return (double)(value - minValue) * (rangeEnd - rangeStart) / (maxValue - minValue) + rangeStart;<br> }<br> <br></div></div></div></blockquote><div>[KS] Please use const ref pass by parameters to avoid pass by value as it is not required here.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div> void Encoder::findSceneCuts(x265_picture *pic, bool& bDup, double maxUVSad, double edgeSad)<br>@@ -1542,20 +1544,13 @@ void Encoder::findSceneCuts(x265_picture *pic, bool& bDup, double maxUVSad, doub<br> {<br> bDup = true;<br> }<br>- else if (edgeSad > m_edgeHistThreshold && maxUVSad >= m_chromaHistThreshold)<br>- {<br>- pic->frameData.bScenecut = true;<br>- bDup = false;<br>- }<br>- else if (edgeSad > m_scaledEdgeThreshold || maxUVSad >= m_scaledChromaThreshold)<br>+ else if (edgeSad > m_scaledEdgeThreshold || maxUVSad >= m_scaledChromaThreshold || (edgeSad > m_edgeHistThreshold && maxUVSad >= m_chromaHistThreshold))<br> {<br> pic->frameData.bScenecut = true;<br> bDup = false;<br>+ x265_log(m_param, X265_LOG_DEBUG, "scene cut at %d \n", pic->poc);<br> }<br> }<br></div></div></div></blockquote><div> </div><div>[KS] Nice improvisation here</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div>-<br>- if (pic->frameData.bScenecut)<br>- x265_log(m_param, X265_LOG_DEBUG, "scene cut at %d \n", pic->poc);<br> }<br> <br> /**<br>diff --git a/source/encoder/encoder.h b/source/encoder/encoder.h<br>old mode 100644<br>new mode 100755<br>index 2c45886..fd6b3e7<br>--- a/source/encoder/encoder.h<br>+++ b/source/encoder/encoder.h<br>@@ -163,7 +163,7 @@ class RateControl;<br> class ThreadPool;<br> class FrameData;<br> <br>-#define MAX_SCENECUT_THRESHOLD 2.0<br>+#define MAX_SCENECUT_THRESHOLD 1.0<br> #define SCENECUT_STRENGTH_FACTOR 2.0<br> <br> class Encoder : public x265_encoder<br>@@ -257,8 +257,7 @@ public:<br> pixel* m_edgePic;<br> pixel* m_inputPic[3];<br> int32_t m_curYUVHist[3][HISTOGRAM_BINS];<br>- int32_t m_curMaxUVHist[HISTOGRAM_BINS];<br>- int32_t m_prevMaxUVHist[HISTOGRAM_BINS];<br>+ int32_t m_prevYUVHist[3][HISTOGRAM_BINS];<br> int32_t m_curEdgeHist[2];<br> int32_t m_prevEdgeHist[2];<br> uint32_t m_planeSizes[3];<br>@@ -373,6 +372,7 @@ public:<br> <br> bool computeHistograms(x265_picture *pic);<br> void computeHistogramSAD(double *maxUVNormalizedSAD, double *edgeNormalizedSAD, int curPoc);<br>+ double normalizeRange(int32_t value, int32_t minValue, int32_t maxValue, double rangeStart, double rangeEnd);<br></div></div></div></blockquote><div>[KS] update function prototype accordingly </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div> void findSceneCuts(x265_picture *pic, bool& bDup, double m_maxUVSADVal, double m_edgeSADVal);<br> <br> void initRefIdx();<br>diff --git a/source/x265cli.cpp b/source/x265cli.cpp<br>old mode 100644<br>new mode 100755<br>index 5b95698..fa0036b<br>--- a/source/x265cli.cpp<br>+++ b/source/x265cli.cpp<br>@@ -175,7 +175,7 @@ namespace X265_NS {<br> H1(" --scenecut-bias <0..100.0> Bias for scenecut detection. Default %.2f\n", param->scenecutBias);<br> H0(" --hist-scenecut Enables histogram based scene-cut detection using histogram based algorithm.\n");<br> H0(" --no-hist-scenecut Disables histogram based scene-cut detection using histogram based algorithm.\n");<br>- H1(" --hist-threshold <0.0..2.0> Luma Edge histogram's Normalized SAD threshold for histogram based scenecut detection Default %.2f\n", param->edgeTransitionThreshold);<br>+ H1(" --hist-threshold <0.0..1.0> Luma Edge histogram's Normalized SAD threshold for histogram based scenecut detection Default %.2f\n", param->edgeTransitionThreshold);<br> H0(" --[no-]fades Enable detection and handling of fade-in regions. Default %s\n", OPT(param->bEnableFades));<br> H1(" --[no-]scenecut-aware-qp Enable increasing QP for frames inside the scenecut window after scenecut. Default %s\n", OPT(param->bEnableSceneCutAwareQp));<br> H1(" --scenecut-window <0..1000> QP incremental duration(in milliseconds) when scenecut-aware-qp is enabled. Default %d\n", param->scenecutWindow);<br>@@ -1059,4 +1059,4 @@ namespace X265_NS {<br> <br> #ifdef __cplusplus<br> }<br>-#endif<br>\ No newline at end of file<br>+#endif<br>-- <br>1.8.3.1</div><div><br></div></div></div>
_______________________________________________<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>OverAll impression: Good</div><div>1. good code optimization techniques.</div><div>2. improvised naming conventions.</div><div>3. fixed normalization and thresholding logic, documentation fixes</div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><b style="background-color:rgb(255,255,255)"><font color="#0b5394">With Regards,</font></b><div><b style="background-color:rgb(255,255,255)"><font color="#0b5394">Srikanth Kurapati.</font></b></div></div></div></div>