[PATCH] media: mediatek: vcodec: Handle VP9 superframe bitstream with 8 sub-frames

Irui Wang (王瑞) Irui.Wang at mediatek.com
Wed Feb 21 18:49:06 PST 2024


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 at 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;
> >  }
> 


More information about the Linux-mediatek mailing list