[Mesa-dev] [PATCH] st/va: remove assert for single slice

[Nayan Deshmukh]

On Thu, Feb 2, 2017 at 3:34 PM, Christian König <deathsimple at vodafone.de> wrote:
> Am 01.02.2017 um 13:59 schrieb Nayan Deshmukh:
>>
>> we anyway allow for multiple slices
>>
>> Signed-off-by: Nayan Deshmukh <nayan26deshmukh at gmail.com>
>> ---
>>   src/gallium/state_trackers/va/picture_mpeg12.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/src/gallium/state_trackers/va/picture_mpeg12.c
>> b/src/gallium/state_trackers/va/picture_mpeg12.c
>> index 812e9e5..4d9c45f 100644
>> --- a/src/gallium/state_trackers/va/picture_mpeg12.c
>> +++ b/src/gallium/state_trackers/va/picture_mpeg12.c
>> @@ -81,6 +81,5 @@ void vlVaHandleIQMatrixBufferMPEG12(vlVaContext
>> *context, vlVaBuffer *buf)
>>     void vlVaHandleSliceParameterBufferMPEG12(vlVaContext *context,
>> vlVaBuffer *buf)
>>   {
>> -   assert(buf->size >= sizeof(VASliceParameterBufferMPEG2) &&
>> buf->num_elements == 1);
>
>
> NAK, we can lower the requirements here, but we shouldn't completely remove
> the assert.
>
> Especially since we ignore all of the fields from the
> VASliceParameterBufferMPEG2 structure and so won't catch errors.
>
I agree, we should have some assert here.

> Please test something like the following:
>
> assert(buf->size >= (sizeof(VASliceParameterBufferMPEG2) *
> buf->num_elements));
>

buf->size represents the size of a single buffer element and hence it
will be equal to sizeof(VASliceParameterBufferMPEG2). So we can have
something like

assert(buf->size >= sizeof(VASliceParameterBufferMPEG2))

I also tried using the fields of the VASliceParameterBufferMPEG2. I
misunderstood
the code first, as mpv sends all the slices at ones and buf->data points to
num_elements number of VASliceParameterBuffers. I tried using this
size fields with the slice data buffers, but surprisingly it leads to no change.
Any ideas why? I am attaching the diff for reference.

Which means that bug https://bugs.freedesktop.org/show_bug.cgi?id=93760
is still open.

Regards,
Nayan

diff --git a/src/gallium/state_trackers/va/picture.c
b/src/gallium/state_trackers/va/picture.c
index 82584ea..8e68e35 100644
--- a/src/gallium/state_trackers/va/picture.c
+++ b/src/gallium/state_trackers/va/picture.c
@@ -262,8 +262,10 @@ handleVASliceDataBufferType(vlVaContext *context,
vlVaBuffer *buf)
 {
    enum pipe_video_format format;
    unsigned num_buffers = 0;
-   void * const *buffers[2];
-   unsigned sizes[2];
+   void * const *buffers[context->desc.mpeg12.num_slices + 1];
+   unsigned sizes[context->desc.mpeg12.num_slices + 1];
+   char *cur;
+   int i;
    static const uint8_t start_code_h264[] = { 0x00, 0x00, 0x01 };
    static const uint8_t start_code_h265[] = { 0x00, 0x00, 0x01 };
    static const uint8_t start_code_vc1[] = { 0x00, 0x00, 0x01, 0x0d };
@@ -306,9 +308,12 @@ handleVASliceDataBufferType(vlVaContext *context,
vlVaBuffer *buf)
       break;
    }

-   buffers[num_buffers] = buf->data;
-   sizes[num_buffers] = buf->size;
-   ++num_buffers;
+   cur = buf->data;
+   for (i = 0; i < context->desc.mpeg12.num_slices; ++i) {
+      buffers[i] = cur;
+      cur = cur + context->sizes[i];
+   }
+

    if (context->needs_begin_frame) {
       context->decoder->begin_frame(context->decoder, context->target,
@@ -316,7 +321,8 @@ handleVASliceDataBufferType(vlVaContext *context,
vlVaBuffer *buf)
       context->needs_begin_frame = false;
    }
    context->decoder->decode_bitstream(context->decoder,
context->target, &context->desc.base,
-      num_buffers, (const void * const*)buffers, sizes);
+      context->desc.mpeg12.num_slices, (const void * const*)buffers,
context->sizes);
+
 }

 static VAStatus
@@ -586,6 +592,7 @@ vlVaEndPicture(VADriverContextP ctx, VAContextID context_id)
    }

    context->decoder->end_frame(context->decoder, context->target,
&context->desc.base);
+   FREE(context->sizes);
    if (context->decoder->entrypoint == PIPE_VIDEO_ENTRYPOINT_ENCODE) {
       int idr_period = context->desc.h264enc.gop_size / context->gop_coeff;
       int p_remain_in_idr = idr_period - context->desc.h264enc.frame_num;
diff --git a/src/gallium/state_trackers/va/picture_mpeg12.c
b/src/gallium/state_trackers/va/picture_mpeg12.c
index 4d9c45f..9e51274 100644
--- a/src/gallium/state_trackers/va/picture_mpeg12.c
+++ b/src/gallium/state_trackers/va/picture_mpeg12.c
@@ -81,5 +81,16 @@ void vlVaHandleIQMatrixBufferMPEG12(vlVaContext
*context, vlVaBuffer *buf)

 void vlVaHandleSliceParameterBufferMPEG12(vlVaContext *context,
vlVaBuffer *buf)
 {
+   VASliceParameterBufferMPEG2 *mpeg2 = buf->data;
+   unsigned *sizes = MALLOC(buf->num_elements * sizeof(unsigned));
+   int i;
+
    context->desc.mpeg12.num_slices += buf->num_elements;
+
+   for(i = 0; i < buf->num_elements; ++i) {
+      sizes[i] = mpeg2->slice_data_size;
+      mpeg2 = mpeg2 + 1;
+   }
+
+   context->sizes = sizes;
 }
diff --git a/src/gallium/state_trackers/va/va_private.h
b/src/gallium/state_trackers/va/va_private.h
index a744461..47cb65e 100644
--- a/src/gallium/state_trackers/va/va_private.h
+++ b/src/gallium/state_trackers/va/va_private.h
@@ -263,6 +263,7 @@ typedef struct {
    bool first_single_submitted;
    int gop_coeff;
    bool needs_begin_frame;
+   unsigned *sizes;
 } vlVaContext;

 typedef struct {