Fix #430: fix condition in residual_lfnst_mode when treeType is DUAL_TREE_CHROMA

Any results on performance impact?

I am a proponent of JVET-O0213. I checked that with this fix correct luma width/height can be accessed for all cases including DUAL_TREE_CHROMA. I also ran simulations for this fix and found no discrepancies (no performance impact) for small sequences (large ones are still running but they may not show any different results, because this fix is a very simple and clear one). I have one comment on the code change: in my opinion, the below seems to be more appropriate than just "int chIdx = CS::isDualITree( *cu.cs ) && cu.chType == CHANNEL_TYPE_CHROMA ? 1 : 0;" because chIdx is used only when JVET-O0213 macro is on.

#if JVET_O0213_RESTRICT_LFNST_TO_MAX_TB_SIZE

int chIdx = CS::isDualITree( *cu.cs ) && cu.chType == CHANNEL_TYPE_CHROMA ? 1 : 0;

#endif

Thanks for the tests. I am merging it.

resolved all discussions

@moonmo.koo I think int int chIdx = CS::isDualITree( *cu.cs ) && cu.chType == CHANNEL_TYPE_CHROMA ? 1 : 0; works no matter whether JVET_O0213_RESTRICT_LFNST_TO_MAX_TB_SIZE is on or off, correct?

added 66 commits

• e7c24cdb...bbf94d32 - 65 commits from branch jvet:master
• 8d802857 - Fix #430: fix condition in residual_lfnst_mode when treeType is DUAL_TREE_CHROMA

Compare with previous version

added 3 commits

• 8d802857...1bee1176 - 2 commits from branch jvet:master
• 04f1ebe4 - Fix #430: fix condition in residual_lfnst_mode when treeType is DUAL_TREE_CHROMA

Compare with previous version

Thank you for your test. I also performed tests for both the current merge request and the one where CS::isDualITree( *cu.cs ) is replaced by cu.isSepTree(). I observed that the two tests have the same results. I think that the difference between CS::isDualITree( *cu.cs ) and cu.isSepTree() lies in SCIPU (related to JVET-O0050) case, but the SCIPU is a small block that must not violate max. TB size condition for LFNST (am I right?), which would be the reason whey the two showed the same results. In my opinion, using cu.isSepTree() seems to be a bit more consistent because all the other parts use cu.isSepTree() instead of CS::isDualITree( *cu.cs ), but I have no objection if the current code implementation is kept. Also, I noticed that the below modification mentioned before was not applied in CABACWriter.cpp

#if JVET_O0213_RESTRICT_LFNST_TO_MAX_TB_SIZE

int chIdx = CS::isDualITree( *cu.cs ) && cu.chType == CHANNEL_TYPE_CHROMA ? 1 : 0;

#endif

added 1 commit

• eef8a821 - Fix #430: fix condition in residual_lfnst_mode when treeType is DUAL_TREE_CHROMA

Compare with previous version

added 49 commits

• eef8a821...af6e9ad0 - 48 commits from branch jvet:master
• ecabd01e - Fix #430: fix condition in residual_lfnst_mode when treeType is DUAL_TREE_CHROMA

Compare with previous version

resolved all discussions

merged

mentioned in commit 5280bded