radeon/uvd: fix poc for hevc encode

Submitted by Zhang, Boyuan on May 27, 2019, 6:41 p.m.

Details

Message ID 20190527184122.26743-1-boyuan.zhang@amd.com
State New
Headers show
Series "radeon/uvd: fix poc for hevc encode" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Zhang, Boyuan May 27, 2019, 6:41 p.m.
From: Boyuan Zhang <boyuan.zhang@amd.com>

MaxPicOrderCntLsb should be at 16 according to the spec,
therefore add minimum value check.

Also use poc value passed from st instead of calculation
in slice header encoding.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110673
Cc: mesa-stable@lists.freedesktop.org

Signed-off-by: Boyuan Zhang <boyuan.zhang@amd.com>
---
 src/gallium/drivers/radeon/radeon_uvd_enc.c     | 3 ++-
 src/gallium/drivers/radeon/radeon_uvd_enc_1_1.c | 3 +--
 2 files changed, 3 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/gallium/drivers/radeon/radeon_uvd_enc.c b/src/gallium/drivers/radeon/radeon_uvd_enc.c
index 521d08f304..9256e43a08 100644
--- a/src/gallium/drivers/radeon/radeon_uvd_enc.c
+++ b/src/gallium/drivers/radeon/radeon_uvd_enc.c
@@ -73,7 +73,8 @@  radeon_uvd_enc_get_param(struct radeon_uvd_encoder *enc,
    enc->enc_pic.general_tier_flag = pic->seq.general_tier_flag;
    enc->enc_pic.general_profile_idc = pic->seq.general_profile_idc;
    enc->enc_pic.general_level_idc = pic->seq.general_level_idc;
-   enc->enc_pic.max_poc = pic->seq.intra_period;
+   enc->enc_pic.max_poc =
+      (pic->seq.intra_period >= 16) ? pic->seq.intra_period : 16;
    enc->enc_pic.log2_max_poc = 0;
    for (int i = enc->enc_pic.max_poc; i != 0; enc->enc_pic.log2_max_poc++)
       i = (i >> 1);
diff --git a/src/gallium/drivers/radeon/radeon_uvd_enc_1_1.c b/src/gallium/drivers/radeon/radeon_uvd_enc_1_1.c
index ddb219792a..8f0e0099e7 100644
--- a/src/gallium/drivers/radeon/radeon_uvd_enc_1_1.c
+++ b/src/gallium/drivers/radeon/radeon_uvd_enc_1_1.c
@@ -768,8 +768,7 @@  radeon_uvd_enc_slice_header_hevc(struct radeon_uvd_encoder *enc)
    if ((enc->enc_pic.nal_unit_type != 19)
        && (enc->enc_pic.nal_unit_type != 20)) {
       radeon_uvd_enc_code_fixed_bits(enc,
-                                     enc->enc_pic.frame_num %
-                                     enc->enc_pic.max_poc,
+                                     enc->enc_pic.pic_order_cnt,
                                      enc->enc_pic.log2_max_poc);
       if (enc->enc_pic.picture_type == PIPE_H265_ENC_PICTURE_TYPE_P)
          radeon_uvd_enc_code_fixed_bits(enc, 0x1, 1);

Comments

It would be better to have those checks in the state tracker than in the 
backend code.

Christian.

Am 27.05.19 um 20:41 schrieb boyuan.zhang@amd.com:
> From: Boyuan Zhang <boyuan.zhang@amd.com>
>
> MaxPicOrderCntLsb should be at 16 according to the spec,
> therefore add minimum value check.
>
> Also use poc value passed from st instead of calculation
> in slice header encoding.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110673
> Cc: mesa-stable@lists.freedesktop.org
>
> Signed-off-by: Boyuan Zhang <boyuan.zhang@amd.com>
> ---
>   src/gallium/drivers/radeon/radeon_uvd_enc.c     | 3 ++-
>   src/gallium/drivers/radeon/radeon_uvd_enc_1_1.c | 3 +--
>   2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/drivers/radeon/radeon_uvd_enc.c b/src/gallium/drivers/radeon/radeon_uvd_enc.c
> index 521d08f304..9256e43a08 100644
> --- a/src/gallium/drivers/radeon/radeon_uvd_enc.c
> +++ b/src/gallium/drivers/radeon/radeon_uvd_enc.c
> @@ -73,7 +73,8 @@ radeon_uvd_enc_get_param(struct radeon_uvd_encoder *enc,
>      enc->enc_pic.general_tier_flag = pic->seq.general_tier_flag;
>      enc->enc_pic.general_profile_idc = pic->seq.general_profile_idc;
>      enc->enc_pic.general_level_idc = pic->seq.general_level_idc;
> -   enc->enc_pic.max_poc = pic->seq.intra_period;
> +   enc->enc_pic.max_poc =
> +      (pic->seq.intra_period >= 16) ? pic->seq.intra_period : 16;
>      enc->enc_pic.log2_max_poc = 0;
>      for (int i = enc->enc_pic.max_poc; i != 0; enc->enc_pic.log2_max_poc++)
>         i = (i >> 1);
> diff --git a/src/gallium/drivers/radeon/radeon_uvd_enc_1_1.c b/src/gallium/drivers/radeon/radeon_uvd_enc_1_1.c
> index ddb219792a..8f0e0099e7 100644
> --- a/src/gallium/drivers/radeon/radeon_uvd_enc_1_1.c
> +++ b/src/gallium/drivers/radeon/radeon_uvd_enc_1_1.c
> @@ -768,8 +768,7 @@ radeon_uvd_enc_slice_header_hevc(struct radeon_uvd_encoder *enc)
>      if ((enc->enc_pic.nal_unit_type != 19)
>          && (enc->enc_pic.nal_unit_type != 20)) {
>         radeon_uvd_enc_code_fixed_bits(enc,
> -                                     enc->enc_pic.frame_num %
> -                                     enc->enc_pic.max_poc,
> +                                     enc->enc_pic.pic_order_cnt,
>                                        enc->enc_pic.log2_max_poc);
>         if (enc->enc_pic.picture_type == PIPE_H265_ENC_PICTURE_TYPE_P)
>            radeon_uvd_enc_code_fixed_bits(enc, 0x1, 1);
Yes, I agree that all interface defined value check should be done in state tracker. However, this value is NOT defined by vaapi interface, so it's like a radeon specific implementation based on the known values we got.

Regards,
Boyuan

-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@gmail.com> 

Sent: May 28, 2019 3:27 AM
To: Zhang, Boyuan <Boyuan.Zhang@amd.com>; mesa-dev@lists.freedesktop.org
Cc: mesa-stable@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH] radeon/uvd: fix poc for hevc encode

It would be better to have those checks in the state tracker than in the backend code.

Christian.

Am 27.05.19 um 20:41 schrieb boyuan.zhang@amd.com:
> From: Boyuan Zhang <boyuan.zhang@amd.com>

>

> MaxPicOrderCntLsb should be at 16 according to the spec, therefore add 

> minimum value check.

>

> Also use poc value passed from st instead of calculation in slice 

> header encoding.

>

> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110673

> Cc: mesa-stable@lists.freedesktop.org

>

> Signed-off-by: Boyuan Zhang <boyuan.zhang@amd.com>

> ---

>   src/gallium/drivers/radeon/radeon_uvd_enc.c     | 3 ++-

>   src/gallium/drivers/radeon/radeon_uvd_enc_1_1.c | 3 +--

>   2 files changed, 3 insertions(+), 3 deletions(-)

>

> diff --git a/src/gallium/drivers/radeon/radeon_uvd_enc.c 

> b/src/gallium/drivers/radeon/radeon_uvd_enc.c

> index 521d08f304..9256e43a08 100644

> --- a/src/gallium/drivers/radeon/radeon_uvd_enc.c

> +++ b/src/gallium/drivers/radeon/radeon_uvd_enc.c

> @@ -73,7 +73,8 @@ radeon_uvd_enc_get_param(struct radeon_uvd_encoder *enc,

>      enc->enc_pic.general_tier_flag = pic->seq.general_tier_flag;

>      enc->enc_pic.general_profile_idc = pic->seq.general_profile_idc;

>      enc->enc_pic.general_level_idc = pic->seq.general_level_idc;

> -   enc->enc_pic.max_poc = pic->seq.intra_period;

> +   enc->enc_pic.max_poc =

> +      (pic->seq.intra_period >= 16) ? pic->seq.intra_period : 16;

>      enc->enc_pic.log2_max_poc = 0;

>      for (int i = enc->enc_pic.max_poc; i != 0; enc->enc_pic.log2_max_poc++)

>         i = (i >> 1);

> diff --git a/src/gallium/drivers/radeon/radeon_uvd_enc_1_1.c 

> b/src/gallium/drivers/radeon/radeon_uvd_enc_1_1.c

> index ddb219792a..8f0e0099e7 100644

> --- a/src/gallium/drivers/radeon/radeon_uvd_enc_1_1.c

> +++ b/src/gallium/drivers/radeon/radeon_uvd_enc_1_1.c

> @@ -768,8 +768,7 @@ radeon_uvd_enc_slice_header_hevc(struct radeon_uvd_encoder *enc)

>      if ((enc->enc_pic.nal_unit_type != 19)

>          && (enc->enc_pic.nal_unit_type != 20)) {

>         radeon_uvd_enc_code_fixed_bits(enc,

> -                                     enc->enc_pic.frame_num %

> -                                     enc->enc_pic.max_poc,

> +                                     enc->enc_pic.pic_order_cnt,

>                                        enc->enc_pic.log2_max_poc);

>         if (enc->enc_pic.picture_type == PIPE_H265_ENC_PICTURE_TYPE_P)

>            radeon_uvd_enc_code_fixed_bits(enc, 0x1, 1);