Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Andy Furniss]

Christian König wrote:

Ah, yes of course. If we delay creating the decoder we need to call
begin_frame() again as well.

Please review and/or test the attached patch. Andy I did understand you
right that this is already a Tested-by from your side, isn't it?


Yes, this patch seems OK.

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

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Nayan Deshmukh]

The patch is Reviewed-by: Nayan Deshmukh 



On Mon, Jan 23, 2017 at 7:01 PM, Christian König
 wrote:
> Ah, yes of course. If we delay creating the decoder we need to call
> begin_frame() again as well.
>
> Please review and/or test the attached patch. Andy I did understand you
> right that this is already a Tested-by from your side, isn't it?
>
>> I am wondering if calling decode_bitstream one at a time for each
>> buffer is similar to
>> calling it with all buffers at once?
>
> Yes, that is correct. It's just not as efficient.
>
> One problem with VA-API is that it doesn't seem to guarantee that buffers
> stays around after handing them of to the decoder (the ownership handling of
> buffers and surfaces is a totally mess).
>
> So we would need to make a copy of the buffer content to submit it again all
> at once.
>
> Regards,
> Christian.
>
>
> Am 21.01.2017 um 20:46 schrieb Andy Furniss:
>>
>> Nayan Deshmukh wrote:
>>>
>>> Hi Christian,
>>>
>>> The new patch leads to seg fault on my system. You forgot to set the
>>> needs_begin_frame to true when the decoder is created. Here's diff for
>>> reference:
>>
>>
>> Setting true there seems to fix, though only a quick test.
>>
>> The patch below sets false :-)
>>
>>> 
>>> diff --git a/src/gallium/state_trackers/va/picture.c
>>> b/src/gallium/state_trackers/va/picture.c
>>> index e75006d..a51e482 100644
>>> --- a/src/gallium/state_trackers/va/picture.c
>>> +++ b/src/gallium/state_trackers/va/picture.c
>>> @@ -178,6 +178,8 @@ handlePictureParameterBuffer(vlVaDriver *drv,
>>> vlVaContext *context, vlVaBuffer *
>>>
>>> if (!context->decoder)
>>>return VA_STATUS_ERROR_ALLOCATION_FAILED;
>>> +
>>> +  context->needs_begin_frame = false;
>>>  }
>>>
>>>  return vaStatus;
>>> --
>>>
>>> I am wondering if calling decode_bitstream one at a time for each
>>> buffer is similar to
>>> calling it with all buffers at once?
>>>
>>> Cheers,
>>> Nayan
>>>
>>> On Thu, Jan 19, 2017 at 8:36 PM, Andy Furniss 
>>> wrote:

 Andy Furniss wrote:
>
>
> Christian König wrote:
>>
>>
>> Hi Andy,
>>
>> Am 19.01.2017 um 11:46 schrieb Andy Furniss:
>>>
>>>
>>> I think you are right about the slices, the failing vids are
>>> blu-ray/tv.
>>>
>>>
>>>
>>> https://drive.google.com/file/d/0BxP5-S1t9VEEZlozcjVUZ1lDbWM/view?usp=sharing
>>>
>>>
>>>
>> Thanks for the link, if you have time please give the attached patch a
>> try.
>>
>> It should fix the issue, but I currently don't have a test system for
>> VAAPI ready so I can't confirm it of hand.
>
>
>
> It doesn't fix properly. The vid will play normally after a second,
> but during that second I get to see many flash frames of gpu mem.
>
> Lucky I chose this sample out of several as the patch does seem to
> fix a couple of other previously failing vids.



 Though more testing shows it also regresses previously working vids,
 some as
 above, but one never "starts" and is total junk.



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

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Christian König]

Ah, yes of course. If we delay creating the decoder we need to call 
begin_frame() again as well.


Please review and/or test the attached patch. Andy I did understand you 
right that this is already a Tested-by from your side, isn't it?



I am wondering if calling decode_bitstream one at a time for each
buffer is similar to
calling it with all buffers at once? 

Yes, that is correct. It's just not as efficient.

One problem with VA-API is that it doesn't seem to guarantee that 
buffers stays around after handing them of to the decoder (the ownership 
handling of buffers and surfaces is a totally mess).


So we would need to make a copy of the buffer content to submit it again 
all at once.


Regards,
Christian.

Am 21.01.2017 um 20:46 schrieb Andy Furniss:

Nayan Deshmukh wrote:

Hi Christian,

The new patch leads to seg fault on my system. You forgot to set the
needs_begin_frame to true when the decoder is created. Here's diff for
reference:


Setting true there seems to fix, though only a quick test.

The patch below sets false :-)



diff --git a/src/gallium/state_trackers/va/picture.c
b/src/gallium/state_trackers/va/picture.c
index e75006d..a51e482 100644
--- a/src/gallium/state_trackers/va/picture.c
+++ b/src/gallium/state_trackers/va/picture.c
@@ -178,6 +178,8 @@ handlePictureParameterBuffer(vlVaDriver *drv,
vlVaContext *context, vlVaBuffer *

if (!context->decoder)
   return VA_STATUS_ERROR_ALLOCATION_FAILED;
+
+  context->needs_begin_frame = false;
 }

 return vaStatus;
--

I am wondering if calling decode_bitstream one at a time for each
buffer is similar to
calling it with all buffers at once?

Cheers,
Nayan

On Thu, Jan 19, 2017 at 8:36 PM, Andy Furniss  
wrote:

Andy Furniss wrote:


Christian König wrote:


Hi Andy,

Am 19.01.2017 um 11:46 schrieb Andy Furniss:


I think you are right about the slices, the failing vids are 
blu-ray/tv.



https://drive.google.com/file/d/0BxP5-S1t9VEEZlozcjVUZ1lDbWM/view?usp=sharing 





Thanks for the link, if you have time please give the attached 
patch a

try.

It should fix the issue, but I currently don't have a test system for
VAAPI ready so I can't confirm it of hand.



It doesn't fix properly. The vid will play normally after a second,
but during that second I get to see many flash frames of gpu mem.

Lucky I chose this sample out of several as the patch does seem to
fix a couple of other previously failing vids.



Though more testing shows it also regresses previously working vids, 
some as

above, but one never "starts" and is total junk.









>From 6d0f9a351fa72d4be56d1e44ad7c347ca93d4420 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20K=C3=B6nig?= 
Date: Thu, 19 Jan 2017 13:44:34 +0100
Subject: [PATCH] st/va: make sure that we call begin_frame() only once v2
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This fixes "st/va: delay calling begin_frame until we have all parameters".

v2: call begin frame after decoder (re)creation as well.

Signed-off-by: Christian König 
---
 src/gallium/state_trackers/va/picture.c| 11 ---
 src/gallium/state_trackers/va/va_private.h |  1 +
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/gallium/state_trackers/va/picture.c b/src/gallium/state_trackers/va/picture.c
index dc7121c..82584ea 100644
--- a/src/gallium/state_trackers/va/picture.c
+++ b/src/gallium/state_trackers/va/picture.c
@@ -81,7 +81,7 @@ vlVaBeginPicture(VADriverContextP ctx, VAContextID context_id, VASurfaceID rende
}
 
if (context->decoder->entrypoint != PIPE_VIDEO_ENTRYPOINT_ENCODE)
-  context->decoder->begin_frame(context->decoder, context->target, >desc.base);
+  context->needs_begin_frame = true;
 
return VA_STATUS_SUCCESS;
 }
@@ -178,6 +178,8 @@ handlePictureParameterBuffer(vlVaDriver *drv, vlVaContext *context, vlVaBuffer *
 
   if (!context->decoder)
  return VA_STATUS_ERROR_ALLOCATION_FAILED;
+
+  context->needs_begin_frame = true;
}
 
return vaStatus;
@@ -308,8 +310,11 @@ handleVASliceDataBufferType(vlVaContext *context, vlVaBuffer *buf)
sizes[num_buffers] = buf->size;
++num_buffers;
 
-   context->decoder->begin_frame(context->decoder, context->target,
-  >desc.base);
+   if (context->needs_begin_frame) {
+  context->decoder->begin_frame(context->decoder, context->target,
+ >desc.base);
+  context->needs_begin_frame = false;
+   }
context->decoder->decode_bitstream(context->decoder, context->target, >desc.base,
   num_buffers, (const void * const*)buffers, sizes);
 }
diff --git a/src/gallium/state_trackers/va/va_private.h b/src/gallium/state_trackers/va/va_private.h
index 8faec10..0877236 100644
--- a/src/gallium/state_trackers/va/va_private.h
+++ b/src/gallium/state_trackers/va/va_private.h
@@ -261,6 +261,7 @@ 

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Andy Furniss]

Nayan Deshmukh wrote:

Hi Christian,

The new patch leads to seg fault on my system. You forgot to set the
needs_begin_frame to true when the decoder is created. Here's diff for
reference:


Setting true there seems to fix, though only a quick test.

The patch below sets false :-)



diff --git a/src/gallium/state_trackers/va/picture.c
b/src/gallium/state_trackers/va/picture.c
index e75006d..a51e482 100644
--- a/src/gallium/state_trackers/va/picture.c
+++ b/src/gallium/state_trackers/va/picture.c
@@ -178,6 +178,8 @@ handlePictureParameterBuffer(vlVaDriver *drv,
vlVaContext *context, vlVaBuffer *

if (!context->decoder)
   return VA_STATUS_ERROR_ALLOCATION_FAILED;
+
+  context->needs_begin_frame = false;
 }

 return vaStatus;
--

I am wondering if calling decode_bitstream one at a time for each
buffer is similar to
calling it with all buffers at once?

Cheers,
Nayan

On Thu, Jan 19, 2017 at 8:36 PM, Andy Furniss  wrote:

Andy Furniss wrote:


Christian König wrote:


Hi Andy,

Am 19.01.2017 um 11:46 schrieb Andy Furniss:


I think you are right about the slices, the failing vids are blu-ray/tv.


https://drive.google.com/file/d/0BxP5-S1t9VEEZlozcjVUZ1lDbWM/view?usp=sharing




Thanks for the link, if you have time please give the attached patch a
try.

It should fix the issue, but I currently don't have a test system for
VAAPI ready so I can't confirm it of hand.



It doesn't fix properly. The vid will play normally after a second,
but during that second I get to see many flash frames of gpu mem.

Lucky I chose this sample out of several as the patch does seem to
fix a couple of other previously failing vids.



Though more testing shows it also regresses previously working vids, some as
above, but one never "starts" and is total junk.







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

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Nayan Deshmukh]

Hi Christian,

The new patch leads to seg fault on my system. You forgot to set the
needs_begin_frame to true when the decoder is created. Here's diff for
reference:

diff --git a/src/gallium/state_trackers/va/picture.c
b/src/gallium/state_trackers/va/picture.c
index e75006d..a51e482 100644
--- a/src/gallium/state_trackers/va/picture.c
+++ b/src/gallium/state_trackers/va/picture.c
@@ -178,6 +178,8 @@ handlePictureParameterBuffer(vlVaDriver *drv,
vlVaContext *context, vlVaBuffer *

   if (!context->decoder)
  return VA_STATUS_ERROR_ALLOCATION_FAILED;
+
+  context->needs_begin_frame = false;
}

return vaStatus;
--

I am wondering if calling decode_bitstream one at a time for each
buffer is similar to
calling it with all buffers at once?

Cheers,
Nayan

On Thu, Jan 19, 2017 at 8:36 PM, Andy Furniss  wrote:
> Andy Furniss wrote:
>>
>> Christian König wrote:
>>>
>>> Hi Andy,
>>>
>>> Am 19.01.2017 um 11:46 schrieb Andy Furniss:

 I think you are right about the slices, the failing vids are blu-ray/tv.


 https://drive.google.com/file/d/0BxP5-S1t9VEEZlozcjVUZ1lDbWM/view?usp=sharing



>>> Thanks for the link, if you have time please give the attached patch a
>>> try.
>>>
>>> It should fix the issue, but I currently don't have a test system for
>>> VAAPI ready so I can't confirm it of hand.
>>
>>
>> It doesn't fix properly. The vid will play normally after a second,
>> but during that second I get to see many flash frames of gpu mem.
>>
>> Lucky I chose this sample out of several as the patch does seem to
>> fix a couple of other previously failing vids.
>
>
> Though more testing shows it also regresses previously working vids, some as
> above, but one never "starts" and is total junk.
>
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Andy Furniss]

Andy Furniss wrote:

Christian König wrote:

Hi Andy,

Am 19.01.2017 um 11:46 schrieb Andy Furniss:

I think you are right about the slices, the failing vids are blu-ray/tv.

https://drive.google.com/file/d/0BxP5-S1t9VEEZlozcjVUZ1lDbWM/view?usp=sharing




Thanks for the link, if you have time please give the attached patch a
try.

It should fix the issue, but I currently don't have a test system for
VAAPI ready so I can't confirm it of hand.


It doesn't fix properly. The vid will play normally after a second,
but during that second I get to see many flash frames of gpu mem.

Lucky I chose this sample out of several as the patch does seem to
fix a couple of other previously failing vids.


Though more testing shows it also regresses previously working vids, some as
above, but one never "starts" and is total junk.



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

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Andy Furniss]

Christian König wrote:

Hi Andy,

Am 19.01.2017 um 11:46 schrieb Andy Furniss:

I think you are right about the slices, the failing vids are blu-ray/tv.

https://drive.google.com/file/d/0BxP5-S1t9VEEZlozcjVUZ1lDbWM/view?usp=sharing



Thanks for the link, if you have time please give the attached patch a try.

It should fix the issue, but I currently don't have a test system for
VAAPI ready so I can't confirm it of hand.


It doesn't fix properly. The vid will play normally after a second,
but during that second I get to see many flash frames of gpu mem.

Lucky I chose this sample out of several as the patch does seem to
fix a couple of other previously failing vids.

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

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Christian König]

Hi Andy,

Am 19.01.2017 um 11:46 schrieb Andy Furniss:

I think you are right about the slices, the failing vids are blu-ray/tv.

https://drive.google.com/file/d/0BxP5-S1t9VEEZlozcjVUZ1lDbWM/view?usp=sharing 




Thanks for the link, if you have time please give the attached patch a try.

It should fix the issue, but I currently don't have a test system for 
VAAPI ready so I can't confirm it of hand.


Thanks,
Christian.
>From 6076a9a095b435add036a01ea1769e29b7e3cddb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20K=C3=B6nig?= 
Date: Thu, 19 Jan 2017 13:44:34 +0100
Subject: [PATCH] st/va: make sure that we call begin_frame() only once
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This fixes "st/va: delay calling begin_frame until we have all parameters".

Signed-off-by: Christian König 
---
 src/gallium/state_trackers/va/picture.c| 9 ++---
 src/gallium/state_trackers/va/va_private.h | 1 +
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/gallium/state_trackers/va/picture.c b/src/gallium/state_trackers/va/picture.c
index dc7121c..e75006d 100644
--- a/src/gallium/state_trackers/va/picture.c
+++ b/src/gallium/state_trackers/va/picture.c
@@ -81,7 +81,7 @@ vlVaBeginPicture(VADriverContextP ctx, VAContextID context_id, VASurfaceID rende
}
 
if (context->decoder->entrypoint != PIPE_VIDEO_ENTRYPOINT_ENCODE)
-  context->decoder->begin_frame(context->decoder, context->target, >desc.base);
+  context->needs_begin_frame = true;
 
return VA_STATUS_SUCCESS;
 }
@@ -308,8 +308,11 @@ handleVASliceDataBufferType(vlVaContext *context, vlVaBuffer *buf)
sizes[num_buffers] = buf->size;
++num_buffers;
 
-   context->decoder->begin_frame(context->decoder, context->target,
-  >desc.base);
+   if (context->needs_begin_frame) {
+  context->decoder->begin_frame(context->decoder, context->target,
+ >desc.base);
+  context->needs_begin_frame = false;
+   }
context->decoder->decode_bitstream(context->decoder, context->target, >desc.base,
   num_buffers, (const void * const*)buffers, sizes);
 }
diff --git a/src/gallium/state_trackers/va/va_private.h b/src/gallium/state_trackers/va/va_private.h
index 8faec10..0877236 100644
--- a/src/gallium/state_trackers/va/va_private.h
+++ b/src/gallium/state_trackers/va/va_private.h
@@ -261,6 +261,7 @@ typedef struct {
int target_id;
bool first_single_submitted;
int gop_coeff;
+   bool needs_begin_frame;
 } vlVaContext;
 
 typedef struct {
-- 
2.7.4

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

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Andy Furniss]

Christian König wrote:

Am 19.01.2017 um 00:20 schrieb Andy Furniss:

Nayan Deshmukh wrote:

On Tue, Jan 17, 2017 at 9:12 PM, Emil Velikov
 wrote:

On 17 January 2017 at 14:55, Nayan Deshmukh
 wrote:

On Tue, Jan 17, 2017 at 6:25 PM, Christian König
 wrote:

Hi Nayan,

I've pushed this patch yesterday and this one just a minute ago.


Thanks for the push. I am planning on sending a similar patch for
vaapi.


If this (and the vaapi one) does not have too many nasty dependencies
might be worth tagging for stable ?
Cc: mesa-sta...@lists.freedesktop.org


This one should be added to stable, I forgot to add the stable tag. I
will send a mail


Unfortunately it seems this does regress some h264 videos.

I did test several h264 - 576, 720, 1080 and 4K, all were OK but I've
just found
a couple that are regressed by this = decode to partial junk.


Making an educated guess I would say everything which uses multiple
slices would now break.

Damn it, I should have thought earlier about this. Trivial to fix, we
just need to make sure to call begin_frame only once for each frame.

Nayan, do you want to provide a patch or should I take care of this?

Andy can you point us to a broken video?


I think you are right about the slices, the failing vids are blu-ray/tv.

https://drive.google.com/file/d/0BxP5-S1t9VEEZlozcjVUZ1lDbWM/view?usp=sharing

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

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Nayan Deshmukh]

On Thu, Jan 19, 2017 at 2:07 PM, Christian König
 wrote:
> Am 19.01.2017 um 00:20 schrieb Andy Furniss:
>>
>> Nayan Deshmukh wrote:
>>>
>>> On Tue, Jan 17, 2017 at 9:12 PM, Emil Velikov 
>>> wrote:

 On 17 January 2017 at 14:55, Nayan Deshmukh 
 wrote:
>
> On Tue, Jan 17, 2017 at 6:25 PM, Christian König
>  wrote:
>>
>> Hi Nayan,
>>
>> I've pushed this patch yesterday and this one just a minute ago.
>>
> Thanks for the push. I am planning on sending a similar patch for
> vaapi.
>
 If this (and the vaapi one) does not have too many nasty dependencies
 might be worth tagging for stable ?
 Cc: mesa-sta...@lists.freedesktop.org

>>> This one should be added to stable, I forgot to add the stable tag. I
>>> will send a mail
>>
>>
>> Unfortunately it seems this does regress some h264 videos.
>>
>> I did test several h264 - 576, 720, 1080 and 4K, all were OK but I've just
>> found
>> a couple that are regressed by this = decode to partial junk.
>
>
> Making an educated guess I would say everything which uses multiple slices
> would now break.
>
> Damn it, I should have thought earlier about this. Trivial to fix, we just
> need to make sure to call begin_frame only once for each frame.
>
> Nayan, do you want to provide a patch or should I take care of this?
>
I am a little busy, so you can take a jab at it.
> Andy can you point us to a broken video?
>
> Thanks in advance,
> Christian.
>
>>
>> ___
>> 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] st/va: delay calling begin_frame until we have all parameters

[Christian König]

Am 19.01.2017 um 00:20 schrieb Andy Furniss:

Nayan Deshmukh wrote:
On Tue, Jan 17, 2017 at 9:12 PM, Emil Velikov 
 wrote:
On 17 January 2017 at 14:55, Nayan Deshmukh 
 wrote:

On Tue, Jan 17, 2017 at 6:25 PM, Christian König
 wrote:

Hi Nayan,

I've pushed this patch yesterday and this one just a minute ago.

Thanks for the push. I am planning on sending a similar patch for 
vaapi.



If this (and the vaapi one) does not have too many nasty dependencies
might be worth tagging for stable ?
Cc: mesa-sta...@lists.freedesktop.org


This one should be added to stable, I forgot to add the stable tag. I
will send a mail


Unfortunately it seems this does regress some h264 videos.

I did test several h264 - 576, 720, 1080 and 4K, all were OK but I've 
just found

a couple that are regressed by this = decode to partial junk.


Making an educated guess I would say everything which uses multiple 
slices would now break.


Damn it, I should have thought earlier about this. Trivial to fix, we 
just need to make sure to call begin_frame only once for each frame.


Nayan, do you want to provide a patch or should I take care of this?

Andy can you point us to a broken video?

Thanks in advance,
Christian.



___
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] st/va: delay calling begin_frame until we have all parameters

[Andy Furniss]

Nayan Deshmukh wrote:

On Tue, Jan 17, 2017 at 9:12 PM, Emil Velikov  wrote:

On 17 January 2017 at 14:55, Nayan Deshmukh  wrote:

On Tue, Jan 17, 2017 at 6:25 PM, Christian König
 wrote:

Hi Nayan,

I've pushed this patch yesterday and this one just a minute ago.


Thanks for the push. I am planning on sending a similar patch for vaapi.


If this (and the vaapi one) does not have too many nasty dependencies
might be worth tagging for stable ?
Cc: mesa-sta...@lists.freedesktop.org


This one should be added to stable, I forgot to add the stable tag. I
will send a mail


Unfortunately it seems this does regress some h264 videos.

I did test several h264 - 576, 720, 1080 and 4K, all were OK but I've 
just found

a couple that are regressed by this = decode to partial junk.

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

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Nayan Deshmukh]

On Tue, Jan 17, 2017 at 9:12 PM, Emil Velikov  wrote:
> On 17 January 2017 at 14:55, Nayan Deshmukh  wrote:
>> On Tue, Jan 17, 2017 at 6:25 PM, Christian König
>>  wrote:
>>> Hi Nayan,
>>>
>>> I've pushed this patch yesterday and this one just a minute ago.
>>>
>> Thanks for the push. I am planning on sending a similar patch for vaapi.
>>
> If this (and the vaapi one) does not have too many nasty dependencies
> might be worth tagging for stable ?
> Cc: mesa-sta...@lists.freedesktop.org
>
This one should be added to stable, I forgot to add the stable tag. I
will send a mail
to the stable list. The vaapi patch that I was talking about in the
previous mail was
in reference to this patch

st/vdpau: use dri3 to directly send the buffer to X(v2)

this avoids an extra copy which occurs in case of dri2

Implementing this for va state tracker too. This is just an improvement.

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

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Emil Velikov]

On 17 January 2017 at 14:55, Nayan Deshmukh  wrote:
> On Tue, Jan 17, 2017 at 6:25 PM, Christian König
>  wrote:
>> Hi Nayan,
>>
>> I've pushed this patch yesterday and this one just a minute ago.
>>
> Thanks for the push. I am planning on sending a similar patch for vaapi.
>
If this (and the vaapi one) does not have too many nasty dependencies
might be worth tagging for stable ?
Cc: mesa-sta...@lists.freedesktop.org

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

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Nayan Deshmukh]

On Tue, Jan 17, 2017 at 6:25 PM, Christian König
 wrote:
> Hi Nayan,
>
> I've pushed this patch yesterday and this one just a minute ago.
>
Thanks for the push. I am planning on sending a similar patch for vaapi.

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

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Christian König]

Hi Nayan,

I've pushed this patch yesterday and this one just a minute ago.

Christian.

Am 16.01.2017 um 14:19 schrieb Nayan Deshmukh:

Hi Christian,

Please push this patch.

There are a couple of patches [1] which are not yet reviewed. They are
trivial and are tested by Andy. Please have a look at them.

Regards,
Nayan

[1] 
https://lists.freedesktop.org/archives/mesa-dev/2017-January/140395.html


On Fri, Jan 13, 2017 at 11:17 PM, Andy Furniss > wrote:


Nayan Deshmukh wrote:

On Fri, Jan 13, 2017 at 9:54 PM, Andy Furniss
> wrote:


Would be interesting to see if you see the same with this vid
which easily shows the corruption.


https://drive.google.com/drive/folders/0BxP5-S1t9VEEbkR4dWhTUFozV2s?usp=sharing



Looks bad --hwdec-vaapi with or without --vo=vaapi

with --hwdec=vaapi and --vo=vaapi I see the corruption. But
without
--vo=vaapi it uses VAAPI EGL interop and leads to this error
unsupported VA image format unknown


Ok and thanks for looking into the buzilla bug.

I don't know why you get egl interop - I get "normal" opengl and don't
know how force mpv to try egl.




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

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Nayan Deshmukh]

Hi Christian,

Please push this patch.

There are a couple of patches [1] which are not yet reviewed. They are
trivial and are tested by Andy. Please have a look at them.

Regards,
Nayan

[1] https://lists.freedesktop.org/archives/mesa-dev/2017-January/140395.html

On Fri, Jan 13, 2017 at 11:17 PM, Andy Furniss  wrote:

> Nayan Deshmukh wrote:
>
>> On Fri, Jan 13, 2017 at 9:54 PM, Andy Furniss 
>> wrote:
>>
>
> Would be interesting to see if you see the same with this vid
>>> which easily shows the corruption.
>>>
>>> https://drive.google.com/drive/folders/0BxP5-S1t9VEEbkR4dWhT
>>> UFozV2s?usp=sharing
>>>
>>> Looks bad --hwdec-vaapi with or without --vo=vaapi
>>>
>>> with --hwdec=vaapi and --vo=vaapi I see the corruption. But without
>> --vo=vaapi it uses VAAPI EGL interop and leads to this error
>> unsupported VA image format unknown
>>
>
> Ok and thanks for looking into the buzilla bug.
>
> I don't know why you get egl interop - I get "normal" opengl and don't
> know how force mpv to try egl.
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Andy Furniss]

Nayan Deshmukh wrote:

On Fri, Jan 13, 2017 at 9:54 PM, Andy Furniss  wrote:



Would be interesting to see if you see the same with this vid
which easily shows the corruption.

https://drive.google.com/drive/folders/0BxP5-S1t9VEEbkR4dWhTUFozV2s?usp=sharing

Looks bad --hwdec-vaapi with or without --vo=vaapi


with --hwdec=vaapi and --vo=vaapi I see the corruption. But without
--vo=vaapi it uses VAAPI EGL interop and leads to this error
unsupported VA image format unknown


Ok and thanks for looking into the buzilla bug.

I don't know why you get egl interop - I get "normal" opengl and don't
know how force mpv to try egl.

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

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Nayan Deshmukh]

On Fri, Jan 13, 2017 at 9:54 PM, Andy Furniss  wrote:
>
> Nayan Deshmukh wrote:
>>
>> On Fri, Jan 13, 2017 at 8:32 PM, Andy Furniss  wrote:
>>
>>> Nayan Deshmukh wrote:
>>>
 Hi Andy,

 Please test this patch for regressions.

>>>
>>> Do you have a testcase to show the fix?
>>>
>>> TBH I've not tested gstreamer with mpeg2 before as vaapi mpeg2
>>> h/w dec never worked properly anyway.
>>>
>>> https://bugs.freedesktop.org/show_bug.cgi?id=93760
>>>
>>> With mpv --hwdec=vaapi it doesn't seem to regress anything.
>>>
>> I was talking about --hwdec=vaapi. Before this patch I was not able to play
>> any mpeg videos with vaapi as mpv --hwdec=vaapi --vo=vaapi always
>> segfaulted. With this patch I can see videos properly. Just wanted to
>> make sure it did not cause any regression when using hardware decoder.
>
>
> Oh, OK, I can't reproduce that with mpv, but it will still just assert with 
> mesa debug build
>
> mpv: picture_mpeg12.c:84: vlVaHandleSliceParameterBufferMPEG12: Assertion 
> `buf->size >= sizeof(VASliceParameterBufferMPEG2) && buf->num_elements == 1' 
> failed.
>
> Or play with non debug build, but depending on source vid may be
> slightly corrupted.
>
> Would be interesting to see if you see the same with this vid
> which easily shows the corruption.
>
> https://drive.google.com/drive/folders/0BxP5-S1t9VEEbkR4dWhTUFozV2s?usp=sharing
>
> Looks bad --hwdec-vaapi with or without --vo=vaapi
>
with --hwdec=vaapi and --vo=vaapi I see the corruption. But without
--vo=vaapi it uses VAAPI EGL interop and leads to this error
unsupported VA image format unknown

> OK with --hwdec=vdpau --vo=vdpau (just --hwdec=vdpau will be slightly wrong
> currently as there is a vdpau gl interop bug that causes half res)
>
Same for me.
>
>>> More generally - it's really good you are working on vaapi - I don't
>>> know what you've discusses with anyone but did you see the old threads
>>> around VAAPI_DISABLE_INTERLACE?
>>>
>> I haven't discussed it with anyone but I will try reading the old threads
>> and the
>> bug reports.
>
>
> Thanks.
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Andy Furniss]

Nayan Deshmukh wrote:

On Fri, Jan 13, 2017 at 8:32 PM, Andy Furniss  wrote:


Nayan Deshmukh wrote:


Hi Andy,

Please test this patch for regressions.



Do you have a testcase to show the fix?

TBH I've not tested gstreamer with mpeg2 before as vaapi mpeg2
h/w dec never worked properly anyway.

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

With mpv --hwdec=vaapi it doesn't seem to regress anything.


I was talking about --hwdec=vaapi. Before this patch I was not able to play
any mpeg videos with vaapi as mpv --hwdec=vaapi --vo=vaapi always
segfaulted. With this patch I can see videos properly. Just wanted to
make sure it did not cause any regression when using hardware decoder.


Oh, OK, I can't reproduce that with mpv, but it will still just assert 
with mesa debug build


mpv: picture_mpeg12.c:84: vlVaHandleSliceParameterBufferMPEG12: 
Assertion `buf->size >= sizeof(VASliceParameterBufferMPEG2) && 
buf->num_elements == 1' failed.


Or play with non debug build, but depending on source vid may be
slightly corrupted.

Would be interesting to see if you see the same with this vid
which easily shows the corruption.

https://drive.google.com/drive/folders/0BxP5-S1t9VEEbkR4dWhTUFozV2s?usp=sharing

Looks bad --hwdec-vaapi with or without --vo=vaapi

OK with --hwdec=vdpau --vo=vdpau (just --hwdec=vdpau will be slightly wrong
currently as there is a vdpau gl interop bug that causes half res)



More generally - it's really good you are working on vaapi - I don't
know what you've discusses with anyone but did you see the old threads
around VAAPI_DISABLE_INTERLACE?


I haven't discussed it with anyone but I will try reading the old threads
and the
bug reports.


Thanks.

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

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Nayan Deshmukh]

On Fri, Jan 13, 2017 at 8:32 PM, Andy Furniss  wrote:

> Nayan Deshmukh wrote:
>
>> Hi Andy,
>>
>> Please test this patch for regressions.
>>
>
> Do you have a testcase to show the fix?
>
> TBH I've not tested gstreamer with mpeg2 before as vaapi mpeg2
> h/w dec never worked properly anyway.
>
> https://bugs.freedesktop.org/show_bug.cgi?id=93760
>
> With mpv --hwdec=vaapi it doesn't seem to regress anything.
>
> I was talking about --hwdec=vaapi. Before this patch I was not able to play
any mpeg videos with vaapi as mpv --hwdec=vaapi --vo=vaapi always
segfaulted. With this patch I can see videos properly. Just wanted to
make sure it did not cause any regression when using hardware decoder.


> With gstreamer - I can display junk and segfault with or without
> the patch.
>
> This is the first time trying though, and I just don't know whether it's
> just me messing up demuxing mpeg container to feed vaapi or whether
> it's using vaapi sink (normally I only test x264/mkv/mp4/raw
> encode/transcode).
>
> If you have a working gstreamer commandline to demux mpegps decode and
> display
> it would be handy :-)
>
>
> More generally - it's really good you are working on vaapi - I don't
> know what you've discusses with anyone but did you see the old threads
> around VAAPI_DISABLE_INTERLACE?
>
> I haven't discussed it with anyone but I will try reading the old threads
and the
bug reports.

Regards,
Nayan

> I was meaning to bring this up via bug/finding replying to old mails.
> It was only meant to be a short term fix and as time goes on it's
> getting more problematic.
>
> Soon ffmpeg will enable de-interlacer, which depending on env, may lock
> peoples GPUs if they paste some example from the wiki.
>
> mpv --vo=vaapi is also borked = vmfaults probably related to interlaces
> vs progressive buffers (though may be more complicated than that as
> hwdec + vo does seem OK)
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Andy Furniss]

Nayan Deshmukh wrote:

Hi Andy,

Please test this patch for regressions.


Do you have a testcase to show the fix?

TBH I've not tested gstreamer with mpeg2 before as vaapi mpeg2
h/w dec never worked properly anyway.

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

With mpv --hwdec=vaapi it doesn't seem to regress anything.

With gstreamer - I can display junk and segfault with or without
the patch.

This is the first time trying though, and I just don't know whether it's
just me messing up demuxing mpeg container to feed vaapi or whether
it's using vaapi sink (normally I only test x264/mkv/mp4/raw 
encode/transcode).


If you have a working gstreamer commandline to demux mpegps decode and 
display

it would be handy :-)


More generally - it's really good you are working on vaapi - I don't
know what you've discusses with anyone but did you see the old threads
around VAAPI_DISABLE_INTERLACE?

I was meaning to bring this up via bug/finding replying to old mails.
It was only meant to be a short term fix and as time goes on it's
getting more problematic.

Soon ffmpeg will enable de-interlacer, which depending on env, may lock
peoples GPUs if they paste some example from the wiki.

mpv --vo=vaapi is also borked = vmfaults probably related to interlaces
vs progressive buffers (though may be more complicated than that as
hwdec + vo does seem OK)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Christian König]

Am 13.01.2017 um 14:15 schrieb Nayan Deshmukh:

If begin_frame is called before setting intra_matrix and
non_intra_matrix it leads to segmentation faults when
vl_mpeg12_decoder.c is used.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92634
Signed-off-by: Nayan Deshmukh 


At one point I would rather like to fix all the codecs (both decoders 
and encoders) to don't rely on the picture info to be complete, but that 
is clearly a different problem.


So that patch is Reviewed-by: Christian König  
for now.


Regards,
Christian.


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

diff --git a/src/gallium/state_trackers/va/picture.c 
b/src/gallium/state_trackers/va/picture.c
index b5b9a83..dc7121c 100644
--- a/src/gallium/state_trackers/va/picture.c
+++ b/src/gallium/state_trackers/va/picture.c
@@ -178,9 +178,6 @@ handlePictureParameterBuffer(vlVaDriver *drv, vlVaContext 
*context, vlVaBuffer *
  
if (!context->decoder)

   return VA_STATUS_ERROR_ALLOCATION_FAILED;
-
-  context->decoder->begin_frame(context->decoder, context->target,
- >desc.base);
 }
  
 return vaStatus;

@@ -310,6 +307,9 @@ handleVASliceDataBufferType(vlVaContext *context, 
vlVaBuffer *buf)
 buffers[num_buffers] = buf->data;
 sizes[num_buffers] = buf->size;
 ++num_buffers;
+
+   context->decoder->begin_frame(context->decoder, context->target,
+  >desc.base);
 context->decoder->decode_bitstream(context->decoder, context->target, 
>desc.base,
num_buffers, (const void * const*)buffers, sizes);
  }



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

Re: [Mesa-dev] [PATCH] st/va: delay calling begin_frame until we have all parameters

[Nayan Deshmukh]

Hi Andy,

Please test this patch for regressions.

Cheers,
Nayan

On Fri, Jan 13, 2017 at 6:45 PM, Nayan Deshmukh 
wrote:

> If begin_frame is called before setting intra_matrix and
> non_intra_matrix it leads to segmentation faults when
> vl_mpeg12_decoder.c is used.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92634
> Signed-off-by: Nayan Deshmukh 
> ---
>  src/gallium/state_trackers/va/picture.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/picture.c
> b/src/gallium/state_trackers/va/picture.c
> index b5b9a83..dc7121c 100644
> --- a/src/gallium/state_trackers/va/picture.c
> +++ b/src/gallium/state_trackers/va/picture.c
> @@ -178,9 +178,6 @@ handlePictureParameterBuffer(vlVaDriver *drv,
> vlVaContext *context, vlVaBuffer *
>
>if (!context->decoder)
>   return VA_STATUS_ERROR_ALLOCATION_FAILED;
> -
> -  context->decoder->begin_frame(context->decoder, context->target,
> - >desc.base);
> }
>
> return vaStatus;
> @@ -310,6 +307,9 @@ handleVASliceDataBufferType(vlVaContext *context,
> vlVaBuffer *buf)
> buffers[num_buffers] = buf->data;
> sizes[num_buffers] = buf->size;
> ++num_buffers;
> +
> +   context->decoder->begin_frame(context->decoder, context->target,
> +  >desc.base);
> context->decoder->decode_bitstream(context->decoder, context->target,
> >desc.base,
>num_buffers, (const void * const*)buffers, sizes);
>  }
> --
> 2.9.3
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev