linux-kernel - Re: [PATCH v2] media: mediatek: vcodec: fix h264 cavlc bitstream fail

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:   Tue, 15 Nov 2022 02:00:24 +0000
From:   Yunfei Dong (董云飞) 
        <Yunfei.Dong@...iatek.com>
To:     "wenst@...omium.org" <wenst@...omium.org>,
        Tiffany Lin (林慧珊) 
        <tiffany.lin@...iatek.com>,
        "nicolas@...fresne.ca" <nicolas@...fresne.ca>,
        "angelogioacchino.delregno@...labora.com" 
        <angelogioacchino.delregno@...labora.com>,
        "benjamin.gaignard@...labora.com" <benjamin.gaignard@...labora.com>,
        "hverkuil-cisco@...all.nl" <hverkuil-cisco@...all.nl>
CC:     Xiaoyong Lu (卢小勇) 
        <Xiaoyong.Lu@...iatek.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        George Sun (孙林) <George.Sun@...iatek.com>,
        "frkoenig@...omium.org" <frkoenig@...omium.org>,
        "stevecho@...omium.org" <stevecho@...omium.org>,
        "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        "mchehab@...nel.org" <mchehab@...nel.org>,
        "daniel@...ll.ch" <daniel@...ll.ch>,
        Project_Global_Chrome_Upstream_Group 
        <Project_Global_Chrome_Upstream_Group@...iatek.com>,
        "hsinyi@...omium.org" <hsinyi@...omium.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "matthias.bgg@...il.com" <matthias.bgg@...il.com>
Subject: Re: [PATCH v2] media: mediatek: vcodec: fix h264 cavlc bitstream fail

Hi AngeloGioacchino,

Thanks for your detail suggestion.
On Mon, 2022-11-14 at 12:08 +0100, AngeloGioacchino Del Regno wrote:
> Il 18/10/22 13:41, Yunfei Dong ha scritto:
> > Some cavlc bistream will decode fail when the frame size is small
> > than
> 
> s/small/smaller/g

Will fix in next patch.
> 
> > 20 bytes. Need to add pending data at the end of the bitstream.
> > 
> > For the minimum size of mapped memory is 256 bytes(16x16), adding
> > four bytes data
> > won't lead to access unknown virtual memory.
> > 
> > Fixes: 59fba9eed5a7 ("media: mediatek: vcodec: support stateless
> > H.264 decoding for mt8192")
> > Signed-off-by: Yunfei Dong <yunfei.dong@...iatek.com>
> > ---
> > compared with v1:
> > - add detail comments for function: vdec_h264_insert_startcode.
> > - re-write commit message.
> > ---
> >   .../vcodec/vdec/vdec_h264_req_multi_if.c      | 32
> > +++++++++++++++++--
> >   1 file changed, 29 insertions(+), 3 deletions(-)
> > 
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_i
> > f.c
> > b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_i
> > f.c
> > index 4cc92700692b..18e048755d11 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_i
> > f.c
> > +++
> > b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_i
> > f.c
> > @@ -539,6 +539,29 @@ static int vdec_h264_slice_core_decode(struct
> > vdec_lat_buf *lat_buf)
> >   	return 0;
> >   }
> >   
> > +static void vdec_h264_insert_startcode(struct mtk_vcodec_dev
> > *vcodec_dev, unsigned char *buf,
> > +				       size_t *bs_size, struct
> > mtk_h264_pps_param *pps)
> > +{
> > +	struct device *dev = &vcodec_dev->plat_dev->dev;
> > +
> > +	/* Need to add pending data at the end of bitstream when bs_sz
> > is small than
> > +	 * 20 bytes for cavlc bitstream, or lat will decode fail. This
> > pending data is
> > +	 * useful for mt8192 and mt8195 platform.
> 
> What is the reason why other SoCs don't need this?
> 
For the hardware not add this feature, and will add in the future Soc.
> > +	 *
> > +	 * cavlc bitstream when entropy_coding_mode_flag is false.
> > +	 */
> > +	if (pps->entropy_coding_mode_flag || *bs_size > 20 ||
> > +	    !(of_device_is_compatible(dev->of_node, "mediatek,mt8192-
> > vcodec-dec") ||
> > +	    of_device_is_compatible(dev->of_node, "mediatek,mt8195-
> > vcodec-dec")))
> 
> I'm not comfortable seeing of_device_is_compatible... this list will
> grow whenever
> a new SoC needing this appears.
> Please think about a good name for a flag/quirk, or a bool, in the
> platform data
> for these two SoCs and use it.
> 
For this feature only need to add in these two Socs, and won't grow
anymore. So just want to use compatible to separate, not add one flags.

So you think that using one flag to separate much better?

Best Regards,
Yunfei Dong

> Regards,
> Angelo
> 
>