lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAAEAJfAvUjtR4w0uaZ5yFueXu8jNbH-gmWUOEZxoJH78771RSA@mail.gmail.com>
Date:   Tue, 3 May 2022 10:12:57 -0300
From:   Ezequiel Garcia <ezequiel@...guardiasur.com.ar>
To:     Benjamin Gaignard <benjamin.gaignard@...labora.com>
Cc:     Philipp Zabel <p.zabel@...gutronix.de>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        linux-media <linux-media@...r.kernel.org>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
        "open list:STAGING SUBSYSTEM" <linux-staging@...ts.linux.dev>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        jon@...ocrew.net, Adam Ford <aford173@...il.com>,
        Collabora Kernel ML <kernel@...labora.com>
Subject: Re: [PATCH v2] media: hantro: HEVC: unconditionnaly set
 pps_{cb/cr}_qp_offset values

Hi Benjamin,

On Tue, Apr 26, 2022 at 10:50 AM Benjamin Gaignard
<benjamin.gaignard@...labora.com> wrote:
>
> Always set pps_cb_qp_offset and pps_cr_qp_offset values in Hantro/G2
> register whatever is V4L2_HEVC_PPS_FLAG_PPS_SLICE_CHROMA_QP_OFFSETS_PRESENT
> flag value.

I would say we need more justification why this is correct, or at least
checking what the reference vendor implementation is doing (and mentioning
in the commit description so we can track it in the future).

> This fix CAINIT_G_SHARP_3 test in fluster.
>

This could sound like a tad a pedantic detail, but I'd say it's
important we stop refering to tests
as "fluster tests", and instead say something more correct as "HEVC
conformance test CAINIT_G_SHARP_3".

Also, when we are fixing conformance tests, let's please add the
Fluster score (in this case, I think it's
OK to refer to Fluster).

PS: Same comments apply to patch "media: hantro: HEVC: Fix reference
frames management".

Thanks,
Ezequiel

> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@...labora.com>
> ---
>  drivers/staging/media/hantro/hantro_g2_hevc_dec.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c b/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
> index 6deb31b7b993..503f4b028bc5 100644
> --- a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
> +++ b/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
> @@ -194,13 +194,8 @@ static void set_params(struct hantro_ctx *ctx)
>                 hantro_reg_write(vpu, &g2_max_cu_qpd_depth, 0);
>         }
>
> -       if (pps->flags & V4L2_HEVC_PPS_FLAG_PPS_SLICE_CHROMA_QP_OFFSETS_PRESENT) {
> -               hantro_reg_write(vpu, &g2_cb_qp_offset, pps->pps_cb_qp_offset);
> -               hantro_reg_write(vpu, &g2_cr_qp_offset, pps->pps_cr_qp_offset);
> -       } else {
> -               hantro_reg_write(vpu, &g2_cb_qp_offset, 0);
> -               hantro_reg_write(vpu, &g2_cr_qp_offset, 0);
> -       }
> +       hantro_reg_write(vpu, &g2_cb_qp_offset, pps->pps_cb_qp_offset);
> +       hantro_reg_write(vpu, &g2_cr_qp_offset, pps->pps_cr_qp_offset);
>
>         hantro_reg_write(vpu, &g2_filt_offset_beta, pps->pps_beta_offset_div2);
>         hantro_reg_write(vpu, &g2_filt_offset_tc, pps->pps_tc_offset_div2);
> --
> 2.32.0
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ