linux-kernel - Re: [PATCH] media: verisilicon: HEVC: Initialize start

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: <0a1b29c5-11f7-4836-b41e-99be75afdfd8@collabora.com>
Date: Mon, 20 Jan 2025 15:19:04 +0100
From: Benjamin Gaignard <benjamin.gaignard@...labora.com>
To: Adam Ford <aford173@...il.com>,
 Nicolas Dufresne <nicolas.dufresne@...labora.com>
Cc: p.zabel@...gutronix.de, mchehab@...nel.org, linux-media@...r.kernel.org,
 linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] media: verisilicon: HEVC: Initialize start_bit field


Le 20/01/2025 à 15:15, Adam Ford a écrit :
> On Mon, Jan 20, 2025 at 8:10 AM Nicolas Dufresne
> <nicolas.dufresne@...labora.com> wrote:
>> Le lundi 20 janvier 2025 à 11:00 +0100, Benjamin Gaignard a écrit :
>>> Le 20/01/2025 à 09:10, Benjamin Gaignard a écrit :
>>>> Always set start_bit field to 0, if not it could lead to corrupted frames
>>>> specially when decoding VP9 bitstreams at the same time since VP9 driver
>>>> set it for it own purpose.
>>                 its
>>
> Does this impact the Fluster score?

No because that happens only when decoding VP9 and HEVC bitstreams at the same time.

>
>>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@...labora.com>
>>> I could add this tag:
>>>
>>> Fixes: cb5dd5a0fa518 ("media: hantro: Introduce G2/HEVC decoder")
>> I have tested this on IMX8MQ board using the following GStreamer pipeline.
>> Before the change, the HEVC window was entirely corrupted. The streams don't
>> matter as long as they use both HEVC and VP9 codec.
>>
>> gst-launch-1.0 \
>>    filesrc location=hevc.mp4 ! parsebin ! v4l2slh265dec ! fakevideosink \
>>    filesrc location=vp9.mkv ! parsebin ! v4l2slvp9dec ! fakevideosink
>>
>> Tested-by: Nicolas Dufresne <nicolas.dufresne@...labora.com>
>>
>>
>>>> ---
>>>>    drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c b/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
>>>> index 85a44143b378..0e212198dd65 100644
>>>> --- a/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
>>>> +++ b/drivers/media/platform/verisilicon/hantro_g2_hevc_dec.c
>>>> @@ -518,6 +518,7 @@ static void set_buffers(struct hantro_ctx *ctx)
>>>>      hantro_reg_write(vpu, &g2_stream_len, src_len);
>>>>      hantro_reg_write(vpu, &g2_strm_buffer_len, src_buf_len);
>>>>      hantro_reg_write(vpu, &g2_strm_start_offset, 0);
>>>> +   hantro_reg_write(vpu, &g2_start_bit, 0);
>>>>      hantro_reg_write(vpu, &g2_write_mvs_e, 1);
>> I've also crossed against "decoder_swreg_map_g2.xlsx" documentation, if you are
>> lucky to have access to that, and within swreg5 there is only g2_tempor_mvp_e
>> that is also shared, and its already being set in both drivers.
>>
>> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@...labora.com>
>>
>>
>>>>      hantro_write_addr(vpu, G2_TILE_SIZES_ADDR, ctx->hevc_dec.tile_sizes.dma);
>>