Mismatch when slice_ts_residual_coding_disabled_flag is enabled

[T.Hashimoto]

If slice_ts_residual_coding_disabled_flag is enabled, encoder / decoder mismatch occurs when sign data hiding (SDH) is enabled.

When slice_ts_residual_coding_disabled_flag is enabled, RRC shall be used instead of TSRC in transfrom skip case. However, encoder RD search does not seem to use RRC in TS case (from line 530 in QuantRDOQ.cpp). A merge requeset will be submitted (based on an attached patch).
Note: In lossless case, the above RD and SDH are not used (thus enc-dec mismatch doesn't occur).

It seems BDPCM encoder still uses TSRC in RD search even if slice_ts_residual_coding_disabled_flag is enabled, but the above fix doesn't cover this case. Thus if BDPCM, RRC (slice_ts_residual_coding_disabled_flag is 1) and SDH are enabled, some encoder mismatch still occurs with the fix.

[XiangLi]

Fixed in ​https://vcgit.hhi.fraunhofer.de/jvet/VVCSoftware_VTM/-/merge_requests/1475.

[sunmi.yoo]

For the first condition change in the attached fix( in DepQuant.cpp ), it can cause another problem.

For example, in the case when ph_dep_quant_enabled_flag=1, transform_skip_flag=1, and slice_ts_residual_coding_disabled_flag=1, the quantization method should be RDOQ, but it goes to DQ using this fix. Using DQ for the transform skip block seems not aligned with the spec.

[analci]

There is another issue with the first condition change in the attached fix (DepQuant.cpp) which appears to be merged. Encoder crashes for lossless coding (lossless.cfg) with additional options DepQuant=1, RDOQ=1 and RDOQTS=1.

[T.Hashimoto]

Thank you for some comments.

Using DQ for the transform skip block seems not aligned with the spec.

In the current specification, if slice_ts_residual_coding_disabled_flag is enabled, RRC is invoked regardless of whether TS is on or off.

if( !transform_skip_flag[ x0 ][ y0 ][ 0 ]  | |  slice_ts_residual_coding_disabled_flag )	
	residual_coding( x0, y0, Log2( tbWidth ), Log2( tbHeight ), 0 )	
else	
	residual_ts_coding( x0, y0, Log2( tbWidth ), Log2( tbHeight ), 0 )	

Thus, QD in RRC could be used for transform skip block in the current specification.

Encoder crashes for lossless coding (lossless.cfg) with additional options DepQuant=1, RDOQ=1 and RDOQTS=1.

It seems to be strange to use RDOQ in lossless case. In my understanding, no need to use RDOQ in lossless coding.

[sunmi.yoo]

I am not sure why DQ is used when the condition I mentioned above (ph_dep_quant_enabled_flag=1, transform_skip_flag=1, and slice_ts_residual_coding_disabled_flag=1).

One point is related to the conditions of equations (1172) and (1173) in the spec. In the source code for DQ, it seems it always adds 1 to the Qp, regardless that the coefficients are transform skipped. According to the condition for the equation (1173), adding 1 to the qp for the transform skip is not appropriate. Another point is that, from the coding efficiency point of view, using DQ for the transform skip might cause some coding loss.

[analci]

Replying to T.Hashimoto:

Encoder crashes for lossless coding (lossless.cfg) with additional options DepQuant=1, RDOQ=1 and RDOQTS=1.

It seems to be strange to use RDOQ in lossless case. In my understanding, no need to use RDOQ in lossless coding.

Thanks for the comments. Ideally the software should not crash if TCQ or RDOQ are enabled in the config level. One such use case is mixed lossless & lossy coding for which there would be a problem with current SW.

[T.Hashimoto]

Thank you for the comments.

I was misunderstanding. Certainly, the changes in DepQuant.cpp is not necessary.
I will submit a new merge requeset which removes the changes in DepQuant.cpp.

[m.sarwer]

Alibaba have also studied this bug. There are few comments about this patch
 First of all, the implementation is not complete. Based on our experience, this fix supposed to introduce significant coding loss since the variables “defaultErrorScale” in the function “xRateDistOptQuant”, is not adjusted based on TS mode.
 Second of all, the BDPCM block still use TS residual coding even slice_ts_residual_coding_disabled_flag == 1.
 The codes in DepQuant.cpp is not required since TS mode will never go there.

Attached you find the patch from our side (tsrdoqpatchAlibaba.diff). I am requesting experts to study the code and let me know if any question. Follow following macros,

#define ALI_ALWAYS_USE_REGULAR_RESIDUAL_CODING 1
#define ALI_FIX_RDOQ_TS_BUG 1

I will submit formal merge request based on your comments. Of course, macro “ALI_ALWAYS_USE_REGULAR_RESIDUAL_CODING” will be disabled in formal merge request.

Best,
Mohammed

[analci]

Thanks to Alibaba for studying this bug and indicating these issues. We agree that “defaultErrorScale” in the function “xRateDistOptQuant” is not adjusted based on TS mode.

We also noticed that BDPCM RDOQ provided in (tsrdoqpatchAlibaba.diff) can be improved for some aspects. For example, in the provided patch forwardRDPCMRRC(.) function appears to be based on RDOQTS for context derivation and coefficient coding parts. It uses cctx.sigCtxIdAbsTS(scanPos, dstCoeff), cctx.templateAbsSumTS(scanPos, dstCoeff) and other TS related contexts when RRC is selected.
It is also likely that residual scan order to be modified to handle RDPCM RDOQ to work with inverse residual scan of RRC.

We would like to provide an alternative patch that addresses these issues.
Please let us know if the provided changes look OK for encoder changes.

[XiangLi]

Thanks for the comments and patch. Due to the issue raised after the merge of MR1475, we reverted MR1475 after discussion. Given different fix solutions on the table , it looks better to discuss the issue in the coming meeting. Proposals are recommended.