[Mesa-dev] [PATCH] st/va: count number of slices

[Julien Isorce]

On 23 December 2015 at 09:42, Christian König <christian.koenig at amd.com>
wrote:

> On 23.12.2015 10:28, Julien Isorce wrote:
>
>> The counter was not set but used by the driver.
>> It is required otherwise visual output is garbage.
>>
>> Signed-off-by: Julien Isorce <j.isorce at samsung.com>
>>
>
> One minor nit pick below, apart from that the patch look good to me.
>
>
> ---
>>   src/gallium/state_trackers/va/picture.c        | 22
>> ++++++++++++++++++++++
>>   src/gallium/state_trackers/va/picture_h264.c   |  2 ++
>>   src/gallium/state_trackers/va/picture_mpeg12.c |  7 +++++++
>>   src/gallium/state_trackers/va/picture_vc1.c    |  8 ++++++++
>>   src/gallium/state_trackers/va/va_private.h     |  2 ++
>>   5 files changed, 41 insertions(+)
>>
>> diff --git a/src/gallium/state_trackers/va/picture.c
>> b/src/gallium/state_trackers/va/picture.c
>> index 7b30bf8..5029eb8 100644
>> --- a/src/gallium/state_trackers/va/picture.c
>> +++ b/src/gallium/state_trackers/va/picture.c
>> @@ -60,6 +60,20 @@ vlVaBeginPicture(VADriverContextP ctx, VAContextID
>> context_id, VASurfaceID rende
>>        context->target = surf->buffer;
>>   +   switch (u_reduce_video_profile(context->templat.profile)) {
>> +   case PIPE_VIDEO_FORMAT_MPEG12:
>> +      context->desc.mpeg12.num_slices = 0;
>> +      break;
>> +   case PIPE_VIDEO_FORMAT_MPEG4_AVC:
>> +      context->desc.h264.slice_count = 0;
>> +      break;
>> +   case PIPE_VIDEO_FORMAT_VC1:
>> +      context->desc.vc1.slice_count = 0;
>> +      break;
>> +   default:
>> +      break;
>> +   }
>> +
>>
>
> We don't have anything in the picture description that needs to be
> preserved from frame to frame, don't we?
>
> So I would say just make nails with heads and clear the whole structure at
> the beginning of each from with zeros.
>
> Regards,
> Christian.


Hi Christian,

Matrix can be common between frames:

struct pipe_mpeg12_picture_desc
{
   ...
   const uint8_t *intra_matrix;
   const uint8_t *non_intra_matrix;

   struct pipe_video_buffer *ref[2];
};

And it is hard to say what will be common or not in general. But you are
right we should clear as much as possible.

For now I'll just move "context->desc.mpeg12.num_slices = 0;" to
vlVaHandlePictureParameterBufferMPEG12. Same for h264 and vc1.

Thx
Julien


>
>
>      if (!context->decoder) {
>>         /* VPP */
>>         if (context->templat.profile == PIPE_VIDEO_PROFILE_UNKNOWN &&
>> @@ -174,6 +188,14 @@ static void
>>   handleSliceParameterBuffer(vlVaContext *context, vlVaBuffer *buf)
>>   {
>>      switch (u_reduce_video_profile(context->templat.profile)) {
>> +   case PIPE_VIDEO_FORMAT_MPEG12:
>> +      vlVaHandleSliceParameterBufferMPEG12(context, buf);
>> +      break;
>> +
>> +   case PIPE_VIDEO_FORMAT_VC1:
>> +      vlVaHandleSliceParameterBufferVC1(context, buf);
>> +      break;
>> +
>>      case PIPE_VIDEO_FORMAT_MPEG4_AVC:
>>         vlVaHandleSliceParameterBufferH264(context, buf);
>>         break;
>> diff --git a/src/gallium/state_trackers/va/picture_h264.c
>> b/src/gallium/state_trackers/va/picture_h264.c
>> index acbfe5d..d98a20a 100644
>> --- a/src/gallium/state_trackers/va/picture_h264.c
>> +++ b/src/gallium/state_trackers/va/picture_h264.c
>> @@ -162,6 +162,8 @@ void vlVaHandleSliceParameterBufferH264(vlVaContext
>> *context, vlVaBuffer *buf)
>>      VASliceParameterBufferH264 *h264 = buf->data;
>>        assert(buf->size >= sizeof(VASliceParameterBufferH264) &&
>> buf->num_elements == 1);
>> +
>> +   context->desc.h264.slice_count += buf->num_elements;
>>      context->desc.h264.num_ref_idx_l0_active_minus1 =
>>         h264->num_ref_idx_l0_active_minus1;
>>      context->desc.h264.num_ref_idx_l1_active_minus1 =
>> diff --git a/src/gallium/state_trackers/va/picture_mpeg12.c
>> b/src/gallium/state_trackers/va/picture_mpeg12.c
>> index e587b1e..f0a2ae0 100644
>> --- a/src/gallium/state_trackers/va/picture_mpeg12.c
>> +++ b/src/gallium/state_trackers/va/picture_mpeg12.c
>> @@ -78,3 +78,10 @@ void vlVaHandleIQMatrixBufferMPEG12(vlVaContext
>> *context, vlVaBuffer *buf)
>>         context->desc.mpeg12.non_intra_matrix = NULL;
>>   }
>>   +void vlVaHandleSliceParameterBufferMPEG12(vlVaContext *context,
>> vlVaBuffer *buf)
>> +{
>> +   assert(buf->size >= sizeof(VASliceParameterBufferMPEG2) &&
>> buf->num_elements == 1);
>> +
>> +   context->desc.mpeg12.num_slices += buf->num_elements;
>> +}
>> +
>> diff --git a/src/gallium/state_trackers/va/picture_vc1.c
>> b/src/gallium/state_trackers/va/picture_vc1.c
>> index f95fd83..496bae3 100644
>> --- a/src/gallium/state_trackers/va/picture_vc1.c
>> +++ b/src/gallium/state_trackers/va/picture_vc1.c
>> @@ -65,3 +65,11 @@ void vlVaHandlePictureParameterBufferVC1(vlVaDriver
>> *drv, vlVaContext *context,
>>      context->desc.vc1.deblockEnable = vc1->post_processing != 0;
>>      context->desc.vc1.pquant =
>> vc1->pic_quantizer_fields.bits.pic_quantizer_scale;
>>   }
>> +
>> +void vlVaHandleSliceParameterBufferVC1(vlVaContext *context, vlVaBuffer
>> *buf)
>> +{
>> +   assert(buf->size >= sizeof(VASliceParameterBufferVC1) &&
>> buf->num_elements == 1);
>> +
>> +   context->desc.vc1.slice_count += buf->num_elements;
>> +}
>> +
>> diff --git a/src/gallium/state_trackers/va/va_private.h
>> b/src/gallium/state_trackers/va/va_private.h
>> index fa6e0fb..bf9d24b 100644
>> --- a/src/gallium/state_trackers/va/va_private.h
>> +++ b/src/gallium/state_trackers/va/va_private.h
>> @@ -351,10 +351,12 @@ VAStatus
>> vlVaHandleVAProcPipelineParameterBufferType(vlVaDriver *drv, vlVaContex
>>   void vlVaGetReferenceFrame(vlVaDriver *drv, VASurfaceID surface_id,
>> struct pipe_video_buffer **ref_frame);
>>   void vlVaHandlePictureParameterBufferMPEG12(vlVaDriver *drv,
>> vlVaContext *context, vlVaBuffer *buf);
>>   void vlVaHandleIQMatrixBufferMPEG12(vlVaContext *context, vlVaBuffer
>> *buf);
>> +void vlVaHandleSliceParameterBufferMPEG12(vlVaContext *context,
>> vlVaBuffer *buf);
>>   void vlVaHandlePictureParameterBufferH264(vlVaDriver *drv, vlVaContext
>> *context, vlVaBuffer *buf);
>>   void vlVaHandleIQMatrixBufferH264(vlVaContext *context, vlVaBuffer
>> *buf);
>>   void vlVaHandleSliceParameterBufferH264(vlVaContext *context,
>> vlVaBuffer *buf);
>>   void vlVaHandlePictureParameterBufferVC1(vlVaDriver *drv, vlVaContext
>> *context, vlVaBuffer *buf);
>> +void vlVaHandleSliceParameterBufferVC1(vlVaContext *context, vlVaBuffer
>> *buf);
>>   void vlVaHandlePictureParameterBufferMPEG4(vlVaDriver *drv, vlVaContext
>> *context, vlVaBuffer *buf);
>>   void vlVaHandleIQMatrixBufferMPEG4(vlVaContext *context, vlVaBuffer
>> *buf);
>>   void vlVaHandleSliceParameterBufferMPEG4(vlVaContext *context,
>> vlVaBuffer *buf);
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151224/9debdbfa/attachment.html>