linux-kernel - RE: [PATCH] Allow S/G_PARM for stateful decoder

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] [day] [month] [year] [list]

Message-ID: <DM6PR02MB5148A6E6D2DE6D56DBC17772F3549@DM6PR02MB5148.namprd02.prod.outlook.com>
Date:   Wed, 28 Sep 2022 06:07:40 +0000
From:   "Dikshita Agarwal (QUIC)" <quic_dikshita@...cinc.com>
To:     Hans Verkuil <hverkuil-cisco@...all.nl>,
        "Dikshita Agarwal (QUIC)" <quic_dikshita@...cinc.com>,
        "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     "stanimir.varbanov@...aro.org" <stanimir.varbanov@...aro.org>,
        "Vikash Garodia (QUIC)" <quic_vgarodia@...cinc.com>,
        "Maheshwar Ajja (QUIC)" <quic_majja@...cinc.com>,
        quic_jdas <quic_jdas@...cinc.com>
Subject: RE: [PATCH] Allow S/G_PARM for stateful decoder

Thanks, Hans, for your comments.

I will take care of all comments regarding this change in next patch.

Will also send a patch to update the documentation.
Venus driver already has support for S_PARM for decoder, will add support for G_PARM as well and send a patch.

Thanks,
Dikshita
-----Original Message-----
From: Hans Verkuil <hverkuil-cisco@...all.nl> 
Sent: Friday, September 9, 2022 5:47 PM
To: Dikshita Agarwal (QUIC) <quic_dikshita@...cinc.com>; linux-media@...r.kernel.org; linux-kernel@...r.kernel.org
Cc: stanimir.varbanov@...aro.org; Vikash Garodia (QUIC) <quic_vgarodia@...cinc.com>; Maheshwar Ajja (QUIC) <quic_majja@...cinc.com>; quic_jdas <quic_jdas@...cinc.com>
Subject: Re: [PATCH] Allow S/G_PARM for stateful decoder

Hi Dikshita,

First I'll comment on this patch, then at the end I have more high-level comments.

On 29/08/2022 09:54, Dikshita Agarwal wrote:
> Some stateful decoder supports S/G_PARM similar to a

decoder -> decoders

BTW, which decoder drivers support this in mainline? Or is this for out-of-tree drivers?

> stateful encoder. S_PARM(OUTPUT) reserves hardware decoder resources, 
> and G_PARM(CAPTURE) returns the embedded frame interval. Allow the 
> same with this change.
> 
> Signed-off-by: Dikshita Agarwal <quic_dikshita@...cinc.com>
> ---
>  utils/v4l2-compliance/v4l2-test-formats.cpp | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/utils/v4l2-compliance/v4l2-test-formats.cpp 
> b/utils/v4l2-compliance/v4l2-test-formats.cpp
> index 269a383..e996558 100644
> --- a/utils/v4l2-compliance/v4l2-test-formats.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-formats.cpp
> @@ -64,8 +64,6 @@ static int testEnumFrameIntervals(struct node *node, __u32 pixfmt,
>  		ret = doioctl(node, VIDIOC_ENUM_FRAMEINTERVALS, &frmival);
>  		if (ret == ENOTTY)
>  			return ret;
> -		// M2M devices don't support this, except for stateful encoders
> -		fail_on_test(node->is_m2m && !(node->codec_mask & STATEFUL_ENCODER));

M2M devices still don't support this, except for stateful codecs. So this test should still be there, just changed to:

!(node->codec_mask & (STATEFUL_ENCODER | STATEFUL_DECODER))

>  		if (f == 0 && ret == EINVAL) {
>  			if (type == V4L2_FRMSIZE_TYPE_DISCRETE)
>  				warn("found framesize %dx%d, but no frame intervals\n", w, h); @@ 
> -1367,8 +1365,6 @@ static int testParmType(struct node *node, unsigned type)
>  	}
>  	if (ret == ENOTTY)
>  		return ret;
> -	// M2M devices don't support this, except for stateful encoders
> -	fail_on_test(node->is_m2m && !is_stateful_enc);

Similar.

>  	if (ret == EINVAL)
>  		return ENOTTY;
>  	if (ret)

But modifying the utility is something you only do if the spec has first been updated.
So the stateful decoder chapter needs to be modified so it explains how G/S_PARM shall be used. And ideally have at least one driver that supports it.

Regards,

	Hans