Incorrect top-left and bottom-right ALF padding

[bheng]

In Sections 8.8.5.2, 8.8.5.3, 8.8.5.4, and 8.8.5.7, the variable hx is defined as a one-dimensional variable. Specifically, the variable hx appears to depend only on location x, with no dependence on location y.

However, for the top-left and bottom-right padding to work correctly (end of Section 8.8.5.6), the values of hx vary depending on the y position as well.

I believe hx for all of those sections should be specified as a 2-dimensional matrix with both an x and a y index.

[bbross]

Thanks for bringing that up but I am not sure whether I can follow.

hx currently only depends on depends on x as well as vy only depends on y. In 8.8.5.6 (hx, vy) are passed as location (x, y) and both x and y are checked for top-left and bottom-right.
How should hx be defined as an array depending on y as well and how should it be addressed in 8.8.5.6?

Anyone familiar with ALF padding likes to comment on this?

[bheng]

I think 8.8.5.6 is fine. It takes in a (x,y) location and modifies it. The problem is in the other sections.

In 8.8.5.2 for example, I think fixing it right would require replacing the 1-D variable hx with a 7x7 variable h[x][y]. But that notation would get somewhat ugly in the large equations like (1437).

As an alternative, I think it might be close enough if the text only refers to h and v jointly, as locations, and never as two independent variables.

For example, combine (1434) and (1435) into a single equation.

(hx+i, vy+j) = ( Clip3(...), Clip3(...) )

And replace the text:

"The variables hx + i and vy + j are modified by ..."

with:

"The locations (hx+i, vy+j) are modified by ..."

As well as similar changes in 8.8.5.3, 8.8.5.4, and 8.8.5.7.

[jlchen]

Hi, is the first suggested change editorial?

The second change (as following) seems good to me. It makes the text more precise compared to the original text.

replacing the text:
"The variables hx + i and vy + j are modified by ..."
with:
"The locations (hx+i, vy+j) are modified by ..."

[fbossen]

I think Brian is correct. The array h_x should be replaced by an array h_(x,y). This isn't just editorial as it fixes an actual bug in the spec (such as to match the software and I believe the intent as well).

The following should work in 8.8.5.2:

– The locations ( h_(x + i, y + j), v_(y + j) ) for each of the corresponding luma samples ( x, y ) inside the given array recPicture of luma samples with i, j = −3..3 are derived as follows:
( h_(x + i, y + j), v_(y + j) ) = ( Clip3( 0, pps_pic_width_in_luma_samples − 1, xCtb + x + i ), Clip3( 0, pps_pic_height_in_luma_samples − 1, yCtb + y + j ) )

– The locations ( h_(x + i, y + j), v_(y + j) ) with i, j = −3..3 are modified by invoking the ALF sample padding process as specified in subclause 8.8.5.6 with ( xCtb, yCtb ), ( h_(x + i, y + j), v_(y + j) ), the variable isChroma set equal to 0, clipLeftPos, clipRightPos, clipTopPos, clipBottomPos, clipTopLeftFlag and clipBotRightFlag as inputs, and ( h_(x + i, y + j), v_(y + j) ) as output.

In the above v_(y+j) may be passed to / modified by 8.8.5.6 multiple times, but I think that's ok.

All the references to [h_a][v_b] in (1436) and (1437) are changed to [h_(a,b)][v_b].

(and as Brian pointed out, similar changes are needed in 8.8.5.3, 8.8.5.4 and 8.8.5.7)

[bbross]

Fixed as discussed/suggested in JVET-T0110-v2.