[x265] [PATCH] SAO: avoid negative indexes in 'x265_lambda2_tab' table

[Mateusz Brzostek]

W dniu 2017-02-16 o 03:54, Pradeep Ramachandran pisze:
>
> On Thu, Feb 16, 2017 at 12:11 AM, Mateusz Brzostek <mateusz at msystem.waw.pl <mailto:mateusz at msystem.waw.pl>> wrote:
>
>     This patch fixes issue #323 -- crash when encoding to 422. The 'qpCb' index is -2 on Selur's sample file and options.
>
>     Please review.
>
>     Mateusz
>
>     # HG changeset patch
>     # User Ma0 <mateuszb at poczta.onet.pl <mailto:mateuszb at poczta.onet.pl>>
>     # Date 1487183578 -3600
>     #      Wed Feb 15 19:32:58 2017 +0100
>     # Node ID 7b55d81f0677b7cbef5a490d90bd9cbbf96127cb
>     # Parent  912dd749bdb53cdd1e251bc3a69e4c41ece3b308
>     SAO: avoid negative indexes in 'x265_lambda2_tab' table
>
>     diff -r 912dd749bdb5 -r 7b55d81f0677 source/encoder/sao.cpp
>     --- a/source/encoder/sao.cpp    Wed Feb 15 12:04:41 2017 +0530
>     +++ b/source/encoder/sao.cpp    Wed Feb 15 19:32:58 2017 +0100
>     @@ -1234,7 +1234,7 @@
>          if (m_param->internalCsp == X265_CSP_I420)
>              qpCb = x265_clip3(m_param->rc.qpMin, m_param->rc.qpMax, (int)g_chromaScale[qp + slice->m_pps->chromaQpOffset[0]]);
>          else
>     -        qpCb = X265_MIN(qp + slice->m_pps->chromaQpOffset[0], QP_MAX_SPEC);
>     +        qpCb = x265_clip3(0, QP_MAX_SPEC, qp + slice->m_pps->chromaQpOffset[0]);
>
>
> This clip should be between m_param->rc.qpMin and m_param->rc.qpMax, as per what is done for the 4:2:0 videos.
>  
>
>
>          lambda[0] = (int64_t)floor(256.0 * x265_lambda2_tab[qp]);
>          lambda[1] = (int64_t)floor(256.0 * x265_lambda2_tab[qpCb]); // Use Cb QP for SAO chroma
>
>
>     _______________________________________________
>     x265-devel mailing list
>     x265-devel at videolan.org <mailto:x265-devel at videolan.org>
>     https://mailman.videolan.org/listinfo/x265-devel <https://mailman.videolan.org/listinfo/x265-devel>
>
>
>
>
> _______________________________________________
> x265-devel mailing list
> x265-devel at videolan.org
> https://mailman.videolan.org/listinfo/x265-devel
Thanks for review.

After new commits there are changes in sao.cpp. I've made more tests and
'qp + slice->m_pps->chromaQpOffset[0] + slice->m_chromaQpOffset[0]' is < 0 for 4:2:0 too.
There is no crash but in this case qpCb is random (in good Min and Max).

In this new patch index in 'g_chromaScale' table is OK and qpCb is between m_param->rc.qpMin and m_param->rc.qpMax.

Please review.

Mateusz

# HG changeset patch
# User Ma0 <mateuszb at poczta.onet.pl>
# Date 1487242844 -3600
#      Thu Feb 16 12:00:44 2017 +0100
# Node ID cac23bf21ede49cb5f98ec3393fb40bbbbda35ba
# Parent  db913efb1a5999a75569628b8a4675e1b6b9e066
SAO: avoid negative indexes in 'x265_lambda2_tab' table

diff -r db913efb1a59 -r cac23bf21ede source/encoder/sao.cpp
--- a/source/encoder/sao.cpp    Wed Feb 01 17:01:29 2017 +0530
+++ b/source/encoder/sao.cpp    Thu Feb 16 12:00:44 2017 +0100
@@ -1230,11 +1230,11 @@
 
     int64_t lambda[2] = { 0 };
 
-    int qpCb = qp;
+    int qpCb = qp + slice->m_pps->chromaQpOffset[0] + slice->m_chromaQpOffset[0];
     if (m_param->internalCsp == X265_CSP_I420)
-        qpCb = x265_clip3(m_param->rc.qpMin, m_param->rc.qpMax, (int)g_chromaScale[qp + slice->m_pps->chromaQpOffset[0] + slice->m_chromaQpOffset[0]]);
+        qpCb = x265_clip3(m_param->rc.qpMin, m_param->rc.qpMax, (int)g_chromaScale[x265_clip3(QP_MIN, QP_MAX_MAX, qpCb)]);
     else
-        qpCb = X265_MIN(qp + slice->m_pps->chromaQpOffset[0] + slice->m_chromaQpOffset[0], QP_MAX_SPEC);
+        qpCb = x265_clip3(m_param->rc.qpMin, m_param->rc.qpMax, qpCb);
     lambda[0] = (int64_t)floor(256.0 * x265_lambda2_tab[qp]);
     lambda[1] = (int64_t)floor(256.0 * x265_lambda2_tab[qpCb]); // Use Cb QP for SAO chroma
 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/x265-devel/attachments/20170216/081f5b46/attachment.html>
-------------- next part --------------
# HG changeset patch
# User Ma0 <mateuszb at poczta.onet.pl>
# Date 1487242844 -3600
#      Thu Feb 16 12:00:44 2017 +0100
# Node ID cac23bf21ede49cb5f98ec3393fb40bbbbda35ba
# Parent  db913efb1a5999a75569628b8a4675e1b6b9e066
SAO: avoid negative indexes in 'x265_lambda2_tab' table

diff -r db913efb1a59 -r cac23bf21ede source/encoder/sao.cpp
--- a/source/encoder/sao.cpp	Wed Feb 01 17:01:29 2017 +0530
+++ b/source/encoder/sao.cpp	Thu Feb 16 12:00:44 2017 +0100
@@ -1230,11 +1230,11 @@
 
     int64_t lambda[2] = { 0 };
 
-    int qpCb = qp;
+    int qpCb = qp + slice->m_pps->chromaQpOffset[0] + slice->m_chromaQpOffset[0];
     if (m_param->internalCsp == X265_CSP_I420)
-        qpCb = x265_clip3(m_param->rc.qpMin, m_param->rc.qpMax, (int)g_chromaScale[qp + slice->m_pps->chromaQpOffset[0] + slice->m_chromaQpOffset[0]]);
+        qpCb = x265_clip3(m_param->rc.qpMin, m_param->rc.qpMax, (int)g_chromaScale[x265_clip3(QP_MIN, QP_MAX_MAX, qpCb)]);
     else
-        qpCb = X265_MIN(qp + slice->m_pps->chromaQpOffset[0] + slice->m_chromaQpOffset[0], QP_MAX_SPEC);
+        qpCb = x265_clip3(m_param->rc.qpMin, m_param->rc.qpMax, qpCb);
     lambda[0] = (int64_t)floor(256.0 * x265_lambda2_tab[qp]);
     lambda[1] = (int64_t)floor(256.0 * x265_lambda2_tab[qpCb]); // Use Cb QP for SAO chroma