This series aims to clean up Reference Picture Set usage and flags.
Long term flag was named with RPS prefix while it is not used for RPS
but for mark long term references in DBP. Remane it and remove the two
other useless RPS flags.
Clarify documentation about RPS lists content and make sure that Hantro
driver use them correctly (i.e without look up in DBP).
Benjamin
Benjamin Gaignard (2):
media: hevc: Remove RPS named flags
media: hevc: Embedded indexes in RPS
.../media/v4l/ext-ctrls-codec.rst | 12 ++++-----
.../staging/media/hantro/hantro_g2_hevc_dec.c | 27 +++++--------------
.../staging/media/sunxi/cedrus/cedrus_h265.c | 2 +-
include/media/hevc-ctrls.h | 4 +--
4 files changed, 14 insertions(+), 31 deletions(-)
--
2.25.1
Reference Picture Set lists provide indexes of short and long term
reference in DBP array.
Fix Hantro to not do a look up in DBP entries.
Make documentation more clear about it.
Signed-off-by: Benjamin Gaignard <[email protected]>
---
.../media/v4l/ext-ctrls-codec.rst | 6 ++---
.../staging/media/hantro/hantro_g2_hevc_dec.c | 25 +++++--------------
2 files changed, 9 insertions(+), 22 deletions(-)
diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
index eff33c511090..4e4892c37723 100644
--- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
+++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst
@@ -3352,15 +3352,15 @@ enum v4l2_mpeg_video_hevc_size_of_length_field -
* - __u8
- ``poc_st_curr_before[V4L2_HEVC_DPB_ENTRIES_NUM_MAX]``
- PocStCurrBefore as described in section 8.3.2 "Decoding process for reference
- picture set.
+ picture set": provides the index of the short term before references in DPB array.
* - __u8
- ``poc_st_curr_after[V4L2_HEVC_DPB_ENTRIES_NUM_MAX]``
- PocStCurrAfter as described in section 8.3.2 "Decoding process for reference
- picture set.
+ picture set": provides the index of the short term after references in DPB array.
* - __u8
- ``poc_lt_curr[V4L2_HEVC_DPB_ENTRIES_NUM_MAX]``
- PocLtCurr as described in section 8.3.2 "Decoding process for reference
- picture set.
+ picture set": provides the index of the long term references in DPB array.
* - __u64
- ``flags``
- See :ref:`Decode Parameters Flags <hevc_decode_params_flags>`
diff --git a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c b/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
index be46b3c28b17..76386e9eb8f4 100644
--- a/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
+++ b/drivers/staging/media/hantro/hantro_g2_hevc_dec.c
@@ -306,24 +306,11 @@ static void set_params(struct hantro_ctx *ctx)
hantro_reg_write(vpu, &g2_apf_threshold, 8);
}
-static int find_ref_pic_index(const struct v4l2_hevc_dpb_entry *dpb, int pic_order_cnt)
-{
- int i;
-
- for (i = 0; i < V4L2_HEVC_DPB_ENTRIES_NUM_MAX; i++) {
- if (dpb[i].pic_order_cnt[0] == pic_order_cnt)
- return i;
- }
-
- return 0x0;
-}
-
static void set_ref_pic_list(struct hantro_ctx *ctx)
{
const struct hantro_hevc_dec_ctrls *ctrls = &ctx->hevc_dec.ctrls;
struct hantro_dev *vpu = ctx->dev;
const struct v4l2_ctrl_hevc_decode_params *decode_params = ctrls->decode_params;
- const struct v4l2_hevc_dpb_entry *dpb = decode_params->dpb;
u32 list0[V4L2_HEVC_DPB_ENTRIES_NUM_MAX] = {};
u32 list1[V4L2_HEVC_DPB_ENTRIES_NUM_MAX] = {};
static const struct hantro_reg ref_pic_regs0[] = {
@@ -367,11 +354,11 @@ static void set_ref_pic_list(struct hantro_ctx *ctx)
/* List 0 contains: short term before, short term after and long term */
j = 0;
for (i = 0; i < decode_params->num_poc_st_curr_before && j < ARRAY_SIZE(list0); i++)
- list0[j++] = find_ref_pic_index(dpb, decode_params->poc_st_curr_before[i]);
+ list0[j++] = decode_params->poc_st_curr_before[i];
for (i = 0; i < decode_params->num_poc_st_curr_after && j < ARRAY_SIZE(list0); i++)
- list0[j++] = find_ref_pic_index(dpb, decode_params->poc_st_curr_after[i]);
+ list0[j++] = decode_params->poc_st_curr_after[i];
for (i = 0; i < decode_params->num_poc_lt_curr && j < ARRAY_SIZE(list0); i++)
- list0[j++] = find_ref_pic_index(dpb, decode_params->poc_lt_curr[i]);
+ list0[j++] = decode_params->poc_lt_curr[i];
/* Fill the list, copying over and over */
i = 0;
@@ -380,11 +367,11 @@ static void set_ref_pic_list(struct hantro_ctx *ctx)
j = 0;
for (i = 0; i < decode_params->num_poc_st_curr_after && j < ARRAY_SIZE(list1); i++)
- list1[j++] = find_ref_pic_index(dpb, decode_params->poc_st_curr_after[i]);
+ list1[j++] = decode_params->poc_st_curr_after[i];
for (i = 0; i < decode_params->num_poc_st_curr_before && j < ARRAY_SIZE(list1); i++)
- list1[j++] = find_ref_pic_index(dpb, decode_params->poc_st_curr_before[i]);
+ list1[j++] = decode_params->poc_st_curr_before[i];
for (i = 0; i < decode_params->num_poc_lt_curr && j < ARRAY_SIZE(list1); i++)
- list1[j++] = find_ref_pic_index(dpb, decode_params->poc_lt_curr[i]);
+ list1[j++] = decode_params->poc_lt_curr[i];
i = 0;
while (j < ARRAY_SIZE(list1))
--
2.25.1
As per discussion with Ezequiel, I think this patch should perhaps be made an
RFC titled "media: uapi: clean and make HEVC uAPI public". Keep it RFC until we
have collected all the needed changes.
Opinion ?
Le mardi 31 août 2021 à 11:48 +0200, Benjamin Gaignard a écrit :
> This series aims to clean up Reference Picture Set usage and flags.
>
> Long term flag was named with RPS prefix while it is not used for RPS
> but for mark long term references in DBP. Remane it and remove the two
> other useless RPS flags.
>
> Clarify documentation about RPS lists content and make sure that Hantro
> driver use them correctly (i.e without look up in DBP).
>
> Benjamin
>
> Benjamin Gaignard (2):
> media: hevc: Remove RPS named flags
> media: hevc: Embedded indexes in RPS
>
> .../media/v4l/ext-ctrls-codec.rst | 12 ++++-----
> .../staging/media/hantro/hantro_g2_hevc_dec.c | 27 +++++--------------
> .../staging/media/sunxi/cedrus/cedrus_h265.c | 2 +-
> include/media/hevc-ctrls.h | 4 +--
> 4 files changed, 14 insertions(+), 31 deletions(-)
>
Hans, Nicolas,
On Tue, 31 Aug 2021 at 16:19, Nicolas Dufresne <[email protected]> wrote:
>
> As per discussion with Ezequiel, I think this patch should perhaps be made an
> RFC titled "media: uapi: clean and make HEVC uAPI public". Keep it RFC until we
> have collected all the needed changes.
>
> Opinion ?
I agree.
IMO, it makes no sense to keep merging anything on Hantro G2 HEVC
support, until the BLK-CTRL part is ready [1]. Without that, we can't
test anything
so we may introduce regressions without knowing.
Also, I think it's time to start discussing the HEVC uAPI as a whole,
with the goal is having a stable interface that can be used by userspace
frameworks (GStreamer).
[1] https://www.spinics.net/lists/arm-kernel/msg913881.html
Thanks,
Ezequiel