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 with stateful vp9 decoder.
Signed-off-by: Irui Wang <[email protected]>
---
changed with v1:
- add a new define 'VP9_MAX_SUPER_FRAMES_NUM' for superframes.
---
.../mediatek/vcodec/decoder/vdec/vdec_vp9_if.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 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..039082f600c8 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
@@ -16,6 +16,7 @@
#include "../vdec_drv_base.h"
#include "../vdec_vpu_if.h"
+#define VP9_MAX_SUPER_FRAMES_NUM 8
#define VP9_SUPER_FRAME_BS_SZ 64
#define MAX_VP9_DPB_SIZE 9
@@ -133,11 +134,11 @@ struct vp9_sf_ref_fb {
*/
struct vdec_vp9_vsi {
unsigned char sf_bs_buf[VP9_SUPER_FRAME_BS_SZ];
- struct vp9_sf_ref_fb sf_ref_fb[VP9_MAX_FRM_BUF_NUM-1];
+ struct vp9_sf_ref_fb sf_ref_fb[VP9_MAX_SUPER_FRAMES_NUM];
int sf_next_ref_fb_idx;
unsigned int sf_frm_cnt;
- unsigned int sf_frm_offset[VP9_MAX_FRM_BUF_NUM-1];
- unsigned int sf_frm_sz[VP9_MAX_FRM_BUF_NUM-1];
+ unsigned int sf_frm_offset[VP9_MAX_SUPER_FRAMES_NUM];
+ unsigned int sf_frm_sz[VP9_MAX_SUPER_FRAMES_NUM];
unsigned int sf_frm_idx;
unsigned int sf_init;
struct vdec_fb fb;
@@ -526,7 +527,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 +736,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_SUPER_FRAMES_NUM) {
mtk_vdec_err(inst->ctx, "Invalid vsi->sf_frm_idx=%u.", vsi->sf_frm_idx);
return -EIO;
}
--
2.18.0
Hey Irui,
On 29.02.2024 11:02, Irui Wang wrote:
>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 with stateful vp9 decoder.
I find this commit message a bit confusing, you say that you couldn't
index the last superframe, but then you say that you modify the index
checking so that you can access the last sub-frame.
I would reword this section, here is my suggestion:
The VP9 bitstream uses superframes, which each contain 8 sub-frames,
enable accessing the last superframe by increasing the range of the
index validation as the maximum number of superframes is 8 and not 7.
The rest looks good as already mentioned by Nicolas.
Greetings,
Sebastian
>
>Signed-off-by: Irui Wang <[email protected]>
>---
>changed with v1:
> - add a new define 'VP9_MAX_SUPER_FRAMES_NUM' for superframes.
>---
> .../mediatek/vcodec/decoder/vdec/vdec_vp9_if.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 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..039082f600c8 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
>@@ -16,6 +16,7 @@
> #include "../vdec_drv_base.h"
> #include "../vdec_vpu_if.h"
>
>+#define VP9_MAX_SUPER_FRAMES_NUM 8
> #define VP9_SUPER_FRAME_BS_SZ 64
> #define MAX_VP9_DPB_SIZE 9
>
>@@ -133,11 +134,11 @@ struct vp9_sf_ref_fb {
> */
> struct vdec_vp9_vsi {
> unsigned char sf_bs_buf[VP9_SUPER_FRAME_BS_SZ];
>- struct vp9_sf_ref_fb sf_ref_fb[VP9_MAX_FRM_BUF_NUM-1];
>+ struct vp9_sf_ref_fb sf_ref_fb[VP9_MAX_SUPER_FRAMES_NUM];
> int sf_next_ref_fb_idx;
> unsigned int sf_frm_cnt;
>- unsigned int sf_frm_offset[VP9_MAX_FRM_BUF_NUM-1];
>- unsigned int sf_frm_sz[VP9_MAX_FRM_BUF_NUM-1];
>+ unsigned int sf_frm_offset[VP9_MAX_SUPER_FRAMES_NUM];
>+ unsigned int sf_frm_sz[VP9_MAX_SUPER_FRAMES_NUM];
> unsigned int sf_frm_idx;
> unsigned int sf_init;
> struct vdec_fb fb;
>@@ -526,7 +527,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 +736,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_SUPER_FRAMES_NUM) {
> mtk_vdec_err(inst->ctx, "Invalid vsi->sf_frm_idx=%u.", vsi->sf_frm_idx);
> return -EIO;
> }
>--
>2.18.0
>
>
Dear Sebastian Sir,
Thanks for your reviewing.
I will reword the commit message according to your suggestion, and send
a new patch v3. Thank you very much.
Thanks
Best Regards
On Fri, 2024-03-01 at 16:13 +0100, Sebastian Fricke wrote:
> Hey Irui,
>
> On 29.02.2024 11:02, Irui Wang wrote:
> > 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 with stateful vp9 decoder.
>
> I find this commit message a bit confusing, you say that you couldn't
> index the last superframe, but then you say that you modify the index
> checking so that you can access the last sub-frame.
>
> I would reword this section, here is my suggestion:
>
> The VP9 bitstream uses superframes, which each contain 8 sub-frames,
> enable accessing the last superframe by increasing the range of the
> index validation as the maximum number of superframes is 8 and not 7.
>
> The rest looks good as already mentioned by Nicolas.
>
> Greetings,
> Sebastian
>
> >
> > Signed-off-by: Irui Wang <[email protected]>
> > ---
> > changed with v1:
> > - add a new define 'VP9_MAX_SUPER_FRAMES_NUM' for superframes.
> > ---
> > .../mediatek/vcodec/decoder/vdec/vdec_vp9_if.c | 11 ++++++
> > -----
> > 1 file changed, 6 insertions(+), 5 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..039082f600c8 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
> > @@ -16,6 +16,7 @@
> > #include "../vdec_drv_base.h"
> > #include "../vdec_vpu_if.h"
> >
> > +#define VP9_MAX_SUPER_FRAMES_NUM 8
> > #define VP9_SUPER_FRAME_BS_SZ 64
> > #define MAX_VP9_DPB_SIZE 9
> >
> > @@ -133,11 +134,11 @@ struct vp9_sf_ref_fb {
> > */
> > struct vdec_vp9_vsi {
> > unsigned char sf_bs_buf[VP9_SUPER_FRAME_BS_SZ];
> > - struct vp9_sf_ref_fb sf_ref_fb[VP9_MAX_FRM_BUF_NUM-1];
> > + struct vp9_sf_ref_fb sf_ref_fb[VP9_MAX_SUPER_FRAMES_NUM];
> > int sf_next_ref_fb_idx;
> > unsigned int sf_frm_cnt;
> > - unsigned int sf_frm_offset[VP9_MAX_FRM_BUF_NUM-1];
> > - unsigned int sf_frm_sz[VP9_MAX_FRM_BUF_NUM-1];
> > + unsigned int sf_frm_offset[VP9_MAX_SUPER_FRAMES_NUM];
> > + unsigned int sf_frm_sz[VP9_MAX_SUPER_FRAMES_NUM];
> > unsigned int sf_frm_idx;
> > unsigned int sf_init;
> > struct vdec_fb fb;
> > @@ -526,7 +527,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 +736,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_SUPER_FRAMES_NUM) {
> > mtk_vdec_err(inst->ctx, "Invalid vsi->sf_frm_idx=%u.",
> > vsi->sf_frm_idx);
> > return -EIO;
> > }
> > --
> > 2.18.0
> >
> >