<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <meta name="Generator" content="Microsoft Exchange Server"> <style></style> </head> <body> <div> <div dir="auto">Ok, that sounds like it makes sense but I'm not so deeply into this handling anyway. <div dir="auto"><br> </div> <div dir="auto">One more comment: Please use the MIN2 or MAX2 macro here instead of open coding it.</div> <div dir="auto"><br> </div> <div dir="auto">Apart from that looks good to me,</div> <div dir="auto">Christian.</div> </div> <div class="x_gmail_extra"><br> <div class="x_gmail_quote">Am 29.05.2019 12:53 schrieb "Zhang, Boyuan" <Boyuan.Zhang@amd.com>:<br type="attribution"> </div> </div> </div> <font size="2"><span style="font-size:11pt;"> <div class="PlainText">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.<br> <br> Regards,<br> Boyuan<br> <br> -----Original Message-----<br> From: Christian König <ckoenig.leichtzumerken@gmail.com> <br> Sent: May 28, 2019 3:27 AM<br> To: Zhang, Boyuan <Boyuan.Zhang@amd.com>; mesa-dev@lists.freedesktop.org<br> Cc: mesa-stable@lists.freedesktop.org<br> Subject: Re: [Mesa-dev] [PATCH] radeon/uvd: fix poc for hevc encode<br> <br> It would be better to have those checks in the state tracker than in the backend code.<br> <br> Christian.<br> <br> Am 27.05.19 um 20:41 schrieb boyuan.zhang@amd.com:<br> > From: Boyuan Zhang <boyuan.zhang@amd.com><br> ><br> > MaxPicOrderCntLsb should be at 16 according to the spec, therefore add <br> > minimum value check.<br> ><br> > Also use poc value passed from st instead of calculation in slice <br> > header encoding.<br> ><br> > Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=110673">https://bugs.freedesktop.org/show_bug.cgi?id=110673</a><br> > Cc: mesa-stable@lists.freedesktop.org<br> ><br> > Signed-off-by: Boyuan Zhang <boyuan.zhang@amd.com><br> > ---<br> > src/gallium/drivers/radeon/radeon_uvd_enc.c | 3 ++-<br> > src/gallium/drivers/radeon/radeon_uvd_enc_1_1.c | 3 +--<br> > 2 files changed, 3 insertions(+), 3 deletions(-)<br> ><br> > diff --git a/src/gallium/drivers/radeon/radeon_uvd_enc.c <br> > b/src/gallium/drivers/radeon/radeon_uvd_enc.c<br> > index 521d08f304..9256e43a08 100644<br> > --- a/src/gallium/drivers/radeon/radeon_uvd_enc.c<br> > +++ b/src/gallium/drivers/radeon/radeon_uvd_enc.c<br> > @@ -73,7 +73,8 @@ radeon_uvd_enc_get_param(struct radeon_uvd_encoder *enc,<br> > enc->enc_pic.general_tier_flag = pic->seq.general_tier_flag;<br> > enc->enc_pic.general_profile_idc = pic->seq.general_profile_idc;<br> > enc->enc_pic.general_level_idc = pic->seq.general_level_idc;<br> > - enc->enc_pic.max_poc = pic->seq.intra_period;<br> > + enc->enc_pic.max_poc =<br> > + (pic->seq.intra_period >= 16) ? pic->seq.intra_period : 16;<br> > enc->enc_pic.log2_max_poc = 0;<br> > for (int i = enc->enc_pic.max_poc; i != 0; enc->enc_pic.log2_max_poc++)<br> > i = (i >> 1);<br> > diff --git a/src/gallium/drivers/radeon/radeon_uvd_enc_1_1.c <br> > b/src/gallium/drivers/radeon/radeon_uvd_enc_1_1.c<br> > index ddb219792a..8f0e0099e7 100644<br> > --- a/src/gallium/drivers/radeon/radeon_uvd_enc_1_1.c<br> > +++ b/src/gallium/drivers/radeon/radeon_uvd_enc_1_1.c<br> > @@ -768,8 +768,7 @@ radeon_uvd_enc_slice_header_hevc(struct radeon_uvd_encoder *enc)<br> > if ((enc->enc_pic.nal_unit_type != 19)<br> > && (enc->enc_pic.nal_unit_type != 20)) {<br> > radeon_uvd_enc_code_fixed_bits(enc,<br> > - enc->enc_pic.frame_num %<br> > - enc->enc_pic.max_poc,<br> > + enc->enc_pic.pic_order_cnt,<br> > enc->enc_pic.log2_max_poc);<br> > if (enc->enc_pic.picture_type == PIPE_H265_ENC_PICTURE_TYPE_P)<br> > radeon_uvd_enc_code_fixed_bits(enc, 0x1, 1);<br> <br> </div> </span></font> </body> </html>