Re: [Mesa-dev] [PATCH] radeon/uvd: fix poc for hevc encode
Ok, that sounds like it makes sense but I'm not so deeply into this handling
anyway.
One more comment: Please use the MIN2 or MAX2 macro here instead of open coding
it.
Apart from that looks good to me,
Christian.
Am 29.05.2019 12:53 schrieb "Zhang, Boyuan" :
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
Sent: May 28, 2019 3:27 AM
To: Zhang, Boyuan ; mesa-dev@lists.freedesktop.org
Cc: mesa-sta...@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.zh...@amd.com:
> From: Boyuan Zhang
>
> 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-sta...@lists.freedesktop.org
>
> Signed-off-by: Boyuan Zhang
> ---
> 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);
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radeon/uvd: fix poc for hevc encode
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
Sent: May 28, 2019 3:27 AM
To: Zhang, Boyuan ; mesa-dev@lists.freedesktop.org
Cc: mesa-sta...@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.zh...@amd.com:
> From: Boyuan Zhang
>
> 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-sta...@lists.freedesktop.org
>
> Signed-off-by: Boyuan Zhang
> ---
> 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);
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
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.zh...@amd.com:
From: Boyuan Zhang
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-sta...@lists.freedesktop.org
Signed-off-by: Boyuan Zhang
---
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);
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
