linux-kernel - Re: [PATCH v1 05/18] media: controls: Add control for HEVC codec

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: <f66a65db46d24beb42b2c286de9e6212b6666978.camel@collabora.com>
Date:   Wed, 17 Feb 2021 16:58:22 -0300
From:   Ezequiel Garcia <ezequiel@...labora.com>
To:     Benjamin Gaignard <benjamin.gaignard@...labora.com>,
        p.zabel@...gutronix.de, mchehab@...nel.org, robh+dt@...nel.org,
        shawnguo@...nel.org, s.hauer@...gutronix.de, kernel@...gutronix.de,
        festevam@...il.com, linux-imx@....com, gregkh@...uxfoundation.org,
        mripard@...nel.org, paul.kocialkowski@...tlin.com, wens@...e.org,
        jernej.skrabec@...l.net, krzk@...nel.org, shengjiu.wang@....com,
        adrian.ratiu@...labora.com, aisheng.dong@....com, peng.fan@....com,
        Anson.Huang@....com, hverkuil-cisco@...all.nl
Cc:     linux-media@...r.kernel.org, linux-rockchip@...ts.infradead.org,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, devel@...verdev.osuosl.org,
        kernel@...labora.com
Subject: Re: [PATCH v1 05/18] media: controls: Add control for HEVC codec

Hi Benjamin,

On Wed, 2021-02-17 at 09:02 +0100, Benjamin Gaignard wrote:
> Add HEVC decode params and scaling matrix controls.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@...labora.com>
> Signed-off-by: Ezequiel Garcia <ezequiel@...labora.com>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@...labora.com>
> ---
>  drivers/media/v4l2-core/v4l2-ctrls.c | 36 ++++++++++++++++++++++------
>  1 file changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 016cf6204cbb..5e45333fd862 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -1028,6 +1028,8 @@ const char *v4l2_ctrl_get_name(u32 id)
>         case V4L2_CID_MPEG_VIDEO_HEVC_SPS:                      return "HEVC Sequence Parameter Set";
>         case V4L2_CID_MPEG_VIDEO_HEVC_PPS:                      return "HEVC Picture Parameter Set";
>         case V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS:             return "HEVC Slice Parameters";
> +       case V4L2_CID_MPEG_VIDEO_HEVC_DECODE_PARAMS:            return "HEVC Decode Parameters";
> +       case V4L2_CID_MPEG_VIDEO_HEVC_SCALING_MATRIX:           return "HEVC Scaling Matrix";

I would move all the SCALING_MATRIX changes to their own patches.
 
>         case V4L2_CID_MPEG_VIDEO_HEVC_DECODE_MODE:              return "HEVC Decode Mode";
>         case V4L2_CID_MPEG_VIDEO_HEVC_START_CODE:               return "HEVC Start Code";
>  
> @@ -1482,6 +1484,12 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>         case V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS:
>                 *type = V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS;
>                 break;
> +       case V4L2_CID_MPEG_VIDEO_HEVC_DECODE_PARAMS:
> +               *type = V4L2_CTRL_TYPE_HEVC_DECODE_PARAMS;
> +               break;
> +       case V4L2_CID_MPEG_VIDEO_HEVC_SCALING_MATRIX:
> +               *type = V4L2_CTRL_TYPE_HEVC_SCALING_MATRIX;
> +               break;
>         case V4L2_CID_UNIT_CELL_SIZE:
>                 *type = V4L2_CTRL_TYPE_AREA;
>                 *flags |= V4L2_CTRL_FLAG_READ_ONLY;
> @@ -1833,6 +1841,7 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
>         struct v4l2_ctrl_hevc_sps *p_hevc_sps;
>         struct v4l2_ctrl_hevc_pps *p_hevc_pps;
>         struct v4l2_ctrl_hevc_slice_params *p_hevc_slice_params;
> +       struct v4l2_ctrl_hevc_decode_params *p_hevc_decode_params;
>         struct v4l2_area *area;
>         void *p = ptr.p + idx * ctrl->elem_size;
>         unsigned int i;
> @@ -2108,26 +2117,33 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
>                 zero_padding(*p_hevc_pps);
>                 break;
>  
> -       case V4L2_CTRL_TYPE_HEVC_SLICE_PARAMS:
> -               p_hevc_slice_params = p;
> +       case V4L2_CTRL_TYPE_HEVC_DECODE_PARAMS:
> +               p_hevc_decode_params = p;
>  
> -               if (p_hevc_slice_params->num_active_dpb_entries >
> +               if (p_hevc_decode_params->num_active_dpb_entries >

I suspect this change should be squashed with the patch that moves
num_active_dpb_entries from the slice control, or otherwise
this code won't compile.

>                     V4L2_HEVC_DPB_ENTRIES_NUM_MAX)
>                         return -EINVAL;
>  
> -               zero_padding(p_hevc_slice_params->pred_weight_table);
> -
> -               for (i = 0; i < p_hevc_slice_params->num_active_dpb_entries;
> +               for (i = 0; i < p_hevc_decode_params->num_active_dpb_entries;
>                      i++) {
>                         struct v4l2_hevc_dpb_entry *dpb_entry =
> -                               &p_hevc_slice_params->dpb[i];
> +                               &p_hevc_decode_params->dpb[i];
>  

Ditto.

Thanks,
Ezequiel