Opened 4 years ago

Closed 4 years ago

#1256 closed defect (fixed)

A typo in in 8.8.3.5 Derivation process of boundary filtering strength

Reported by: zhangkai Owned by:
Priority: minor Milestone:
Component: spec Version: VVC D10 vE
Keywords: Cc: ksuehring, bbross, XiangLi, fbossen, jvet@…

Description

In 8.8.3.5
...
– The variable bS[ xDi ][ yDj ] is derived as follows:
– If cIdx is equal to 0 and both samples p0 and q0 are in a coding block with intra_bdpcm_luma_flag equal to 1, bS[ xDi ][ yDj ] is set equal to 0.
– Otherwise, if cIdx is greater than 0 and both samples p0 and q0 are in a coding block with intra_bdpcm_chroma_flag equal to 1, bS[ xDi ][ yDj ] is set equal to 0.
– Otherwise, if the sample p0 or q0 is in the coding block of a coding unit coded with CuPredMode equal to MODE_INTRA, bS[ xDi ][ yDj ] is set equal to 2.
– Otherwise, if the sample p0 or q0 is in a coding block with ciip_flag equal to 1, bS[ xDi ][ yDj ] is set equal to 2.
– Otherwise, if the block edge is also a transform block edge and the sample p0 or q0 is in a transform block which contains one or more non-zero transform coefficient levels, bS[ xDi ][ yDj ] is set equal to 1.
– Otherwise, if cIdx is equal to 0, edgeFlags[ xDi ][ yDj ] is equal to 2, and one or more of the following conditions are true, bS[ xDi ][ yDj ] is set equal to 1:
...

should be changed to
...
– The variable bS[ xDi ][ yDj ] is derived as follows:
– If cIdx is equal to 0 and both samples p0 and q0 are in a coding block with intra_bdpcm_luma_flag equal to 1, bS[ xDi ][ yDj ] is set equal to 0.
– Otherwise, if cIdx is greater than 0 and both samples p0 and q0 are in a coding block with intra_bdpcm_chroma_flag equal to 1, bS[ xDi ][ yDj ] is set equal to 0.
– Otherwise, if the sample p0 or q0 is in the coding block of a coding unit coded with CuPredMode equal to MODE_INTRA, bS[ xDi ][ yDj ] is set equal to 2.
– Otherwise, if the sample p0 or q0 is in a coding block with ciip_flag equal to 1, bS[ xDi ][ yDj ] is set equal to 2.
– Otherwise, if the block edge is also a transform block edge and the sample p0 or q0 is in a transform block which contains one or more non-zero transform coefficient levels, bS[ xDi ][ yDj ] is set equal to 1.
– Otherwise, if cIdx is equal to 0, edgeFlags[ xDi ][ yDj ] is equal to 1 or 2, and one or more of the following conditions are true, bS[ xDi ][ yDj ] is set equal to 1:
...

Change history (8)

comment:1 Changed 4 years ago by jlchen

This is somehow related to ticket #1237.

It seems to me that there is no difference between edgeFlags[ xDi ][ yDj ] being 1 and 2. If this is the case, edgeFlags can be set as 1 in equation (1206) and (1217). Can other deblocking filter experts study and two tickets and confirm the solution in both ticket #1237 and this ticket.

comment:2 Changed 4 years ago by chiaming

According to my understanding, the intention is to only allow "edgeFlags[ xDi ][ yDj ] is equal to 2" for checking PB related conditions.
If "edgeFlags[ xDi ][ yDj ] is equal to 1", then the edge is just a TB edge (i.e., not also a PB edge). PB related conditions shall not be applied.
Please also check the adoption of JVET-P0043, and correct me if I am wrong.

comment:3 follow-up: Changed 4 years ago by fbossen

This bug report is invalid as far as I can tell, as the proposed fix would change the behaviour in ways inconsistent with the SW. I will repeat here what I previously said in #1236 regarding edgeFlags:

The current use of edgeFlags is a bit strange:
value 0 means no transform block edge and no subblock edge
value 1 means transform block edge and no subblock edge
value 2 means subblock edge (but doesn't say anything about transform block edge)

If the use of edgeFlags is changed to allow for 4 values to distinguish between the 4 possible combinations, say:
value 0 means no transform block edge and no subblock edge
value 1 means transform block edge and no subblock edge
value 2 means no transform block edge and subblock edge
value 3 means transform block edge and subblock edge
Note: this can be achieved by changing edgeFlags[x][y] = 2 to edgeFlags[x][y]+=2 in 8.8.3.4

Then the checks in 8.8.3.5 can be
edgeFlags[ xDi ][ yDj ] % 2 is equal to 1 when checking for transform block edge, and
edgeFlags[ xDi ][ yDj ] / 2 is equal to 1 when checking for subblock edge

instead of currently
"the block edge is also a transform block edge", and
edgeFlags[ xDi ][ yDj ] is equal to 2

comment:4 Changed 4 years ago by bbross

I kind of like Franks suggestion, however I we only have one day left to finalize the spec and I guess there are more changes needed for that in order to not break anything, e.g.:

  1. What about the following check after having incorporated edgeFlags[x][y] += 2 in 8.8.3.4"

– When edgeFlags[ x ][ y ] is equal to 1 or 2, the values of maxFilterLengthPs[ x ][ y ] and maxFilterLengthQs[ x ][ y ] are modified as follows:
Does that need to be changed to edgeFlags[x][y] is greater than 0?
Same of course for the horizontal edges.

  1. What about the checks on edgeTbFlags used in 8.8.3.4?

I guess this variable could be removed now and checks could be replaced by "edgeFlags[ xDi ][ yDj ] % 2 is equal to 1"

  1. What places do need to be changed in 8.8.3.5 exactly? I only identified two so far:

– Otherwise, if the block edge is also a transform block edge and one of the following conditions is true, bS[ xDi ][ yDj ] is set equal to 1.
-> edgeFlags[ xDi ][ yDj ] % 2 is equal to 1

– Otherwise, if cIdx is equal to 0, edgeFlags[ xDi ][ yDj ] is equal to 2, and one or more of the following conditions are true, bS[ xDi ][ yDj ] is set equal to 1:
-> edgeFlags[ xDi ][ yDj ] / 2 is equal to 1

  1. Editorial: We should change edgeFlags[][] to edgeIdc[][] since the array does not contain flags but indices.

comment:5 Changed 4 years ago by fbossen

Ben, Regarding the points you raise:

  1. Yes, the check after updating edgeFlags in 8.8.3.4 would have to be changed from

– When edgeFlags[ x ][ y ] is equal to 1 or 2
to something like
– When edgeFlags[ x ][ y ] is not equal to 0

  1. You can make the change that you are describing but don't have to
  1. You correctly identified the 2 places in 8.8.3.5 that are affected
  1. Sounds good.

comment:6 in reply to: ↑ 3 Changed 4 years ago by zhangkai

I agree with you that the bug is not valid after #1237 is fixed. Thank you for your clarification.

Replying to fbossen:

This bug report is invalid as far as I can tell, as the proposed fix would change the behaviour in ways inconsistent with the SW. I will repeat here what I previously said in #1236 regarding edgeFlags:

The current use of edgeFlags is a bit strange:
value 0 means no transform block edge and no subblock edge
value 1 means transform block edge and no subblock edge
value 2 means subblock edge (but doesn't say anything about transform block edge)

If the use of edgeFlags is changed to allow for 4 values to distinguish between the 4 possible combinations, say:
value 0 means no transform block edge and no subblock edge
value 1 means transform block edge and no subblock edge
value 2 means no transform block edge and subblock edge
value 3 means transform block edge and subblock edge
Note: this can be achieved by changing edgeFlags[x][y] = 2 to edgeFlags[x][y]+=2 in 8.8.3.4

Then the checks in 8.8.3.5 can be
edgeFlags[ xDi ][ yDj ] % 2 is equal to 1 when checking for transform block edge, and
edgeFlags[ xDi ][ yDj ] / 2 is equal to 1 when checking for subblock edge

instead of currently
"the block edge is also a transform block edge", and
edgeFlags[ xDi ][ yDj ] is equal to 2

comment:7 Changed 4 years ago by jlchen

Probably a safe way at this moment is only have action on item 4, and leave other parts as they are.

Editorial: change edgeFlags[][] to edgeIdc[][] since the array does not contain flags but indices.

comment:8 Changed 4 years ago by bbross

  • Resolution set to fixed
  • Status changed from new to closed

Ok, although I started to really like the improvements suggested by Frank I agree that it is most safe at this stage to just rename the array.

Note: See TracTickets for help on using tickets.