Thank your explanation, you are right, we uninitialize when merge mode unavailable, but we never read value in that time. For reset function, I think we can merge into one function, we forgot cleanup it.
At 2015-11-23 21:16:11,"Ashok Kumar Mishra" <[email protected]> wrote: Yes, I removed directly because of the following reason. mergeSaoParam array is declared in rdoSaoUnitRow() function. It is getting initialized inside saoComponentParamDist() and sao2ChromaParamDist() function when merge up/left available. And again in rdoSaoUnitRow() function we are calculating the best merge candidate using this array only if the merge up/left available. So it is not required to initialize. Beacuse we are double checking if up/left merge block is available. For your conformance I ran valgrind, there is no issue. It is not affecting the output. I was just trying to remove the duplicate code like resetSaoUnit(), because we are using the same code with different name reset(). Below are the two functions: void SAO::resetSaoUnit(SaoCtuParam* saoUnit) { saoUnit->mergeMode = SAO_MERGE_NONE; saoUnit->typeIdx = -1; saoUnit->bandPos = 0; for (int i = 0; i < SAO_NUM_OFFSET; i++) saoUnit->offset[i] = 0; } void reset() { mergeMode = SAO_MERGE_NONE; typeIdx = -1; bandPos = 0; offset[0] = 0; offset[1] = 0; offset[2] = 0; offset[3] = 0; } Really do we need two functions which serve same purpose ? On Fri, Nov 20, 2015 at 9:12 PM, chen <[email protected]> wrote: At 2015-11-20 19:58:21,[email protected] wrote: ># HG changeset patch ># User Ashok Kumar Mishra<[email protected]> ># Date 1445505834 -19800 ># Thu Oct 22 14:53:54 2015 +0530 ># Node ID 5d320d99e14fed8e64505d73f01505cdf0d94861 ># Parent c1575815b250fb02c1fdae803aead10a4997c835 >SAO: remove resetSaoUnit() > >diff -r c1575815b250 -r 5d320d99e14f source/encoder/sao.cpp >--- a/source/encoder/sao.cpp Thu Oct 29 17:15:15 2015 +0530 >+++ b/source/encoder/sao.cpp Thu Oct 22 14:53:54 2015 +0530 >@@ -704,16 +704,6 @@ > std::swap(m_tmpU1[plane], m_tmpU2[plane]); > } > >-void SAO::resetSaoUnit(SaoCtuParam* saoUnit) >-{ >- saoUnit->mergeMode = SAO_MERGE_NONE; >- saoUnit->typeIdx = -1; >- saoUnit->bandPos = 0; >- >- for (int i = 0; i < SAO_NUM_OFFSET; i++) >- saoUnit->offset[i] = 0; >-} >- > void SAO::copySaoUnit(SaoCtuParam* saoUnitDst, const SaoCtuParam* saoUnitSrc) > { > saoUnitDst->mergeMode = saoUnitSrc->mergeMode; >@@ -1172,9 +1162,7 @@ > } > } > >- saoParam->ctuParam[plane][addr].mergeMode = SAO_MERGE_NONE; >- saoParam->ctuParam[plane][addr].typeIdx = -1; >- saoParam->ctuParam[plane][addr].bandPos = 0; >+ saoParam->ctuParam[plane][addr].reset(); > if (saoParam->bSaoFlag[plane > 0]) > calcSaoStatsCu(addr, plane); > } >@@ -1326,7 +1314,6 @@ > int currentDistortionTableBo[MAX_NUM_SAO_CLASS]; > double currentRdCostTableBo[MAX_NUM_SAO_CLASS]; > >- resetSaoUnit(lclCtuParam); > m_entropyCoder.load(m_rdContexts.temp); > m_entropyCoder.resetBits(); > m_entropyCoder.codeSaoOffset(*lclCtuParam, 0); >@@ -1386,7 +1373,6 @@ > m_entropyCoder.store(m_rdContexts.temp); > > // merge left or merge up >- > for (int mergeIdx = 0; mergeIdx < 2; mergeIdx++) > { > SaoCtuParam* mergeSrcParam = NULL; >@@ -1413,8 +1399,6 @@ > > mergeDist[mergeIdx + 1] = ((double)estDist / m_lumaLambda); > } >- else >- resetSaoUnit(&mergeSaoParam[mergeIdx]); remove directly? mergeSaoParam declare in rdoSaoUnitRow() and never initialize before. _______________________________________________ x265-devel mailing list [email protected] https://mailman.videolan.org/listinfo/x265-devel
_______________________________________________ x265-devel mailing list [email protected] https://mailman.videolan.org/listinfo/x265-devel
