| Message ID | 01020191dc45365b-26b103cd-153a-4b74-a663-ed7beecc1713-000000@eu-west-1.amazonses.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: verisilicon: av1: Fix reference video buffer pointer assignment | expand |
Hi, Le mardi 10 septembre 2024 à 14:10 +0000, Benjamin Gaignard a écrit : > Always get new destination buffer for reference frame because nothing > garanty the one set previously is still valid or unused. Mind documenting here which tests got fixed with this change ? > > Fixes: 727a400686a2 ("media: verisilicon: Add Rockchip AV1 decoder") > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> > --- > .../media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c > index 372dfcd0fcd9..2b9a1047479c 100644 > --- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c > +++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c > @@ -161,8 +161,7 @@ static int rockchip_vpu981_av1_dec_frame_ref(struct hantro_ctx *ctx, > av1_dec->frame_refs[i].timestamp = timestamp; > av1_dec->frame_refs[i].frame_type = frame->frame_type; > av1_dec->frame_refs[i].order_hint = frame->order_hint; > - if (!av1_dec->frame_refs[i].vb2_ref) > - av1_dec->frame_refs[i].vb2_ref = hantro_get_dst_buf(ctx); > + av1_dec->frame_refs[i].vb2_ref = hantro_get_dst_buf(ctx); Good catch, would still be nice to improve the commit message. Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > for (j = 0; j < V4L2_AV1_TOTAL_REFS_PER_FRAME; j++) > av1_dec->frame_refs[i].order_hints[j] = frame->order_hints[j];
Le 10/09/2024 à 21:44, Nicolas Dufresne a écrit : > Hi, > > Le mardi 10 septembre 2024 à 14:10 +0000, Benjamin Gaignard a écrit : >> Always get new destination buffer for reference frame because nothing >> garanty the one set previously is still valid or unused. > Mind documenting here which tests got fixed with this change ? Only one from chromium test suite: https://chromium.googlesource.com/chromium/src/media/+/refs/heads/main/test/data/test-25fps.av1.ivf Fluster AV1 score remains unchanged. > >> Fixes: 727a400686a2 ("media: verisilicon: Add Rockchip AV1 decoder") >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> >> --- >> .../media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c >> index 372dfcd0fcd9..2b9a1047479c 100644 >> --- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c >> +++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c >> @@ -161,8 +161,7 @@ static int rockchip_vpu981_av1_dec_frame_ref(struct hantro_ctx *ctx, >> av1_dec->frame_refs[i].timestamp = timestamp; >> av1_dec->frame_refs[i].frame_type = frame->frame_type; >> av1_dec->frame_refs[i].order_hint = frame->order_hint; >> - if (!av1_dec->frame_refs[i].vb2_ref) >> - av1_dec->frame_refs[i].vb2_ref = hantro_get_dst_buf(ctx); >> + av1_dec->frame_refs[i].vb2_ref = hantro_get_dst_buf(ctx); > Good catch, would still be nice to improve the commit message. > > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > >> >> for (j = 0; j < V4L2_AV1_TOTAL_REFS_PER_FRAME; j++) >> av1_dec->frame_refs[i].order_hints[j] = frame->order_hints[j]; >
Le mercredi 11 septembre 2024 à 10:36 +0200, Benjamin Gaignard a écrit : > Le 10/09/2024 à 21:44, Nicolas Dufresne a écrit : > > Hi, > > > > Le mardi 10 septembre 2024 à 14:10 +0000, Benjamin Gaignard a écrit : > > > Always get new destination buffer for reference frame because nothing > > > garanty the one set previously is still valid or unused. > > Mind documenting here which tests got fixed with this change ? > > Only one from chromium test suite: > https://chromium.googlesource.com/chromium/src/media/+/refs/heads/main/test/data/test-25fps.av1.ivf > > Fluster AV1 score remains unchanged. We already integrated a lot of chromium tests in Fluster, would make sense to include some more. Was this one missed ? or added later ? > > > > > > Fixes: 727a400686a2 ("media: verisilicon: Add Rockchip AV1 decoder") > > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> > > > --- > > > .../media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c > > > index 372dfcd0fcd9..2b9a1047479c 100644 > > > --- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c > > > +++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c > > > @@ -161,8 +161,7 @@ static int rockchip_vpu981_av1_dec_frame_ref(struct hantro_ctx *ctx, > > > av1_dec->frame_refs[i].timestamp = timestamp; > > > av1_dec->frame_refs[i].frame_type = frame->frame_type; > > > av1_dec->frame_refs[i].order_hint = frame->order_hint; > > > - if (!av1_dec->frame_refs[i].vb2_ref) > > > - av1_dec->frame_refs[i].vb2_ref = hantro_get_dst_buf(ctx); > > > + av1_dec->frame_refs[i].vb2_ref = hantro_get_dst_buf(ctx); > > Good catch, would still be nice to improve the commit message. > > > > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > > > > > > for (j = 0; j < V4L2_AV1_TOTAL_REFS_PER_FRAME; j++) > > > av1_dec->frame_refs[i].order_hints[j] = frame->order_hints[j]; > >
diff --git a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c index 372dfcd0fcd9..2b9a1047479c 100644 --- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c +++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c @@ -161,8 +161,7 @@ static int rockchip_vpu981_av1_dec_frame_ref(struct hantro_ctx *ctx, av1_dec->frame_refs[i].timestamp = timestamp; av1_dec->frame_refs[i].frame_type = frame->frame_type; av1_dec->frame_refs[i].order_hint = frame->order_hint; - if (!av1_dec->frame_refs[i].vb2_ref) - av1_dec->frame_refs[i].vb2_ref = hantro_get_dst_buf(ctx); + av1_dec->frame_refs[i].vb2_ref = hantro_get_dst_buf(ctx); for (j = 0; j < V4L2_AV1_TOTAL_REFS_PER_FRAME; j++) av1_dec->frame_refs[i].order_hints[j] = frame->order_hints[j];
Always get new destination buffer for reference frame because nothing garanty the one set previously is still valid or unused. Fixes: 727a400686a2 ("media: verisilicon: Add Rockchip AV1 decoder") Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> --- .../media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)