linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] media: mediatek: vcodec: fix h264 cavlc bitstream fail
@ 2022-10-17  9:02 Yunfei Dong
  2022-10-18  9:55 ` Chen-Yu Tsai
  0 siblings, 1 reply; 3+ messages in thread
From: Yunfei Dong @ 2022-10-17  9:02 UTC (permalink / raw)
  To: Yunfei Dong, Chen-Yu Tsai, Nicolas Dufresne, Hans Verkuil,
	AngeloGioacchino Del Regno, Benjamin Gaignard, Tiffany Lin
  Cc: Mauro Carvalho Chehab, Matthias Brugger, George Sun, Xiaoyong Lu,
	Hsin-Yi Wang, Fritz Koenig, Daniel Vetter, Steve Cho,
	linux-media, devicetree, linux-kernel, linux-arm-kernel,
	linux-mediatek, Project_Global_Chrome_Upstream_Group

Some cavlc bistream will decode fail when the frame size is small than
20 bytes. Need to add pending data at the end of the bitstream.

For the size of mapped memory is at least one page, 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@mediatek.com>
---
 .../vcodec/vdec/vdec_h264_req_multi_if.c      | 27 ++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_if.c b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_if.c
index 4cc92700692b..c1583dddcb04 100644
--- a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_if.c
+++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_if.c
@@ -539,6 +539,24 @@ 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;
+
+	/* 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")))
+		return;
+
+	buf[*bs_size] = 0;
+	buf[*bs_size + 1] = 0;
+	buf[*bs_size + 2] = 1;
+	buf[*bs_size + 3] = 0xff;
+	(*bs_size) += 4;
+}
+
 static int vdec_h264_slice_lat_decode(void *h_vdec, struct mtk_vcodec_mem *bs,
 				      struct vdec_fb *fb, bool *res_chg)
 {
@@ -582,9 +600,6 @@ static int vdec_h264_slice_lat_decode(void *h_vdec, struct mtk_vcodec_mem *bs,
 	}
 
 	inst->vsi->dec.nal_info = buf[nal_start_idx];
-	inst->vsi->dec.bs_buf_addr = (u64)bs->dma_addr;
-	inst->vsi->dec.bs_buf_size = bs->size;
-
 	lat_buf->src_buf_req = src_buf_info->m2m_buf.vb.vb2_buf.req_obj.req;
 	v4l2_m2m_buf_copy_metadata(&src_buf_info->m2m_buf.vb, &lat_buf->ts_info, true);
 
@@ -592,6 +607,12 @@ static int vdec_h264_slice_lat_decode(void *h_vdec, struct mtk_vcodec_mem *bs,
 	if (err)
 		goto err_free_fb_out;
 
+	vdec_h264_insert_startcode(inst->ctx->dev, buf, &bs->size,
+				   &share_info->h264_slice_params.pps);
+
+	inst->vsi->dec.bs_buf_addr = (uint64_t)bs->dma_addr;
+	inst->vsi->dec.bs_buf_size = bs->size;
+
 	*res_chg = inst->resolution_changed;
 	if (inst->resolution_changed) {
 		mtk_vcodec_debug(inst, "- resolution changed -");
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] media: mediatek: vcodec: fix h264 cavlc bitstream fail
  2022-10-17  9:02 [PATCH] media: mediatek: vcodec: fix h264 cavlc bitstream fail Yunfei Dong
@ 2022-10-18  9:55 ` Chen-Yu Tsai
  2022-10-18 11:44   ` yunfei.dong
  0 siblings, 1 reply; 3+ messages in thread
From: Chen-Yu Tsai @ 2022-10-18  9:55 UTC (permalink / raw)
  To: Yunfei Dong
  Cc: Nicolas Dufresne, Hans Verkuil, AngeloGioacchino Del Regno,
	Benjamin Gaignard, Tiffany Lin, Mauro Carvalho Chehab,
	Matthias Brugger, George Sun, Xiaoyong Lu, Hsin-Yi Wang,
	Fritz Koenig, Daniel Vetter, Steve Cho, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

On Mon, Oct 17, 2022 at 5:02 PM Yunfei Dong <yunfei.dong@mediatek.com> wrote:
>
> Some cavlc bistream will decode fail when the frame size is small than
> 20 bytes. Need to add pending data at the end of the bitstream.

"magic terminating pattern" instead of "pending data"?

> For the size of mapped memory is at least one page, adding four bytes data
> won't lead to access unknown virtual memory.

Actually we can narrow this down a bit. The minimum dimension (16x16)
sets the minimum size of the buffer at 256 bytes.

> Fixes: 59fba9eed5a7 ("media: mediatek: vcodec: support stateless H.264 decoding for mt8192")
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> ---
>  .../vcodec/vdec/vdec_h264_req_multi_if.c      | 27 ++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_if.c b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_if.c
> index 4cc92700692b..c1583dddcb04 100644
> --- a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_multi_if.c
> @@ -539,6 +539,24 @@ 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;
> +
> +       /* 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")))
> +               return;
> +

There should be a comment here describing what is added.


ChenYu

> +       buf[*bs_size] = 0;
> +       buf[*bs_size + 1] = 0;
> +       buf[*bs_size + 2] = 1;
> +       buf[*bs_size + 3] = 0xff;
> +       (*bs_size) += 4;
> +}
> +
>  static int vdec_h264_slice_lat_decode(void *h_vdec, struct mtk_vcodec_mem *bs,
>                                       struct vdec_fb *fb, bool *res_chg)
>  {
> @@ -582,9 +600,6 @@ static int vdec_h264_slice_lat_decode(void *h_vdec, struct mtk_vcodec_mem *bs,
>         }
>
>         inst->vsi->dec.nal_info = buf[nal_start_idx];
> -       inst->vsi->dec.bs_buf_addr = (u64)bs->dma_addr;
> -       inst->vsi->dec.bs_buf_size = bs->size;
> -
>         lat_buf->src_buf_req = src_buf_info->m2m_buf.vb.vb2_buf.req_obj.req;
>         v4l2_m2m_buf_copy_metadata(&src_buf_info->m2m_buf.vb, &lat_buf->ts_info, true);
>
> @@ -592,6 +607,12 @@ static int vdec_h264_slice_lat_decode(void *h_vdec, struct mtk_vcodec_mem *bs,
>         if (err)
>                 goto err_free_fb_out;
>
> +       vdec_h264_insert_startcode(inst->ctx->dev, buf, &bs->size,
> +                                  &share_info->h264_slice_params.pps);
> +
> +       inst->vsi->dec.bs_buf_addr = (uint64_t)bs->dma_addr;
> +       inst->vsi->dec.bs_buf_size = bs->size;
> +
>         *res_chg = inst->resolution_changed;
>         if (inst->resolution_changed) {
>                 mtk_vcodec_debug(inst, "- resolution changed -");
> --
> 2.25.1
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] media: mediatek: vcodec: fix h264 cavlc bitstream fail
  2022-10-18  9:55 ` Chen-Yu Tsai
@ 2022-10-18 11:44   ` yunfei.dong
  0 siblings, 0 replies; 3+ messages in thread
From: yunfei.dong @ 2022-10-18 11:44 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Nicolas Dufresne, Hans Verkuil, AngeloGioacchino Del Regno,
	Benjamin Gaignard, Tiffany Lin, Mauro Carvalho Chehab,
	Matthias Brugger, George Sun, Xiaoyong Lu, Hsin-Yi Wang,
	Fritz Koenig, Daniel Vetter, Steve Cho, linux-media, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek,
	Project_Global_Chrome_Upstream_Group

Hi Chen-Yu,

Thanks for your suggestion.
On Tue, 2022-10-18 at 17:55 +0800, Chen-Yu Tsai wrote:
> On Mon, Oct 17, 2022 at 5:02 PM Yunfei Dong <yunfei.dong@mediatek.com
> > wrote:
> > 
> > Some cavlc bistream will decode fail when the frame size is small
> > than
> > 20 bytes. Need to add pending data at the end of the bitstream.
> 
> "magic terminating pattern" instead of "pending data"?
> 
> > For the size of mapped memory is at least one page, adding four
> > bytes data
> > won't lead to access unknown virtual memory.
> 
> Actually we can narrow this down a bit. The minimum dimension (16x16)
> sets the minimum size of the buffer at 256 bytes.
> 
Fix in patch v2.
> > Fixes: 59fba9eed5a7 ("media: mediatek: vcodec: support stateless
> > H.264 decoding for mt8192")
> > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> > ---
> >  .../vcodec/vdec/vdec_h264_req_multi_if.c      | 27
> > ++++++++++++++++---
> >  1 file changed, 24 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..c1583dddcb04 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,24 @@ 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;
> > +
> > +       /* 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")))
> > +               return;
> > +
> 
> There should be a comment here describing what is added.
> 
Fix in patch v2.
> 
> ChenYu
> 
Best Regards,
Yunfei Dong
> > +       buf[*bs_size] = 0;
> > +       buf[*bs_size + 1] = 0;
> > +       buf[*bs_size + 2] = 1;
> > +       buf[*bs_size + 3] = 0xff;
> > +       (*bs_size) += 4;
> > +}
> > +
> >  static int vdec_h264_slice_lat_decode(void *h_vdec, struct
> > mtk_vcodec_mem *bs,
> >                                       struct vdec_fb *fb, bool
> > *res_chg)
> >  {
> > @@ -582,9 +600,6 @@ static int vdec_h264_slice_lat_decode(void
> > *h_vdec, struct mtk_vcodec_mem *bs,
> >         }
> > 
> >         inst->vsi->dec.nal_info = buf[nal_start_idx];
> > -       inst->vsi->dec.bs_buf_addr = (u64)bs->dma_addr;
> > -       inst->vsi->dec.bs_buf_size = bs->size;
> > -
> >         lat_buf->src_buf_req = src_buf_info-
> > >m2m_buf.vb.vb2_buf.req_obj.req;
> >         v4l2_m2m_buf_copy_metadata(&src_buf_info->m2m_buf.vb,
> > &lat_buf->ts_info, true);
> > 
> > @@ -592,6 +607,12 @@ static int vdec_h264_slice_lat_decode(void
> > *h_vdec, struct mtk_vcodec_mem *bs,
> >         if (err)
> >                 goto err_free_fb_out;
> > 
> > +       vdec_h264_insert_startcode(inst->ctx->dev, buf, &bs->size,
> > +                                  &share_info-
> > >h264_slice_params.pps);
> > +
> > +       inst->vsi->dec.bs_buf_addr = (uint64_t)bs->dma_addr;
> > +       inst->vsi->dec.bs_buf_size = bs->size;
> > +
> >         *res_chg = inst->resolution_changed;
> >         if (inst->resolution_changed) {
> >                 mtk_vcodec_debug(inst, "- resolution changed -");
> > --
> > 2.25.1
> > 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-10-18 11:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-17  9:02 [PATCH] media: mediatek: vcodec: fix h264 cavlc bitstream fail Yunfei Dong
2022-10-18  9:55 ` Chen-Yu Tsai
2022-10-18 11:44   ` yunfei.dong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).