Re: [Mesa-dev] [PATCH] st/va: add mpeg4 startcode workaround

[Zhang, Boyuan]

Hi Emil,

Please see the following information about this patch.

- Issue: For Mpeg4, the VOP and GOV headers were truncated. With the existing 
workaround in st/va, playback shows massive corruptions.
- This Patch: Provide another way to get the truncated headers back. Massive 
corruptions are gone with this patch. At the same time, add an environmental 
variable to allow user to decide whether to use this patch.

Regards,
Boyuan

-Original Message-
From: Liu, Leo 
Sent: November-05-15 10:15 PM
To: Emil Velikov; Zhang, Boyuan
Cc: ML mesa-dev
Subject: RE: [Mesa-dev] [PATCH] st/va: add mpeg4 startcode workaround

+Boyuan, forgot to Cc him when I sent.

Regards,
Leo


>-Original Message-
>From: Emil Velikov [mailto:emil.l.veli...@gmail.com]
>Sent: Thursday, November 05, 2015 7:00 PM
>To: Liu, Leo
>Cc: ML mesa-dev
>Subject: Re: [Mesa-dev] [PATCH] st/va: add mpeg4 startcode workaround
>
>On 5 November 2015 at 21:00, Leo Liu  wrote:
>> From: Boyuan Zhang 
>>
>> Signed-off-by: Boyuan Zhang 
>> Reviewed-by: Christian König 
>> ---
>>  src/gallium/state_trackers/va/buffer.c |  24 +-
>>  src/gallium/state_trackers/va/context.c|   7 ++
>>  src/gallium/state_trackers/va/picture.c| 117 
>> +
>>  src/gallium/state_trackers/va/va_private.h |   3 +
>>  4 files changed, 102 insertions(+), 49 deletions(-)
>>
>Guys can get some commit message please ? What is the workaround why is 
>it needed ?
>
>It's a bit sad that one has to ask for such a thing.
>
>
>> @@ -59,8 +70,17 @@ vlVaCreateBuffer(VADriverContextP ctx, VAContextID
>context, VABufferType type,
>>return VA_STATUS_ERROR_ALLOCATION_FAILED;
>> }
>>
>> -   if (data)
>> -  memcpy(buf->data, data, size * num_elements);
>> +   uint8_t* pExternalData = (uint8_t*) data;
>> +   if (data) {
>> +  if ((u_reduce_video_profile(pContext->desc.base.profile) ==
>PIPE_VIDEO_FORMAT_MPEG4)
>> +&& (pContext->mpeg4.vaapi_mpeg4_workaround == true)
>> +&& (buf->type == VASliceDataBufferType)) {
>Please follow st/va coding style - drop the explicit comparison against 
>true, && should be at the end of the line.
>
>
>> --- a/src/gallium/state_trackers/va/context.c
>> +++ b/src/gallium/state_trackers/va/context.c
>> @@ -35,6 +35,8 @@
>>
>>  #include "va_private.h"
>>
>> +DEBUG_GET_ONCE_BOOL_OPTION(mpeg4, "VAAPI_MPEG4_WORKAROUND",
>FALSE);
>> +
>You do realise that defined as it, one can only use 
>VAAPI_MPEG4_WORKAROUND on debug mesa builds ?
>
>
>> @@ -275,6 +277,11 @@ vlVaCreateContext(VADriverContextP ctx, 
>> VAConfigID
>config_id, int picture_width,
>>  return VA_STATUS_ERROR_ALLOCATION_FAILED;
>>   }
>>}
>> +
>> +  if (u_reduce_video_profile(context->decoder->profile) ==
>> +PIPE_VIDEO_FORMAT_MPEG4) {
>u_reduce_video_profile() is called three times already. Stash it into a 
>local variable and keep the comparison on a single line ?
>
>
>> --- a/src/gallium/state_trackers/va/picture.c
>> +++ b/src/gallium/state_trackers/va/picture.c
>> @@ -584,60 +584,83 @@ vlVaDecoderFixMPEG4Startcode(vlVaContext
>> *context)
>
>> +  for (int i = 0 ; i < VL_VA_MPEG4_BYTES_FOR_LOOKUP ; i++) {
>> + if (memcmp (p, start_code, sizeof(start_code)) == 0) {
>> +found = true;
>Just use startcode_available directly ?
>
>> +break;
>> + }
>> + p += 1;
>> + extraSize += 1;
>> +  }
>> +  if (found) {
>> + startcode_available = true;
>
>
>-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/va: enable vbr rate control for vaapi encode

[Zhang, Boyuan]

Hi Andy,

For the VBR target/max bit-rate, yes, this is gstreamer-vaapi's current design. 
User typed bit-rate is actually the max bit-rate not the actual bit-rate, which 
is a bit confused.

For the overflow concern, unsigned int can handle about 4294Mbit/s, which we 
thought is big enough for real life cases, right?

Regards,
Boyuan

-Original Message-
From: Andy Furniss [mailto:adf.li...@gmail.com] 
Sent: September-25-16 6:46 AM
To: Liu, Leo; Christian König; Zhang, Boyuan; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH] st/va: enable vbr rate control for vaapi encode

Leo Liu wrote:
>
>
> On 09/07/2016 03:02 AM, Christian König wrote:
>> Am 06.09.2016 um 22:39 schrieb boyuan.zh...@amd.com:
>>> From: Boyuan Zhang 
>>>
>>> This patch enables variable bit-rate for vaapi encoding. According 
>>> to va.h, target bit-rate equals to maximum bit-rate multiplies by 
>>> target_percentage.
>>>
>>> Signed-off-by: Boyuan Zhang 
>>
>> That was astonishing simple to fix :)

Bit late on this but I am not sure this is VBR as it's constrained and that has 
it's own setting.

Maybe I am taking too much notice of what libx264/broadcast encoders do, but it 
would be good if VCE could throw way more bits at IDR frames as they really 
need it to avoid pulsing with default gop 30.

Of course I don't know how things work/what's possible.

The names help of course but not always eg. what is -

peak_bits_picture_fraction seems to be always 0.

and

rate_ctrl.vbv_buf_lv 0 or 48 is this some level in a spec, or is it initial 
buffer fullness for vbv?

One issue I found and sent a patch for is the bitrate calc can overflow.

Generally as a user asking for a rate and getting 70% seems a bit strange 
anyway but maybe that's a different discussion.

https://patchwork.freedesktop.org/patch/112040/

>>
>> Patch is Reviewed-by: Christian König .
>>
>> Leo do you want to push it or should I take care of this?
>
> I will take care of it.
>
> Regards,
> Leo
>
>>
>> Regards,
>> Christian.
>>
>>> ---
>>>   src/gallium/state_trackers/va/config.c  | 2 +-
>>>   src/gallium/state_trackers/va/picture.c | 2 +-
>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/gallium/state_trackers/va/config.c
>>> b/src/gallium/state_trackers/va/config.c
>>> index 84bf913..4052316 100644
>>> --- a/src/gallium/state_trackers/va/config.c
>>> +++ b/src/gallium/state_trackers/va/config.c
>>> @@ -120,7 +120,7 @@ vlVaGetConfigAttributes(VADriverContextP ctx, 
>>> VAProfile profile, VAEntrypoint en
>>>value = VA_RT_FORMAT_YUV420;
>>>break;
>>> case VAConfigAttribRateControl:
>>> - value = VA_RC_CQP | VA_RC_CBR;
>>> + value = VA_RC_CQP | VA_RC_CBR | VA_RC_VBR;
>>>break;
>>> default:
>>>value = VA_ATTRIB_NOT_SUPPORTED; diff --git 
>>> a/src/gallium/state_trackers/va/picture.c
>>> b/src/gallium/state_trackers/va/picture.c
>>> index a283e83..7f3d96d 100644
>>> --- a/src/gallium/state_trackers/va/picture.c
>>> +++ b/src/gallium/state_trackers/va/picture.c
>>> @@ -322,7 +322,7 @@
>>> handleVAEncMiscParameterTypeRateControl(vlVaContext *context, 
>>> VAEncMiscParameter
>>>  PIPE_H264_ENC_RATE_CONTROL_METHOD_CONSTANT)
>>> context->desc.h264enc.rate_ctrl.target_bitrate =
>>> rc->bits_per_second;
>>>  else
>>> -  context->desc.h264enc.rate_ctrl.target_bitrate =
>>> rc->bits_per_second * rc->target_percentage;
>>> +  context->desc.h264enc.rate_ctrl.target_bitrate =
>>> rc->bits_per_second * rc->target_percentage / 100;
>>>  context->desc.h264enc.rate_ctrl.peak_bitrate = rc->bits_per_second;
>>>  if (context->desc.h264enc.rate_ctrl.target_bitrate < 200)
>>> context->desc.h264enc.rate_ctrl.vbv_buffer_size = 
>>> MIN2((context->desc.h264enc.rate_ctrl.target_bitrate * 2.75), 
>>> 200);
>>
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] st/va: enable dual instances encode by sync surface

[Zhang, Boyuan]

Hi Andy,

Thanks for the testings.

Regarding to the inconsistencies, the current Vaapi dual instances encoding 
behaviour is random. Whether or not the dual instances is being used depends on 
how early the player calls sync_surface function according to the current 
gstreamer-vaapi's mechanism. e.g. if the player calls sync_surface too early, 
then we don't have enough time to receive and process the 2nd frame and we can 
only end up with single instance encoding for this frame as a result. And the 
random player behaviours depends on the current environment, for example 
cpufreq might be one factor. As a result, we don't guarantee the consistency 
especially when rate control is enabled for dual instances Vaapi encoding, 
since the randomness could result different calculations.

For the corruption/wrong frame order issue, could you please provide more 
detailed information for reproduction? e.g. the clips and commands that can 
reproduce the issue. Does this issue only happen after enabling dual instance 
patch?

Regards,
Boyuan

-Original Message-
From: Andy Furniss [mailto:adf.li...@gmail.com] 
Sent: October-03-16 7:35 AM
To: Zhang, Boyuan; mesa-dev@lists.freedesktop.org
Cc: deathsim...@vodafone.de
Subject: Re: [PATCH 1/2] st/va: enable dual instances encode by sync surface

Boyuan Zhang wrote:
> This patch improves the performance of Vaapi Encode by enabling dual 
> instances encoding. flush function is not called after each end_frame 
> call. radeon/vce will do flush whenever 2 frames are submitted for 
> encoding. Implement sync surface function to flush only if the frame 
> hasn't been flushed yet.

I filed a bug about this, pinging here as I couldn't add Boyuan to the cc on 
bugzilla.

https://bugs.freedesktop.org/show_bug.cgi?id=98005
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] st/va: Remove else case in vlVaEndPicture() made superfluous by c59628d11b

[Zhang, Boyuan]

Patch is Reviewed-by: Boyuan Zhang 

Regards,
Boyuan

-Original Message-
From: Kai Wasserbäch [mailto:k...@dev.carbon-project.org] 
Sent: August-20-16 12:15 PM
To: mesa-dev@lists.freedesktop.org
Cc: Zhang, Boyuan
Subject: [PATCH 1/2] st/va: Remove else case in vlVaEndPicture() made 
superfluous by c59628d11b

Commit c59628d11b134fc016388a170880f7646e100d6f made the else statement and 
duplication of the context->decoder->end_frame() call superfluous.

Cc: Boyuan Zhang 
Signed-off-by: Kai Wasserbäch 
---
 src/gallium/state_trackers/va/picture.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/gallium/state_trackers/va/picture.c 
b/src/gallium/state_trackers/va/picture.c
index 87567be..bbb5595 100644
--- a/src/gallium/state_trackers/va/picture.c
+++ b/src/gallium/state_trackers/va/picture.c
@@ -576,11 +576,9 @@ vlVaEndPicture(VADriverContextP ctx, VAContextID 
context_id)
   surf->frame_num_cnt = context->desc.h264enc.frame_num_cnt;
   surf->feedback = feedback;
   surf->coded_buf = coded_buf;
-  context->decoder->end_frame(context->decoder, context->target, 
&context->desc.base);
}
-   else
-  context->decoder->end_frame(context->decoder, context->target, 
&context->desc.base);
 
+   context->decoder->end_frame(context->decoder, context->target, 
+ &context->desc.base);
pipe_mutex_unlock(drv->mutex);
return VA_STATUS_SUCCESS;
 }
--
2.8.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] st/va: Remove unused variable coded_size from vlVaEndPicture()

[Zhang, Boyuan]

Patch is Reviewed-by: Boyuan Zhang 

Regards,
Boyuan

-Original Message-
From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf Of Kai 
Wasserbäch
Sent: August-20-16 12:15 PM
To: mesa-dev@lists.freedesktop.org
Subject: [Mesa-dev] [PATCH 2/2] st/va: Remove unused variable coded_size from 
vlVaEndPicture()

Removes the following GCC warning:
 ../../../../../src/gallium/state_trackers/va/picture.c:542:17: warning:
  unused variable 'coded_size' [-Wunused-variable]
unsigned int coded_size;
 ^~

Signed-off-by: Kai Wasserbäch 
---
 src/gallium/state_trackers/va/picture.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/gallium/state_trackers/va/picture.c 
b/src/gallium/state_trackers/va/picture.c
index bbb5595..a283e83 100644
--- a/src/gallium/state_trackers/va/picture.c
+++ b/src/gallium/state_trackers/va/picture.c
@@ -539,7 +539,6 @@ vlVaEndPicture(VADriverContextP ctx, VAContextID context_id)
vlVaContext *context;
vlVaBuffer *coded_buf;
vlVaSurface *surf;
-   unsigned int coded_size;
void *feedback;
 
if (!ctx)
-- 
2.8.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/2] st/va: minor clean-ups

[Zhang, Boyuan]

Hi Kai,

Thanks for cleaning up the codes. The duplicated end_frame and unused variable 
are due to the new patch submitted last Friday. I have reviewed both patches.

Regards,
Boyuan

-Original Message-
From: Kai Wasserbäch [mailto:k...@dev.carbon-project.org] 
Sent: August-20-16 12:15 PM
To: mesa-dev@lists.freedesktop.org
Cc: Zhang, Boyuan
Subject: [PATCH 0/2] st/va: minor clean-ups

Hey,
just noticed a duplicate call to context->decoder->end_frame() while looking 
through the recent changes. This is just a trivial clean-up and no functional 
change is intended. And while I was there, I also noticed an unused variable, 
which the second patch removes.

Cheers,
Kai

P.S.: If this "series" gets accepted, please commit it for me, since I do not 
have commit access.


Kai Wasserbäch (3):
  st/va: Remove else case in vlVaEndPicture() made superfluous by
c59628d11b
  st/va: Remove unused variable coded_size from vlVaEndPicture()

 src/gallium/state_trackers/va/picture.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

--
2.8.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH mesa] st/va: add missing mutex_unlock

[Zhang, Boyuan]

Patch is Reviewed-by: Boyuan Zhang 

Regards,
Boyuan

-Original Message-
From: Eric Engestrom [mailto:e...@engestrom.ch] 
Sent: August-21-16 5:12 PM
To: mesa-dev@lists.freedesktop.org
Cc: Zhang, Boyuan; Koenig, Christian; Eric Engestrom
Subject: [PATCH mesa] st/va: add missing mutex_unlock

Fixes: c59628d11b134fc01638 ("st/va: enable dual instances encode by sync 
surface")
Signed-off-by: Eric Engestrom 
---
 src/gallium/state_trackers/va/surface.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/gallium/state_trackers/va/surface.c 
b/src/gallium/state_trackers/va/surface.c
index 012e48e..3ee1cdd 100644
--- a/src/gallium/state_trackers/va/surface.c
+++ b/src/gallium/state_trackers/va/surface.c
@@ -106,8 +106,10 @@ vlVaSyncSurface(VADriverContextP ctx, VASurfaceID 
render_target)
pipe_mutex_lock(drv->mutex);
surf = handle_table_get(drv->htab, render_target);
 
-   if (!surf || !surf->buffer)
+   if (!surf || !surf->buffer) {
+  pipe_mutex_unlock(drv->mutex);
   return VA_STATUS_ERROR_INVALID_SURFACE;
+   }
 
context = handle_table_get(drv->htab, surf->ctx);
if (!context) {
-- 
2.9.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH mesa] st/va: add missing mutex_unlock

[Zhang, Boyuan]

Hi Eric,

Thanks for catching it. The patch is Reviewed-by: Boyuan Zhang 


Regards,
Boyuan

-Original Message-
From: Eric Engestrom [mailto:eric.engest...@imgtec.com] 
Sent: August-22-16 12:17 PM
To: Zhang, Boyuan
Cc: mesa-dev@lists.freedesktop.org; Koenig, Christian; Eric Engestrom
Subject: Re: [Mesa-dev] [PATCH mesa] st/va: add missing mutex_unlock

CC'ing Boyuan Zhang (author of the original patch), who got somehow removed 
from the CC list when sending my patch.


On Sun, Aug 21, 2016 at 10:11:48PM +0100, Eric Engestrom wrote:
> Fixes: c59628d11b134fc01638 ("st/va: enable dual instances encode by 
> sync surface")
> Signed-off-by: Eric Engestrom 
> ---
>  src/gallium/state_trackers/va/surface.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/gallium/state_trackers/va/surface.c 
> b/src/gallium/state_trackers/va/surface.c
> index 012e48e..3ee1cdd 100644
> --- a/src/gallium/state_trackers/va/surface.c
> +++ b/src/gallium/state_trackers/va/surface.c
> @@ -106,8 +106,10 @@ vlVaSyncSurface(VADriverContextP ctx, VASurfaceID 
> render_target)
> pipe_mutex_lock(drv->mutex);
> surf = handle_table_get(drv->htab, render_target);
>  
> -   if (!surf || !surf->buffer)
> +   if (!surf || !surf->buffer) {
> +  pipe_mutex_unlock(drv->mutex);
>return VA_STATUS_ERROR_INVALID_SURFACE;
> +   }
>  
> context = handle_table_get(drv->htab, surf->ctx);
> if (!context) {
> --
> 2.9.3
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/va: also honors interlaced preference when providing a video format

[Zhang, Boyuan]

Hi Leo, Christian and Julien,

I tested the patch with Vaapi Encoding and Transcoding, it seems working fine. 
We are using "VAAPI_DISABLE_INTERLACE" env, so interlaced is always disabled.

Regards,
Boyuan

-Original Message-
From: Liu, Leo 
Sent: September-08-16 9:50 AM
To: Koenig, Christian; Julien Isorce; mesa-dev@lists.freedesktop.org
Cc: mesa-sta...@lists.freedesktop.org; Zhang, Boyuan; Julien Isorce
Subject: Re: [PATCH] st/va: also honors interlaced preference when providing a 
video format



On 09/08/2016 03:50 AM, Christian König wrote:
> Am 08.09.2016 um 09:34 schrieb Julien Isorce:
>> This fixes a crash when using the prefered video format with 
>> vaapisink on Nvidia hardwares.
>> Also caught by the following assert:
>>nouveau_vp3_video.c:91: Assertion `templat->interlaced' failed.
>>
>> TEST= gst-launch-1.0 videotestsrc ! video/x-raw, format=NV12 ! 
>> vaapisink
>>
>> Signed-off-by: Julien Isorce 
>> Tested-by: Víctor Manuel Jáquez Leal 
>
> Reviewed-by: Christian König .
>
> But somebody should double check if that doesn't break transcoding for 
> AMD GPUs.
>
> We had some problems with that in the past.

VA-API encode use "VAAPI_DISABLE_INTERLACE" env for making sure not interlaced, 
but better to double check.

Boyuan, can you test on this patch?

Regards,
Leo

>
> Regards,
> Christian.
>
>> ---
>>   src/gallium/state_trackers/va/surface.c | 36
>> +
>>   1 file changed, 19 insertions(+), 17 deletions(-)
>>
>> diff --git a/src/gallium/state_trackers/va/surface.c
>> b/src/gallium/state_trackers/va/surface.c
>> index 3ee1cdd..00df69d 100644
>> --- a/src/gallium/state_trackers/va/surface.c
>> +++ b/src/gallium/state_trackers/va/surface.c
>> @@ -632,24 +632,26 @@ vlVaCreateSurfaces2(VADriverContextP ctx, 
>> unsigned int format,
>>memset(&templat, 0, sizeof(templat));
>>   +   templat.buffer_format = pscreen->get_video_param(
>> +  pscreen,
>> +  PIPE_VIDEO_PROFILE_UNKNOWN,
>> +  PIPE_VIDEO_ENTRYPOINT_BITSTREAM,
>> +  PIPE_VIDEO_CAP_PREFERED_FORMAT
>> +   );
>> +   templat.interlaced = pscreen->get_video_param(
>> +  pscreen,
>> +  PIPE_VIDEO_PROFILE_UNKNOWN,
>> +  PIPE_VIDEO_ENTRYPOINT_BITSTREAM,
>> +  PIPE_VIDEO_CAP_PREFERS_INTERLACED
>> +   );
>> +
>>  if (expected_fourcc) {
>> -  templat.buffer_format = VaFourccToPipeFormat(expected_fourcc);
>> -  templat.interlaced = 0;
>> -   } else {
>> -  templat.buffer_format = pscreen->get_video_param
>> -(
>> -   pscreen,
>> -   PIPE_VIDEO_PROFILE_UNKNOWN,
>> -   PIPE_VIDEO_ENTRYPOINT_BITSTREAM,
>> -   PIPE_VIDEO_CAP_PREFERED_FORMAT
>> -   );
>> -  templat.interlaced = pscreen->get_video_param
>> -(
>> -   pscreen,
>> -   PIPE_VIDEO_PROFILE_UNKNOWN,
>> -   PIPE_VIDEO_ENTRYPOINT_BITSTREAM,
>> -   PIPE_VIDEO_CAP_PREFERS_INTERLACED
>> -   );
>> +  enum pipe_format expected_format =
>> VaFourccToPipeFormat(expected_fourcc);
>> +
>> +  if (expected_format != templat.buffer_format || memory_attibute)
>> +templat.interlaced = 0;
>> +
>> +  templat.buffer_format = expected_format;
>>  }
>>templat.chroma_format = ChromaToPipe(format);
>
>

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [Bug 97261] vaapi u/v wrong order since vl/util: add copy func for yv12image to nv12surface

[Zhang, Boyuan]

Hi Andy,

I verified the bug. You are correct. The u and v are inversed. I checked your 
patch, and confirmed it fixes the issue. Patch is Reviewed-by: Boyuan Zhang 

Thanks a lot for the help!

Regards,
Boyuan

From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf Of 
bugzilla-dae...@freedesktop.org
Sent: August-09-16 9:52 AM
To: mesa-dev@lists.freedesktop.org
Subject: [Mesa-dev] [Bug 97261] vaapi u/v wrong order since vl/util: add copy 
func for yv12image to nv12surface

Bug ID

97261

Summary

vaapi u/v wrong order since vl/util: add copy func for yv12image to nv12surface

Product

Mesa

Version

git

Hardware

Other

OS

All

Status

NEW

Severity

normal

Priority

medium

Component

Mesa core

Assignee

mesa-dev@lists.freedesktop.org

Reporter

adf.li...@gmail.com

QA Contact

mesa-dev@lists.freedesktop.org


Created attachment 125638 
[details]

small test vid



As noted st the time, though Boyuan said he couldn't reproduce, for me



vl/util: add copy func for yv12image to nv12surface



gets u and v for both yv12 and I420 inputs reversed whether encoding or

playing.



Both gstreamer and mpv affected.



Testing playback using attached small test vid that instantly shows the issue

either



VAAPI_DISABLE_INTERLACE=true mpv --vo=vaapi uvtest.mkv



or



gst-launch-1.0 filesrc location=uvtest.mkv ! matroskademux ! avdec_h264 !

vaapisink



Of course any test that outputs nv12 works OK as it avoids the conversion.



It seems that the new util function expects input to be yuv, but it actually

gets yvu.



I sent a patch to the list for this -



https://lists.freedesktop.org/archives/mesa-dev/2016-July/124695.html



Filing bug/test to see if anyone else reproduce.


You are receiving this mail because:

  *   You are the QA Contact for the bug.
  *   You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 01/12] vl: add parameters for VAAPI encode

[Zhang, Boyuan]

Hi Emil,


Thanks for the suggestion. I added brief message to each of the patch to 
explain what the patch does. Please see the new patch set I just submitted.



Hi Christian,


The un-used ref_pic related definitions are removed from this patch.


For the concern of is_idr flag , I checked the behavior of Vaapi and the codes 
again. Vaapi treats both "idr-iframe" and "non-idr-iframe" same as i-frame in 
picture type, and it uses a separate flag to tell driver whether this iframe is 
idr or not. So from only the picture type, we can't tell whether it's idr or 
not. Since VCE needs this information, so I think we still need to add this 
flag.


Regards,

Boyuan


From: Christian König 
Sent: July 1, 2016 8:21 AM
To: Zhang, Boyuan; mesa-dev@lists.freedesktop.org
Subject: Re: [PATCH 01/12] vl: add parameters for VAAPI encode

Hi Boyuan,

as Emil wrote as well try to add some commit messages to the set. For
this patch something like the following should do it:

Allow to specify more parameters in the encoding interface which where
previously just hardcoded in the encoder.

Additional to that we need to reorder the patches a bit. First the
interface changes, then the OMX changes to fill in the previously
hardcoded values, then the radeon backend changes and then last the
VA-API changes to use the new interface.

Additional to that a few notes below.

Am 30.06.2016 um 20:30 schrieb Boyuan Zhang:
> Signed-off-by: Boyuan Zhang 
> ---
>   src/gallium/include/pipe/p_video_state.h | 36 
> 
>   1 file changed, 36 insertions(+)
>
> diff --git a/src/gallium/include/pipe/p_video_state.h 
> b/src/gallium/include/pipe/p_video_state.h
> index d353be6..9cd489b 100644
> --- a/src/gallium/include/pipe/p_video_state.h
> +++ b/src/gallium/include/pipe/p_video_state.h
> @@ -352,9 +352,29 @@ struct pipe_h264_enc_rate_control
>  unsigned frame_rate_num;
>  unsigned frame_rate_den;
>  unsigned vbv_buffer_size;
> +   unsigned vbv_buf_lv;
>  unsigned target_bits_picture;
>  unsigned peak_bits_picture_integer;
>  unsigned peak_bits_picture_fraction;
> +   unsigned fill_data_enable;
> +   unsigned enforce_hrd;
> +};
> +
> +struct pipe_h264_enc_motion_estimation
> +{
> +   unsigned motion_est_quarter_pixel;
> +   unsigned enc_disable_sub_mode;
> +   unsigned lsmvert;
> +   unsigned enc_en_ime_overw_dis_subm;
> +   unsigned enc_ime_overw_dis_subm_no;
> +   unsigned enc_ime2_search_range_x;
> +   unsigned enc_ime2_search_range_y;
> +};
> +
> +struct pipe_h264_enc_pic_control
> +{
> +   unsigned enc_cabac_enable;
> +   unsigned enc_constraint_set_flags;
>   };
>
>   struct pipe_h264_enc_picture_desc
> @@ -363,17 +383,33 @@ struct pipe_h264_enc_picture_desc
>
>  struct pipe_h264_enc_rate_control rate_ctrl;
>
> +   struct pipe_h264_enc_motion_estimation motion_est;
> +   struct pipe_h264_enc_pic_control pic_ctrl;
> +
>  unsigned quant_i_frames;
>  unsigned quant_p_frames;
>  unsigned quant_b_frames;
>
>  enum pipe_h264_enc_picture_type picture_type;
>  unsigned frame_num;
> +   unsigned frame_num_cnt;
> +   unsigned p_remain;
> +   unsigned i_remain;
> +   unsigned idr_pic_id;
> +   unsigned gop_cnt;
>  unsigned pic_order_cnt;
>  unsigned ref_idx_l0;
>  unsigned ref_idx_l1;
> +   unsigned gop_size;
> +   unsigned ref_pic_mode;
>
>  bool not_referenced;
> +   bool is_idr;

Why can't this be inferred from the encoded picture type?

> +   bool has_ref_pic_list;
> +   bool enable_vui;
> +   unsigned int ref_pic_list_0[32];
> +   unsigned int ref_pic_list_1[32];
> +   unsigned int frame_idx[32];

I thought we wanted to drop the ref_pic_list handling for now. If that
is still the case please drop those fields here as well.

Regards,
Christian.

>   };
>
>   struct pipe_h265_sps

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 06/12] st/va: colorspace conversion when image is yv12 and surface is nv12

[Zhang, Boyuan]

Hi Christian,


Style issue is fixed.


Also, I checked the utility function, it seems that the existing yv12 to nv12 
function can't be used for "copying from image to surface" case, so I added a 
new function in the utility function to do this job. Please see the new 
submitted patch set.


For IYUV case, it's already converted to yv12 (by swapping u and v field) 
before the colorspace conversion call, so IYUV case should also work.


Regards,

Boyuan



From: Christian König 
Sent: July 1, 2016 8:51 AM
To: Zhang, Boyuan; mesa-dev@lists.freedesktop.org
Subject: Re: [PATCH 06/12] st/va: colorspace conversion when image is yv12 and 
surface is nv12

Am 30.06.2016 um 20:30 schrieb Boyuan Zhang:
> Signed-off-by: Boyuan Zhang 
> ---
>   src/gallium/state_trackers/va/image.c | 48 
> +--
>   1 file changed, 40 insertions(+), 8 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/image.c 
> b/src/gallium/state_trackers/va/image.c
> index 3c8cc9c..1f68169 100644
> --- a/src/gallium/state_trackers/va/image.c
> +++ b/src/gallium/state_trackers/va/image.c
> @@ -499,7 +499,7 @@ vlVaPutImage(VADriverContextP ctx, VASurfaceID surface, 
> VAImageID image,
>  VAImage *vaimage;
>  struct pipe_sampler_view **views;
>  enum pipe_format format;
> -   void *data[3];
> +   uint8_t *data[3];
>  unsigned pitches[3], i, j;
>
>  if (!ctx)
> @@ -539,7 +539,9 @@ vlVaPutImage(VADriverContextP ctx, VASurfaceID surface, 
> VAImageID image,
> return VA_STATUS_ERROR_OPERATION_FAILED;
>  }
>
> -   if (format != surf->buffer->buffer_format) {
> +   if ((format != surf->buffer->buffer_format) &&
> +  ((format != PIPE_FORMAT_YV12) || (surf->buffer->buffer_format != 
> PIPE_FORMAT_NV12)) &&
> +  ((format != PIPE_FORMAT_IYUV) || (surf->buffer->buffer_format != 
> PIPE_FORMAT_NV12))) {
> struct pipe_video_buffer *tmp_buf;
> struct pipe_video_buffer templat = surf->templat;
>
> @@ -581,12 +583,42 @@ vlVaPutImage(VADriverContextP ctx, VASurfaceID surface, 
> VAImageID image,
> unsigned width, height;
> if (!views[i]) continue;
> vlVaVideoSurfaceSize(surf, i, &width, &height);
> -  for (j = 0; j < views[i]->texture->array_size; ++j) {
> - struct pipe_box dst_box = {0, 0, j, width, height, 1};
> - drv->pipe->transfer_inline_write(drv->pipe, views[i]->texture, 0,
> -PIPE_TRANSFER_WRITE, &dst_box,
> -data[i] + pitches[i] * j,
> -pitches[i] * views[i]->texture->array_size, 0);
> +  if ((format == PIPE_FORMAT_YV12) || (format == PIPE_FORMAT_IYUV) &&
> + (surf->buffer->buffer_format == PIPE_FORMAT_NV12) && (i == 1)) {
> + struct pipe_transfer *transfer = NULL;
> + uint8_t *map = NULL;
> + struct pipe_box dst_box_1 = {0, 0, 0, width, height, 1};
> + map = drv->pipe->transfer_map(drv->pipe,
> +   views[i]->texture,
> +   0,
> +   PIPE_TRANSFER_DISCARD_RANGE,
> +   &dst_box_1, &transfer);
> + if (map == NULL)
> +return VA_STATUS_ERROR_OPERATION_FAILED;
> +
> + bool odd = false;
> + for (unsigned int k = 0; k < ((vaimage->offsets[1])/2) ; k++){
> +if (odd == false) {
> +   map[k] = data[i][k/2];
> +   odd = true;
> +}
> +else {
> +   map[k] = data[i+1][k/2];
> +   odd = false;
> +}
> + }
> + pipe_transfer_unmap(drv->pipe, transfer);
> + pipe_mutex_unlock(drv->mutex);
> + return VA_STATUS_SUCCESS;
> +  }
> +  else {

Style issue, the "}" and the "else {" should be on the same line.

Apart from that please use the u_copy_yv12_to_nv12() functions for the
conversion instead of coding it manually.

Also the code doesn't looks like it handles IYUV correctly.

Regards,
Christian.

> + for (j = 0; j < views[i]->texture->array_size; ++j) {
> +struct pipe_box dst_box = {0, 0, j, width, height, 1};
> +drv->pipe->transfer_inline_write(drv->pipe, views[i]->texture, 0,
> +   PIPE_TRANSFER_WRITE, &dst_box,
> +   data[i] + pitches[i] * j,
> +   pitches[i] * views[i]->texture->array_size, 0);
> + }
> }
>  }
>  pipe_mutex_unlock(drv->mutex);

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 08/12] st/va: add functions for VAAPI encode

[Zhang, Boyuan]

As discussed, we will improve this in a separate patch later.


Regards,

Boyuan


From: Christian König 
Sent: July 1, 2016 9:03:13 AM
To: Zhang, Boyuan; mesa-dev@lists.freedesktop.org
Subject: Re: [PATCH 08/12] st/va: add functions for VAAPI encode

Am 30.06.2016 um 20:30 schrieb Boyuan Zhang:
> Signed-off-by: Boyuan Zhang 
> ---
>   src/gallium/state_trackers/va/buffer.c |   6 +
>   src/gallium/state_trackers/va/picture.c| 170 
> -
>   src/gallium/state_trackers/va/va_private.h |   3 +
>   3 files changed, 177 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/buffer.c 
> b/src/gallium/state_trackers/va/buffer.c
> index 7d3167b..dfcebbe 100644
> --- a/src/gallium/state_trackers/va/buffer.c
> +++ b/src/gallium/state_trackers/va/buffer.c
> @@ -133,6 +133,12 @@ vlVaMapBuffer(VADriverContextP ctx, VABufferID buf_id, 
> void **pbuff)
> if (!buf->derived_surface.transfer || !*pbuff)
>return VA_STATUS_ERROR_INVALID_BUFFER;
>
> +  if (buf->type == VAEncCodedBufferType) {
> + ((VACodedBufferSegment*)buf->data)->buf = *pbuff;
> + ((VACodedBufferSegment*)buf->data)->size = buf->coded_size;
> + ((VACodedBufferSegment*)buf->data)->next = NULL;
> + *pbuff = buf->data;
> +  }
>  } else {
> pipe_mutex_unlock(drv->mutex);
> *pbuff = buf->data;
> diff --git a/src/gallium/state_trackers/va/picture.c 
> b/src/gallium/state_trackers/va/picture.c
> index 89ac024..26205b1 100644
> --- a/src/gallium/state_trackers/va/picture.c
> +++ b/src/gallium/state_trackers/va/picture.c
> @@ -78,7 +78,8 @@ vlVaBeginPicture(VADriverContextP ctx, VAContextID 
> context_id, VASurfaceID rende
> return VA_STATUS_SUCCESS;
>  }
>
> -   context->decoder->begin_frame(context->decoder, context->target, 
> &context->desc.base);
> +   if (context->decoder->entrypoint != PIPE_VIDEO_ENTRYPOINT_ENCODE)
> +  context->decoder->begin_frame(context->decoder, context->target, 
> &context->desc.base);
>
>  return VA_STATUS_SUCCESS;
>   }
> @@ -278,6 +279,140 @@ handleVASliceDataBufferType(vlVaContext *context, 
> vlVaBuffer *buf)
> num_buffers, (const void * const*)buffers, sizes);
>   }
>
> +static VAStatus
> +handleVAEncMiscParameterTypeRateControl(vlVaContext *context, 
> VAEncMiscParameterBuffer *misc)
> +{
> +   VAEncMiscParameterRateControl *rc = (VAEncMiscParameterRateControl 
> *)misc->data;
> +   if (context->desc.h264enc.rate_ctrl.rate_ctrl_method ==
> +   PIPE_H264_ENC_RATE_CONTROL_METHOD_CONSTANT)
> +  context->desc.h264enc.rate_ctrl.target_bitrate = rc->bits_per_second;
> +   else
> +  context->desc.h264enc.rate_ctrl.target_bitrate = rc->bits_per_second * 
> rc->target_percentage;
> +   context->desc.h264enc.rate_ctrl.peak_bitrate = rc->bits_per_second;
> +   if (context->desc.h264enc.rate_ctrl.target_bitrate < 200)
> +  context->desc.h264enc.rate_ctrl.vbv_buffer_size = 
> MIN2((context->desc.h264enc.rate_ctrl.target_bitrate * 2.75), 200);
> +   else
> +  context->desc.h264enc.rate_ctrl.vbv_buffer_size = 
> context->desc.h264enc.rate_ctrl.target_bitrate;
> +
> +   return VA_STATUS_SUCCESS;
> +}
> +
> +static VAStatus
> +handleVAEncSequenceParameterBufferType(vlVaDriver *drv, vlVaContext 
> *context, vlVaBuffer *buf)
> +{
> +   VAEncSequenceParameterBufferH264 *h264 = 
> (VAEncSequenceParameterBufferH264 *)buf->data;
> +   if (!context->decoder) {
> +  context->templat.max_references = h264->max_num_ref_frames;
> +  context->templat.level = h264->level_idc;
> +  context->decoder = drv->pipe->create_video_codec(drv->pipe, 
> &context->templat);
> +  if (!context->decoder)
> + return VA_STATUS_ERROR_ALLOCATION_FAILED;
> +   }
> +   context->desc.h264enc.gop_size = h264->intra_idr_period;
> +   return VA_STATUS_SUCCESS;
> +}
> +
> +static VAStatus
> +handleVAEncMiscParameterBufferType(vlVaContext *context, vlVaBuffer *buf)
> +{
> +   VAStatus vaStatus = VA_STATUS_SUCCESS;
> +   VAEncMiscParameterBuffer *misc;
> +   misc = buf->data;
> +
> +   switch (misc->type) {
> +   case VAEncMiscParameterTypeRateControl:
> +  vaStatus = handleVAEncMiscParameterTypeRateControl(context, misc);
> +  break;
> +
> +   default:
> +  break;
> +   }
> +
> +   return vaStatus;
> +}
> +
> +static VAStatus
> +handleVAEncPictureParameterBufferType(vlVaDriver *drv, vlVaContext *context, 
> vlVaBuffer *buf)

Re: [Mesa-dev] [PATCH 02/11] vl: add entry point

[Zhang, Boyuan]

For example, in patch 5/11 when " VaCreateContext", we used to CALLOC_STRUCT 
for "pps" and "sps" whenever we see video format is H.264. This is fine for 
decode ONLY case. Now, since we added H.264 encoding, "pps" and "sps" shouldn't 
be allocated. So we need to use the entry_point to determine this is H.264 
decode or H.264 encode. We can use config to determine the entrypoint since 
config_id is passed to us for VaCreateContext call. However, for 
VaDestoyContext call, only context_id is passed to us. So we need to know the 
entrypoint in order to not free the pps/sps for encoding case.

Regard,
Boyuan

case PIPE_VIDEO_FORMAT_MPEG4_AVC:
  context->templat.max_references = 0;
- context->desc.h264.pps = CALLOC_STRUCT(pipe_h264_pps);
- if (!context->desc.h264.pps) {
-FREE(context);
-return VA_STATUS_ERROR_ALLOCATION_FAILED;
- }
- context->desc.h264.pps->sps = CALLOC_STRUCT(pipe_h264_sps);
- if (!context->desc.h264.pps->sps) {
-FREE(context->desc.h264.pps);
-FREE(context);
-return VA_STATUS_ERROR_ALLOCATION_FAILED;
+ if (config->entrypoint != PIPE_VIDEO_ENTRYPOINT_ENCODE) {
+context->desc.h264.pps = CALLOC_STRUCT(pipe_h264_pps);
+if (!context->desc.h264.pps) {
+   FREE(context);
+   return VA_STATUS_ERROR_ALLOCATION_FAILED;
+}
+context->desc.h264.pps->sps = CALLOC_STRUCT(pipe_h264_sps);
+if (!context->desc.h264.pps->sps) {
+   FREE(context->desc.h264.pps);
+   FREE(context);
+   return VA_STATUS_ERROR_ALLOCATION_FAILED;
+}
  }
  break;

-Original Message-
From: Christian König [mailto:deathsim...@vodafone.de] 
Sent: July-14-16 4:07 AM
To: Zhang, Boyuan; mesa-dev@lists.freedesktop.org
Subject: Re: [PATCH 02/11] vl: add entry point

Am 14.07.2016 um 00:51 schrieb Boyuan Zhang:
> Add entry point for encoding which previously hardcoded for decoding 
> purpose only

I still can't figure out why we would want this? The variable doesn't seem to 
be used in the whole patchset.

Christian.

>
> Signed-off-by: Boyuan Zhang 
> ---
>   src/gallium/include/pipe/p_video_state.h | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/src/gallium/include/pipe/p_video_state.h 
> b/src/gallium/include/pipe/p_video_state.h
> index 754d013..39b3905 100644
> --- a/src/gallium/include/pipe/p_video_state.h
> +++ b/src/gallium/include/pipe/p_video_state.h
> @@ -131,6 +131,7 @@ enum pipe_h264_enc_rate_control_method
>   struct pipe_picture_desc
>   {
>  enum pipe_video_profile profile;
> +   enum pipe_video_entrypoint entry_point;
>   };
>   
>   struct pipe_quant_matrix

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 02/11] vl: add entry point

[Zhang, Boyuan]

Thanks for the suggestion Emil. I modified each un-submitted patch based on the 
article you provided. Please see the newly submitted patch set.

Regards,
Boyuan

-Original Message-
From: Emil Velikov [mailto:emil.l.veli...@gmail.com] 
Sent: July-14-16 1:07 PM
To: Zhang, Boyuan
Cc: Christian König; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 02/11] vl: add entry point

On 14 July 2016 at 16:06, Zhang, Boyuan  wrote:
> For example, in patch 5/11 when " VaCreateContext", we used to CALLOC_STRUCT 
> for "pps" and "sps" whenever we see video format is H.264. This is fine for 
> decode ONLY case. Now, since we added H.264 encoding, "pps" and "sps" 
> shouldn't be allocated. So we need to use the entry_point to determine this 
> is H.264 decode or H.264 encode. We can use config to determine the 
> entrypoint since config_id is passed to us for VaCreateContext call. However, 
> for VaDestoyContext call, only context_id is passed to us. So we need to know 
> the entrypoint in order to not free the pps/sps for encoding case.
>
This is a perfect example (one might call it too verbose, but that shouldn't be 
an issue) of a good information to add in the commit message :-) If a question 
is coming from a person familiar with the code this is a dead giveaway that the 
commit message is subpar.

I would kindly ask that you check through the readings suggested earlier [1] 
[2]. It seems like you did not have the time/chance.

Thanks
Emil

[1] http://who-t.blogspot.co.uk/2009/12/on-commit-messages.html
[2] http://chris.beams.io/posts/git-commit/
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 06/11] st/va: add copy function for yv12 image to nv12 surface

[Zhang, Boyuan]

Fixed both subject and coding style in newly submitted patch set.

Regards,
Boyuan

-Original Message-
From: Christian König [mailto:deathsim...@vodafone.de] 
Sent: July-14-16 3:56 AM
To: Zhang, Boyuan; mesa-dev@lists.freedesktop.org
Subject: Re: [PATCH 06/11] st/va: add copy function for yv12 image to nv12 
surface

Am 14.07.2016 um 00:51 schrieb Boyuan Zhang:
> Add function to copy from yv12 image to nv12 surface for VAAPI putimage call. 
> Existing function only work for copying from yv12 surface to nv12 image.
>
> Signed-off-by: Boyuan Zhang 

Two nitpicks here, the subject should read "vl/util: ..." and the coding style 
below.

> ---
>   src/gallium/auxiliary/util/u_video.h | 22 ++
>   1 file changed, 22 insertions(+)
>
> diff --git a/src/gallium/auxiliary/util/u_video.h 
> b/src/gallium/auxiliary/util/u_video.h
> index 9196afc..6e835d8 100644
> --- a/src/gallium/auxiliary/util/u_video.h
> +++ b/src/gallium/auxiliary/util/u_video.h
> @@ -130,6 +130,28 @@ u_copy_yv12_to_nv12(void *const *destination_data,
>   }
>   
>   static inline void
> +u_copy_yv12_img_to_nv12_surf(uint8_t *const *src, uint8_t *dest, int 
> +*offset, int field) {
> +   if (field == 0) {
> +for (int i = 0; i < offset[1] ; i++)
> +dest[i] = src[field][i];
> +   }
> +   else if (field == 1) {

"}" and "else if" should be on the same line.

Christian.

> +bool odd = false;
> +for (int k = 0; k < (offset[1]/2) ; k++){
> +if (odd == false) {
> +   dest[k] = src[field][k/2];
> +   odd = true;
> +}
> +else {
> +   dest[k] = src[field+1][k/2];
> +   odd = false;
> +}
> +}
> +   }
> +}
> +
> +static inline void
>   u_copy_swap422_packed(void *const *destination_data,
>  uint32_t const *destination_pitches,
>  int src_plane, int src_field,

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 01/12] vl: add parameters for VAAPI encode

[Zhang, Boyuan]

Hi Andy,

Thanks for the testing. I will look into the memory issue. For the cbr test, 
what was the bitrate you set? And by saying testing high rates, how high was 
the rate approximately?

Regards,
Boyuan

-Original Message-
From: Andy Furniss [mailto:adf.li...@gmail.com] 
Sent: July-15-16 9:42 AM
To: Christian König; Zhang, Boyuan; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 01/12] vl: add parameters for VAAPI encode

Christian König wrote:
> Am 05.07.2016 um 13:17 schrieb Christian König:
>> Am 01.07.2016 um 18:18 schrieb Andy Furniss:
>>> Christian König wrote:
>>>> Am 01.07.2016 um 18:02 schrieb Andy Furniss:
>>>>> Christian König wrote:
>>>>>> Am 01.07.2016 um 16:42 schrieb Andy Furniss:
>>>>>>> Boyuan Zhang wrote:
>>>>>>>> Signed-off-by: Boyuan Zhang 
>>>>>>>
>>>>>>> Is this supposed to be the same functionally as the first version?
>>>>>>>
>>>>>>> I notice on Tonga that I previously got cabac, but don't now.
>>>>>>>
>>>>>>> I see the new options are now in radeon_vce_52.c whereas before 
>>>>>>> radeon_vce_40_2_2.c was changed - is this why?
>>>>>>
>>>>>> We wanted to keep the code for the old firmware versions as it 
>>>>>> is, because we otherwise would need to test them again.
>>>>>>
>>>>>> Because of this we moved all modifications into radeon_vce_52.c.
>>>>>
>>>>> Oh OK, I guess it's early days but cabac was one thing that seemed OK.
>>>>>
>>>>> There are many issued on Tonga using vaapi - but then I suppose 
>>>>> gstreamer and ffmpeg are not really set up for it yet.
>>>>>
>>>>> eg. worse first = gpu vm fault/lock with gstreamer but not ffmpeg.
>>>>>
>>>>> to provoke do
>>>>>
>>>>> gst-launch-1.0 videotestsrc ! video/x-raw, format=NV12, 
>>>>> width=1920, height=1080, framerate=60/1 ! vaapih264enc | fakesink
>>>>>
>>>>> then flip away/back to another desktop soon will lock.
>>>>>
>>>>> Maybe I am too early eg. other patches needed? or does what you 
>>>>> say above mean that vaapienc shouldn't even be enabled for older chips?
>>>>
>>>> Tonga should be perfectly supported, you just need recent enough 
>>>> firmware.
>>>>
>>>> What does "dmesg | grep 'Found VCE firmware Version'" give you?
>>>
>>> Found VCE firmware Version: 50.17 Binary ID: 3
>>
>> Mhm, well looks like we haven't released the right firmware for Tonga 
>> yet :(
>>
>> Going to leave you a note when we have everything together.
>
> The new firmware landed on Monday and the final patchset was released 
> yesterday night.
>
> Have fun testing it,

Found VCE firmware Version: 52.4 Binary ID: 3

No luck I'm afraid. The above test will still hard lock me after about
30 seconds.

I also notice it's eating memory 2.6 gig by the time I lock.

Replace vaapih264enc with omxh264enc and all is OK, will run for ages no 
increasing memory usage. In fact I think this firmware & or kernel has fixed a 
powerplay omx issue - will update my bug for that when I have tested more.

FWIW the above test would make cqp with a qp of 0 - testing cbr does also 
leak/lock, though specifying bitrates does sort of work, the result is much 
higher than requested (maybe some framerate assumption - I am testing high 
rates.




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 01/12] vl: add parameters for VAAPI encode

[Zhang, Boyuan]

Hi Andy,

The memory issue is fixed. Please try the new patch set I just submitted. 
Should NOT have hard lock anymore.

And thanks for providing the rate control info, I will do some test about it.

Regards,
Boyuan

-Original Message-
From: Andy Furniss [mailto:adf.li...@gmail.com] 
Sent: July-15-16 1:32 PM
To: Zhang, Boyuan; Christian König; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 01/12] vl: add parameters for VAAPI encode

Zhang, Boyuan wrote:
> Hi Andy,
>
> Thanks for the testing. I will look into the memory issue. For the cbr 
> test, what was the bitrate you set? And by saying testing high rates, 
> how high was the rate approximately?

Testing more shows that I get the same size file independent of framerate. An 
example inputting rawvideo and asking gstreamer for 30mbit with 500 frames 
2560x1440 for both 25fps and 50fps produces 60M byte files for both.

omx doesn't have this issue, for 30mbit the files produced are 53M and 26M.

Also noted that omx takes 3.8 seconds and vaapi 13.8 for the same input.




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 06/11] vl/util: add copy func for yv12image to nv12surface

[Zhang, Boyuan]

Hi Andy,

I just submitted another patch set, most of the issues you reported are solved, 
please see the information below:

- Giving different frame rate should result different output size. The final 
result from my side is very close to the CBR I set. Please give a try with 
different frame rate and bit rate.

- Picture corruption (half height pic) is caused by interlaced setting. 
Interlace encoding is not supported. However, for transcoding case, VAAPI 
decode will use interlace mode, which will cause this issue. The temp solution 
is to use an Environmental Variable to disable interlace when doing 
transcoding. Please try the following command with the new patch:
DISABLE_INTERLACE=true gst-launch-1.0 filesrc 
location=~/big_buck_bunny_720p_1mb.mp4 ! qtdemux ! h264parse ! vaapidecode ! 
vaapih264enc ! filesink location=out.264

- I420 yuv -> nv12 case seems working fine on my side, can you please provide 
the testing raw file and command you were using? I want to reproduce the issue 
from my side and try to fix it if possible. Thanks a lot!


Hi Christian,

Besides fixing those issue listed above, I also modified the code based on your 
suggestions, e.g. adding mutex lock/unlock. Please take a look at the new patch 
set, and feel free to give any suggestions/comments. Thanks!

Regards,
Boyuan

-Original Message-
From: Christian König [mailto:deathsim...@vodafone.de]
Sent: July-18-16 10:15 AM
To: Zhang, Boyuan; mesa-dev@lists.freedesktop.org
Cc: adf.li...@gmail.com
Subject: Re: [PATCH 06/11] vl/util: add copy func for yv12image to nv12surface

Am 16.07.2016 um 00:41 schrieb Boyuan Zhang:
> Add function to copy from yv12 image to nv12 surface for VAAPI putimage call. 
> We need this function in VaPutImage call where copying from yv12 image to 
> nv12 surface for encoding. Existing function can't be used because it only 
> work for copying from yv12 surface to nv12 image in Vaapi.

I think we can keep the patches mostly as they are now, but I would like to get 
a bit more positive feedback from Andy and maybe others.

E.g. at least we should be able to encode something without crashing on Tonga 
and other hardware generations as well before we push it upstream.

Regards,
Christian.

>
> Signed-off-by: Boyuan Zhang 
> mailto:boyuan.zh...@amd.com>>
> ---
>   src/gallium/auxiliary/util/u_video.h | 23 +++
>   1 file changed, 23 insertions(+)
>
> diff --git a/src/gallium/auxiliary/util/u_video.h
> b/src/gallium/auxiliary/util/u_video.h
> index 9196afc..d147295 100644
> --- a/src/gallium/auxiliary/util/u_video.h
> +++ b/src/gallium/auxiliary/util/u_video.h
> @@ -130,6 +130,29 @@ u_copy_yv12_to_nv12(void *const *destination_data,
>   }
>
>   static inline void
> +u_copy_yv12_img_to_nv12_surf(uint8_t *const *src,
> + uint8_t *dest,
> + int *offset,
> + int field) {
> +   if (field == 0) {
> +  for (int i = 0; i < offset[1] ; i++)
> + dest[i] = src[field][i];
> +   } else if (field == 1) {
> +  bool odd = false;
> +  for (int i = 0; i < (offset[1]/2) ; i++){
> + if (odd == false) {
> +dest[i] = src[field][i/2];
> +odd = true;
> + } else {
> +dest[i] = src[field+1][i/2];
> +odd = false;
> + }
> +  }
> +   }
> +}
> +
> +static inline void
>   u_copy_swap422_packed(void *const *destination_data,
>  uint32_t const *destination_pitches,
>  int src_plane, int src_field,


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 05/12] st/va: add encode entrypoint

[Zhang, Boyuan]

>> @@ -150,7 +167,16 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile 
>> profile, VAEntrypoint entrypoin
>>  if (entrypoint != VAEntrypointVLD)
>> return VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>>
>> -   *config_id = p;
>> +   if (entrypoint == VAEntrypointEncSlice || entrypoint == 
>> VAEntrypointEncPicture)
>> +  config->entrypoint = PIPE_VIDEO_ENTRYPOINT_ENCODE;
>> +   else
>> +  config->entrypoint = PIPE_VIDEO_ENTRYPOINT_BITSTREAM;

>Well that doesn't make much sense here.

>First we return and error if the entrypoint isn't VAEntrypointVLD and
>then check if it's an encoding entry point.

>Additional to that I already wondered if we are really going to support
>slice level as well as picture level encoding.

>I think that it should only be one of the two.

>Regards,
>Christian.


Hi Christian,


Sorry for the confusion, The first 2 lines of codes

>>  if (entrypoint != VAEntrypointVLD)
>> return VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;

will actually be removed in the last patch where we enable the VAAPI Encode 
(Patch 12/12). In other word, we don't accept VAEncode entrypoint until the 
time we enable VAAPI Encode. Therefore, we still only accept VAEntrypointVLD at 
this patch.


And we need to accept both picture level and slice level entrypoint. For some 
application, e.g. libva h264encode test, if we don't enable slice level encode, 
it will fail the call and report h264 encode is not supported. If we enable 
both, it will still use picture level encode. That's why I put both here.


Regards,

Boyuan


From: Christian König 
Sent: July 19, 2016 4:52 AM
To: Zhang, Boyuan; mesa-dev@lists.freedesktop.org
Cc: adf.li...@gmail.com
Subject: Re: [PATCH 05/12] st/va: add encode entrypoint

Am 19.07.2016 um 00:43 schrieb Boyuan Zhang:
> VAAPI passes PIPE_VIDEO_ENTRYPOINT_ENCODE as entry point for encoding case. 
> We will save this encode entry point in config. config_id was used as profile 
> previously. Now, config has both profile and entrypoint field, and config_id 
> is used to get the config object. Later on, we pass this entrypoint to 
> context->templat.entrypoint instead of always hardcoded to 
> PIPE_VIDEO_ENTRYPOINT_BITSTREAM for decoding case previously.
>
> Signed-off-by: Boyuan Zhang 
> ---
>   src/gallium/state_trackers/va/config.c | 69 
> +++---
>   src/gallium/state_trackers/va/context.c| 59 ++---
>   src/gallium/state_trackers/va/surface.c| 14 --
>   src/gallium/state_trackers/va/va_private.h |  5 +++
>   4 files changed, 115 insertions(+), 32 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/config.c 
> b/src/gallium/state_trackers/va/config.c
> index 9ca0aa8..7ea7e24 100644
> --- a/src/gallium/state_trackers/va/config.c
> +++ b/src/gallium/state_trackers/va/config.c
> @@ -34,6 +34,8 @@
>
>   #include "va_private.h"
>
> +#include "util/u_handle_table.h"
> +
>   DEBUG_GET_ONCE_BOOL_OPTION(mpeg4, "VAAPI_MPEG4_ENABLED", false)
>
>   VAStatus
> @@ -128,14 +130,29 @@ VAStatus
>   vlVaCreateConfig(VADriverContextP ctx, VAProfile profile, VAEntrypoint 
> entrypoint,
>VAConfigAttrib *attrib_list, int num_attribs, VAConfigID 
> *config_id)
>   {
> +   vlVaDriver *drv;
> +   vlVaConfig *config;
>  struct pipe_screen *pscreen;
>  enum pipe_video_profile p;
>
>  if (!ctx)
> return VA_STATUS_ERROR_INVALID_CONTEXT;
>
> +   drv = VL_VA_DRIVER(ctx);
> +
> +   if (!drv)
> +  return VA_STATUS_ERROR_INVALID_CONTEXT;
> +
> +   config = CALLOC(1, sizeof(vlVaConfig));
> +   if (!config)
> +  return VA_STATUS_ERROR_ALLOCATION_FAILED;
> +
>  if (profile == VAProfileNone && entrypoint == VAEntrypointVideoProc) {
> -  *config_id = PIPE_VIDEO_PROFILE_UNKNOWN;
> +  config->entrypoint = VAEntrypointVideoProc;
> +  config->profile = PIPE_VIDEO_PROFILE_UNKNOWN;
> +  pipe_mutex_lock(drv->mutex);
> +  *config_id = handle_table_add(drv->htab, config);
> +  pipe_mutex_unlock(drv->mutex);
> return VA_STATUS_SUCCESS;
>  }
>
> @@ -150,7 +167,16 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile 
> profile, VAEntrypoint entrypoin
>  if (entrypoint != VAEntrypointVLD)
> return VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>
> -   *config_id = p;
> +   if (entrypoint == VAEntrypointEncSlice || entrypoint == 
> VAEntrypointEncPicture)
> +  config->entrypoint = PIPE_VIDEO_ENTRYPOINT_ENCODE;
> +   else
> +  config->entrypoint = PIPE_VIDEO_ENTRYPOINT_BITSTREAM;

Well that

Re: [Mesa-dev] [PATCH 09/12] st/va: add functions for VAAPI encode

[Zhang, Boyuan]

>> -   context->decoder->begin_frame(context->decoder, context->target, 
>> &context->desc.base);
>> +   if (context->decoder->entrypoint != PIPE_VIDEO_ENTRYPOINT_ENCODE)
>> +  context->decoder->begin_frame(context->decoder, context->target, 
>> &context->desc.base);

>Why do we do so here? Could we avoid that?

>I would rather like to keep the begin_frame()/end_frame() handling as it is.

>Christian.


This is on purpose. Based on my testing, application will call begin_frame 
first, then call PictureParameter/SequenceParameter/... to pass us all picture 
related parameters. However, some of those values are actually required by 
begin_picture call in radeon_vce. So we have to delay the call until we receive 
all the parameters that needed. Same applies to encode_bitstream call. That's 
why I delay both calls to end_frame where we get all necessary values.


Regards,

Boyuan

________
From: Christian König 
Sent: July 19, 2016 4:55:43 AM
To: Zhang, Boyuan; mesa-dev@lists.freedesktop.org
Cc: adf.li...@gmail.com
Subject: Re: [PATCH 09/12] st/va: add functions for VAAPI encode

Am 19.07.2016 um 00:43 schrieb Boyuan Zhang:
> Add necessary functions/changes for VAAPI encoding to buffer and picture. 
> These changes will allow driver to handle all Vaapi encode related 
> operations. This patch doesn't change the Vaapi decode behaviour.
>
> Signed-off-by: Boyuan Zhang 
> ---
>   src/gallium/state_trackers/va/buffer.c |   6 +
>   src/gallium/state_trackers/va/picture.c| 169 
> -
>   src/gallium/state_trackers/va/va_private.h |   3 +
>   3 files changed, 176 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/buffer.c 
> b/src/gallium/state_trackers/va/buffer.c
> index 7d3167b..dfcebbe 100644
> --- a/src/gallium/state_trackers/va/buffer.c
> +++ b/src/gallium/state_trackers/va/buffer.c
> @@ -133,6 +133,12 @@ vlVaMapBuffer(VADriverContextP ctx, VABufferID buf_id, 
> void **pbuff)
> if (!buf->derived_surface.transfer || !*pbuff)
>return VA_STATUS_ERROR_INVALID_BUFFER;
>
> +  if (buf->type == VAEncCodedBufferType) {
> + ((VACodedBufferSegment*)buf->data)->buf = *pbuff;
> + ((VACodedBufferSegment*)buf->data)->size = buf->coded_size;
> + ((VACodedBufferSegment*)buf->data)->next = NULL;
> + *pbuff = buf->data;
> +  }
>  } else {
> pipe_mutex_unlock(drv->mutex);
> *pbuff = buf->data;
> diff --git a/src/gallium/state_trackers/va/picture.c 
> b/src/gallium/state_trackers/va/picture.c
> index 89ac024..4793194 100644
> --- a/src/gallium/state_trackers/va/picture.c
> +++ b/src/gallium/state_trackers/va/picture.c
> @@ -78,7 +78,8 @@ vlVaBeginPicture(VADriverContextP ctx, VAContextID 
> context_id, VASurfaceID rende
> return VA_STATUS_SUCCESS;
>  }
>
> -   context->decoder->begin_frame(context->decoder, context->target, 
> &context->desc.base);
> +   if (context->decoder->entrypoint != PIPE_VIDEO_ENTRYPOINT_ENCODE)
> +  context->decoder->begin_frame(context->decoder, context->target, 
> &context->desc.base);

Why do we do so here? Could we avoid that?

I would rather like to keep the begin_frame()/end_frame() handling as it is.

Christian.

>
>  return VA_STATUS_SUCCESS;
>   }
> @@ -278,6 +279,139 @@ handleVASliceDataBufferType(vlVaContext *context, 
> vlVaBuffer *buf)
> num_buffers, (const void * const*)buffers, sizes);
>   }
>
> +static VAStatus
> +handleVAEncMiscParameterTypeRateControl(vlVaContext *context, 
> VAEncMiscParameterBuffer *misc)
> +{
> +   VAEncMiscParameterRateControl *rc = (VAEncMiscParameterRateControl 
> *)misc->data;
> +   if (context->desc.h264enc.rate_ctrl.rate_ctrl_method ==
> +   PIPE_H264_ENC_RATE_CONTROL_METHOD_CONSTANT)
> +  context->desc.h264enc.rate_ctrl.target_bitrate = rc->bits_per_second;
> +   else
> +  context->desc.h264enc.rate_ctrl.target_bitrate = rc->bits_per_second * 
> rc->target_percentage;
> +   context->desc.h264enc.rate_ctrl.peak_bitrate = rc->bits_per_second;
> +   if (context->desc.h264enc.rate_ctrl.target_bitrate < 200)
> +  context->desc.h264enc.rate_ctrl.vbv_buffer_size = 
> MIN2((context->desc.h264enc.rate_ctrl.target_bitrate * 2.75), 200);
> +   else
> +  context->desc.h264enc.rate_ctrl.vbv_buffer_size = 
> context->desc.h264enc.rate_ctrl.target_bitrate;
> +   context->desc.h264enc.rate_ctrl.target_bits_picture =
> +context->desc.h264enc.rate_ctrl.target_bitrate / 
> cont

Re: [Mesa-dev] [PATCH 06/11] vl/util: add copy func for yv12image to nv12surface

[Zhang, Boyuan]

Hi Andy,


Thanks for the confirmation. It seems like all basic functionality is working 
now.


I will look into the cqp issue. And for the speed related issue, we understand 
the reason and have already planned to work on it after this bring-up. However, 
it will be a separate patch in future, and won't be included in this bring-up 
patch set.


Regards,

Boyuan


From: Andy Furniss 
Sent: July 19, 2016 2:39:44 PM
To: Zhang, Boyuan; 'Christian König'; mesa-dev@lists.freedesktop.org
Subject: Re: [PATCH 06/11] vl/util: add copy func for yv12image to nv12surface

Andy Furniss wrote:
> Zhang, Boyuan wrote:
>> Hi Andy,
>>
>> I just submitted another patch set, most of the issues you reported
>>  are solved, please see the information below:
>>
>> - Giving different frame rate should result different output size.
>> The final result from my side is very close to the CBR I set.
>> Please give a try with different frame rate and bit rate.
>>
>> - Picture corruption (half height pic) is caused by interlaced
>> setting. Interlace encoding is not supported. However, for
>> transcoding case, VAAPI decode will use interlace mode, which will
>> cause this issue. The temp solution is to use an Environmental
>> Variable to disable interlace when doing transcoding. Please try
>> the following command with the new patch: DISABLE_INTERLACE=true
>> gst-launch-1.0 filesrc location=~/big_buck_bunny_720p_1mb.mp4 !
>> qtdemux ! h264parse ! vaapidecode ! vaapih264enc ! filesink
>> location=out.264
>>
>> - I420 yuv -> nv12 case seems working fine on my side, can you
>> please provide the testing raw file and command you were using? I
>> want to reproduce the issue from my side and try to fix it if
>> possible. Thanks a lot!
>
> Will try new patches tomorrow.

DISABLE_INTERLACE=true does fix the decode -> encode issue.

bitrate seems to be working OK now with different fps and various rates
I tested. Gstreamer apparently can't count > 102M so that was as high
as I could go.

Stability on Tonga is good.

Remaining issues -

The default people will get just using ... ! vaapih264enc ! ... is not
sane - it encodes with a qp=0 so is huge.
vaapih264enc parameters init-qp and min-qp have no effect, though I am
not sure they would be the right ones to specify cqp anyway.

Speed - though omxh264dec has issues with bitrates, so a direct
comparison is hard, it's always 3x faster than vaapi.

Speed 2 - there seems to be an issue in the case where the bitrate
requested is higher than can be achieved WRT the content to be encoded.

It's up to twice as slow as it would be encoding something that had the
detail to be constrained by the bitrate. This leads to the strange
situation when say screen recording 1080p60 that when nothing much is
happening the framerate can't be reached, but if there is a lot going
on then it can. This is at very high rates = 100M, but then to record
an fps type  game the higher rate may be needed for the fast action.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 05/12] st/va: add encode entrypoint

[Zhang, Boyuan]

>Makes sense, but I suggest that in this case we should add at least a comment 
>why this is still disabled.
>And it would look better if we have an "#if 0" or something like this in the 
>code which gets explicitly removed with the last patch.

Sure, I agree. I will submit a new patch set to add this and other minor 
changes.

>The problem with slice level encoding is that we haven't fully implemented it. 
>E.g. the last time I checked the h264encode test it would try to add an SPS 
>and PPS in front of the slice data returned from our VA-API driver.
>Since our VA-API driver doesn't return slice data, but rather a full blown 
>elementary stream you end up with a complete mess which looks something like 
>this:
>SPS (added by the application), PPS (added by the application), Slice Header, 
>SPS (added by the driver), PPS(added by the driver), Slice Header/Slice Data.
>That might work in some if not most cases, but is certainly not complaint to 
>the VA-API specification.
>
>Christian.

I just tried to disable slice encoding support, and even Gstreamer is not 
working. It will give error message saying "unsupported HW profile".
On the other hand, by exposing slice encoding, Gstreamer will still work as 
"Frame-in, Frame-out" mode. I didn't see that Gstreamer will add extra headers. 
And by dumping the output 264 output, it seems gstreamer is using picture 
encoding. However, as my test shown, gstreamer will not work at all if we don't 
expose slice encoding. Do you have any suggestions how we should do for this 
situation?

Regards,
Boyuan

From: Christian König [mailto:deathsim...@vodafone.de]
Sent: July-20-16 4:48 AM
To: Zhang, Boyuan; mesa-dev@lists.freedesktop.org
Cc: adf.li...@gmail.com
Subject: Re: [PATCH 05/12] st/va: add encode entrypoint

Am 20.07.2016 um 06:12 schrieb Zhang, Boyuan:

>> @@ -150,7 +167,16 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile 
>> profile, VAEntrypoint entrypoin
>>  if (entrypoint != VAEntrypointVLD)
>> return VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>>
>> -   *config_id = p;
>> +   if (entrypoint == VAEntrypointEncSlice || entrypoint == 
>> VAEntrypointEncPicture)
>> +  config->entrypoint = PIPE_VIDEO_ENTRYPOINT_ENCODE;
>> +   else
>> +  config->entrypoint = PIPE_VIDEO_ENTRYPOINT_BITSTREAM;

>Well that doesn't make much sense here.

>First we return and error if the entrypoint isn't VAEntrypointVLD and
>then check if it's an encoding entry point.

>Additional to that I already wondered if we are really going to support
>slice level as well as picture level encoding.

>I think that it should only be one of the two.

>Regards,
>Christian.



Hi Christian,



Sorry for the confusion, The first 2 lines of codes

>>  if (entrypoint != VAEntrypointVLD)
>> return VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;

will actually be removed in the last patch where we enable the VAAPI Encode 
(Patch 12/12). In other word, we don't accept VAEncode entrypoint until the 
time we enable VAAPI Encode. Therefore, we still only accept VAEntrypointVLD at 
this patch.

Makes sense, but I suggest that in this case we should add at least a comment 
why this is still disabled.

And it would look better if we have an "#if 0" or something like this in the 
code which gets explicitly removed with the last patch.





And we need to accept both picture level and slice level entrypoint. For some 
application, e.g. libva h264encode test, if we don't enable slice level encode, 
it will fail the call and report h264 encode is not supported. If we enable 
both, it will still use picture level encode. That's why I put both here.

The problem with slice level encoding is that we haven't fully implemented it. 
E.g. the last time I checked the h264encode test it would try to add an SPS and 
PPS in front of the slice data returned from our VA-API driver.

Since our VA-API driver doesn't return slice data, but rather a full blown 
elementary stream you end up with a complete mess which looks something like 
this:

SPS (added by the application), PPS (added by the application), Slice Header, 
SPS (added by the driver), PPS(added by the driver), Slice Header/Slice Data.

That might work in some if not most cases, but is certainly not complaint to 
the VA-API specification.

Christian.





Regards,

Boyuan


From: Christian König <mailto:deathsim...@vodafone.de>
Sent: July 19, 2016 4:52 AM
To: Zhang, Boyuan; 
mesa-dev@lists.freedesktop.org<mailto:mesa-dev@lists.freedesktop.org>
Cc: adf.li...@gmail.com<mailto:adf.li...@gmail.com>
Subject: Re: [PATCH 05/12] st/va: add encode entrypoint

Am 19.07.2016 um 00:43 schrieb Boyuan Zhang:
> VAAPI passes PIPE_VIDEO_ENTRYPOINT_ENCODE as

Re: [Mesa-dev] [PATCH 06/11] vl/util: add copy func for yv12image to nv12surface

[Zhang, Boyuan]

Hi Andy,

Thanks very much for providing all the information.

The I420 U V swapping issue still can't be reproduced from my side, I will try 
it again later. CQP issue is fixed in the new patch set I just submitted. 
Please use " ... vaapiencodeh264 rate-control=cqp init-qp=x ..." command, where 
x can be any value b/w 0--51. Please give a try and let me know the result. 
Other issues, e.g. encoding speed, ffmpeg, will be addressed/investigated later 
in separate patch as I mentioned. This initial patch set is to bring up VAAPI 
encode for gstreamer with basic functionality working. I will update with you 
once we make progress.

Regards,
Boyuan 



-Original Message-
From: Andy Furniss [mailto:adf.li...@gmail.com] 
Sent: July-20-16 5:31 AM
To: Zhang, Boyuan; 'Christian König'; mesa-dev@lists.freedesktop.org
Subject: Re: [PATCH 06/11] vl/util: add copy func for yv12image to nv12surface

Zhang, Boyuan wrote:
> Hi Andy,
>
>
> Thanks for the confirmation. It seems like all basic functionality is 
> working now.
>
>
> I will look into the cqp issue. And for the speed related issue, we 
> understand the reason and have already planned to work on it after 
> this bring-up. However, it will be a separate patch in future, and 
> won't be included in this bring-up patch set.

OK, though I didn't list it as remaining below due to sending a separate mail - 
the I420 issue is still present. I don't know what other s/w decoders output, 
but ffmpeg (so  avdec_h264 in a gatreamer pipeline) always seem to use I420 
(yuv420p) rather than YV12 which the conversion assumes.

So far I've been testing with gstreamer. ffmpeg needs patches from libav and 
always had some issues.

I tried again, and there's a possible further regression - though I have no 
idea whether it's ffmpeg or the newer patches here.

Just thought I would mention it, as I haven't had time to look into it further 
yet.

The regression is that when asking for a bitrate there is now a floating point 
exception, -qp still works (comes out at 0 like gst).

andy [vce-tests]$ gdb /mnt/sdb1/Gits/ffmpeg/ffmpeg_g GNU gdb (GDB) 7.10.1 
Copyright (C) 2015 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-unknown-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /mnt/sdb1/Gits/ffmpeg/ffmpeg_g...done.
(gdb) run -vaapi_device /dev/dri/renderD128 -f rawvideo -framerate 50 -s
2560x1440 -pix_fmt nv12 -i /mnt/ramdisk/trees-1440p50.nv12 -vf 'hwupload' -c:v 
h264_vaapi -profile:v 66 -b:v 40M  -bf 0 -y
/mnt/ramdisk/ffm-40M.264
Starting program: /mnt/sdb1/Gits/ffmpeg/ffmpeg_g -vaapi_device
/dev/dri/renderD128 -f rawvideo -framerate 50 -s 2560x1440 -pix_fmt nv12 -i 
/mnt/ramdisk/trees-1440p50.nv12 -vf 'hwupload' -c:v h264_vaapi -profile:v 66 
-b:v 40M  -bf 0 -y /mnt/ramdisk/ffm-40M.264 [Thread debugging using 
libthread_db enabled] Using host libthread_db library "/lib/libthread_db.so.1".
[New Thread 0x7fffea690700 (LWP 907)]
[New Thread 0x7fffe9c8b700 (LWP 908)]
[New Thread 0x7fffe948a700 (LWP 909)]
[New Thread 0x7fffe8c89700 (LWP 910)]
[New Thread 0x7fffe8488700 (LWP 911)]
[New Thread 0x7fffe7c87700 (LWP 912)]
[New Thread 0x7fffe7486700 (LWP 913)]
[New Thread 0x7fffe6c85700 (LWP 914)]
[New Thread 0x7fffe6484700 (LWP 915)]
[Thread 0x7fffe6484700 (LWP 915) exited] [Thread 0x7fffe6c85700 (LWP 914) 
exited] [Thread 0x7fffe7486700 (LWP 913) exited] [Thread 0x7fffe7c87700 (LWP 
912) exited] ffmpeg version N-81050-g9bf3fdc Copyright (c) 2000-2016 the FFmpeg 
developers
   built with gcc 5.3.0 (GCC)
   configuration: --prefix=/usr --disable-doc --enable-gpl --enable-omx 
--enable-opencl --enable-libzimg --enable-libvpx --enable-libx265 
--enable-libmp3lame --enable-libx264 --enable-gnutls
   libavutil  55. 28.100 / 55. 28.100
   libavcodec 57. 50.100 / 57. 50.100
   libavformat57. 43.100 / 57. 43.100
   libavdevice57.  0.102 / 57.  0.102
   libavfilter 6. 47.100 /  6. 47.100
   libswscale  4.  1.100 /  4.  1.100
   libswresample   2.  1.100 /  2.  1.100
   libpostproc54.  0.100 / 54.  0.100
libva info: VA-API version 0.38.1
libva info: va_getDriverName() returns -1 libva info: User requested driver 
'radeonsi'
libva info: Trying to open /usr/lib/dri/

Re: [Mesa-dev] [PATCH 09/12] st/va: add functions for VAAPI encode

[Zhang, Boyuan]

>We can keep it like this for now, but I would prefer that we clean this up and 
>change the radeon_vce so that it matches the begin/encode/end calls from 
>VA-API.

>We should probably work on this together with the performance improvements.

>Regards,
>Christian.

Hi Christian,

Sure, I agree, we can do that together with the performance improvements.

I just submitted another patch set, which addressed all your commends, as well 
as fixed a cqp issue reported by Andy. Please review.

Regards,
Boyuan

From: Christian König [mailto:deathsim...@vodafone.de]
Sent: July-20-16 4:49 AM
To: Zhang, Boyuan; mesa-dev@lists.freedesktop.org
Cc: adf.li...@gmail.com
Subject: Re: [PATCH 09/12] st/va: add functions for VAAPI encode

Am 20.07.2016 um 06:21 schrieb Zhang, Boyuan:

>> -   context->decoder->begin_frame(context->decoder, context->target, 
>> &context->desc.base);
>> +   if (context->decoder->entrypoint != PIPE_VIDEO_ENTRYPOINT_ENCODE)
>> +  context->decoder->begin_frame(context->decoder, context->target, 
>> &context->desc.base);

>Why do we do so here? Could we avoid that?

>I would rather like to keep the begin_frame()/end_frame() handling as it is.

>Christian.



This is on purpose. Based on my testing, application will call begin_frame 
first, then call PictureParameter/SequenceParameter/... to pass us all picture 
related parameters. However, some of those values are actually required by 
begin_picture call in radeon_vce. So we have to delay the call until we receive 
all the parameters that needed. Same applies to encode_bitstream call. That's 
why I delay both calls to end_frame where we get all necessary values.

We can keep it like this for now, but I would prefer that we clean this up and 
change the radeon_vce so that it matches the begin/encode/end calls from VA-API.

We should probably work on this together with the performance improvements.

Regards,
Christian.





Regards,

Boyuan

________
From: Christian König <mailto:deathsim...@vodafone.de>
Sent: July 19, 2016 4:55:43 AM
To: Zhang, Boyuan; 
mesa-dev@lists.freedesktop.org<mailto:mesa-dev@lists.freedesktop.org>
Cc: adf.li...@gmail.com<mailto:adf.li...@gmail.com>
Subject: Re: [PATCH 09/12] st/va: add functions for VAAPI encode

Am 19.07.2016 um 00:43 schrieb Boyuan Zhang:
> Add necessary functions/changes for VAAPI encoding to buffer and picture. 
> These changes will allow driver to handle all Vaapi encode related 
> operations. This patch doesn't change the Vaapi decode behaviour.
>
> Signed-off-by: Boyuan Zhang 
> <mailto:boyuan.zh...@amd.com>
> ---
>   src/gallium/state_trackers/va/buffer.c |   6 +
>   src/gallium/state_trackers/va/picture.c| 169 
> -
>   src/gallium/state_trackers/va/va_private.h |   3 +
>   3 files changed, 176 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/buffer.c 
> b/src/gallium/state_trackers/va/buffer.c
> index 7d3167b..dfcebbe 100644
> --- a/src/gallium/state_trackers/va/buffer.c
> +++ b/src/gallium/state_trackers/va/buffer.c
> @@ -133,6 +133,12 @@ vlVaMapBuffer(VADriverContextP ctx, VABufferID buf_id, 
> void **pbuff)
> if (!buf->derived_surface.transfer || !*pbuff)
>return VA_STATUS_ERROR_INVALID_BUFFER;
>
> +  if (buf->type == VAEncCodedBufferType) {
> + ((VACodedBufferSegment*)buf->data)->buf = *pbuff;
> + ((VACodedBufferSegment*)buf->data)->size = buf->coded_size;
> + ((VACodedBufferSegment*)buf->data)->next = NULL;
> + *pbuff = buf->data;
> +  }
>  } else {
> pipe_mutex_unlock(drv->mutex);
> *pbuff = buf->data;
> diff --git a/src/gallium/state_trackers/va/picture.c 
> b/src/gallium/state_trackers/va/picture.c
> index 89ac024..4793194 100644
> --- a/src/gallium/state_trackers/va/picture.c
> +++ b/src/gallium/state_trackers/va/picture.c
> @@ -78,7 +78,8 @@ vlVaBeginPicture(VADriverContextP ctx, VAContextID 
> context_id, VASurfaceID rende
> return VA_STATUS_SUCCESS;
>  }
>
> -   context->decoder->begin_frame(context->decoder, context->target, 
> &context->desc.base);
> +   if (context->decoder->entrypoint != PIPE_VIDEO_ENTRYPOINT_ENCODE)
> +  context->decoder->begin_frame(context->decoder, context->target, 
> &context->desc.base);

Why do we do so here? Could we avoid that?

I would rather like to keep the begin_frame()/end_frame() handling as it is.

Christian.

>
>  return VA_STATUS_SUCCESS;
>   }
> @@ -278,6 +279,139 @@ handleVASliceDataBufferType(vlVaContext *context, 
> vlVaBuffer *buf)
> num_buffers, (const void * cons

Re: [Mesa-dev] [PATCH 05/12] st/va: add encode entrypoint

[Zhang, Boyuan]

>> @@ -150,7 +167,18 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile 
>> profile, VAEntrypoint entrypoin
>>  if (entrypoint != VAEntrypointVLD)
>> return VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>>  
>> -   *config_id = p;
>> +#if 0
>> +   if (entrypoint == VAEntrypointEncSlice || entrypoint == 
>> VAEntrypointEncPicture)
>> +  config->entrypoint = PIPE_VIDEO_ENTRYPOINT_ENCODE;
>> +   else
>> +#endif
>> +  config->entrypoint = PIPE_VIDEO_ENTRYPOINT_BITSTREAM;

>If you don't mind I'm just going ahead and replacing this with a switch/case 
>statement.
>
>Going to commit the patch with that change in the evening if you don't have 
>any objections, but you probably have to rebase your set on top of master then.
>
>Christian.

Yes sure, I'm totally fine with it. Thanks for the suggestion.

Regards,
Boyuan


-Original Message-
From: Christian König [mailto:deathsim...@vodafone.de] 
Sent: July-21-16 4:46 AM
To: Zhang, Boyuan; mesa-dev@lists.freedesktop.org
Cc: adf.li...@gmail.com
Subject: Re: [PATCH 05/12] st/va: add encode entrypoint

Am 21.07.2016 um 00:13 schrieb Boyuan Zhang:
> VAAPI passes PIPE_VIDEO_ENTRYPOINT_ENCODE as entry point for encoding case. 
> We will save this encode entry point in config. config_id was used as profile 
> previously. Now, config has both profile and entrypoint field, and config_id 
> is used to get the config object. Later on, we pass this entrypoint to 
> context->templat.entrypoint instead of always hardcoded to 
> PIPE_VIDEO_ENTRYPOINT_BITSTREAM for decoding case previously. Encode 
> entrypoint is not accepted by driver until we enable Vaapi encode in later 
> patch.
>
> Signed-off-by: Boyuan Zhang 
> ---
>   src/gallium/state_trackers/va/config.c | 71 
> +++---
>   src/gallium/state_trackers/va/context.c| 59 +++--
>   src/gallium/state_trackers/va/surface.c| 14 --
>   src/gallium/state_trackers/va/va_private.h |  5 +++
>   4 files changed, 117 insertions(+), 32 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/config.c 
> b/src/gallium/state_trackers/va/config.c
> index 9ca0aa8..3aacc63 100644
> --- a/src/gallium/state_trackers/va/config.c
> +++ b/src/gallium/state_trackers/va/config.c
> @@ -34,6 +34,8 @@
>   
>   #include "va_private.h"
>   
> +#include "util/u_handle_table.h"
> +
>   DEBUG_GET_ONCE_BOOL_OPTION(mpeg4, "VAAPI_MPEG4_ENABLED", false)
>   
>   VAStatus
> @@ -128,14 +130,29 @@ VAStatus
>   vlVaCreateConfig(VADriverContextP ctx, VAProfile profile, VAEntrypoint 
> entrypoint,
>VAConfigAttrib *attrib_list, int num_attribs, VAConfigID 
> *config_id)
>   {
> +   vlVaDriver *drv;
> +   vlVaConfig *config;
>  struct pipe_screen *pscreen;
>  enum pipe_video_profile p;
>   
>  if (!ctx)
> return VA_STATUS_ERROR_INVALID_CONTEXT;
>   
> +   drv = VL_VA_DRIVER(ctx);
> +
> +   if (!drv)
> +  return VA_STATUS_ERROR_INVALID_CONTEXT;
> +
> +   config = CALLOC(1, sizeof(vlVaConfig));
> +   if (!config)
> +  return VA_STATUS_ERROR_ALLOCATION_FAILED;
> +
>  if (profile == VAProfileNone && entrypoint == VAEntrypointVideoProc) {
> -  *config_id = PIPE_VIDEO_PROFILE_UNKNOWN;
> +  config->entrypoint = VAEntrypointVideoProc;
> +  config->profile = PIPE_VIDEO_PROFILE_UNKNOWN;
> +  pipe_mutex_lock(drv->mutex);
> +  *config_id = handle_table_add(drv->htab, config);
> +  pipe_mutex_unlock(drv->mutex);
> return VA_STATUS_SUCCESS;
>  }
>   
> @@ -150,7 +167,18 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile 
> profile, VAEntrypoint entrypoin
>  if (entrypoint != VAEntrypointVLD)
> return VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>   
> -   *config_id = p;
> +#if 0
> +   if (entrypoint == VAEntrypointEncSlice || entrypoint == 
> VAEntrypointEncPicture)
> +  config->entrypoint = PIPE_VIDEO_ENTRYPOINT_ENCODE;
> +   else
> +#endif
> +  config->entrypoint = PIPE_VIDEO_ENTRYPOINT_BITSTREAM;

If you don't mind I'm just going ahead and replacing this with a switch/case 
statement.

Going to commit the patch with that change in the evening if you don't have any 
objections, but you probably have to rebase your set on top of master then.

Christian.

> +
> +   config->profile = p;
> +
> +   pipe_mutex_lock(drv->mutex);
> +   *config_id = handle_table_add(drv->htab, config);
> +   pipe_mutex_unlock(drv->mutex);
>   
>  return VA_STATUS_SUCCESS;
>   }
> @@ -158,9 +186,27 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile

Re: [Mesa-dev] [PATCH 05/12] st/va: add encode entrypoint

[Zhang, Boyuan]

And I forgot to say, all the patches I submitted for code review is based on 
the latest Master. And confirmed from my side, all patches can be applied to 
the latest Master without any problem.

Regards,
Boyuan

-Original Message-
From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf Of 
Zhang, Boyuan
Sent: July-21-16 10:52 AM
To: 'Christian König'; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 05/12] st/va: add encode entrypoint

>> @@ -150,7 +167,18 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile 
>> profile, VAEntrypoint entrypoin
>>  if (entrypoint != VAEntrypointVLD)
>> return VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>>  
>> -   *config_id = p;
>> +#if 0
>> +   if (entrypoint == VAEntrypointEncSlice || entrypoint == 
>> VAEntrypointEncPicture)
>> +  config->entrypoint = PIPE_VIDEO_ENTRYPOINT_ENCODE;
>> +   else
>> +#endif
>> +  config->entrypoint = PIPE_VIDEO_ENTRYPOINT_BITSTREAM;

>If you don't mind I'm just going ahead and replacing this with a switch/case 
>statement.
>
>Going to commit the patch with that change in the evening if you don't have 
>any objections, but you probably have to rebase your set on top of master then.
>
>Christian.

Yes sure, I'm totally fine with it. Thanks for the suggestion.

Regards,
Boyuan


-Original Message-
From: Christian König [mailto:deathsim...@vodafone.de]
Sent: July-21-16 4:46 AM
To: Zhang, Boyuan; mesa-dev@lists.freedesktop.org
Cc: adf.li...@gmail.com
Subject: Re: [PATCH 05/12] st/va: add encode entrypoint

Am 21.07.2016 um 00:13 schrieb Boyuan Zhang:
> VAAPI passes PIPE_VIDEO_ENTRYPOINT_ENCODE as entry point for encoding case. 
> We will save this encode entry point in config. config_id was used as profile 
> previously. Now, config has both profile and entrypoint field, and config_id 
> is used to get the config object. Later on, we pass this entrypoint to 
> context->templat.entrypoint instead of always hardcoded to 
> PIPE_VIDEO_ENTRYPOINT_BITSTREAM for decoding case previously. Encode 
> entrypoint is not accepted by driver until we enable Vaapi encode in later 
> patch.
>
> Signed-off-by: Boyuan Zhang 
> ---
>   src/gallium/state_trackers/va/config.c | 71 
> +++---
>   src/gallium/state_trackers/va/context.c| 59 +++--
>   src/gallium/state_trackers/va/surface.c| 14 --
>   src/gallium/state_trackers/va/va_private.h |  5 +++
>   4 files changed, 117 insertions(+), 32 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/config.c
> b/src/gallium/state_trackers/va/config.c
> index 9ca0aa8..3aacc63 100644
> --- a/src/gallium/state_trackers/va/config.c
> +++ b/src/gallium/state_trackers/va/config.c
> @@ -34,6 +34,8 @@
>   
>   #include "va_private.h"
>   
> +#include "util/u_handle_table.h"
> +
>   DEBUG_GET_ONCE_BOOL_OPTION(mpeg4, "VAAPI_MPEG4_ENABLED", false)
>   
>   VAStatus
> @@ -128,14 +130,29 @@ VAStatus
>   vlVaCreateConfig(VADriverContextP ctx, VAProfile profile, VAEntrypoint 
> entrypoint,
>VAConfigAttrib *attrib_list, int num_attribs, VAConfigID 
> *config_id)
>   {
> +   vlVaDriver *drv;
> +   vlVaConfig *config;
>  struct pipe_screen *pscreen;
>  enum pipe_video_profile p;
>   
>  if (!ctx)
> return VA_STATUS_ERROR_INVALID_CONTEXT;
>   
> +   drv = VL_VA_DRIVER(ctx);
> +
> +   if (!drv)
> +  return VA_STATUS_ERROR_INVALID_CONTEXT;
> +
> +   config = CALLOC(1, sizeof(vlVaConfig));
> +   if (!config)
> +  return VA_STATUS_ERROR_ALLOCATION_FAILED;
> +
>  if (profile == VAProfileNone && entrypoint == VAEntrypointVideoProc) {
> -  *config_id = PIPE_VIDEO_PROFILE_UNKNOWN;
> +  config->entrypoint = VAEntrypointVideoProc;
> +  config->profile = PIPE_VIDEO_PROFILE_UNKNOWN;
> +  pipe_mutex_lock(drv->mutex);
> +  *config_id = handle_table_add(drv->htab, config);
> +  pipe_mutex_unlock(drv->mutex);
> return VA_STATUS_SUCCESS;
>  }
>   
> @@ -150,7 +167,18 @@ vlVaCreateConfig(VADriverContextP ctx, VAProfile 
> profile, VAEntrypoint entrypoin
>  if (entrypoint != VAEntrypointVLD)
> return VA_STATUS_ERROR_UNSUPPORTED_ENTRYPOINT;
>   
> -   *config_id = p;
> +#if 0
> +   if (entrypoint == VAEntrypointEncSlice || entrypoint == 
> VAEntrypointEncPicture)
> +  config->entrypoint = PIPE_VIDEO_ENTRYPOINT_ENCODE;
> +   else
> +#endif
> +  config->entrypoint = PIPE_VIDEO_ENTRYPOINT_BITSTREAM;

If you don't mind I'm just going ahead and replacing this with a switch/case 
statement.

Going t

Re: [Mesa-dev] [PATCH 06/11] vl/util: add copy func for yv12image to nv12surface

[Zhang, Boyuan]

Hi Andy,

I just submitted another patch set.

1. Fixed previously reported regression when using ffmpeg to encode.
2. Fixed I420 "width=720,height=480" garbage output issue.

Please give a try from your side.

Regards,
Boyuan


Hi Christian,

As stated above, this new patch set just submitted fixed the 2 issue reported 
by Andy. Main change is adding a new patch (8/9) to fix the regression issue, 
detailed information is written in the patch. I believe all basic functionality 
is working fine now if Andy confirmed the 2 fixes. And as discussed, other 
changes/issues will be addressed with the performance improvement in future. 

Please let me know whether this patch set is good enough for pushing to 
upstream?

Regards,
Boyuan 


-Original Message-
From: Andy Furniss [mailto:adf.li...@gmail.com] 
Sent: July-21-16 8:57 AM
To: Zhang, Boyuan; 'Christian König'; mesa-dev@lists.freedesktop.org
Subject: Re: [PATCH 06/11] vl/util: add copy func for yv12image to nv12surface

Zhang, Boyuan wrote:
> Hi Andy,
>
> Thanks very much for providing all the information.
>
> The I420 U V swapping issue still can't be reproduced from my side, I 
> will try it again later. CQP issue is fixed in the new patch set I 
> just submitted. Please use " ... vaapiencodeh264 rate-control=cqp 
> init-qp=x ..." command, where x can be any value b/w 0--51. Please 
> give a try and let me know the result. Other issues, e.g. encoding 
> speed, ffmpeg, will be addressed/investigated later in separate patch 
> as I mentioned. This initial patch set is to bring up VAAPI encode for 
> gstreamer with basic functionality working. I will update with you 
> once we make progress.

CQP is working OK now.

On the I420 I still see it whatever I try and have just managed to produce a 
totally trashed output.

Below produces "expected" output = colors are wrong for I420 but the vid is OK 
apart from that.

gst-launch-1.0 videotestsrc num-buffers=5 ! 
video/x-raw,format=I420,width=1280,height=720,framerate=1/1 ! 
vaapih264enc ! h264parse ! mp4mux ! filesink location=I420.mp4

gst-launch-1.0 videotestsrc num-buffers=5 ! 
video/x-raw,format=NV12,width=1280,height=720,framerate=1/1 ! 
vaapih264enc ! h264parse ! mp4mux ! filesink location=NV12.mp4

I then decided I would attach a png showing both outputs, to get it to fit I 
repeated above with width=720,height=480 and the result for
I420 was totally trashed, NV12 OK.

Replacing vaapih264enc with x264enc for the trashed case produces good output - 
so I don't think it's the input that is trashed at that res/pix_fmt.




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] st/va: fix the incorrect max profiles report

[Zhang, Boyuan]

This patch is 
Reviewed-by: Boyuan Zhang 

-Original Message-
From: mesa-dev  On Behalf Of Liu, Leo
Sent: February 8, 2019 10:11 AM
To: mesa-dev@lists.freedesktop.org
Cc: 19 . 0 
Subject: [Mesa-dev] [PATCH 1/2] st/va: fix the incorrect max profiles report

Add "PIPE_VIDEO_PROFILE_MAX" to enum, so it will make sure here will be correct 
when adding more profiles in the future.

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

Signed-off-by: Leo Liu 
Cc: 19.0 
---
 src/gallium/include/pipe/p_video_enums.h | 3 ++-  
src/gallium/state_trackers/va/context.c  | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/gallium/include/pipe/p_video_enums.h 
b/src/gallium/include/pipe/p_video_enums.h
index b5b8b062285..adbe7858d0f 100644
--- a/src/gallium/include/pipe/p_video_enums.h
+++ b/src/gallium/include/pipe/p_video_enums.h
@@ -70,7 +70,8 @@ enum pipe_video_profile
PIPE_VIDEO_PROFILE_HEVC_MAIN_444,
PIPE_VIDEO_PROFILE_JPEG_BASELINE,
PIPE_VIDEO_PROFILE_VP9_PROFILE0,
-   PIPE_VIDEO_PROFILE_VP9_PROFILE2
+   PIPE_VIDEO_PROFILE_VP9_PROFILE2,
+   PIPE_VIDEO_PROFILE_MAX
 };
 
 /* Video caps, can be different for each codec/profile */ diff --git 
a/src/gallium/state_trackers/va/context.c 
b/src/gallium/state_trackers/va/context.c
index 14e904ee490..47a5e7be230 100644
--- a/src/gallium/state_trackers/va/context.c
+++ b/src/gallium/state_trackers/va/context.c
@@ -175,7 +175,7 @@ VA_DRIVER_INIT_FUNC(VADriverContextP ctx)
ctx->version_minor = 1;
*ctx->vtable = vtable;
*ctx->vtable_vpp = vtable_vpp;
-   ctx->max_profiles = PIPE_VIDEO_PROFILE_MPEG4_AVC_HIGH - 
PIPE_VIDEO_PROFILE_UNKNOWN;
+   ctx->max_profiles = PIPE_VIDEO_PROFILE_MAX - 
+ PIPE_VIDEO_PROFILE_UNKNOWN - 1;
ctx->max_entrypoints = 2;
ctx->max_attributes = 1;
ctx->max_image_formats = VL_VA_MAX_IMAGE_FORMATS;
--
2.17.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] st/va/vp9: set max reference as default of VP9 reference number

[Zhang, Boyuan]

This patch is 
Reviewed-by: Boyuan Zhang 

-Original Message-
From: mesa-dev  On Behalf Of Liu, Leo
Sent: February 8, 2019 10:11 AM
To: mesa-dev@lists.freedesktop.org
Cc: 19 . 0 
Subject: [Mesa-dev] [PATCH 2/2] st/va/vp9: set max reference as default of VP9 
reference number

If there is no information about number of render targets

Signed-off-by: Leo Liu 
Cc: 19.0 
---
 src/gallium/state_trackers/va/picture_vp9.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/gallium/state_trackers/va/picture_vp9.c 
b/src/gallium/state_trackers/va/picture_vp9.c
index c1ca54cd008..b5aca9a513c 100644
--- a/src/gallium/state_trackers/va/picture_vp9.c
+++ b/src/gallium/state_trackers/va/picture_vp9.c
@@ -28,6 +28,8 @@
 #include "vl/vl_vlc.h"
 #include "va_private.h"
 
+#define NUM_VP9_REFS 8
+
 void vlVaHandlePictureParameterBufferVP9(vlVaDriver *drv, vlVaContext 
*context, vlVaBuffer *buf)  {
VADecPictureParameterBufferVP9 *vp9 = buf->data; @@ -79,8 +81,11 @@ void 
vlVaHandlePictureParameterBufferVP9(vlVaDriver *drv, vlVaContext *context,
 
context->desc.vp9.picture_parameter.bit_depth = vp9->bit_depth;
 
-   for (i = 0 ; i < 8 ; i++)
+   for (i = 0 ; i < NUM_VP9_REFS ; i++)
   vlVaGetReferenceFrame(drv, vp9->reference_frames[i], 
&context->desc.vp9.ref[i]);
+
+   if (!context->decoder && !context->templat.max_references)
+  context->templat.max_references = NUM_VP9_REFS;
 }
 
 void vlVaHandleSliceParameterBufferVP9(vlVaContext *context, vlVaBuffer *buf)
--
2.17.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/3] radeon/vce: use vce structures for vce_52 firmware

[Zhang, Boyuan]

> We should write the encode structure directly without the use of the
> RVCE_CS() macros.
> 
> Otherwise all of that doesn't make much sense and is just another layer of 
> abstraction.

Different from UVD where firmware takes the address of the IB structure, VCE 
firmware directly takes the value of IB, not the address. The encode structure 
here is used for storing value. We need this layer is because we want to assign 
different values to some of the IB in VAAPI which had previously hardcoded 
values for OMX. Therefore, we still want to keep the RVCE_CS() macros. By 
keeping this, all firmware version can work, even the structure changes b/w 
different version of firmware, it still works because we only take the value of 
IB not the structure itself.

Regards,
Boyuan

-Original Message-
From: Christian König [mailto:deathsim...@vodafone.de] 
Sent: June-22-16 3:34 AM
To: Zhang, Boyuan; mesa-dev@lists.freedesktop.org
Subject: Re: [PATCH 3/3] radeon/vce: use vce structures for vce_52 firmware

Am 21.06.2016 um 16:50 schrieb Boyuan Zhang:
> Signed-off-by: Boyuan Zhang 
> ---
>   src/gallium/drivers/radeon/radeon_vce.c| 171 +++
>   src/gallium/drivers/radeon/radeon_vce.h|   1 +
>   src/gallium/drivers/radeon/radeon_vce_52.c | 447 
> +++--
>   3 files changed, 533 insertions(+), 86 deletions(-)
>
> diff --git a/src/gallium/drivers/radeon/radeon_vce.c 
> b/src/gallium/drivers/radeon/radeon_vce.c
> index e16e0cf..0d96085 100644
> --- a/src/gallium/drivers/radeon/radeon_vce.c
> +++ b/src/gallium/drivers/radeon/radeon_vce.c
> @@ -139,6 +139,176 @@ static void sort_cpb(struct rvce_encoder *enc)
>   }
>   }
>   
> +static void get_rate_control_param(struct rvce_encoder *enc, struct 
> +pipe_h264_enc_picture_desc *pic) {

Move all of this into the firmware specific file. Don't add anything to the 
common file since we don't want to implement this for the older firmware 
versions.

> + enc->enc_pic.rc.rc_method = pic->rate_ctrl.rate_ctrl_method;
> + enc->enc_pic.rc.target_bitrate = pic->rate_ctrl.target_bitrate;
> + enc->enc_pic.rc.peak_bitrate = pic->rate_ctrl.peak_bitrate;
> + enc->enc_pic.rc.quant_i_frames = pic->quant_i_frames;
> + enc->enc_pic.rc.quant_p_frames = pic->quant_p_frames;
> + enc->enc_pic.rc.quant_b_frames = pic->quant_b_frames;
> + enc->enc_pic.rc.gop_size = pic->gop_size;
> + enc->enc_pic.rc.frame_rate_num = pic->rate_ctrl.frame_rate_num;
> + enc->enc_pic.rc.frame_rate_den = pic->rate_ctrl.frame_rate_den;
> + enc->enc_pic.rc.max_qp = 51;
> +
> + if (pic->enable_low_level_control == true) {
> + enc->enc_pic.rc.vbv_buffer_size = 2000;
> + if (pic->rate_ctrl.frame_rate_num == 0)
> + enc->enc_pic.rc.frame_rate_num = 30;
> + if (pic->rate_ctrl.frame_rate_den == 0)
> + enc->enc_pic.rc.frame_rate_den = 1;
> + enc->enc_pic.rc.vbv_buf_lv = 48;
> + enc->enc_pic.rc.fill_data_enable = 1;
> + enc->enc_pic.rc.enforce_hrd = 1;
> + enc->enc_pic.rc.target_bits_picture = 
> enc->enc_pic.rc.target_bitrate / enc->enc_pic.rc.frame_rate_num;
> + enc->enc_pic.rc.peak_bits_picture_integer = 
> enc->enc_pic.rc.peak_bitrate / enc->enc_pic.rc.frame_rate_num;
> + enc->enc_pic.rc.peak_bits_picture_fraction = 0;
> + } else {
> + enc->enc_pic.rc.vbv_buffer_size = 
> pic->rate_ctrl.vbv_buffer_size;
> + enc->enc_pic.rc.vbv_buf_lv = 0;
> + enc->enc_pic.rc.fill_data_enable = 0;
> + enc->enc_pic.rc.enforce_hrd = 0;
> + enc->enc_pic.rc.target_bits_picture = 
> pic->rate_ctrl.target_bits_picture;
> + enc->enc_pic.rc.peak_bits_picture_integer = 
> pic->rate_ctrl.peak_bits_picture_integer;
> + enc->enc_pic.rc.peak_bits_picture_fraction = 
> pic->rate_ctrl.peak_bits_picture_fraction;
> + }
> +}
> +
> +static void get_motion_estimation_param(struct rvce_encoder *enc, 
> +struct pipe_h264_enc_picture_desc *pic) {
> + if (pic->enable_low_level_control == true) {
> + enc->enc_pic.me.motion_est_quarter_pixel = 0x0001;
> + enc->enc_pic.me.enc_disable_sub_mode = 0x0078;
> + enc->enc_pic.me.lsmvert = 0x0002;
> + enc->enc_pic.me.enc_en_ime_overw_dis_subm = 0x0001;
> + enc->enc_pic.me.enc_ime_overw_dis_subm_no = 0x0001;
> + enc->enc_pic.me.enc_ime2_search_range_x = 0x0004;
> + enc->enc_p

Re: [Mesa-dev] [PATCH 2/3] vl: add parameters for VAAPI encode

[Zhang, Boyuan]

Agree, please see the new patch set I just sent.

-Original Message-
From: Christian König [mailto:deathsim...@vodafone.de] 
Sent: June-22-16 3:29 AM
To: Zhang, Boyuan; mesa-dev@lists.freedesktop.org
Subject: Re: [PATCH 2/3] vl: add parameters for VAAPI encode

Am 21.06.2016 um 16:50 schrieb Boyuan Zhang:
> Signed-off-by: Boyuan Zhang 

Please move that patch to the end of the series. E.g. implement the existing 
interface first, then add the new one and make the changes to support the new 
one.

Regards,
Christian.

> ---
>   src/gallium/include/pipe/p_video_state.h | 13 +
>   1 file changed, 13 insertions(+)
>
> diff --git a/src/gallium/include/pipe/p_video_state.h 
> b/src/gallium/include/pipe/p_video_state.h
> index d353be6..d519d17 100644
> --- a/src/gallium/include/pipe/p_video_state.h
> +++ b/src/gallium/include/pipe/p_video_state.h
> @@ -131,6 +131,7 @@ enum pipe_h264_enc_rate_control_method
>   struct pipe_picture_desc
>   {
>  enum pipe_video_profile profile;
> +   enum pipe_video_entrypoint entry_point;
>   };
>   
>   struct pipe_quant_matrix
> @@ -369,11 +370,23 @@ struct pipe_h264_enc_picture_desc
>   
>  enum pipe_h264_enc_picture_type picture_type;
>  unsigned frame_num;
> +   unsigned frame_num_cnt;
> +   unsigned p_remain;
> +   unsigned i_remain;
> +   unsigned idr_pic_id;
> +   unsigned gop_cnt;
>  unsigned pic_order_cnt;
>  unsigned ref_idx_l0;
>  unsigned ref_idx_l1;
> +   unsigned gop_size;
>   
>  bool not_referenced;
> +   bool is_idr;
> +   bool has_ref_pic_list;
> +   bool enable_low_level_control;
> +   unsigned int ref_pic_list_0[32];
> +   unsigned int ref_pic_list_1[32];
> +   unsigned int frame_idx[32];
>   };
>   
>   struct pipe_h265_sps

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 3/3] radeon/vce: use vce structures for vce_52 firmware

[Zhang, Boyuan]

OK, so I added get parameters call for each firmware versions, and moved all 
the value assignments to firmware specific file. As a result, changes made for 
specific version won't affect other version. For future firmware verison, we 
still can use the same structure but assign different values in version 
specific calls. Please see the new patch set I just sent.

-Original Message-
From: Christian König [mailto:deathsim...@vodafone.de] 
Sent: June-22-16 11:55 AM
To: Zhang, Boyuan; mesa-dev@lists.freedesktop.org
Subject: Re: [PATCH 3/3] radeon/vce: use vce structures for vce_52 firmware

Am 22.06.2016 um 17:43 schrieb Zhang, Boyuan:
>> We should write the encode structure directly without the use of the
>> RVCE_CS() macros.
>>
>> Otherwise all of that doesn't make much sense and is just another layer of 
>> abstraction.
> Different from UVD where firmware takes the address of the IB structure, VCE 
> firmware directly takes the value of IB, not the address. The encode 
> structure here is used for storing value. We need this layer is because we 
> want to assign different values to some of the IB in VAAPI which had 
> previously hardcoded values for OMX. Therefore, we still want to keep the 
> RVCE_CS() macros. By keeping this, all firmware version can work, even the 
> structure changes b/w different version of firmware, it still works because 
> we only take the value of IB not the structure itself.

And exactly that's what we don't want.

Each firmware version should have a complete separate implementation of mapping 
the values from the pipe description into the binary representation of the IB.

Otherwise we would need to test with all the older firmware versions as well 
when we make a change.

Adding different values to the IB is also possible completely without the 
structure by just using the values from the picture descriptor directly.

Regards,
Christian.

>
> Regards,
> Boyuan
>
> -Original Message-
> From: Christian König [mailto:deathsim...@vodafone.de]
> Sent: June-22-16 3:34 AM
> To: Zhang, Boyuan; mesa-dev@lists.freedesktop.org
> Subject: Re: [PATCH 3/3] radeon/vce: use vce structures for vce_52 
> firmware
>
> Am 21.06.2016 um 16:50 schrieb Boyuan Zhang:
>> Signed-off-by: Boyuan Zhang 
>> ---
>>src/gallium/drivers/radeon/radeon_vce.c| 171 +++
>>src/gallium/drivers/radeon/radeon_vce.h|   1 +
>>src/gallium/drivers/radeon/radeon_vce_52.c | 447 
>> +++--
>>3 files changed, 533 insertions(+), 86 deletions(-)
>>
>> diff --git a/src/gallium/drivers/radeon/radeon_vce.c
>> b/src/gallium/drivers/radeon/radeon_vce.c
>> index e16e0cf..0d96085 100644
>> --- a/src/gallium/drivers/radeon/radeon_vce.c
>> +++ b/src/gallium/drivers/radeon/radeon_vce.c
>> @@ -139,6 +139,176 @@ static void sort_cpb(struct rvce_encoder *enc)
>>  }
>>}
>>
>> +static void get_rate_control_param(struct rvce_encoder *enc, struct 
>> +pipe_h264_enc_picture_desc *pic) {
> Move all of this into the firmware specific file. Don't add anything to the 
> common file since we don't want to implement this for the older firmware 
> versions.
>
>> +enc->enc_pic.rc.rc_method = pic->rate_ctrl.rate_ctrl_method;
>> +enc->enc_pic.rc.target_bitrate = pic->rate_ctrl.target_bitrate;
>> +enc->enc_pic.rc.peak_bitrate = pic->rate_ctrl.peak_bitrate;
>> +enc->enc_pic.rc.quant_i_frames = pic->quant_i_frames;
>> +enc->enc_pic.rc.quant_p_frames = pic->quant_p_frames;
>> +enc->enc_pic.rc.quant_b_frames = pic->quant_b_frames;
>> +enc->enc_pic.rc.gop_size = pic->gop_size;
>> +enc->enc_pic.rc.frame_rate_num = pic->rate_ctrl.frame_rate_num;
>> +enc->enc_pic.rc.frame_rate_den = pic->rate_ctrl.frame_rate_den;
>> +enc->enc_pic.rc.max_qp = 51;
>> +
>> +if (pic->enable_low_level_control == true) {
>> +enc->enc_pic.rc.vbv_buffer_size = 2000;
>> +if (pic->rate_ctrl.frame_rate_num == 0)
>> +enc->enc_pic.rc.frame_rate_num = 30;
>> +if (pic->rate_ctrl.frame_rate_den == 0)
>> +enc->enc_pic.rc.frame_rate_den = 1;
>> +enc->enc_pic.rc.vbv_buf_lv = 48;
>> +enc->enc_pic.rc.fill_data_enable = 1;
>> +enc->enc_pic.rc.enforce_hrd = 1;
>> +enc->enc_pic.rc.target_bits_picture = 
>> enc->enc_pic.rc.target_bitrate / enc->enc_pic.rc.frame_rate_num;
>> +enc->enc_pic.rc.peak_bits_picture_integer = 
>> enc->enc_pic.rc.peak_bitrate / enc->enc_

Re: [Mesa-dev] [PATCH 01/18] radeon/vcn: add vcn encode interface

[Zhang, Boyuan]

- Added simple descriptions for all patches.
- Fixed all typos.
- Agreed and changed all types in radeon_vcn_enc.h to uintX_t/intX_t

Thanks,
Boyuan

-Original Message-
From: Alex Deucher [mailto:alexdeuc...@gmail.com] 
Sent: November-07-17 5:42 PM
To: Zhang, Boyuan
Cc: mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 01/18] radeon/vcn: add vcn encode interface

On Tue, Nov 7, 2017 at 4:58 PM,   wrote:
> From: Boyuan Zhang 
>
> Signed-off-by: Boyuan Zhang 
> ---
>  src/gallium/drivers/radeon/radeon_vcn_enc.h | 325 
> 
>  1 file changed, 325 insertions(+)
>  create mode 100644 src/gallium/drivers/radeon/radeon_vcn_enc.h
>
> diff --git a/src/gallium/drivers/radeon/radeon_vcn_enc.h 
> b/src/gallium/drivers/radeon/radeon_vcn_enc.h
> new file mode 100644
> index 000..a58ff6b
> --- /dev/null
> +++ b/src/gallium/drivers/radeon/radeon_vcn_enc.h
> @@ -0,0 +1,325 @@
> +/
> +**
> + *
> + * Copyright 2017 Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person 
> +obtaining a
> + * copy of this software and associated documentation files (the
> +   + * "Software"), to deal in the Software without restriction, 
> +including

Extras '+' here.

Oops, fixed.
Boyuan

> + * without limitation the rights to use, copy, modify, merge, 
> + publish,
> + * distribute, sub license, and/or sell copies of the Software, and 
> + to
> + * permit persons to whom the Software is furnished to do so, subject 
> + to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> +   + * next paragraph) shall be included in all copies or 
> + substantial portions

and here.

Fixed as well.
Boyuan

> + * of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
> + EXPRESS
> + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
> + * IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE 
> + FOR
> + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF 
> + CONTRACT,
> + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
> + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + 
> + **/
> +
> +#ifndef _RADEON_VCN_ENC_H
> +#define _RADEON_VCN_ENC_H
> +
> +#define RENCODE_FW_INTERFACE_MAJOR_VERSION 1
> +#define RENCODE_FW_INTERFACE_MINOR_VERSION 2
> +
> +#define RENCODE_IB_PARAM_SESSION_INFO  0x0001
> +#define RENCODE_IB_PARAM_TASK_INFO 0x0002
> +#define RENCODE_IB_PARAM_SESSION_INIT  0x0003
> +#define RENCODE_IB_PARAM_LAYER_CONTROL 0x0004
> +#define RENCODE_IB_PARAM_LAYER_SELECT  0x0005
> +#define RENCODE_IB_PARAM_RATE_CONTROL_SESSION_INIT 0x0006
> +#define RENCODE_IB_PARAM_RATE_CONTROL_LAYER_INIT   0x0007
> +#define RENCODE_IB_PARAM_RATE_CONTROL_PER_PICTURE  0x0008
> +#define RENCODE_IB_PARAM_QUALITY_PARAMS0x0009
> +#define RENCODE_IB_PARAM_SLICE_HEADER  0x000a
> +#define RENCODE_IB_PARAM_ENCODE_PARAMS 0x000b
> +#define RENCODE_IB_PARAM_INTRA_REFRESH 0x000c
> +#define RENCODE_IB_PARAM_ENCODE_CONTEXT_BUFFER 0x000d
> +#define RENCODE_IB_PARAM_VIDEO_BITSTREAM_BUFFER0x000e
> +#define RENCODE_IB_PARAM_FEEDBACK_BUFFER   0x0010
> +#define RENCODE_IB_PARAM_DIRECT_OUTPUT_NALU0x0020
> +
> +#define RENCODE_H264_IB_PARAM_SLICE_CONTROL0x0021
> +#define RENCODE_H264_IB_PARAM_SPEC_MISC0x0022
> +#define RENCODE_H264_IB_PARAM_ENCODE_PARAMS0x0023
> +#define RENCODE_H264_IB_PARAM_DEBLOCKING_FILTER0x0024
> +
> +#define RENCODE_IB_OP_INITIALIZE   0x0101
> +#define RENCODE_IB_OP_CLOSE_SESSION0x0102
> +#define RENCODE_IB_OP_ENCODE   0x0103
> +#define RENCODE_IB_OP_INIT_RC  0x0104
> +#define RENCODE_IB_OP_INIT_RC_VBV_BUFFER_LEVEL 0x0105
> +#define RENCODE_IB_OP_SET_SPEED_ENCODING_MODE   

Re: [Mesa-dev] [PATCH 07/18] radeon/vcn: add common encode part

[Zhang, Boyuan]

Thanks for pointing out, fixed in new patch sets.

Regards,
Boyuan

-Original Message-
From: Dylan Baker [mailto:dy...@pnwbakers.com] 
Sent: November-07-17 5:59 PM
To: Zhang, Boyuan; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 07/18] radeon/vcn: add common encode part

Quoting boyuan.zh...@amd.com (2017-11-07 13:59:02)
> From: Boyuan Zhang 
> 
> Signed-off-by: Boyuan Zhang 
> ---
>  src/gallium/drivers/radeon/Makefile.sources |   3 +
>  src/gallium/drivers/radeon/radeon_vcn_enc.c | 166 +
>  src/gallium/drivers/radeon/radeon_vcn_enc.h |  82 
>  src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c | 237 
> 
>  4 files changed, 488 insertions(+)
>  create mode 100644 src/gallium/drivers/radeon/radeon_vcn_enc.c
>  create mode 100644 src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
> 
> diff --git a/src/gallium/drivers/radeon/Makefile.sources 
> b/src/gallium/drivers/radeon/Makefile.sources
> index 22de129..0871666 100644
> --- a/src/gallium/drivers/radeon/Makefile.sources
> +++ b/src/gallium/drivers/radeon/Makefile.sources
> @@ -13,6 +13,9 @@ C_SOURCES := \
> radeon_uvd.h \
> radeon_vcn_dec.c \
> radeon_vcn_dec.h \
> +   radeon_vcn_enc.c \
> +   radeon_vcn_enc_1_2.c \
> +   radeon_vcn_enc.h \

Please add the .c files to src/gallium/drivers/radeon/meson.build as well.

Dylan
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 03/18] radeon/common: add vcn enc ip info query

[Zhang, Boyuan]

Oops, thanks for pointing out. Fixed in new patch sets.

Regards,
Boyuan

-Original Message-
From: Marek Olšák [mailto:mar...@gmail.com] 
Sent: November-08-17 11:53 AM
To: Zhang, Boyuan
Cc: mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 03/18] radeon/common: add vcn enc ip info query

On Tue, Nov 7, 2017 at 10:58 PM,   wrote:
> From: Boyuan Zhang 
>
> Signed-off-by: Boyuan Zhang 
> ---
>  src/amd/common/ac_gpu_info.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/src/amd/common/ac_gpu_info.c 
> b/src/amd/common/ac_gpu_info.c index 2e56012..b0c0a08 100644
> --- a/src/amd/common/ac_gpu_info.c
> +++ b/src/amd/common/ac_gpu_info.c
> @@ -98,7 +98,7 @@ bool ac_query_gpu_info(int fd, amdgpu_device_handle 
> dev,  {
> struct amdgpu_buffer_size_alignments alignment_info = {};
> struct amdgpu_heap_info vram, vram_vis, gtt;
> -   struct drm_amdgpu_info_hw_ip dma = {}, compute = {}, uvd = {}, vce = 
> {}, vcn_dec = {};
> +   struct drm_amdgpu_info_hw_ip dma = {}, compute = {}, uvd = {}, 
> + vce = {}, vcn_dec = {}, vcn_enc = {};
> uint32_t vce_version = 0, vce_feature = 0, uvd_version = 0, 
> uvd_feature = 0;
> int r, i, j;
> drmDevicePtr devinfo;
> @@ -174,6 +174,14 @@ bool ac_query_gpu_info(int fd, amdgpu_device_handle dev,
> }
> }
>
> +   if (info->drm_major == 3 && info->drm_minor >= 17) {
> +   r = amdgpu_query_hw_ip_info(dev, AMDGPU_HW_IP_VCN_ENC, 0, 
> &vcn_enc);
> +   if (r) {
> +   fprintf(stderr, "amdgpu: 
> + amdgpu_query_hw_ip_info(vcn_dec) failed.\n");

should be "vcn_enc" in the string.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/va: change frame_idx from array to hash table

[Zhang, Boyuan]

The code looks good to me. I also did a few local vaapi-enc tests and confirmed 
that patch works fine.

Regards,
Boyuan

-Original Message-
From: Liu, Leo 
Sent: July-26-17 12:01 PM
To: Andy Furniss; Koenig, Christian; Julien Isorce; 
mesa-dev@lists.freedesktop.org; Zhang, Boyuan
Subject: Re: [Mesa-dev] [PATCH] st/va: change frame_idx from array to hash table



On 07/25/2017 06:04 PM, Andy Furniss wrote:
> Christian König wrote:
>
>> Leo and Boyuan can you take a quick look as well? On first glance 
>> looks totally sane to me.
>
> This reminds me .
>
> I don't know what's special about my setup, but I haven't been able to 
> use gst + vce properly since March.
>
> As I said at the time -
>
> https://lists.freedesktop.org/archives/mesa-dev/2017-March/148216.html
>
> I did ping as well, but never quite got round to doing a bug.
>
> I guess devs who tests vce with gstreamer don't see this?

Personally, haven't been working on encode too much recently. esp for VA-API 
encode.

Cheers,
Leo


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/va: change frame_idx from array to hash table

[Zhang, Boyuan]

Oops, I thought I did it in previous email. Sorry about that. Patches are
Reviewed-and-tested-by: Boyuan Zhang 

Regards,
Boyuan

From: Julien Isorce [mailto:julien.iso...@gmail.com]
Sent: August-10-17 11:37 AM
To: Christian König
Cc: Zhang, Boyuan; Liu, Leo; Andy Furniss; Koenig, Christian; 
mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH] st/va: change frame_idx from array to hash table

Boyuan, gentle ping ?



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 10/18] radeon/vcn: add encode header implementations

[Zhang, Boyuan]

From: Boyuan Zhang 

Implement encoding of sps, pps, and silce headers using the newly added h.264 
header coding descriptors functions based on h.264 specs.

Signed-off-by: Boyuan Zhang 
---
 src/gallium/drivers/radeon/radeon_vcn_enc.h |   1 +
 src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c | 240 
 2 files changed, 241 insertions(+)

diff --git a/src/gallium/drivers/radeon/radeon_vcn_enc.h 
b/src/gallium/drivers/radeon/radeon_vcn_enc.h
index c04652b..0385860 100644
--- a/src/gallium/drivers/radeon/radeon_vcn_enc.h
+++ b/src/gallium/drivers/radeon/radeon_vcn_enc.h
@@ -346,6 +346,7 @@ struct radeon_enc_h264_enc_pic {
 
boolnot_referenced;
boolis_idr;
+   boolis_even_frame;
 
rvcn_enc_task_info_ttask_info;
rvcn_enc_session_init_t session_init;
diff --git a/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c 
b/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
index 5170c67..b985921 100644
--- a/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
+++ b/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
@@ -362,6 +362,239 @@ static void radeon_enc_quality_params(struct 
radeon_encoder *enc)
RADEON_ENC_END();
 }
 
+static void radeon_enc_nalu_sps(struct radeon_encoder *enc)
+{
+   RADEON_ENC_BEGIN(RENCODE_IB_PARAM_DIRECT_OUTPUT_NALU);
+   RADEON_ENC_CS(RENCODE_DIRECT_OUTPUT_NALU_TYPE_SPS);
+   uint32_t *size_in_bytes = &enc->cs->current.buf[enc->cs->current.cdw++];
+   radeon_enc_reset(enc);
+   radeon_enc_set_emulation_prevention(enc, false);
+   radeon_enc_code_fixed_bits(enc, 0x0001, 32);
+   radeon_enc_code_fixed_bits(enc, 0x67, 8);
+   radeon_enc_byte_align(enc);
+   radeon_enc_set_emulation_prevention(enc, true);
+   radeon_enc_code_fixed_bits(enc, enc->enc_pic.spec_misc.profile_idc, 8);
+   radeon_enc_code_fixed_bits(enc, 0x44, 8); //hardcode to constrained 
baseline
+   radeon_enc_code_fixed_bits(enc, enc->enc_pic.spec_misc.level_idc, 8);
+   radeon_enc_code_ue(enc, 0x0);
+
+   if(enc->enc_pic.spec_misc.profile_idc == 100 || 
enc->enc_pic.spec_misc.profile_idc == 110 || enc->enc_pic.spec_misc.profile_idc 
== 122 ||
+   enc->enc_pic.spec_misc.profile_idc == 244 || 
enc->enc_pic.spec_misc.profile_idc == 44 || enc->enc_pic.spec_misc.profile_idc 
== 83 ||
+   enc->enc_pic.spec_misc.profile_idc == 86 || 
enc->enc_pic.spec_misc.profile_idc == 118 || enc->enc_pic.spec_misc.profile_idc 
== 128 ||
+   enc->enc_pic.spec_misc.profile_idc == 138) {
+   radeon_enc_code_ue(enc, 0x1);
+   radeon_enc_code_ue(enc, 0x0);
+   radeon_enc_code_ue(enc, 0x0);
+   radeon_enc_code_fixed_bits(enc, 0x0, 2);
+   }
+
+   radeon_enc_code_ue(enc, 1);
+   radeon_enc_code_ue(enc, enc->enc_pic.pic_order_cnt_type);
+
+   if (enc->enc_pic.pic_order_cnt_type == 0)
+   radeon_enc_code_ue(enc, 1);
+
+   radeon_enc_code_ue(enc, (enc->base.max_references + 1));
+   radeon_enc_code_fixed_bits(enc, 
enc->enc_pic.layer_ctrl.max_num_temporal_layers > 1 ? 0x1 : 0x0, 1);
+   radeon_enc_code_ue(enc, 
(enc->enc_pic.session_init.aligned_picture_width / 16 - 1));
+   radeon_enc_code_ue(enc, 
(enc->enc_pic.session_init.aligned_picture_height / 16 - 1));
+   bool progressive_only = true;
+   radeon_enc_code_fixed_bits(enc, progressive_only ? 0x1 : 0x0, 1);
+
+   if (!progressive_only)
+   radeon_enc_code_fixed_bits(enc, 0x0, 1);
+
+   radeon_enc_code_fixed_bits(enc, 0x1, 1);
+
+   if ((enc->enc_pic.crop_left != 0) || (enc->enc_pic.crop_right != 0) ||
+   (enc->enc_pic.crop_top != 0) || 
(enc->enc_pic.crop_bottom != 0)) {
+   radeon_enc_code_fixed_bits(enc, 0x1, 1);
+   radeon_enc_code_ue(enc, enc->enc_pic.crop_left);
+   radeon_enc_code_ue(enc, enc->enc_pic.crop_right);
+   radeon_enc_code_ue(enc, enc->enc_pic.crop_top);
+   radeon_enc_code_ue(enc, enc->enc_pic.crop_bottom);
+   } else
+   radeon_enc_code_fixed_bits(enc, 0x0, 1);
+
+   radeon_enc_code_fixed_bits(enc, 0x1, 1);
+   radeon_enc_code_fixed_bits(enc, 0x0, 1);
+   radeon_enc_code_fixed_bits(enc, 0x0, 1);
+   radeon_enc_code_fixed_bits(enc, 0x0, 1);
+   radeon_enc_code_fixed_bits(enc, 0x0, 1);
+   radeon_enc_code_fixed_bits(enc, 0x0, 1);
+   radeon_enc_code_fixed_bits(enc, 0x0, 1);
+   radeon_enc_code_fixed_bits(enc, 0x0, 1);
+   radeon_enc_code_fixed_bits(enc, 0x0, 1);
+   radeon_enc_code_fixed_bits(enc, 0x1, 1);
+   radeon_enc_code_fixed_bits(enc, 0x1, 1);
+   radeon_enc_code_ue(enc, 0x0);
+   radeon_enc_code_ue(enc, 0x0);
+   radeon_enc_code_ue(enc, 16);
+   radeon_enc_code_ue(enc, 16);
+   radeon_enc_code_ue(enc, 0x0);
+   radeon_enc_code_ue(enc, (enc->base.max_references + 1));
+
+   radeon_enc_code_fixed_bits(en

Re: [Mesa-dev] [PATCH 10/18] radeon/vcn: add encode header implementations

[Zhang, Boyuan]

-Original Message-
From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf Of 
Christian König
Sent: November-09-17 12:19 PM
To: Mark Thompson; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 10/18] radeon/vcn: add encode header 
implementations

Am 09.11.2017 um 17:57 schrieb Mark Thompson:
>> On 08/11/17 18:08, boyuan.zh...@amd.com wrote:
>>> From: Boyuan Zhang 
>>>
>>> Implement encoding of sps, pps, and silce headers using the newly 
>>> added h.264 header coding descriptors functions based on h.264 specs.
>>>
>>> Signed-off-by: Boyuan Zhang 
>>> ---
>>>   src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c | 234 
>>> 
>>>   1 file changed, 234 insertions(+)
>> So, does this mean we could actually implement VAAPI encode properly with 
>> packed headers now rather than hard-coding all of this in the driver?

[Boyuan] Generally speaking, we are doing part of the slice header encoding 
work in driver side now. Previously for vce encode, all those works are done by 
firmware, and driver only provides required variables . Now for vcn encode, 
both driver and firmware do part of the work, and at the end firmware generate 
the entire slice header. For example, one of the questions you asked below that 
"who will set the value for slice_qp_delta?" It's actually that firmware will 
set the value, while driver side needs to instructs it. So in conclusion, we 
can't encode everything in driver side. All other questions/comments are 
answered below.

> At least that's the intention here.
> 
> BTW: Boyuan we already have a bitstream write in picture_mpeg4.c in the VA 
> state tracker.
> 
> Please unify all that stuff in a header in src/gallium/auxiliary/vl/.
> 
> Regards,
> Christian.

[Boyuan] I see. I would like to create a separate patch and move those stuff 
after these bringup patches set, does it sound fine to you? 

>>
>>> diff --git a/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c 
>>> b/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>> index 5170c67..c6dc420 100644
>>> --- a/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>> +++ b/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>> @@ -362,6 +362,233 @@ static void radeon_enc_quality_params(struct 
>>> radeon_encoder *enc)
>>> RADEON_ENC_END();
>>>   }
>>>   
>>> +static void radeon_enc_nalu_sps(struct radeon_encoder *enc) {
>>> +   RADEON_ENC_BEGIN(RENCODE_IB_PARAM_DIRECT_OUTPUT_NALU);
>>> +   RADEON_ENC_CS(RENCODE_DIRECT_OUTPUT_NALU_TYPE_SPS);
>>> +   uint32_t *size_in_bytes = &enc->cs->current.buf[enc->cs->current.cdw++];
>>> +   radeon_enc_reset(enc);
>>> +   radeon_enc_set_emulation_prevention(enc, false);
>>> +   radeon_enc_code_fixed_bits(enc, 0x0001, 32);
>>> +   radeon_enc_code_fixed_bits(enc, 0x67, 8);
>>> +   radeon_enc_byte_align(enc);
>>> +   radeon_enc_set_emulation_prevention(enc, true);
>>> +   radeon_enc_code_fixed_bits(enc, enc->enc_pic.spec_misc.profile_idc, 8);
>>> +   radeon_enc_code_fixed_bits(enc, 0x04, 8);
> >Please always set constraint_set1_flag when profile_idc is 66.  There are 
> >enough actually-constrained-baseline-but-not-marked-as-such streams in the 
> >world already to catch out decoders without full baseline support (that is, 
> >all of them).
>>
> >Also, "constraint_set5_flag shall be equal to 0 when profile_idc is not 
> >equal to 77, 88, 100, or 118".

[Boyuan] Thanks for pointing out. I modified the value to be 0x44 in the new 
patch (set1=1, and set5=1) since we only support constrained baseline for now.

>>
>>> +   radeon_enc_code_fixed_bits(enc, enc->enc_pic.spec_misc.level_idc, 8);
>>> +   radeon_enc_code_ue(enc, 0x0);
>>> +
>>> +   if(enc->enc_pic.spec_misc.profile_idc == 100 || 
>>> enc->enc_pic.spec_misc.profile_idc == 110 || 
>>> enc->enc_pic.spec_misc.profile_idc == 122 ||
>>> +   enc->enc_pic.spec_misc.profile_idc == 244 || 
>>> enc->enc_pic.spec_misc.profile_idc == 44 || 
>>> enc->enc_pic.spec_misc.profile_idc == 83 ||
>>> +   enc->enc_pic.spec_misc.profile_idc == 86 || 
>>> enc->enc_pic.spec_misc.profile_idc == 118 || 
>>> enc->enc_pic.spec_misc.profile_idc == 128 ||
>>> +   enc->enc_pic.spec_misc.profile_idc == 138) {
>>> +   radeon_enc_code_ue(enc, 0x1);
>>> +   radeon_enc_code_ue(enc, 0x0);
>>> +   radeon_enc_code_ue(enc, 0x0);
>>> +   radeon_enc_code_fixed_bits(enc, 0x0, 2);
>>> +   }
>>> +
>>> +   radeon_enc_code_ue(enc, 1);
>>> +   radeon_enc_code_ue(enc, enc->enc_pic.pic_order_cnt_type);
>>> +
>>> +   if (enc->enc_pic.pic_order_cnt_type == 0)
>>> +   radeon_enc_code_ue(enc, 1);
>> POC type can be 1 as well, which has more associated syntax.

[Boyuan] For now, we only support 0 and 2. And the implicit "else" is for 
poc_type==2 case.

>>
>>> +
>>> +   radeon_enc_code_ue(enc, (enc->base.max_references + 1));
>>> +   radeon_enc_code_fixed_bits(enc, 
>>> enc->enc_pic.layer_ctrl.max_num_temporal_layers > 1 ? 0x1 : 0x0, 1);
>>> +   radeon_enc_cod

Re: [Mesa-dev] [PATCH 10/18] radeon/vcn: add encode header implementations

[Zhang, Boyuan]

Zhang, Boyuan wrote:

>>>>> diff --git a/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>>>> b/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>>>> index 5170c67..c6dc420 100644
>>>>> --- a/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>>>> +++ b/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>>>> @@ -362,6 +362,233 @@ static void radeon_enc_quality_params(struct 
>>>>> radeon_encoder *enc)
>>>>>   RADEON_ENC_END();
>>>>>}
>>>>>
>>>>> +static void radeon_enc_nalu_sps(struct radeon_encoder *enc) {
>>>>> + RADEON_ENC_BEGIN(RENCODE_IB_PARAM_DIRECT_OUTPUT_NALU);
>>>>> + RADEON_ENC_CS(RENCODE_DIRECT_OUTPUT_NALU_TYPE_SPS);
>>>>> + uint32_t *size_in_bytes = &enc->cs->current.buf[enc->cs->current.cdw++];
>>>>> + radeon_enc_reset(enc);
>>>>> + radeon_enc_set_emulation_prevention(enc, false);
>>>>> + radeon_enc_code_fixed_bits(enc, 0x0001, 32);
>>>>> + radeon_enc_code_fixed_bits(enc, 0x67, 8);
>>>>> + radeon_enc_byte_align(enc);
>>>>> + radeon_enc_set_emulation_prevention(enc, true);
>>>>> + radeon_enc_code_fixed_bits(enc, enc->enc_pic.spec_misc.profile_idc, 8);
>>>>> + radeon_enc_code_fixed_bits(enc, 0x04, 8);
>>>> Please always set constraint_set1_flag when profile_idc is 66.  There are 
>>>> enough actually-constrained-baseline-but-not-marked-as-such streams in the 
>>>> world already to catch out decoders without full baseline support (that 
>>>> is, all of them).
>>>>
>>>> Also, "constraint_set5_flag shall be equal to 0 when profile_idc is not 
>>>> equal to 77, 88, 100, or 118".
>> 
>> [Boyuan] Thanks for pointing out. I modified the value to be 0x44 in the new 
>> patch (set1=1, and set5=1) since we only support constrained baseline for 
>> now.
>
>It's not really with cabac though. I know there was a patch to turn it off - 
>but that would have been wasteful and make linux < windows.
>
>Why not use 77 if cabac is on + not set constrained bits, windows seems to set 
>main.
>
>Currently with vce trying to set main manually from ffmeg/gst in order to get 
>something "correct" still sets flags = something that's not seen as main (but 
>works).

Yes, but still the problem is cabac is not allowed for baseline profile and we 
only support baseline for now. I'm not quite sure about windows test you 
mentioned, I'm just guessing that we might have some main profile features 
support in some closed test environment on windows side, but definitely now all 
main profile features, no b frame support. 
On linux side, we only support baseline profile for this vcn enc.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 10/18] radeon/vcn: add encode header implementations

[Zhang, Boyuan]

-Original Message-
From: Zhang, Boyuan 
Sent: November-13-17 11:41 AM
To: Andy Furniss; Koenig, Christian; Mark Thompson; 
mesa-dev@lists.freedesktop.org
Subject: RE: [Mesa-dev] [PATCH 10/18] radeon/vcn: add encode header 
implementations

Zhang, Boyuan wrote:

>>>>> diff --git a/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>>>> b/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>>>> index 5170c67..c6dc420 100644
>>>>> --- a/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>>>> +++ b/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>>>> @@ -362,6 +362,233 @@ static void radeon_enc_quality_params(struct 
>>>>> radeon_encoder *enc)
>>>>>   RADEON_ENC_END();
>>>>>}
>>>>>
>>>>> +static void radeon_enc_nalu_sps(struct radeon_encoder *enc) {
>>>>> + RADEON_ENC_BEGIN(RENCODE_IB_PARAM_DIRECT_OUTPUT_NALU);
>>>>> + RADEON_ENC_CS(RENCODE_DIRECT_OUTPUT_NALU_TYPE_SPS);
>>>>> + uint32_t *size_in_bytes = &enc->cs->current.buf[enc->cs->current.cdw++];
>>>>> + radeon_enc_reset(enc);
>>>>> + radeon_enc_set_emulation_prevention(enc, false);
>>>>> + radeon_enc_code_fixed_bits(enc, 0x0001, 32);
>>>>> + radeon_enc_code_fixed_bits(enc, 0x67, 8);
>>>>> + radeon_enc_byte_align(enc);
>>>>> + radeon_enc_set_emulation_prevention(enc, true);
>>>>> + radeon_enc_code_fixed_bits(enc, enc->enc_pic.spec_misc.profile_idc, 8);
>>>>> + radeon_enc_code_fixed_bits(enc, 0x04, 8);
>>>> Please always set constraint_set1_flag when profile_idc is 66.  There are 
>>>> enough actually-constrained-baseline-but-not-marked-as-such streams in the 
>>>> world already to catch out decoders without full baseline support (that 
>>>> is, all of them).
>>>>
>>>> Also, "constraint_set5_flag shall be equal to 0 when profile_idc is not 
>>>> equal to 77, 88, 100, or 118".
>> 
>> [Boyuan] Thanks for pointing out. I modified the value to be 0x44 in the new 
>> patch (set1=1, and set5=1) since we only support constrained baseline for 
>> now.
>
>It's not really with cabac though. I know there was a patch to turn it off - 
>but that would have been wasteful and make linux < windows.
>
>Why not use 77 if cabac is on + not set constrained bits, windows seems to set 
>main.
>
>Currently with vce trying to set main manually from ffmeg/gst in order to get 
>something "correct" still sets flags = something that's not seen as main (but 
>works).

Yes, but still the problem is cabac is not allowed for baseline profile and we 
only support baseline for now. I'm not quite sure about windows test you 
mentioned, I'm just guessing that we might have some main profile features 
support in some closed test environment on windows side, but definitely not all 
main profile features, no b frame support. 
On linux side, we only support baseline profile for this vcn enc.
(fixed a typo)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 10/18] radeon/vcn: add encode header implementations

[Zhang, Boyuan]

On 14/11/17 10:53, Andy Furniss wrote:
>> Zhang, Boyuan wrote:
>>>
>>>
>>> -Original Message-
>>> From: Zhang, Boyuan
>>> Sent: November-13-17 11:41 AM
>>> To: Andy Furniss; Koenig, Christian; Mark Thompson; 
>>> mesa-dev@lists.freedesktop.org
>>> Subject: RE: [Mesa-dev] [PATCH 10/18] radeon/vcn: add encode header 
>>> implementations
>>>
>>> Zhang, Boyuan wrote:
>>>
>>>>>>>> diff --git a/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>>>>>>> b/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>>>>>>> index 5170c67..c6dc420 100644
>>>>>>>> --- a/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>>>>>>> +++ b/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>>>>>>> @@ -362,6 +362,233 @@ static void radeon_enc_quality_params(struct 
>>>>>>>> radeon_encoder *enc)
>>>>>>>>     RADEON_ENC_END();
>>>>>>>>     }
>>>>>>>>     +static void radeon_enc_nalu_sps(struct radeon_encoder *enc) {
>>>>>>>> +    RADEON_ENC_BEGIN(RENCODE_IB_PARAM_DIRECT_OUTPUT_NALU);
>>>>>>>> +    RADEON_ENC_CS(RENCODE_DIRECT_OUTPUT_NALU_TYPE_SPS);
>>>>>>>> +    uint32_t *size_in_bytes = 
>>>>>>>> &enc->cs->current.buf[enc->cs->current.cdw++];
>>>>>>>> +    radeon_enc_reset(enc);
>>>>>>>> +    radeon_enc_set_emulation_prevention(enc, false);
>>>>>>>> +    radeon_enc_code_fixed_bits(enc, 0x0001, 32);
>>>>>>>> +    radeon_enc_code_fixed_bits(enc, 0x67, 8);
>>>>>>>> +    radeon_enc_byte_align(enc);
>>>>>>>> +    radeon_enc_set_emulation_prevention(enc, true);
>>>>>>>> +    radeon_enc_code_fixed_bits(enc, 
>>>>>>>> enc->enc_pic.spec_misc.profile_idc, 8);
>>>>>>>> +    radeon_enc_code_fixed_bits(enc, 0x04, 8);
>>>>>>> Please always set constraint_set1_flag when profile_idc is 66.  There 
>>>>>>> are enough actually-constrained-baseline-but-not-marked-as-such streams 
>>>>>>> in the world already to catch out decoders without full baseline 
>>>>>>> support (that is, all of them).
>>>>>>>
>>>>>>> Also, "constraint_set5_flag shall be equal to 0 when profile_idc is not 
>>>>>>> equal to 77, 88, 100, or 118".
>>>>>
>>>>> [Boyuan] Thanks for pointing out. I modified the value to be 0x44 in the 
>>>>> new patch (set1=1, and set5=1) since we only support constrained baseline 
>>>>> for now.
>>>>
>>>> It's not really with cabac though. I know there was a patch to turn it off 
>>>> - but that would have been wasteful and make linux < windows.
>>>>
>>>> Why not use 77 if cabac is on + not set constrained bits, windows seems to 
>>>> set main.
>>>>
>>>> Currently with vce trying to set main manually from ffmeg/gst in order to 
>>>> get something "correct" still sets flags = something that's not seen as 
>>>> main (but works).
>>>
>>> Yes, but still the problem is cabac is not allowed for baseline profile and 
>>> we only support baseline for now. I'm not quite sure about windows test you 
>>> mentioned, I'm just guessing that we might have some main profile features 
>>> support in some closed test environment on windows side, but definitely not 
>>> all main profile features, no b frame support.
>>> On linux side, we only support baseline profile for this vcn enc.
>>> (fixed a typo)
>> 
>>  Yea, it's tricky, FWIW windows does not use b-frames for the test I've done 
>> (re-live turned up high = 50mbit 60fps).
>
> I can get B-frames from AMF with a GCN 2 card on Windows.

Yes, we used to support b-frame for GCN2 (e.g. like Bonaire, Kabini, etc...) 
cards. But for later Asics, we dropped the b-frame support. Including this 
raven vcn encode, b-frame is still not supported according to our firmware team.
- Boyuan

>
>> It is flagged as main and uses cabac.
>> I guess main is the "correct" way to describe constrained baseline + cabac 
>> even if there are no other main features like b-frames.

Yes, I agree, that's the correct way in that case. However, we can't advertise 
main profile since we don't have b-frame support. It would cause serious 
problem if player tries to use b-frame when seeing that main profile is 
supported. So as explained before, we only support constrained baseline profile 
for this raven vcn encode now. And as a result, cabac is forced to be off in 
this bring-up patch.
- Boyuan

>
> Yes, it must be flagged as main in this case.
>
> - Mark
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 10/18] radeon/vcn: add encode header implementations

[Zhang, Boyuan]

Zhang, Boyuan wrote:
>> 
>> On 14/11/17 10:53, Andy Furniss wrote:
>>>> Zhang, Boyuan wrote:
>>>>>
>>>>>
>>>>> -Original Message-
>>>>> From: Zhang, Boyuan
>>>>> Sent: November-13-17 11:41 AM
>>>>> To: Andy Furniss; Koenig, Christian; Mark Thompson; 
>>>>> mesa-dev@lists.freedesktop.org
>>>>> Subject: RE: [Mesa-dev] [PATCH 10/18] radeon/vcn: add encode header 
>>>>> implementations
>>>>>
>>>>> Zhang, Boyuan wrote:
>>>>>
>>>>>>>>>> diff --git a/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>>>>>>>>> b/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>>>>>>>>> index 5170c67..c6dc420 100644
>>>>>>>>>> --- a/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>>>>>>>>> +++ b/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
>>>>>>>>>> @@ -362,6 +362,233 @@ static void radeon_enc_quality_params(struct 
>>>>>>>>>> radeon_encoder *enc)
>>>>>>>>>>  RADEON_ENC_END();
>>>>>>>>>>  }
>>>>>>>>>>  +static void radeon_enc_nalu_sps(struct radeon_encoder 
>>>>>>>>>> *enc) {
>>>>>>>>>> +RADEON_ENC_BEGIN(RENCODE_IB_PARAM_DIRECT_OUTPUT_NALU);
>>>>>>>>>> +RADEON_ENC_CS(RENCODE_DIRECT_OUTPUT_NALU_TYPE_SPS);
>>>>>>>>>> +uint32_t *size_in_bytes = 
>>>>>>>>>> &enc->cs->current.buf[enc->cs->current.cdw++];
>>>>>>>>>> +radeon_enc_reset(enc);
>>>>>>>>>> +radeon_enc_set_emulation_prevention(enc, false);
>>>>>>>>>> +radeon_enc_code_fixed_bits(enc, 0x0001, 32);
>>>>>>>>>> +radeon_enc_code_fixed_bits(enc, 0x67, 8);
>>>>>>>>>> +radeon_enc_byte_align(enc);
>>>>>>>>>> +radeon_enc_set_emulation_prevention(enc, true);
>>>>>>>>>> +radeon_enc_code_fixed_bits(enc, 
>>>>>>>>>> enc->enc_pic.spec_misc.profile_idc, 8);
>>>>>>>>>> +radeon_enc_code_fixed_bits(enc, 0x04, 8);
>>>>>>>>> Please always set constraint_set1_flag when profile_idc is 66.  There 
>>>>>>>>> are enough actually-constrained-baseline-but-not-marked-as-such 
>>>>>>>>> streams in the world already to catch out decoders without full 
>>>>>>>>> baseline support (that is, all of them).
>>>>>>>>>
>>>>>>>>> Also, "constraint_set5_flag shall be equal to 0 when profile_idc is 
>>>>>>>>> not equal to 77, 88, 100, or 118".
>>>>>>>>
>>>>>>> [Boyuan] Thanks for pointing out. I modified the value to be 0x44 in 
>>>>>>> the new patch (set1=1, and set5=1) since we only support constrained 
>>>>>>> baseline for now.
>>>>>>
>>>>>> It's not really with cabac though. I know there was a patch to turn it 
>>>>>> off - but that would have been wasteful and make linux < windows.
>>>>>>
>>>>>> Why not use 77 if cabac is on + not set constrained bits, windows seems 
>>>>>> to set main.
>>>>>>
>>>>>> Currently with vce trying to set main manually from ffmeg/gst in order 
>>>>>> to get something "correct" still sets flags = something that's not seen 
>>>>>> as main (but works).
>>>>>
>>>>> Yes, but still the problem is cabac is not allowed for baseline profile 
>>>>> and we only support baseline for now. I'm not quite sure about windows 
>>>>> test you mentioned, I'm just guessing that we might have some main 
>>>>> profile features support in some closed test environment on windows side, 
>>>>> but definitely not all main profile features, no b frame support.
>>>>> On linux side, we only support baseline profile for this vcn enc.
>>>>> (fixed a typo)
>>>>
>>>>   Yea, it's tricky, FWIW windows does not use b-frames for the test I've 
>>>> done (re-live turned

Re: [Mesa-dev] [PATCH 01/18] radeon/vcn: add vcn encode interface

[Zhang, Boyuan]

If there is no objection, the patches will be pushed by the end of the week.

Thanks,
Boyuan

-Original Message-
From: Zhang, Boyuan 
Sent: November-08-17 1:08 PM
To: mesa-dev@lists.freedesktop.org
Cc: alexdeuc...@gmail.com; dy...@pnwbakers.com; mar...@gmail.com; 
ckoenig.leichtzumer...@gmail.com; Zhang, Boyuan
Subject: [PATCH 01/18] radeon/vcn: add vcn encode interface

From: Boyuan Zhang 

Add a new header file for vcn encode interface

Signed-off-by: Boyuan Zhang 
---
 src/gallium/drivers/radeon/radeon_vcn_enc.h | 325 
 1 file changed, 325 insertions(+)
 create mode 100644 src/gallium/drivers/radeon/radeon_vcn_enc.h

diff --git a/src/gallium/drivers/radeon/radeon_vcn_enc.h 
b/src/gallium/drivers/radeon/radeon_vcn_enc.h
new file mode 100644
index 000..f9fa168
--- /dev/null
+++ b/src/gallium/drivers/radeon/radeon_vcn_enc.h
@@ -0,0 +1,325 @@
+/**
+
+ *
+ * Copyright 2017 Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person 
+obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject 
+to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial 
+portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
+EXPRESS
+ * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
+ * IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR
+ * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF 
+CONTRACT,
+ * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+ 
+***
+***/
+
+#ifndef _RADEON_VCN_ENC_H
+#define _RADEON_VCN_ENC_H
+
+#define RENCODE_FW_INTERFACE_MAJOR_VERSION 1
+#define RENCODE_FW_INTERFACE_MINOR_VERSION 2
+
+#define RENCODE_IB_PARAM_SESSION_INFO  0x0001
+#define RENCODE_IB_PARAM_TASK_INFO 0x0002
+#define RENCODE_IB_PARAM_SESSION_INIT  0x0003
+#define RENCODE_IB_PARAM_LAYER_CONTROL 0x0004
+#define RENCODE_IB_PARAM_LAYER_SELECT  0x0005
+#define RENCODE_IB_PARAM_RATE_CONTROL_SESSION_INIT 0x0006
+#define RENCODE_IB_PARAM_RATE_CONTROL_LAYER_INIT   0x0007
+#define RENCODE_IB_PARAM_RATE_CONTROL_PER_PICTURE  0x0008
+#define RENCODE_IB_PARAM_QUALITY_PARAMS0x0009
+#define RENCODE_IB_PARAM_SLICE_HEADER  0x000a
+#define RENCODE_IB_PARAM_ENCODE_PARAMS 0x000b
+#define RENCODE_IB_PARAM_INTRA_REFRESH 0x000c
+#define RENCODE_IB_PARAM_ENCODE_CONTEXT_BUFFER 0x000d
+#define RENCODE_IB_PARAM_VIDEO_BITSTREAM_BUFFER0x000e
+#define RENCODE_IB_PARAM_FEEDBACK_BUFFER   0x0010
+#define RENCODE_IB_PARAM_DIRECT_OUTPUT_NALU0x0020
+
+#define RENCODE_H264_IB_PARAM_SLICE_CONTROL0x0021
+#define RENCODE_H264_IB_PARAM_SPEC_MISC0x0022
+#define RENCODE_H264_IB_PARAM_ENCODE_PARAMS0x0023
+#define RENCODE_H264_IB_PARAM_DEBLOCKING_FILTER0x0024
+
+#define RENCODE_IB_OP_INITIALIZE   0x0101
+#define RENCODE_IB_OP_CLOSE_SESSION0x0102
+#define RENCODE_IB_OP_ENCODE   0x0103
+#define RENCODE_IB_OP_INIT_RC  0x0104
+#define RENCODE_IB_OP_INIT_RC_VBV_BUFFER_LEVEL 0x0105
+#define RENCODE_IB_OP_SET_SPEED_ENCODING_MODE  0x0106
+#define RENCODE_IB_OP_SET_BALANCE_ENCODING_MODE0x0107
+#define RENCODE_IB_OP_SET_QUALITY_ENCODING_MODE0x0108
+
+#define RENCODE_IF_MAJOR_VERSION_MASK  0x
+#define RENCODE_IF_MAJOR_VERSION_SHIFT 16
+#define RENCODE_IF_MINOR_VERSION_MASK  0x
+#define RENCODE_IF_MINOR_VERSION_SHIFT 0
+
+#define RENCODE_ENCODE_STANDARD_H264   1
+
+#define RENCODE_PREENCODE_MODE_NONE  

Re: [Mesa-dev] [PATCH] radeon/vcn: determine idr by pic type

[Zhang, Boyuan]

> Am 30.11.2017 um 22:18 schrieb Leo Liu:
>>
>>
>> On 11/30/2017 04:12 PM, boyuan.zh...@amd.com wrote:
>>> From: Boyuan Zhang 
>>>
>>> Vaapi encode interface provides idr frame flags, where omx interface 
>>> doesn't.
>>> Therefore, change to use picture type to determine idr frame, which 
>>> will work for both interfaces.
>>>
>>> Signed-off-by: Boyuan Zhang 
>> Reviewed-by: Leo Liu 
>
> Reviewed-by: Christian König 
>
> As a consequence could you remove the is_idr flag from the picture structure 
> or is that used somewhere else as well?
>
> Regards,
> Christian.

Since Vaapi interface provides the idr flag directly, it's better to have a 1:1 
mapping in driver side and use it directly. And actually, we are using this 
"is_idr" flag in some functions in st/va now. So I think it's better to keep it 
for now.

Regards,
Boyuan

>
>>
>>> ---
>>>   src/gallium/drivers/radeon/radeon_vcn_enc.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/gallium/drivers/radeon/radeon_vcn_enc.c
>>> b/src/gallium/drivers/radeon/radeon_vcn_enc.c
>>> index 9806a69..5fc9fc7 100644
>>> --- a/src/gallium/drivers/radeon/radeon_vcn_enc.c
>>> +++ b/src/gallium/drivers/radeon/radeon_vcn_enc.c
>>> @@ -47,7 +47,7 @@ static void radeon_vcn_enc_get_param(struct 
>>> radeon_encoder *enc, struct pipe_h26
>>>   enc->enc_pic.ref_idx_l0 = pic->ref_idx_l0;
>>>   enc->enc_pic.ref_idx_l1 = pic->ref_idx_l1;
>>>   enc->enc_pic.not_referenced = pic->not_referenced;
>>> -    enc->enc_pic.is_idr = pic->is_idr;
>>> +    enc->enc_pic.is_idr = (pic->picture_type ==
>>> PIPE_H264_ENC_PICTURE_TYPE_IDR);
>>>   enc->enc_pic.crop_left = 0;
>>>   enc->enc_pic.crop_right = (align(enc->base.width, 16) -
>>> enc->base.width) / 2;
>>>   enc->enc_pic.crop_top = 0;
>>

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] radeon/vcn: determine idr by pic type

[Zhang, Boyuan]

Am 04.12.2017 um 21:12 schrieb Zhang, Boyuan:
>>> Am 30.11.2017 um 22:18 schrieb Leo Liu:
>>>>
>>>> On 11/30/2017 04:12 PM, boyuan.zh...@amd.com wrote:
>>>>> From: Boyuan Zhang 
>>>>>
>>>>> Vaapi encode interface provides idr frame flags, where omx 
>>>>> interface doesn't.
>>>>> Therefore, change to use picture type to determine idr frame, which 
>>>>> will work for both interfaces.
>>>>>
>>>>> Signed-off-by: Boyuan Zhang 
>>>> Reviewed-by: Leo Liu 
>>> Reviewed-by: Christian König 
>>>
>>> As a consequence could you remove the is_idr flag from the picture 
>>> structure or is that used somewhere else as well?
>>>
>>> Regards,
>>> Christian.
>> Since Vaapi interface provides the idr flag directly, it's better to have a 
>> 1:1 mapping in driver side and use it directly.
>
> I strongly disagree. The VA-API interface for codecs is broken in quite a 
> number of ways.
>
> We should rather use the specifications as source for internal structures and 
> interfaces.

OK, that makes sense. I managed to remove the flag.

>
>>   And actually, we are using this "is_idr" flag in some functions in st/va 
>> now. So I think it's better to keep it for now.
>
> I think we should fix this as well and use the picture type here as well.

This is fixed as well, all "is_idr" flags are removed. 

Please review the new patch sets that just submitted.

Regards,
Boyuan

>
> Regards,
> Christian.
>
>>
>> Regards,
>> Boyuan
>>
>>>>> ---
>>>>>    src/gallium/drivers/radeon/radeon_vcn_enc.c | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/src/gallium/drivers/radeon/radeon_vcn_enc.c
>>>>> b/src/gallium/drivers/radeon/radeon_vcn_enc.c
>>>>> index 9806a69..5fc9fc7 100644
>>>>> --- a/src/gallium/drivers/radeon/radeon_vcn_enc.c
>>>>> +++ b/src/gallium/drivers/radeon/radeon_vcn_enc.c
>>>>> @@ -47,7 +47,7 @@ static void radeon_vcn_enc_get_param(struct 
>>>>> radeon_encoder *enc, struct pipe_h26
>>>>>    enc->enc_pic.ref_idx_l0 = pic->ref_idx_l0;
>>>>>    enc->enc_pic.ref_idx_l1 = pic->ref_idx_l1;
>>>>>    enc->enc_pic.not_referenced = pic->not_referenced;
>>>>> -    enc->enc_pic.is_idr = pic->is_idr;
>>>>> +    enc->enc_pic.is_idr = (pic->picture_type ==
>>>>> PIPE_H264_ENC_PICTURE_TYPE_IDR);
>>>>>    enc->enc_pic.crop_left = 0;
>>>>>    enc->enc_pic.crop_right = (align(enc->base.width, 16) -
>>>>> enc->base.width) / 2;
>>>>>    enc->enc_pic.crop_top = 0;
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/3] st/va: directly use idr pic flag

[Zhang, Boyuan]

> Am 09.12.2017 um 00:33 schrieb boyuan.zh...@amd.com:
>> From: Boyuan Zhang 
>>
>> Remove is_idr flag, and use idr_pic_flag provided by vaapi directly
>>
>> Signed-off-by: Boyuan Zhang 
>
> Reviewed-by: Christian König  for this one and 
> patch #3.
>
> But where is patch #1 in this series?

Patch #1 can be found here: 
https://lists.freedesktop.org/archives/mesa-dev/2017-December/179680.html
For some unknown reason, it hasn't been marked as [1/3]. Sorry for the 
confusion.

Regards,
Boyuan

>
> Regards,
> Christian.
>
>> ---
>>   src/gallium/state_trackers/va/picture.c | 8 +++-
>>   1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/gallium/state_trackers/va/picture.c 
>> b/src/gallium/state_trackers/va/picture.c
>> index 55ca16e..8951573 100644
>> --- a/src/gallium/state_trackers/va/picture.c
>> +++ b/src/gallium/state_trackers/va/picture.c
>> @@ -432,7 +432,6 @@ handleVAEncPictureParameterBufferType(vlVaDriver *drv, 
>> vlVaContext *context, vlV
>>  h264 = buf->data;
>>  context->desc.h264enc.frame_num = h264->frame_num;
>>  context->desc.h264enc.not_referenced = false;
>> -   context->desc.h264enc.is_idr = (h264->pic_fields.bits.idr_pic_flag == 1);
>>  context->desc.h264enc.pic_order_cnt = h264->CurrPic.TopFieldOrderCnt;
>>  if (context->desc.h264enc.gop_cnt == 0)
>> context->desc.h264enc.i_remain = context->gop_coeff; @@ -451,7 
>> +450,7 @@ handleVAEncPictureParameterBufferType(vlVaDriver *drv, vlVaContext 
>> *context, vlV
>> UINT_TO_PTR(h264->CurrPic.picture_id),
>> UINT_TO_PTR(h264->frame_num));
>>   
>> -   if (context->desc.h264enc.is_idr)
>> +   if (h264->pic_fields.bits.idr_pic_flag == 1)
>> context->desc.h264enc.picture_type = PIPE_H264_ENC_PICTURE_TYPE_IDR;
>>  else
>> context->desc.h264enc.picture_type = 
>> PIPE_H264_ENC_PICTURE_TYPE_P; @@ -493,10 +492,9 @@ 
>> handleVAEncSliceParameterBufferType(vlVaDriver *drv, vlVaContext *context, 
>> vlVaB
>>  else if (h264->slice_type == 0)
>> context->desc.h264enc.picture_type = PIPE_H264_ENC_PICTURE_TYPE_P;
>>  else if (h264->slice_type == 2) {
>> -  if (context->desc.h264enc.is_idr){
>> - context->desc.h264enc.picture_type = 
>> PIPE_H264_ENC_PICTURE_TYPE_IDR;
>> +  if (context->desc.h264enc.picture_type == 
>> + PIPE_H264_ENC_PICTURE_TYPE_IDR)
>>context->desc.h264enc.idr_pic_id++;
>> -   } else
>> +   else
>>context->desc.h264enc.picture_type = PIPE_H264_ENC_PICTURE_TYPE_I;
>>  } else
>> context->desc.h264enc.picture_type = 
>> PIPE_H264_ENC_PICTURE_TYPE_SKIP;

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] vl: reorder H264 profiles

[Zhang, Boyuan]

Yes, I was thinking either using a switch helper function or doing this 1 line 
change since there will be no more profile to be added.

But no problem, I agree that using switch helper function is a safer way.

New patch just sent.

Regards,
Boyuan


-Original Message-
From: Christian König  
Sent: September 26, 2018 4:05 AM
To: Zhang, Boyuan ; mesa-dev@lists.freedesktop.org
Subject: Re: [PATCH] vl: reorder H264 profiles

Am 26.09.2018 um 00:11 schrieb boyuan.zh...@amd.com:
> From: Boyuan Zhang 
>
> Fix the wrong h264 profiles order. Previously, the constrained 
> baseline was added in between baseline and main profiles, which 
> breaked the logic in radeon/vce when converting from 
> pipe_video_profile to profile_idc

I think it would be better to use a switch/case in radeon/vce or even better 
make a helper function which converts between
PIPE_VIDEO_PROFILE_MPEG4_AVC_* and the profile_idc from the specification.

Christian.

>
> Signed-off-by: Boyuan Zhang 
> ---
>   src/gallium/include/pipe/p_video_enums.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/gallium/include/pipe/p_video_enums.h 
> b/src/gallium/include/pipe/p_video_enums.h
> index b5b8b06228..260f47ea8a 100644
> --- a/src/gallium/include/pipe/p_video_enums.h
> +++ b/src/gallium/include/pipe/p_video_enums.h
> @@ -55,8 +55,8 @@ enum pipe_video_profile
>  PIPE_VIDEO_PROFILE_VC1_SIMPLE,
>  PIPE_VIDEO_PROFILE_VC1_MAIN,
>  PIPE_VIDEO_PROFILE_VC1_ADVANCED,
> -   PIPE_VIDEO_PROFILE_MPEG4_AVC_BASELINE,
>  PIPE_VIDEO_PROFILE_MPEG4_AVC_CONSTRAINED_BASELINE,
> +   PIPE_VIDEO_PROFILE_MPEG4_AVC_BASELINE,
>  PIPE_VIDEO_PROFILE_MPEG4_AVC_MAIN,
>  PIPE_VIDEO_PROFILE_MPEG4_AVC_EXTENDED,
>  PIPE_VIDEO_PROFILE_MPEG4_AVC_HIGH,

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] radeon/vcn: add and manage render picture list

[Zhang, Boyuan]

Hi Emil,

I cherry-picked these 2 patches over to the 17.3 branch, then tested locally 
and confirmed that it works fine.

I also added bugzilla reference to the patch as well. Please see the details in 
the links below:

https://lists.freedesktop.org/archives/mesa-dev/2018-January/183485.html
https://lists.freedesktop.org/archives/mesa-dev/2018-January/183484.html

Regards,
Boyuan


-Original Message-
From: Emil Velikov [mailto:emil.l.veli...@gmail.com] 
Sent: January-24-18 8:38 AM
To: Zhang, Boyuan; ML mesa-stable
Cc: ML mesa-dev
Subject: Re: [Mesa-dev] [PATCH 1/2] radeon/vcn: add and manage render picture 
list

On 11 December 2017 at 16:47,   wrote:
> From: Boyuan Zhang 
>
> Create a list in decoder to store all render picture buffer pointers 
> that currently being used in reference picture lists.
>
> During get message buffer call, check each pointer in 
> render_pic_list[] within given pic->ref[] list, remove pointer that no 
> longer being used by
> pic->ref[]. Then add current render surface pointer to the 
> pic->render_pic_list[]
> and assign the associated index to result.curr_idx.
>
> As a result, result.curr_idx will have the correct index to represent 
> the current render picture, instead of the previous increamenting values.
>
> Signed-off-by: Boyuan Zhang 
> Reviewed-by: Christian König 
> ---
We'd want this and 2/2 (sha's below) in stable. Otherwise people will 
experience regressions when updating their firmware.

f2bfd1cbb7e radeon/vcn: add and manage render picture list 2ec48039b8a 
radeon/uvd: add and manage render picture list

Including the bugzilla reference will be great.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104745

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] meson: Add new picture_{h264, hevc}_enc.c files to meson too

[Zhang, Boyuan]

Thanks a lot for pointing out, I add meson changes too.


Regards,

Boyuan


From: Boyuan Zhang 

Subject: [PATCH 06/12] st/va: move H264 enc functions into separate file

Signed-off-by: Boyuan Zhang 
---
 src/gallium/state_trackers/va/Makefile.sources   |   1 +
 src/gallium/state_trackers/va/meson.build|   2 +-
 src/gallium/state_trackers/va/picture.c  | 146 +++-
 src/gallium/state_trackers/va/picture_h264_enc.c | 163 +++
 src/gallium/state_trackers/va/va_private.h   |   5 +
 5 files changed, 219 insertions(+), 98 deletions(-)
 create mode 100644 src/gallium/state_trackers/va/picture_h264_enc.c

diff --git a/src/gallium/state_trackers/va/Makefile.sources 
b/src/gallium/state_trackers/va/Makefile.sources
index 2d6546b..8a69828 100644
--- a/src/gallium/state_trackers/va/Makefile.sources
+++ b/src/gallium/state_trackers/va/Makefile.sources
@@ -8,6 +8,7 @@ C_SOURCES := \
 picture_mpeg12.c \
 picture_mpeg4.c \
 picture_h264.c \
+picture_h264_enc.c \
 picture_hevc.c \
 picture_vc1.c \
 picture_mjpeg.c \
diff --git a/src/gallium/state_trackers/va/meson.build 
b/src/gallium/state_trackers/va/meson.build
index 56e68e9..bddd5ef 100644
--- a/src/gallium/state_trackers/va/meson.build
+++ b/src/gallium/state_trackers/va/meson.build
@@ -26,7 +26,7 @@ libva_st = static_library(
 'buffer.c', 'config.c', 'context.c', 'display.c', 'image.c', 'picture.c',
 'picture_mpeg12.c', 'picture_mpeg4.c', 'picture_h264.c', 'picture_hevc.c',
 'picture_vc1.c', 'picture_mjpeg.c', 'postproc.c', 'subpicture.c',
-'surface.c',
+'surface.c', 'picture_h264_enc.c', 'picture_hevc_enc.c',
   ),
   c_args : [
 c_vis_args,
diff --git a/src/gallium/state_trackers/va/picture.c 
b/src/gallium/state_trackers/va/picture.c
index 8951573..77d379b 100644
--- a/src/gallium/state_trackers/va/picture.c
+++ b/src/gallium/state_trackers/va/picture.c
@@ -349,55 +349,52 @@ handleVASliceDataBufferType(vlVaContext *context, 
vlVaBuffer *buf)
 static VAStatus
 handleVAEncMiscParameterTypeRateControl(vlVaContext *context, 
VAEncMiscParameterBuffer *misc)
 {
-   VAEncMiscParameterRateControl *rc = (VAEncMiscParameterRateControl 
*)misc->data;
-   if (context->desc.h264enc.rate_ctrl.rate_ctrl_method ==
-   PIPE_H264_ENC_RATE_CONTROL_METHOD_CONSTANT)
-  context->desc.h264enc.rate_ctrl.target_bitrate = rc->bits_per_second;
-   else
-  context->desc.h264enc.rate_ctrl.target_bitrate = rc->bits_per_second * 
(rc->target_percentage / 100.0);
-   context->desc.h264enc.rate_ctrl.peak_bitrate = rc->bits_per_second;
-   if (context->desc.h264enc.rate_ctrl.target_bitrate < 200)
-  context->desc.h264enc.rate_ctrl.vbv_buffer_size = 
MIN2((context->desc.h264enc.rate_ctrl.target_bitrate * 2.75), 200);
-   else
-  context->desc.h264enc.rate_ctrl.vbv_buffer_size = 
context->desc.h264enc.rate_ctrl.target_bitrate;
+   VAStatus status = VA_STATUS_SUCCESS;

-   return VA_STATUS_SUCCESS;
+   switch (u_reduce_video_profile(context->templat.profile)) {
+   case PIPE_VIDEO_FORMAT_MPEG4_AVC:
+  status = vlVaHandleVAEncMiscParameterTypeRateControlH264(context, misc);
+  break;
+
+   default:
+  break;
+   }
+
+   return status;
 }

 static VAStatus
 handleVAEncMiscParameterTypeFrameRate(vlVaContext *context, 
VAEncMiscParameterBuffer *misc)
 {
-   VAEncMiscParameterFrameRate *fr = (VAEncMiscParameterFrameRate *)misc->data;
-   if (fr->framerate & 0x) {
-  context->desc.h264enc.rate_ctrl.frame_rate_num = fr->framerate   & 
0x;
-  context->desc.h264enc.rate_ctrl.frame_rate_den = fr->framerate >> 16 & 
0x;
-   } else {
-  context->desc.h264enc.rate_ctrl.frame_rate_num = fr->framerate;
-  context->desc.h264enc.rate_ctrl.frame_rate_den = 1;
+   VAStatus status = VA_STATUS_SUCCESS;
+
+   switch (u_reduce_video_profile(context->templat.profile)) {
+   case PIPE_VIDEO_FORMAT_MPEG4_AVC:
+  status = vlVaHandleVAEncMiscParameterTypeFrameRateH264(context, misc);
+  break;
+
+   default:
+  break;
}
-   return VA_STATUS_SUCCESS;
+
+   return status;
 }

 static VAStatus
 handleVAEncSequenceParameterBufferType(vlVaDriver *drv, vlVaContext *context, 
vlVaBuffer *buf)
 {
-   VAEncSequenceParameterBufferH264 *h264 = (VAEncSequenceParameterBufferH264 
*)buf->data;
-   if (!context->decoder) {
-  context->templat.max_references = h264->max_num_ref_frames;
-  context->templat.level = h264->level_idc;
-  context->decoder = drv->pipe->create_video_codec(drv->pipe, 
&context->templat);
-  if (!context->decoder)
- return VA_STATUS_ERROR_ALLOCATION_FAILED;
+   VAStatus status = VA_STATUS_SUCCESS;
+
+   switch (u_reduce_video_profile(context->templat.profile)) {
+   case PIPE_VIDEO_FORMAT_MPEG4_AVC:
+  status = vlVaHandleVAEncSequenceParameterBufferTypeH264(drv, context, 
buf);
+  break;
+
+   default:
+  break;
}

-   context->gop_coeff = ((1024 + h264->intra_idr_period - 1) / 
h264->

Re: [Mesa-dev] [PATCH 01/12] vl: add parameters for HEVC encode

[Zhang, Boyuan]

The whole series are the updated version. Changes are made mainly based on the 
comments from previous code review, plus fixing a few typos.

-Original Message-
From: Zhang, Boyuan 
Sent: February-02-18 11:11 AM
To: mesa-dev@lists.freedesktop.org
Cc: Zhang, Boyuan
Subject: [PATCH 01/12] vl: add parameters for HEVC encode

From: Boyuan Zhang 

Add HEVC encode interface

Signed-off-by: Boyuan Zhang 
Acked-by: Christian König 
---
 src/gallium/include/pipe/p_video_state.h | 99 
 1 file changed, 99 insertions(+)

diff --git a/src/gallium/include/pipe/p_video_state.h 
b/src/gallium/include/pipe/p_video_state.h
index 5a88e6c..2533ba4 100644
--- a/src/gallium/include/pipe/p_video_state.h
+++ b/src/gallium/include/pipe/p_video_state.h
@@ -120,6 +120,15 @@ enum pipe_h264_enc_picture_type
PIPE_H264_ENC_PICTURE_TYPE_SKIP = 0x04  };
 
+enum pipe_h265_enc_picture_type
+{
+   PIPE_H265_ENC_PICTURE_TYPE_P = 0x00,
+   PIPE_H265_ENC_PICTURE_TYPE_B = 0x01,
+   PIPE_H265_ENC_PICTURE_TYPE_I = 0x02,
+   PIPE_H265_ENC_PICTURE_TYPE_IDR = 0x03,
+   PIPE_H265_ENC_PICTURE_TYPE_SKIP = 0x04 };
+
 enum pipe_h264_enc_rate_control_method
 {
PIPE_H264_ENC_RATE_CONTROL_METHOD_DISABLE = 0x00, @@ -129,6 +138,15 @@ enum 
pipe_h264_enc_rate_control_method
PIPE_H264_ENC_RATE_CONTROL_METHOD_VARIABLE = 0x04  };
 
+enum pipe_h265_enc_rate_control_method
+{
+   PIPE_H265_ENC_RATE_CONTROL_METHOD_DISABLE = 0x00,
+   PIPE_H265_ENC_RATE_CONTROL_METHOD_CONSTANT_SKIP = 0x01,
+   PIPE_H265_ENC_RATE_CONTROL_METHOD_VARIABLE_SKIP = 0x02,
+   PIPE_H265_ENC_RATE_CONTROL_METHOD_CONSTANT = 0x03,
+   PIPE_H265_ENC_RATE_CONTROL_METHOD_VARIABLE = 0x04 };
+
 struct pipe_picture_desc
 {
enum pipe_video_profile profile;
@@ -412,6 +430,87 @@ struct pipe_h264_enc_picture_desc
 
 };
 
+struct pipe_h265_enc_seq_param
+{
+   uint8_t  general_profile_idc;
+   uint8_t  general_level_idc;
+   uint8_t  general_tier_flag;
+   uint32_t intra_period;
+   uint16_t pic_width_in_luma_samples;
+   uint16_t pic_height_in_luma_samples;
+   uint32_t chroma_format_idc;
+   uint32_t bit_depth_luma_minus8;
+   uint32_t bit_depth_chroma_minus8;
+   bool strong_intra_smoothing_enabled_flag;
+   bool amp_enabled_flag;
+   bool sample_adaptive_offset_enabled_flag;
+   bool pcm_enabled_flag;
+   bool sps_temporal_mvp_enabled_flag;
+   uint8_t  log2_min_luma_coding_block_size_minus3;
+   uint8_t  log2_diff_max_min_luma_coding_block_size;
+   uint8_t  log2_min_transform_block_size_minus2;
+   uint8_t  log2_diff_max_min_transform_block_size;
+   uint8_t  max_transform_hierarchy_depth_inter;
+   uint8_t  max_transform_hierarchy_depth_intra;
+};
+
+struct pipe_h265_enc_pic_param
+{
+   uint8_t log2_parallel_merge_level_minus2;
+   uint8_t nal_unit_type;
+   bool constrained_intra_pred_flag;
+};
+
+struct pipe_h265_enc_slice_param
+{
+   uint8_t max_num_merge_cand;
+   int8_t slice_cb_qp_offset;
+   int8_t slice_cr_qp_offset;
+   int8_t slice_beta_offset_div2;
+   int8_t slice_tc_offset_div2;
+   bool cabac_init_flag;
+   uint32_t slice_deblocking_filter_disabled_flag;
+   bool slice_loop_filter_across_slices_enabled_flag;
+};
+
+struct pipe_h265_enc_rate_control
+{
+   enum pipe_h265_enc_rate_control_method rate_ctrl_method;
+   unsigned target_bitrate;
+   unsigned peak_bitrate;
+   unsigned frame_rate_num;
+   unsigned frame_rate_den;
+   unsigned quant_i_frames;
+   unsigned vbv_buffer_size;
+   unsigned vbv_buf_lv;
+   unsigned target_bits_picture;
+   unsigned peak_bits_picture_integer;
+   unsigned peak_bits_picture_fraction;
+   unsigned fill_data_enable;
+   unsigned enforce_hrd;
+};
+
+struct pipe_h265_enc_picture_desc
+{
+   struct pipe_picture_desc base;
+
+   struct pipe_h265_enc_seq_param seq;
+   struct pipe_h265_enc_pic_param pic;
+   struct pipe_h265_enc_slice_param slice;
+   struct pipe_h265_enc_rate_control rc;
+
+   enum pipe_h265_enc_picture_type picture_type;
+   unsigned decoded_curr_pic;
+   unsigned reference_frames[16];
+   unsigned frame_num;
+   unsigned pic_order_cnt;
+   unsigned pic_order_cnt_type;
+   unsigned ref_idx_l0;
+   unsigned ref_idx_l1;
+   bool not_referenced;
+   struct util_hash_table *frame_idx;
+};
+
 struct pipe_h265_sps
 {
uint8_t chroma_format_idc;
--
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 05/12] radeon/vcn: add header implementations for HEVC

[Zhang, Boyuan]

Update patch 05/12 with a fix.

From: Boyuan Zhang 

Implement encoding of sps, pps, vps, aud, and slice headers for HEVC
based on HEVC specs.

Signed-off-by: Boyuan Zhang 
Acked-by: Christian König 
---
 src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c | 348 +++-
 1 file changed, 347 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c 
b/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
index a651f7e..c86c2f3 100644
--- a/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
+++ b/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
@@ -551,6 +551,86 @@ static void radeon_enc_nalu_sps(struct radeon_encoder *enc)
RADEON_ENC_END();
 }
 
+static void radeon_enc_nalu_sps_hevc(struct radeon_encoder *enc)
+{
+   RADEON_ENC_BEGIN(RENCODE_IB_PARAM_DIRECT_OUTPUT_NALU);
+   RADEON_ENC_CS(RENCODE_DIRECT_OUTPUT_NALU_TYPE_SPS);
+   uint32_t *size_in_bytes = &enc->cs->current.buf[enc->cs->current.cdw++];
+   int i;
+
+   radeon_enc_reset(enc);
+   radeon_enc_set_emulation_prevention(enc, false);
+   radeon_enc_code_fixed_bits(enc, 0x0001, 32);
+   radeon_enc_code_fixed_bits(enc, 0x4201, 16);
+   radeon_enc_byte_align(enc);
+   radeon_enc_set_emulation_prevention(enc, true);
+   radeon_enc_code_fixed_bits(enc, 0x0, 4);
+   radeon_enc_code_fixed_bits(enc, 
enc->enc_pic.layer_ctrl.max_num_temporal_layers - 1, 3);
+   radeon_enc_code_fixed_bits(enc, 0x1, 1);
+   radeon_enc_code_fixed_bits(enc, 0x0, 2);
+   radeon_enc_code_fixed_bits(enc, enc->enc_pic.general_tier_flag, 1);
+   radeon_enc_code_fixed_bits(enc, enc->enc_pic.general_profile_idc, 5);
+   radeon_enc_code_fixed_bits(enc, 0x6000, 32);
+   radeon_enc_code_fixed_bits(enc, 0xb000, 32);
+   radeon_enc_code_fixed_bits(enc, 0x0, 16);
+   radeon_enc_code_fixed_bits(enc, enc->enc_pic.general_level_idc, 8);
+
+   for (i = 0; i < (enc->enc_pic.layer_ctrl.max_num_temporal_layers - 1) ; 
i++)
+   radeon_enc_code_fixed_bits(enc, 0x0, 2);
+
+   if ((enc->enc_pic.layer_ctrl.max_num_temporal_layers - 1) > 0) {
+   for (i = (enc->enc_pic.layer_ctrl.max_num_temporal_layers - 1); 
i < 8; i++)
+   radeon_enc_code_fixed_bits(enc, 0x0, 2);
+   }
+
+   radeon_enc_code_ue(enc, 0x0);
+   radeon_enc_code_ue(enc, enc->enc_pic.chroma_format_idc);
+   radeon_enc_code_ue(enc, 
enc->enc_pic.session_init.aligned_picture_width);
+   radeon_enc_code_ue(enc, 
enc->enc_pic.session_init.aligned_picture_height);
+   radeon_enc_code_fixed_bits(enc, 0x0, 1);
+   radeon_enc_code_ue(enc, enc->enc_pic.bit_depth_luma_minus8);
+   radeon_enc_code_ue(enc, enc->enc_pic.bit_depth_chroma_minus8);
+   radeon_enc_code_ue(enc, enc->enc_pic.log2_max_poc - 4);
+   radeon_enc_code_fixed_bits(enc, 0x0, 1);
+   radeon_enc_code_ue(enc, 1);
+   radeon_enc_code_ue(enc, 0x0);
+   radeon_enc_code_ue(enc, 0x0);
+   radeon_enc_code_ue(enc, 
enc->enc_pic.hevc_spec_misc.log2_min_luma_coding_block_size_minus3);
+   //Only support CTBSize 64
+   radeon_enc_code_ue(enc, 6 - 
(enc->enc_pic.hevc_spec_misc.log2_min_luma_coding_block_size_minus3 + 3));
+   radeon_enc_code_ue(enc, 
enc->enc_pic.log2_min_transform_block_size_minus2);
+   radeon_enc_code_ue(enc, 
enc->enc_pic.log2_diff_max_min_transform_block_size);
+   radeon_enc_code_ue(enc, 
enc->enc_pic.max_transform_hierarchy_depth_inter);
+   radeon_enc_code_ue(enc, 
enc->enc_pic.max_transform_hierarchy_depth_intra);
+
+   radeon_enc_code_fixed_bits(enc, 0x0, 1);
+   radeon_enc_code_fixed_bits(enc, 
!enc->enc_pic.hevc_spec_misc.amp_disabled, 1);
+   radeon_enc_code_fixed_bits(enc, 
enc->enc_pic.sample_adaptive_offset_enabled_flag, 1);
+   radeon_enc_code_fixed_bits(enc, enc->enc_pic.pcm_enabled_flag, 1);
+
+   radeon_enc_code_ue(enc, 1);
+   radeon_enc_code_ue(enc, 1);
+   radeon_enc_code_ue(enc, 0);
+   radeon_enc_code_ue(enc, 0);
+   radeon_enc_code_fixed_bits(enc, 0x1, 1);
+
+   radeon_enc_code_fixed_bits(enc, 0x0, 1);
+
+   radeon_enc_code_fixed_bits(enc, 0, 1);
+   radeon_enc_code_fixed_bits(enc, 
enc->enc_pic.hevc_spec_misc.strong_intra_smoothing_enabled, 1);
+
+   radeon_enc_code_fixed_bits(enc, 0x0, 1);
+
+   radeon_enc_code_fixed_bits(enc, 0x0, 1);
+
+   radeon_enc_code_fixed_bits(enc, 0x1, 1);
+
+   radeon_enc_byte_align(enc);
+   radeon_enc_flush_headers(enc);
+   *size_in_bytes = (enc->bits_output + 7) / 8;
+   RADEON_ENC_END();
+}
+
 static void radeon_enc_nalu_pps(struct radeon_encoder *enc)
 {
RADEON_ENC_BEGIN(RENCODE_IB_PARAM_DIRECT_OUTPUT_NALU);
@@ -586,6 +666,150 @@ static void radeon_enc_nalu_pps(struct radeon_encoder 
*enc)
RADEON_ENC_END();
 }
 
+static void radeon_enc_nalu_pps_hevc(struct radeon_encoder *enc)
+{
+   RADEON_ENC_BEGIN(RENCODE_IB_PARAM_DIRECT_OUTPUT_NALU);
+   RADEON_ENC_CS(REN

Re: [Mesa-dev] [PATCH] radeon/vcn: add RENOIR VCN decode support

[Zhang, Boyuan]

Patch is
Reviewed-by: Boyuan Zhang 

-Original Message-
From: mesa-dev  On Behalf Of Liu, Leo
Sent: September 4, 2019 1:40 PM
To: mesa-dev@lists.freedesktop.org
Subject: [Mesa-dev] [PATCH] radeon/vcn: add RENOIR VCN decode support

It has same VCN2.x block as Navi1x

Signed-off-by: Leo Liu 
---
 src/gallium/drivers/radeon/radeon_vcn_dec.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/gallium/drivers/radeon/radeon_vcn_dec.c 
b/src/gallium/drivers/radeon/radeon_vcn_dec.c
index 77bfe187674..0743d47f703 100644
--- a/src/gallium/drivers/radeon/radeon_vcn_dec.c
+++ b/src/gallium/drivers/radeon/radeon_vcn_dec.c
@@ -840,7 +840,7 @@ static struct pb_buffer *rvcn_dec_message_decode(struct 
radeon_decoder *dec,
decode->sc_coeff_size = 0;
 
decode->sw_ctxt_size = RDECODE_SESSION_CONTEXT_SIZE;
-   decode->db_pitch = (((struct si_screen*)dec->screen)->info.family >= 
CHIP_ARCTURUS &&
+   decode->db_pitch = (((struct si_screen*)dec->screen)->info.family >= 
CHIP_RENOIR &&
dec->base.width > 32 && dec->stream_type == 
RDECODE_CODEC_VP9) ?
align(dec->base.width, 64) :
align(dec->base.width, 32) ;
@@ -938,7 +938,7 @@ static struct pb_buffer *rvcn_dec_message_decode(struct 
radeon_decoder *dec,
/* default probability + probability data */
ctx_size = 2304 * 5;
 
-   if (((struct si_screen*)dec->screen)->info.family >= 
CHIP_ARCTURUS) {
+   if (((struct si_screen*)dec->screen)->info.family >= 
CHIP_RENOIR) {
/* SRE collocated context data */
ctx_size += 32 * 2 * 128 * 68;
/* SMP collocated context data */
@@ -1263,7 +1263,7 @@ static unsigned calc_dpb_size(struct radeon_decoder *dec)
case PIPE_VIDEO_FORMAT_VP9:
max_references = MAX2(max_references, 9);
 
-   dpb_size = (((struct si_screen*)dec->screen)->info.family >= 
CHIP_ARCTURUS) ?
+   dpb_size = (((struct si_screen*)dec->screen)->info.family >= 
CHIP_RENOIR) ?
(8192 * 4320 * 3 / 2) * max_references :
(4096 * 3000 * 3 / 2) * max_references;
 
@@ -1607,7 +1607,7 @@ struct pipe_video_codec *radeon_create_decoder(struct 
pipe_context *context,
dec->reg.data1 = RDECODE_VCN2_5_GPCOM_VCPU_DATA1;
dec->reg.cmd = RDECODE_VCN2_5_GPCOM_VCPU_CMD;
dec->reg.cntl = RDECODE_VCN2_5_ENGINE_CNTL;
-   } else if (sctx->family >= CHIP_NAVI10) {
+   } else if (sctx->family >= CHIP_NAVI10 || sctx->family == CHIP_RENOIR) {
dec->reg.data0 = RDECODE_VCN2_GPCOM_VCPU_DATA0;
dec->reg.data1 = RDECODE_VCN2_GPCOM_VCPU_DATA1;
dec->reg.cmd = RDECODE_VCN2_GPCOM_VCPU_CMD;
-- 
2.17.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] radeonsi: Add support for midstream bitrate change in encoder

[Zhang, Boyuan]

Patch is
Reviewed-by: Boyuan Zhang mailto:boyuan.zh...@amd.com>>

Regards,
Boyuan

From: Liu, Leo 
Sent: December 3, 2019 9:08 PM
To: Marek Olšák ; Sahu, Satyajit ; 
Zhang, Boyuan 
Cc: ML Mesa-dev ; Sharma, Deepak 

Subject: Re: [Mesa-dev] [PATCH] radeonsi: Add support for midstream bitrate 
change in encoder

+Boyuan

From: Marek Olšák mailto:mar...@gmail.com>>
Sent: December 3, 2019 7:21:23 PM
To: Sahu, Satyajit mailto:satyajit.s...@amd.com>>; Liu, 
Leo mailto:leo@amd.com>>
Cc: ML Mesa-dev 
mailto:mesa-dev@lists.freedesktop.org>>; 
Sharma, Deepak mailto:deepak.sha...@amd.com>>
Subject: Re: [Mesa-dev] [PATCH] radeonsi: Add support for midstream bitrate 
change in encoder

+Leo

On Mon, Dec 2, 2019 at 11:31 PM Satyajit Sahu 
mailto:satyajit.s...@amd.com>> wrote:
Added support for bitrate change in between encoding.

Signed-off-by: Satyajit Sahu 
mailto:satyajit.s...@amd.com>>

diff --git a/src/gallium/drivers/radeon/radeon_vce.c 
b/src/gallium/drivers/radeon/radeon_vce.c
index 84d3c1e2fa4..7d7a2fa4eb3 100644
--- a/src/gallium/drivers/radeon/radeon_vce.c
+++ b/src/gallium/drivers/radeon/radeon_vce.c
@@ -268,7 +268,8 @@ static void rvce_begin_frame(struct pipe_video_codec 
*encoder,
enc->pic.rate_ctrl.rate_ctrl_method != 
pic->rate_ctrl.rate_ctrl_method ||
enc->pic.quant_i_frames != pic->quant_i_frames ||
enc->pic.quant_p_frames != pic->quant_p_frames ||
-   enc->pic.quant_b_frames != pic->quant_b_frames;
+   enc->pic.quant_b_frames != pic->quant_b_frames ||
+   enc->pic.rate_ctrl.target_bitrate != 
pic->rate_ctrl.target_bitrate;

enc->pic = *pic;
si_get_pic_param(enc, pic);
diff --git a/src/gallium/drivers/radeon/radeon_vcn_enc.c 
b/src/gallium/drivers/radeon/radeon_vcn_enc.c
index aa9182f273b..c4fb9a7bd92 100644
--- a/src/gallium/drivers/radeon/radeon_vcn_enc.c
+++ b/src/gallium/drivers/radeon/radeon_vcn_enc.c
@@ -247,6 +247,17 @@ static void radeon_enc_begin_frame(struct pipe_video_codec 
*encoder,
 {
struct radeon_encoder *enc = (struct radeon_encoder*)encoder;
struct vl_video_buffer *vid_buf = (struct vl_video_buffer *)source;
+   bool need_rate_control = false;
+
+   if (u_reduce_video_profile(enc->base.profile) == 
PIPE_VIDEO_FORMAT_MPEG4_AVC) {
+   struct pipe_h264_enc_picture_desc *pic = (struct 
pipe_h264_enc_picture_desc *)picture;
+   need_rate_control =
+   enc->enc_pic.rc_layer_init.target_bit_rate != 
pic->rate_ctrl.target_bitrate;
+   } else if (u_reduce_video_profile(picture->profile) == 
PIPE_VIDEO_FORMAT_HEVC) {
+struct pipe_h265_enc_picture_desc *pic = (struct 
pipe_h265_enc_picture_desc *)picture;
+   need_rate_control =
+   enc->enc_pic.rc_layer_init.target_bit_rate != 
pic->rc.target_bitrate;
+   }

radeon_vcn_enc_get_param(enc, picture);

@@ -266,6 +277,10 @@ static void radeon_enc_begin_frame(struct pipe_video_codec 
*encoder,
flush(enc);
si_vid_destroy_buffer(&fb);
}
+   if (need_rate_control) {
+   enc->begin(enc, picture);
+   flush(enc);
+   }
 }

 static void radeon_enc_encode_bitstream(struct pipe_video_codec *encoder,
--
2.17.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org<mailto:mesa-dev@lists.freedesktop.org>
https://lists.freedesktop.org/mailman/listinfo/mesa-dev<https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-dev&data=02%7C01%7Cleo.liu%40amd.com%7Cae84ee699f3246cb915708d7784ffad3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637110157225190450&sdata=dw2yJa%2BgZ3t1BlOH87PypwWIhmH2y7pZ8vU5SiWBkO8%3D&reserved=0>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] radeon/vcn: enable rate control for hevc encoding

[Zhang, Boyuan]

Port changes from radeon_vcn_enc_1_2.c to radeon_vcn_enc_2_0.c

Set cu_qp_delta_enable_flag on when rate control is enabled, and set it
off when rate control is disabled (e.g. constant qp).

Signed-off-by: Boyuan Zhang 
---
 src/gallium/drivers/radeon/radeon_vcn_enc_2_0.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/radeon/radeon_vcn_enc_2_0.c 
b/src/gallium/drivers/radeon/radeon_vcn_enc_2_0.c
index d2c6378a055..580a084a1c1 100644
--- a/src/gallium/drivers/radeon/radeon_vcn_enc_2_0.c
+++ b/src/gallium/drivers/radeon/radeon_vcn_enc_2_0.c
@@ -196,7 +196,13 @@ static void radeon_enc_nalu_pps_hevc(struct radeon_encoder 
*enc)
  radeon_enc_code_se(enc, 0x0);
  radeon_enc_code_fixed_bits(enc, 
enc->enc_pic.hevc_spec_misc.constrained_intra_pred_flag, 1);
  radeon_enc_code_fixed_bits(enc, 0x0, 1);
- radeon_enc_code_fixed_bits(enc, 0x0, 1);
+ if (enc->enc_pic.rc_session_init.rate_control_method ==
+ RENCODE_RATE_CONTROL_METHOD_NONE)
+ radeon_enc_code_fixed_bits(enc, 0x0, 1);
+ else {
+ radeon_enc_code_fixed_bits(enc, 0x1, 1);
+ radeon_enc_code_ue(enc, 0x0);
+ }
  radeon_enc_code_se(enc, enc->enc_pic.hevc_deblock.cb_qp_offset);
  radeon_enc_code_se(enc, enc->enc_pic.hevc_deblock.cr_qp_offset);
  radeon_enc_code_fixed_bits(enc, 0x0, 1);
--
2.17.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] radeon: Fix width align for hevc encode

[Zhang, Boyuan]

Hi Lei,

You are right. The width alignment for HEVC encoding should be 64. This is 
actually a hardware requirement that we missed here. Thanks for catching it.

Just a small suggestion, can we use macros to define width alignment for hevc 
and h264 instead of the number 64 and 16? So that we can clearly notice the 
differences b/w this two codecs. Then we can replace the number with macros on 
radeon_vcn_enc.c:54&69, as well as radeon_vcn_enc_1_2.c:215

Thanks,
Boyuan


-Original Message-
From: mesa-dev  On Behalf Of Lei Zhou
Sent: April 15, 2019 2:24 AM
To: mesa-dev@lists.freedesktop.org
Subject: [Mesa-dev] [PATCH] radeon: Fix width align for hevc encode

Before, width is aligned to 16, we get 512x800 when encoding with 480x800, and 
conformance_window_flag=0 in sps.

Signed-off-by: Lei Zhou 
---
 src/gallium/drivers/radeon/radeon_uvd_enc.c | 2 +-  
src/gallium/drivers/radeon/radeon_vcn_enc.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/radeon/radeon_uvd_enc.c 
b/src/gallium/drivers/radeon/radeon_uvd_enc.c
index 3164dbb2c20..f0bf756ec50 100644
--- a/src/gallium/drivers/radeon/radeon_uvd_enc.c
+++ b/src/gallium/drivers/radeon/radeon_uvd_enc.c
@@ -66,7 +66,7 @@ radeon_uvd_enc_get_param(struct radeon_uvd_encoder *enc,
   || (pic->picture_type == PIPE_H265_ENC_PICTURE_TYPE_I);
enc->enc_pic.crop_left = 0;
enc->enc_pic.crop_right =
-  (align(enc->base.width, 16) - enc->base.width) / 2;
+  (align(enc->base.width, 64) - enc->base.width) / 2;
enc->enc_pic.crop_top = 0;
enc->enc_pic.crop_bottom =
   (align(enc->base.height, 16) - enc->base.height) / 2; diff --git 
a/src/gallium/drivers/radeon/radeon_vcn_enc.c 
b/src/gallium/drivers/radeon/radeon_vcn_enc.c
index 7d64a28a405..248f4c7d99e 100644
--- a/src/gallium/drivers/radeon/radeon_vcn_enc.c
+++ b/src/gallium/drivers/radeon/radeon_vcn_enc.c
@@ -66,7 +66,7 @@ static void radeon_vcn_enc_get_param(struct radeon_encoder 
*enc, struct pipe_pic
   enc->enc_pic.is_idr = (pic->picture_type == 
PIPE_H265_ENC_PICTURE_TYPE_IDR) ||
 (pic->picture_type == 
PIPE_H265_ENC_PICTURE_TYPE_I);
   enc->enc_pic.crop_left = 0;
-  enc->enc_pic.crop_right = (align(enc->base.width, 16) - enc->base.width) 
/ 2;
+  enc->enc_pic.crop_right = (align(enc->base.width, 64) - 
+ enc->base.width) / 2;
   enc->enc_pic.crop_top = 0;
   enc->enc_pic.crop_bottom = (align(enc->base.height, 16) - 
enc->base.height) / 2;
   enc->enc_pic.general_tier_flag = pic->seq.general_tier_flag;
--
2.21.0



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 2/2] radeon/vcn: fix width alignment for hevc encoding

[Zhang, Boyuan]

Both patches are
Reviewed-by: Boyuan Zhang 

Thanks,
Boyuan

-Original Message-
From: mesa-dev  On Behalf Of Lei Zhou
Sent: April 18, 2019 12:14 PM
To: mesa-dev@lists.freedesktop.org
Subject: [Mesa-dev] [PATCH v2 2/2] radeon/vcn: fix width alignment for hevc 
encoding

The width alignment for HEVC encoding should be 64 due to hardware requirement. 
This will fix conformance_window_flag in SPS.

v2 (Zhang, Boyuan):
 - add marcos to define width alignment for hevc and h264

Signed-off-by: Lei Zhou 
---
 src/gallium/drivers/radeon/radeon_vcn_enc.c | 4 ++--
 src/gallium/drivers/radeon/radeon_vcn_enc.h | 3 +++
 src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c | 4 ++--
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/gallium/drivers/radeon/radeon_vcn_enc.c 
b/src/gallium/drivers/radeon/radeon_vcn_enc.c
index 7d64a28a405..a882bba502c 100644
--- a/src/gallium/drivers/radeon/radeon_vcn_enc.c
+++ b/src/gallium/drivers/radeon/radeon_vcn_enc.c
@@ -51,7 +51,7 @@ static void radeon_vcn_enc_get_param(struct radeon_encoder 
*enc, struct pipe_pic
   enc->enc_pic.not_referenced = pic->not_referenced;
   enc->enc_pic.is_idr = (pic->picture_type == 
PIPE_H264_ENC_PICTURE_TYPE_IDR);
   enc->enc_pic.crop_left = 0;
-  enc->enc_pic.crop_right = (align(enc->base.width, 16) - enc->base.width) 
/ 2;
+  enc->enc_pic.crop_right = (align(enc->base.width, 
+ RENCODE_H264_WIDTH_ALIGN) - enc->base.width) / 2;
   enc->enc_pic.crop_top = 0;
   enc->enc_pic.crop_bottom = (align(enc->base.height, 16) - 
enc->base.height) / 2;
} else if (u_reduce_video_profile(picture->profile) == 
PIPE_VIDEO_FORMAT_HEVC) { @@ -66,7 +66,7 @@ static void 
radeon_vcn_enc_get_param(struct radeon_encoder *enc, struct pipe_pic
   enc->enc_pic.is_idr = (pic->picture_type == 
PIPE_H265_ENC_PICTURE_TYPE_IDR) ||
 (pic->picture_type == 
PIPE_H265_ENC_PICTURE_TYPE_I);
   enc->enc_pic.crop_left = 0;
-  enc->enc_pic.crop_right = (align(enc->base.width, 16) - enc->base.width) 
/ 2;
+  enc->enc_pic.crop_right = (align(enc->base.width, 
+ RENCODE_HEVC_WIDTH_ALIGN) - enc->base.width) / 2;
   enc->enc_pic.crop_top = 0;
   enc->enc_pic.crop_bottom = (align(enc->base.height, 16) - 
enc->base.height) / 2;
   enc->enc_pic.general_tier_flag = pic->seq.general_tier_flag; diff --git 
a/src/gallium/drivers/radeon/radeon_vcn_enc.h 
b/src/gallium/drivers/radeon/radeon_vcn_enc.h
index 04685c69af1..fadc74cc898 100644
--- a/src/gallium/drivers/radeon/radeon_vcn_enc.h
+++ b/src/gallium/drivers/radeon/radeon_vcn_enc.h
@@ -148,6 +148,9 @@
 #define RENCODE_FEEDBACK_BUFFER_MODE_LINEAR0
 #define RENCODE_FEEDBACK_BUFFER_MODE_CIRCULAR  1
 
+#define RENCODE_H264_WIDTH_ALIGN16
+#define RENCODE_HEVC_WIDTH_ALIGN64
+
 typedef struct rvcn_enc_session_info_s
 {
 uint32_t   interface_version;
diff --git a/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c 
b/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
index 7f5b1909344..388df42909f 100644
--- a/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
+++ b/src/gallium/drivers/radeon/radeon_vcn_enc_1_2.c
@@ -212,7 +212,7 @@ static void radeon_enc_task_info(struct radeon_encoder 
*enc, bool need_feedback)  static void radeon_enc_session_init(struct 
radeon_encoder *enc)  {
enc->enc_pic.session_init.encode_standard = 
RENCODE_ENCODE_STANDARD_H264;
-   enc->enc_pic.session_init.aligned_picture_width = 
align(enc->base.width, 16);
+   enc->enc_pic.session_init.aligned_picture_width = 
+align(enc->base.width, RENCODE_H264_WIDTH_ALIGN);
enc->enc_pic.session_init.aligned_picture_height = 
align(enc->base.height, 16);
enc->enc_pic.session_init.padding_width = 
enc->enc_pic.session_init.aligned_picture_width - enc->base.width;
enc->enc_pic.session_init.padding_height = 
enc->enc_pic.session_init.aligned_picture_height - enc->base.height; @@ -233,7 
+233,7 @@ static void radeon_enc_session_init(struct radeon_encoder *enc)  
static void radeon_enc_session_init_hevc(struct radeon_encoder *enc)  {
enc->enc_pic.session_init.encode_standard = 
RENCODE_ENCODE_STANDARD_HEVC;
-   enc->enc_pic.session_init.aligned_picture_width = 
align(enc->base.width, 64);
+   enc->enc_pic.session_init.aligned_picture_width = 
+align(enc->base.width, RENCODE_HEVC_WIDTH_ALIGN);
enc->enc_pic.session_init.aligned_picture_height = 
align(enc->base.height, 16);
enc->enc_pic.session_init.padding_width = 
enc->enc_pic.session_init.aligned_picture_width - enc->base.width;
enc->enc_pic.session_init.padding_height = 
enc->enc_pic.session_init.aligned_picture_height - enc->base.height;
--
2.21.0



___
mesa

Re: [Mesa-dev] [PATCH] radeon/uvd: fix poc for hevc encode

[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