diff mbox series

[FFmpeg-devel,v2,3/3] tests/checkasm/vvc_alf: change alf step size to 8

Message ID OSZP286MB21730479462A17DB43A2416ECAF32@OSZP286MB2173.JPNP286.PROD.OUTLOOK.COM
State New
Headers show
Series [FFmpeg-devel,v2,1/3] avcodec/x86/vvc/vvc_alf: fix integer overflow | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Wu Jianhua May 30, 2024, 4:28 p.m. UTC 
From: Wu Jianhua <toqsxw@outlook.com>

From Benjamin Bross:
> for ALF where functions are in increments of 4 while 8 should be sufficient according to the spec.

Signed-off-by: Wu Jianhua <toqsxw@outlook.com>
---
 tests/checkasm/vvc_alf.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Bross, Benjamin June 3, 2024, 10:42 a.m. UTC | #1 
> From Benjamin Bross:
>> for ALF where functions are in increments of 4 while 8 should be sufficient according to the spec.

Actually, it is not only the increment but the size has to be a multiple of 8, hence in addition loops should start at 8 instead of at 4.

ALF filter and classification is applied on CTU-level.
According to VVC spec, CTU sizes can be: 32x32, 64x64, 128x128 luma samples.
However, if width or height are not a multiple of CTU size, the larger CTU is "forced to split" until width or height fits.
E.g. width=840 luma samples fits 26.25 CTUs, i.e. 26 32x32 and one 8x32. 
Width and height are restricted to be at least a multiple of 8 (see H.266 09/2023, 7.4.3.4 Sequence parameter set RBSP semantics)

sps_pic_width_max_in_luma_samples shall not be equal to 0 and shall be an integer multiple of
Max( 8, MinCbSizeY )
…
sps_pic_height_max_in_luma_samples shall not be equal to 0 and shall be an integer multiple of
Max( 8, MinCbSizeY ).

Please note that this applies only for classification and 7x7 luma filtering. For chroma 5x5 filtering and 420 subsampling, CTU sizes can be as small as multiples of 4 chroma samples.


> On 30. May 2024, at 18:28, toqsxw@outlook.com wrote:
> 
> From: Wu Jianhua <toqsxw@outlook.com>
> 
> From Benjamin Bross:
>> for ALF where functions are in increments of 4 while 8 should be sufficient according to the spec.
> 
> Signed-off-by: Wu Jianhua <toqsxw@outlook.com>
> ---
> tests/checkasm/vvc_alf.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/checkasm/vvc_alf.c b/tests/checkasm/vvc_alf.c
> index f35fd2cd3e..84b0f9da15 100644
> --- a/tests/checkasm/vvc_alf.c
> +++ b/tests/checkasm/vvc_alf.c
> @@ -90,8 +90,8 @@ static void check_alf_filter(VVCDSPContext *c, const int bit_depth)
>     randomize_buffers2(filter, LUMA_PARAMS_SIZE, 1);
>     randomize_buffers2(clip, LUMA_PARAMS_SIZE, 0);
> 
> -    for (int h = 4; h <= MAX_CTU_SIZE; h += 4) {
> -        for (int w = 4; w <= MAX_CTU_SIZE; w += 4) {
> +    for (int h = 4; h <= MAX_CTU_SIZE; h += 8) {
> +        for (int w = 4; w <= MAX_CTU_SIZE; w += 8) {
>             const int ctu_size = MAX_CTU_SIZE;
>             if (check_func(c->alf.filter[LUMA], "vvc_alf_filter_luma_%dx%d_%d", w, h, bit_depth)) {
>                 const int vb_pos = ctu_size - ALF_VB_POS_ABOVE_LUMA;
> @@ -142,8 +142,8 @@ static void check_alf_classify(VVCDSPContext *c, const int bit_depth)
> 
>     randomize_buffers(src0, src1, SRC_BUF_SIZE);
> 
> -    for (int h = 4; h <= MAX_CTU_SIZE; h += 4) {
> -        for (int w = 4; w <= MAX_CTU_SIZE; w += 4) {
> +    for (int h = 4; h <= MAX_CTU_SIZE; h += 8) {
> +        for (int w = 4; w <= MAX_CTU_SIZE; w += 8) {
>             const int id_size = w * h / ALF_BLOCK_SIZE / ALF_BLOCK_SIZE * sizeof(int);
>             const int vb_pos  = MAX_CTU_SIZE - ALF_BLOCK_SIZE;
>             if (check_func(c->alf.classify, "vvc_alf_classify_%dx%d_%d", w, h, bit_depth)) {
> -- 
> 2.44.0.windows.1
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Wu Jianhua June 6, 2024, 6:24 p.m. UTC | #2 
Bross, Benjamin:
> 发件人: Bross, Benjamin <benjamin.bross@hhi.fraunhofer.de>
> 发送时间: 2024年6月3日 3:42
> 收件人: FFmpeg development discussions and patches
> 抄送: Wu Jianhua
> 主题: Re: [FFmpeg-devel] [PATCH v2 3/3] tests/checkasm/vvc_alf: change alf step size to 8
> 
>> From Benjamin Bross:
>>> for ALF where functions are in increments of 4 while 8 should be sufficient according to the spec.
> 
> Actually, it is not only the increment but the size has to be a multiple of 8, hence in addition loops should start at 8 instead of at 4.
> 
> ALF filter and classification is applied on CTU-level.
> According to VVC spec, CTU sizes can be: 32x32, 64x64, 128x128 luma samples.
> However, if width or height are not a multiple of CTU size, the larger CTU is "forced to split" until width or height fits.
> E.g. width=840 luma samples fits 26.25 CTUs, i.e. 26 32x32 and one 8x32.
> Width and height are restricted to be at least a multiple of 8 (see H.266 09/2023, 7.4.3.4 Sequence parameter set RBSP semantics)

> sps_pic_width_max_in_luma_samples shall not be equal to 0 and shall be an integer multiple of
> Max( 8, MinCbSizeY )
> …
> sps_pic_height_max_in_luma_samples shall not be equal to 0 and shall be an integer multiple of
> Max( 8, MinCbSizeY ).
> 
> Please note that this applies only for classification and 7x7 luma filtering. For chroma 5x5 filtering and 420 subsampling, CTU sizes can be as small as multiples of 4 chroma samples.


Hi Benjamin,

I will update some patches according to your comments.

Thanks for your suggestion!
Jianhua
diff mbox series

Patch

diff --git a/tests/checkasm/vvc_alf.c b/tests/checkasm/vvc_alf.c
index f35fd2cd3e..84b0f9da15 100644
--- a/tests/checkasm/vvc_alf.c
+++ b/tests/checkasm/vvc_alf.c
@@ -90,8 +90,8 @@  static void check_alf_filter(VVCDSPContext *c, const int bit_depth)
     randomize_buffers2(filter, LUMA_PARAMS_SIZE, 1);
     randomize_buffers2(clip, LUMA_PARAMS_SIZE, 0);
 
-    for (int h = 4; h <= MAX_CTU_SIZE; h += 4) {
-        for (int w = 4; w <= MAX_CTU_SIZE; w += 4) {
+    for (int h = 4; h <= MAX_CTU_SIZE; h += 8) {
+        for (int w = 4; w <= MAX_CTU_SIZE; w += 8) {
             const int ctu_size = MAX_CTU_SIZE;
             if (check_func(c->alf.filter[LUMA], "vvc_alf_filter_luma_%dx%d_%d", w, h, bit_depth)) {
                 const int vb_pos = ctu_size - ALF_VB_POS_ABOVE_LUMA;
@@ -142,8 +142,8 @@  static void check_alf_classify(VVCDSPContext *c, const int bit_depth)
 
     randomize_buffers(src0, src1, SRC_BUF_SIZE);
 
-    for (int h = 4; h <= MAX_CTU_SIZE; h += 4) {
-        for (int w = 4; w <= MAX_CTU_SIZE; w += 4) {
+    for (int h = 4; h <= MAX_CTU_SIZE; h += 8) {
+        for (int w = 4; w <= MAX_CTU_SIZE; w += 8) {
             const int id_size = w * h / ALF_BLOCK_SIZE / ALF_BLOCK_SIZE * sizeof(int);
             const int vb_pos  = MAX_CTU_SIZE - ALF_BLOCK_SIZE;
             if (check_func(c->alf.classify, "vvc_alf_classify_%dx%d_%d", w, h, bit_depth)) {