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