linux-kernel - Re: [PATCH 4/4] venus: return P010 as preferred format for 10 bit decode

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]

Date:   Fri, 5 May 2023 14:33:20 +0530
From:   Dikshita Agarwal <quic_dikshita@...cinc.com>
To:     Konrad Dybcio <konrad.dybcio@...aro.org>,
        <linux-media@...r.kernel.org>, <stanimir.k.varbanov@...il.com>,
        <quic_vgarodia@...cinc.com>, <agross@...nel.org>,
        <andersson@...nel.org>, <mchehab@...nel.org>
CC:     <linux-kernel@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>
Subject: Re: [PATCH 4/4] venus: return P010 as preferred format for 10 bit
 decode


On 5/4/2023 10:50 PM, Konrad Dybcio wrote:
>
> On 4.05.2023 12:36, Dikshita Agarwal wrote:
>> If bit depth is detected as 10 bit by firmware, return
>> P010 as preferred decoder format to the client.
>>
>> Signed-off-by: Dikshita Agarwal <quic_dikshita@...cinc.com>
>> ---
>>   drivers/media/platform/qcom/venus/vdec.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
>> index 69f7f6e..ed11dc2 100644
>> --- a/drivers/media/platform/qcom/venus/vdec.c
>> +++ b/drivers/media/platform/qcom/venus/vdec.c
>> @@ -1468,8 +1468,13 @@ static void vdec_event_change(struct venus_inst *inst,
>>   	inst->out_width = ev_data->width;
>>   	inst->out_height = ev_data->height;
>>   
>> -	if (inst->bit_depth != ev_data->bit_depth)
>> +	if (inst->bit_depth != ev_data->bit_depth) {
>>   		inst->bit_depth = ev_data->bit_depth;
>> +		if (inst->bit_depth == VIDC_BITDEPTH_10)
>> +			inst->fmt_cap = &vdec_formats[3];
>> +		else
>> +			inst->fmt_cap = &vdec_formats[0];
> This doesn't scale and is very error-prone, please enumerate the
> entries and assign it using the enumerator, like:
>
> enum {
> 	VDEC_FORMAT_FOO,
> 	...
> };
>
> ... vdec_formats[] = {
> 	[VDEC_FORMAT_FOO] = { foo, bar, baz }
> }
>
> Konrad

I agree, this can be improved but I would prefer making that change as 
separate patch.

As this is not only related to HDR 10 decoding, there are other places 
in the code which will require similar change.

Thanks,

Dikshita

>> +	}
>>   
>>   	if (inst->pic_struct != ev_data->pic_struct)
>>   		inst->pic_struct = ev_data->pic_struct;