PoC shall be int the range of -2^31 to 2^31 -1
(HEVC spec section 8.3.1 Decoding process for picture order count).
The current way to know if an entry in reference picture array is free
is to test if PoC = UNUSED_REF. Since UNUSED_REF is defined as '-1' that
could lead to decode issue if one PoC also equal '-1'.
PoC with value = '-1' exists in conformance test SLIST_B_Sony_9.
Change the way unused entries are managed in reference pictures array to
avoid using PoC to detect then.
This patch doesn't change fluster HEVC score.
Signed-off-by: Benjamin Gaignard <[email protected]>
---
.../staging/media/hantro/hantro_g2_hevc_dec.c | 6 ++---
drivers/staging/media/hantro/hantro_hevc.c | 27 +++----------------
drivers/staging/media/hantro/hantro_hw.h | 2 +-
3 files changed, 6 insertions(+), 29 deletions(-)
diff --git a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c b/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
index 0a8c01ff2fa7..b7835bbf5e98 100644
--- a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
+++ b/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
@@ -473,8 +473,8 @@ static int set_ref(struct hantro_ctx *ctx)
set_ref_pic_list(ctx);
- /* We will only keep the references picture that are still used */
- ctx->hevc_dec.ref_bufs_used = 0;
+ /* We will only keep the references pictures that are still used */
+ hantro_hevc_ref_init(ctx);
/* Set up addresses of DPB buffers */
dpb_longterm_e = 0;
@@ -515,8 +515,6 @@ static int set_ref(struct hantro_ctx *ctx)
hantro_write_addr(vpu, G2_OUT_CHROMA_ADDR, chroma_addr);
hantro_write_addr(vpu, G2_OUT_MV_ADDR, mv_addr);
- hantro_hevc_ref_remove_unused(ctx);
-
for (; i < V4L2_HEVC_DPB_ENTRIES_NUM_MAX; i++) {
hantro_write_addr(vpu, G2_REF_LUMA_ADDR(i), 0);
hantro_write_addr(vpu, G2_REF_CHROMA_ADDR(i), 0);
diff --git a/drivers/staging/media/hantro/hantro_hevc.c b/drivers/staging/media/hantro/hantro_hevc.c
index 7d4b1d72255c..7fdec50dc853 100644
--- a/drivers/staging/media/hantro/hantro_hevc.c
+++ b/drivers/staging/media/hantro/hantro_hevc.c
@@ -25,15 +25,11 @@
#define MAX_TILE_COLS 20
#define MAX_TILE_ROWS 22
-#define UNUSED_REF -1
-
-static void hantro_hevc_ref_init(struct hantro_ctx *ctx)
+void hantro_hevc_ref_init(struct hantro_ctx *ctx)
{
struct hantro_hevc_dec_hw_ctx *hevc_dec = &ctx->hevc_dec;
- int i;
- for (i = 0; i < NUM_REF_PICTURES; i++)
- hevc_dec->ref_bufs_poc[i] = UNUSED_REF;
+ hevc_dec->ref_bufs_used = 0;
}
dma_addr_t hantro_hevc_get_ref_buf(struct hantro_ctx *ctx,
@@ -60,7 +56,7 @@ int hantro_hevc_add_ref_buf(struct hantro_ctx *ctx, int poc, dma_addr_t addr)
/* Add a new reference buffer */
for (i = 0; i < NUM_REF_PICTURES; i++) {
- if (hevc_dec->ref_bufs_poc[i] == UNUSED_REF) {
+ if (!(hevc_dec->ref_bufs_used & 1 << i)) {
hevc_dec->ref_bufs_used |= 1 << i;
hevc_dec->ref_bufs_poc[i] = poc;
hevc_dec->ref_bufs[i].dma = addr;
@@ -71,23 +67,6 @@ int hantro_hevc_add_ref_buf(struct hantro_ctx *ctx, int poc, dma_addr_t addr)
return -EINVAL;
}
-void hantro_hevc_ref_remove_unused(struct hantro_ctx *ctx)
-{
- struct hantro_hevc_dec_hw_ctx *hevc_dec = &ctx->hevc_dec;
- int i;
-
- /* Just tag buffer as unused, do not free them */
- for (i = 0; i < NUM_REF_PICTURES; i++) {
- if (hevc_dec->ref_bufs_poc[i] == UNUSED_REF)
- continue;
-
- if (hevc_dec->ref_bufs_used & (1 << i))
- continue;
-
- hevc_dec->ref_bufs_poc[i] = UNUSED_REF;
- }
-}
-
static int tile_buffer_reallocate(struct hantro_ctx *ctx)
{
struct hantro_dev *vpu = ctx->dev;
diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
index 9f31cce609d6..5de558386179 100644
--- a/drivers/staging/media/hantro/hantro_hw.h
+++ b/drivers/staging/media/hantro/hantro_hw.h
@@ -337,9 +337,9 @@ int hantro_hevc_dec_init(struct hantro_ctx *ctx);
void hantro_hevc_dec_exit(struct hantro_ctx *ctx);
int hantro_g2_hevc_dec_run(struct hantro_ctx *ctx);
int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx);
+void hantro_hevc_ref_init(struct hantro_ctx *ctx);
dma_addr_t hantro_hevc_get_ref_buf(struct hantro_ctx *ctx, s32 poc);
int hantro_hevc_add_ref_buf(struct hantro_ctx *ctx, int poc, dma_addr_t addr);
-void hantro_hevc_ref_remove_unused(struct hantro_ctx *ctx);
static inline unsigned short hantro_vp9_num_sbs(unsigned short dimension)
{
--
2.32.0
On Tue, May 03, 2022 at 03:51:38PM +0200, Benjamin Gaignard wrote:
> PoC shall be int the range of -2^31 to 2^31 -1
> (HEVC spec section 8.3.1 Decoding process for picture order count).
> The current way to know if an entry in reference picture array is free
> is to test if PoC = UNUSED_REF. Since UNUSED_REF is defined as '-1' that
> could lead to decode issue if one PoC also equal '-1'.
> PoC with value = '-1' exists in conformance test SLIST_B_Sony_9.
>
> Change the way unused entries are managed in reference pictures array to
> avoid using PoC to detect then.
>
> This patch doesn't change fluster HEVC score.
>
> Signed-off-by: Benjamin Gaignard <[email protected]>
Good catch!
Thanks,
Reviewed-by: Ezequiel Garcia <[email protected]>
> ---
> .../staging/media/hantro/hantro_g2_hevc_dec.c | 6 ++---
> drivers/staging/media/hantro/hantro_hevc.c | 27 +++----------------
> drivers/staging/media/hantro/hantro_hw.h | 2 +-
> 3 files changed, 6 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c b/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
> index 0a8c01ff2fa7..b7835bbf5e98 100644
> --- a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
> +++ b/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
> @@ -473,8 +473,8 @@ static int set_ref(struct hantro_ctx *ctx)
>
> set_ref_pic_list(ctx);
>
> - /* We will only keep the references picture that are still used */
> - ctx->hevc_dec.ref_bufs_used = 0;
> + /* We will only keep the references pictures that are still used */
> + hantro_hevc_ref_init(ctx);
>
> /* Set up addresses of DPB buffers */
> dpb_longterm_e = 0;
> @@ -515,8 +515,6 @@ static int set_ref(struct hantro_ctx *ctx)
> hantro_write_addr(vpu, G2_OUT_CHROMA_ADDR, chroma_addr);
> hantro_write_addr(vpu, G2_OUT_MV_ADDR, mv_addr);
>
> - hantro_hevc_ref_remove_unused(ctx);
> -
> for (; i < V4L2_HEVC_DPB_ENTRIES_NUM_MAX; i++) {
> hantro_write_addr(vpu, G2_REF_LUMA_ADDR(i), 0);
> hantro_write_addr(vpu, G2_REF_CHROMA_ADDR(i), 0);
> diff --git a/drivers/staging/media/hantro/hantro_hevc.c b/drivers/staging/media/hantro/hantro_hevc.c
> index 7d4b1d72255c..7fdec50dc853 100644
> --- a/drivers/staging/media/hantro/hantro_hevc.c
> +++ b/drivers/staging/media/hantro/hantro_hevc.c
> @@ -25,15 +25,11 @@
> #define MAX_TILE_COLS 20
> #define MAX_TILE_ROWS 22
>
> -#define UNUSED_REF -1
> -
> -static void hantro_hevc_ref_init(struct hantro_ctx *ctx)
> +void hantro_hevc_ref_init(struct hantro_ctx *ctx)
> {
> struct hantro_hevc_dec_hw_ctx *hevc_dec = &ctx->hevc_dec;
> - int i;
>
> - for (i = 0; i < NUM_REF_PICTURES; i++)
> - hevc_dec->ref_bufs_poc[i] = UNUSED_REF;
> + hevc_dec->ref_bufs_used = 0;
> }
>
> dma_addr_t hantro_hevc_get_ref_buf(struct hantro_ctx *ctx,
> @@ -60,7 +56,7 @@ int hantro_hevc_add_ref_buf(struct hantro_ctx *ctx, int poc, dma_addr_t addr)
>
> /* Add a new reference buffer */
> for (i = 0; i < NUM_REF_PICTURES; i++) {
> - if (hevc_dec->ref_bufs_poc[i] == UNUSED_REF) {
> + if (!(hevc_dec->ref_bufs_used & 1 << i)) {
> hevc_dec->ref_bufs_used |= 1 << i;
> hevc_dec->ref_bufs_poc[i] = poc;
> hevc_dec->ref_bufs[i].dma = addr;
> @@ -71,23 +67,6 @@ int hantro_hevc_add_ref_buf(struct hantro_ctx *ctx, int poc, dma_addr_t addr)
> return -EINVAL;
> }
>
> -void hantro_hevc_ref_remove_unused(struct hantro_ctx *ctx)
> -{
> - struct hantro_hevc_dec_hw_ctx *hevc_dec = &ctx->hevc_dec;
> - int i;
> -
> - /* Just tag buffer as unused, do not free them */
> - for (i = 0; i < NUM_REF_PICTURES; i++) {
> - if (hevc_dec->ref_bufs_poc[i] == UNUSED_REF)
> - continue;
> -
> - if (hevc_dec->ref_bufs_used & (1 << i))
> - continue;
> -
> - hevc_dec->ref_bufs_poc[i] = UNUSED_REF;
> - }
> -}
> -
> static int tile_buffer_reallocate(struct hantro_ctx *ctx)
> {
> struct hantro_dev *vpu = ctx->dev;
> diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
> index 9f31cce609d6..5de558386179 100644
> --- a/drivers/staging/media/hantro/hantro_hw.h
> +++ b/drivers/staging/media/hantro/hantro_hw.h
> @@ -337,9 +337,9 @@ int hantro_hevc_dec_init(struct hantro_ctx *ctx);
> void hantro_hevc_dec_exit(struct hantro_ctx *ctx);
> int hantro_g2_hevc_dec_run(struct hantro_ctx *ctx);
> int hantro_hevc_dec_prepare_run(struct hantro_ctx *ctx);
> +void hantro_hevc_ref_init(struct hantro_ctx *ctx);
> dma_addr_t hantro_hevc_get_ref_buf(struct hantro_ctx *ctx, s32 poc);
> int hantro_hevc_add_ref_buf(struct hantro_ctx *ctx, int poc, dma_addr_t addr);
> -void hantro_hevc_ref_remove_unused(struct hantro_ctx *ctx);
>
> static inline unsigned short hantro_vp9_num_sbs(unsigned short dimension)
> {
> --
> 2.32.0
>