* [PATCH] media: mediatek: vcodec: Handle VP9 superframe bitstream with 8 sub-frames
@ 2024-02-18 11:59 ` Irui Wang
0 siblings, 0 replies; 6+ messages in thread
From: Irui Wang @ 2024-02-18 11:59 UTC (permalink / raw)
To: Hans Verkuil, Mauro Carvalho Chehab, Matthias Brugger,
angelogioacchino.delregno, Tiffany Lin, Yunfei Dong,
Maoguang Meng, Longfei Wang
Cc: Irui Wang, Project_Global_Chrome_Upstream_Group, linux-media,
linux-arm-kernel, linux-mediatek
The VP9 bitstream has 8 sub-frames into one superframe, the superframe
index validate failed when reach 8, modify the index checking so that the
last sub-frame can be decoded normally.
Signed-off-by: Irui Wang <irui.wang@mediatek.com>
---
.../media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
index 55355fa70090..4a9ced7348ee 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
@@ -526,7 +526,7 @@ static void vp9_swap_frm_bufs(struct vdec_vp9_inst *inst)
/* if this super frame and it is not last sub-frame, get next fb for
* sub-frame decode
*/
- if (vsi->sf_frm_cnt > 0 && vsi->sf_frm_idx != vsi->sf_frm_cnt - 1)
+ if (vsi->sf_frm_cnt > 0 && vsi->sf_frm_idx != vsi->sf_frm_cnt)
vsi->sf_next_ref_fb_idx = vp9_get_sf_ref_fb(inst);
}
@@ -735,7 +735,7 @@ static void get_free_fb(struct vdec_vp9_inst *inst, struct vdec_fb **out_fb)
static int validate_vsi_array_indexes(struct vdec_vp9_inst *inst,
struct vdec_vp9_vsi *vsi) {
- if (vsi->sf_frm_idx >= VP9_MAX_FRM_BUF_NUM - 1) {
+ if (vsi->sf_frm_idx >= VP9_MAX_FRM_BUF_NUM) {
mtk_vdec_err(inst->ctx, "Invalid vsi->sf_frm_idx=%u.", vsi->sf_frm_idx);
return -EIO;
}
--
2.18.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] media: mediatek: vcodec: Handle VP9 superframe bitstream with 8 sub-frames
@ 2024-02-18 11:59 ` Irui Wang
0 siblings, 0 replies; 6+ messages in thread
From: Irui Wang @ 2024-02-18 11:59 UTC (permalink / raw)
To: Hans Verkuil, Mauro Carvalho Chehab, Matthias Brugger,
angelogioacchino.delregno, Tiffany Lin, Yunfei Dong,
Maoguang Meng, Longfei Wang
Cc: Irui Wang, Project_Global_Chrome_Upstream_Group, linux-media,
linux-arm-kernel, linux-mediatek
The VP9 bitstream has 8 sub-frames into one superframe, the superframe
index validate failed when reach 8, modify the index checking so that the
last sub-frame can be decoded normally.
Signed-off-by: Irui Wang <irui.wang@mediatek.com>
---
.../media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
index 55355fa70090..4a9ced7348ee 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
@@ -526,7 +526,7 @@ static void vp9_swap_frm_bufs(struct vdec_vp9_inst *inst)
/* if this super frame and it is not last sub-frame, get next fb for
* sub-frame decode
*/
- if (vsi->sf_frm_cnt > 0 && vsi->sf_frm_idx != vsi->sf_frm_cnt - 1)
+ if (vsi->sf_frm_cnt > 0 && vsi->sf_frm_idx != vsi->sf_frm_cnt)
vsi->sf_next_ref_fb_idx = vp9_get_sf_ref_fb(inst);
}
@@ -735,7 +735,7 @@ static void get_free_fb(struct vdec_vp9_inst *inst, struct vdec_fb **out_fb)
static int validate_vsi_array_indexes(struct vdec_vp9_inst *inst,
struct vdec_vp9_vsi *vsi) {
- if (vsi->sf_frm_idx >= VP9_MAX_FRM_BUF_NUM - 1) {
+ if (vsi->sf_frm_idx >= VP9_MAX_FRM_BUF_NUM) {
mtk_vdec_err(inst->ctx, "Invalid vsi->sf_frm_idx=%u.", vsi->sf_frm_idx);
return -EIO;
}
--
2.18.0
_______________________________________________
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] 6+ messages in thread
* Re: [PATCH] media: mediatek: vcodec: Handle VP9 superframe bitstream with 8 sub-frames
2024-02-18 11:59 ` Irui Wang
@ 2024-02-19 21:09 ` Nicolas Dufresne
-1 siblings, 0 replies; 6+ messages in thread
From: Nicolas Dufresne @ 2024-02-19 21:09 UTC (permalink / raw)
To: Irui Wang, Hans Verkuil, Mauro Carvalho Chehab, Matthias Brugger,
angelogioacchino.delregno, Tiffany Lin, Yunfei Dong,
Maoguang Meng, Longfei Wang
Cc: Project_Global_Chrome_Upstream_Group, linux-media,
linux-arm-kernel, linux-mediatek
Hi,
Le dimanche 18 février 2024 à 19:59 +0800, Irui Wang a écrit :
> The VP9 bitstream has 8 sub-frames into one superframe, the superframe
> index validate failed when reach 8, modify the index checking so that the
> last sub-frame can be decoded normally.
When I first saw this patch I was concerned, since we don't allow bundling super
frame into the stateless API, but now I realize that this is for the stateful
decoder. Perhaps you can help me and possibly other reviewers with simply
stating that this is for stateful decoders.
>
> Signed-off-by: Irui Wang <irui.wang@mediatek.com>
> ---
> .../media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> index 55355fa70090..4a9ced7348ee 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> @@ -526,7 +526,7 @@ static void vp9_swap_frm_bufs(struct vdec_vp9_inst *inst)
> /* if this super frame and it is not last sub-frame, get next fb for
> * sub-frame decode
> */
> - if (vsi->sf_frm_cnt > 0 && vsi->sf_frm_idx != vsi->sf_frm_cnt - 1)
> + if (vsi->sf_frm_cnt > 0 && vsi->sf_frm_idx != vsi->sf_frm_cnt)
> vsi->sf_next_ref_fb_idx = vp9_get_sf_ref_fb(inst);
> }
>
> @@ -735,7 +735,7 @@ static void get_free_fb(struct vdec_vp9_inst *inst, struct vdec_fb **out_fb)
>
> static int validate_vsi_array_indexes(struct vdec_vp9_inst *inst,
> struct vdec_vp9_vsi *vsi) {
> - if (vsi->sf_frm_idx >= VP9_MAX_FRM_BUF_NUM - 1) {
> + if (vsi->sf_frm_idx >= VP9_MAX_FRM_BUF_NUM) {
nit: I'd propose to define a new maximum (contractions allowed):
#define VP9_MAX_NUM_SUPER_FRAMES 8
This way you can revisit bunch of `VP9_MAX_FRM_BUF_NUM-1`, and make the overall
code a bit more human readable. There is no relation between VP9_MAX_FRM_BUF_NUM
and this maximum. The limits simply comes from the fact
frames_in_superframe_minus_1 is expressed with 3 bits.
regards,
Nicolas
p.s. your change looks good otherwise.
> mtk_vdec_err(inst->ctx, "Invalid vsi->sf_frm_idx=%u.", vsi->sf_frm_idx);
> return -EIO;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] media: mediatek: vcodec: Handle VP9 superframe bitstream with 8 sub-frames
@ 2024-02-19 21:09 ` Nicolas Dufresne
0 siblings, 0 replies; 6+ messages in thread
From: Nicolas Dufresne @ 2024-02-19 21:09 UTC (permalink / raw)
To: Irui Wang, Hans Verkuil, Mauro Carvalho Chehab, Matthias Brugger,
angelogioacchino.delregno, Tiffany Lin, Yunfei Dong,
Maoguang Meng, Longfei Wang
Cc: Project_Global_Chrome_Upstream_Group, linux-media,
linux-arm-kernel, linux-mediatek
Hi,
Le dimanche 18 février 2024 à 19:59 +0800, Irui Wang a écrit :
> The VP9 bitstream has 8 sub-frames into one superframe, the superframe
> index validate failed when reach 8, modify the index checking so that the
> last sub-frame can be decoded normally.
When I first saw this patch I was concerned, since we don't allow bundling super
frame into the stateless API, but now I realize that this is for the stateful
decoder. Perhaps you can help me and possibly other reviewers with simply
stating that this is for stateful decoders.
>
> Signed-off-by: Irui Wang <irui.wang@mediatek.com>
> ---
> .../media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> index 55355fa70090..4a9ced7348ee 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> @@ -526,7 +526,7 @@ static void vp9_swap_frm_bufs(struct vdec_vp9_inst *inst)
> /* if this super frame and it is not last sub-frame, get next fb for
> * sub-frame decode
> */
> - if (vsi->sf_frm_cnt > 0 && vsi->sf_frm_idx != vsi->sf_frm_cnt - 1)
> + if (vsi->sf_frm_cnt > 0 && vsi->sf_frm_idx != vsi->sf_frm_cnt)
> vsi->sf_next_ref_fb_idx = vp9_get_sf_ref_fb(inst);
> }
>
> @@ -735,7 +735,7 @@ static void get_free_fb(struct vdec_vp9_inst *inst, struct vdec_fb **out_fb)
>
> static int validate_vsi_array_indexes(struct vdec_vp9_inst *inst,
> struct vdec_vp9_vsi *vsi) {
> - if (vsi->sf_frm_idx >= VP9_MAX_FRM_BUF_NUM - 1) {
> + if (vsi->sf_frm_idx >= VP9_MAX_FRM_BUF_NUM) {
nit: I'd propose to define a new maximum (contractions allowed):
#define VP9_MAX_NUM_SUPER_FRAMES 8
This way you can revisit bunch of `VP9_MAX_FRM_BUF_NUM-1`, and make the overall
code a bit more human readable. There is no relation between VP9_MAX_FRM_BUF_NUM
and this maximum. The limits simply comes from the fact
frames_in_superframe_minus_1 is expressed with 3 bits.
regards,
Nicolas
p.s. your change looks good otherwise.
> mtk_vdec_err(inst->ctx, "Invalid vsi->sf_frm_idx=%u.", vsi->sf_frm_idx);
> return -EIO;
> }
_______________________________________________
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] 6+ messages in thread
* Re: [PATCH] media: mediatek: vcodec: Handle VP9 superframe bitstream with 8 sub-frames
2024-02-19 21:09 ` Nicolas Dufresne
@ 2024-02-22 2:49 ` Irui Wang (王瑞)
-1 siblings, 0 replies; 6+ messages in thread
From: Irui Wang (王瑞) @ 2024-02-22 2:49 UTC (permalink / raw)
To: nicolas, Longfei Wang (王龙飞),
Tiffany Lin (林慧珊),
Maoguang Meng (孟毛广),
mchehab, Yunfei Dong (董云飞),
hverkuil-cisco, matthias.bgg, angelogioacchino.delregno
Cc: linux-arm-kernel, linux-media, linux-mediatek,
Project_Global_Chrome_Upstream_Group
Dear Nicolas,
Thanks for your reviewing.
Yes, this patch is just for the VP9 stateful decoder process, so the
super frame is handled in stateful driver.
On Mon, 2024-02-19 at 16:09 -0500, Nicolas Dufresne wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> Hi,
>
> Le dimanche 18 février 2024 à 19:59 +0800, Irui Wang a écrit :
> > The VP9 bitstream has 8 sub-frames into one superframe, the
> superframe
> > index validate failed when reach 8, modify the index checking so
> that the
> > last sub-frame can be decoded normally.
>
> When I first saw this patch I was concerned, since we don't allow
> bundling super
> frame into the stateless API, but now I realize that this is for the
> stateful
> decoder. Perhaps you can help me and possibly other reviewers with
> simply
> stating that this is for stateful decoders.
>
> >
> > Signed-off-by: Irui Wang <irui.wang@mediatek.com>
> > ---
> > .../media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c | 4
> ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git
> a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> > index 55355fa70090..4a9ced7348ee 100644
> > ---
> a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> > +++
> b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> > @@ -526,7 +526,7 @@ static void vp9_swap_frm_bufs(struct
> vdec_vp9_inst *inst)
> > /* if this super frame and it is not last sub-frame, get next fb
> for
> > * sub-frame decode
> > */
> > -if (vsi->sf_frm_cnt > 0 && vsi->sf_frm_idx != vsi->sf_frm_cnt - 1)
> > +if (vsi->sf_frm_cnt > 0 && vsi->sf_frm_idx != vsi->sf_frm_cnt)
> > vsi->sf_next_ref_fb_idx = vp9_get_sf_ref_fb(inst);
> > }
> >
> > @@ -735,7 +735,7 @@ static void get_free_fb(struct vdec_vp9_inst
> *inst, struct vdec_fb **out_fb)
> >
> > static int validate_vsi_array_indexes(struct vdec_vp9_inst *inst,
> > struct vdec_vp9_vsi *vsi) {
> > -if (vsi->sf_frm_idx >= VP9_MAX_FRM_BUF_NUM - 1) {
> > +if (vsi->sf_frm_idx >= VP9_MAX_FRM_BUF_NUM) {
>
> nit: I'd propose to define a new maximum (contractions allowed):
>
> #define VP9_MAX_NUM_SUPER_FRAMES 8
>
> This way you can revisit bunch of `VP9_MAX_FRM_BUF_NUM-1`, and make
> the overall
> code a bit more human readable. There is no relation between
> VP9_MAX_FRM_BUF_NUM
> and this maximum. The limits simply comes from the fact
> frames_in_superframe_minus_1 is expressed with 3 bits.
>
> regards,
> Nicolas
>
Yes, define a new maximum makes code more readable, we will check it.
Thanks
Best Regards
> p.s. your change looks good otherwise.
>
> > mtk_vdec_err(inst->ctx, "Invalid vsi->sf_frm_idx=%u.", vsi-
> >sf_frm_idx);
> > return -EIO;
> > }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] media: mediatek: vcodec: Handle VP9 superframe bitstream with 8 sub-frames
@ 2024-02-22 2:49 ` Irui Wang (王瑞)
0 siblings, 0 replies; 6+ messages in thread
From: Irui Wang (王瑞) @ 2024-02-22 2:49 UTC (permalink / raw)
To: nicolas, Longfei Wang (王龙飞),
Tiffany Lin (林慧珊),
Maoguang Meng (孟毛广),
mchehab, Yunfei Dong (董云飞),
hverkuil-cisco, matthias.bgg, angelogioacchino.delregno
Cc: linux-arm-kernel, linux-media, linux-mediatek,
Project_Global_Chrome_Upstream_Group
Dear Nicolas,
Thanks for your reviewing.
Yes, this patch is just for the VP9 stateful decoder process, so the
super frame is handled in stateful driver.
On Mon, 2024-02-19 at 16:09 -0500, Nicolas Dufresne wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> Hi,
>
> Le dimanche 18 février 2024 à 19:59 +0800, Irui Wang a écrit :
> > The VP9 bitstream has 8 sub-frames into one superframe, the
> superframe
> > index validate failed when reach 8, modify the index checking so
> that the
> > last sub-frame can be decoded normally.
>
> When I first saw this patch I was concerned, since we don't allow
> bundling super
> frame into the stateless API, but now I realize that this is for the
> stateful
> decoder. Perhaps you can help me and possibly other reviewers with
> simply
> stating that this is for stateful decoders.
>
> >
> > Signed-off-by: Irui Wang <irui.wang@mediatek.com>
> > ---
> > .../media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c | 4
> ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git
> a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> > index 55355fa70090..4a9ced7348ee 100644
> > ---
> a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> > +++
> b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_if.c
> > @@ -526,7 +526,7 @@ static void vp9_swap_frm_bufs(struct
> vdec_vp9_inst *inst)
> > /* if this super frame and it is not last sub-frame, get next fb
> for
> > * sub-frame decode
> > */
> > -if (vsi->sf_frm_cnt > 0 && vsi->sf_frm_idx != vsi->sf_frm_cnt - 1)
> > +if (vsi->sf_frm_cnt > 0 && vsi->sf_frm_idx != vsi->sf_frm_cnt)
> > vsi->sf_next_ref_fb_idx = vp9_get_sf_ref_fb(inst);
> > }
> >
> > @@ -735,7 +735,7 @@ static void get_free_fb(struct vdec_vp9_inst
> *inst, struct vdec_fb **out_fb)
> >
> > static int validate_vsi_array_indexes(struct vdec_vp9_inst *inst,
> > struct vdec_vp9_vsi *vsi) {
> > -if (vsi->sf_frm_idx >= VP9_MAX_FRM_BUF_NUM - 1) {
> > +if (vsi->sf_frm_idx >= VP9_MAX_FRM_BUF_NUM) {
>
> nit: I'd propose to define a new maximum (contractions allowed):
>
> #define VP9_MAX_NUM_SUPER_FRAMES 8
>
> This way you can revisit bunch of `VP9_MAX_FRM_BUF_NUM-1`, and make
> the overall
> code a bit more human readable. There is no relation between
> VP9_MAX_FRM_BUF_NUM
> and this maximum. The limits simply comes from the fact
> frames_in_superframe_minus_1 is expressed with 3 bits.
>
> regards,
> Nicolas
>
Yes, define a new maximum makes code more readable, we will check it.
Thanks
Best Regards
> p.s. your change looks good otherwise.
>
> > mtk_vdec_err(inst->ctx, "Invalid vsi->sf_frm_idx=%u.", vsi-
> >sf_frm_idx);
> > return -EIO;
> > }
>
_______________________________________________
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] 6+ messages in thread
end of thread, other threads:[~2024-02-22 2:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-18 11:59 [PATCH] media: mediatek: vcodec: Handle VP9 superframe bitstream with 8 sub-frames Irui Wang
2024-02-18 11:59 ` Irui Wang
2024-02-19 21:09 ` Nicolas Dufresne
2024-02-19 21:09 ` Nicolas Dufresne
2024-02-22 2:49 ` Irui Wang (王瑞)
2024-02-22 2:49 ` Irui Wang (王瑞)
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.