[Mesa-dev] [PATCH v3 2/4] nvc0: add support for st/va

[Ilia Mirkin]

On Fri, Sep 11, 2015 at 7:54 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Mon, Sep 7, 2015 at 8:15 AM, Julien Isorce <j.isorce at samsung.com> wrote:
>> - split nvc0_decoder_bsp in begin/next/end
>> - preserve content buffer when calling nvc0_decoder_bsp_next
>> - implement pipe_video_codec::begin_frame/end_frame
>>
>> https://bugs.freedesktop.org/show_bug.cgi?id=89969
>>
>> Signed-off-by: Julien Isorce <j.isorce at samsung.com>
>> ---
>>  src/gallium/drivers/nouveau/nouveau_vp3_video.h    |   3 +
>>  .../drivers/nouveau/nouveau_vp3_video_bsp.c        |   2 +
>>  src/gallium/drivers/nouveau/nvc0/nvc0_video.c      |  44 ++++++-
>>  src/gallium/drivers/nouveau/nvc0/nvc0_video.h      |  18 ++-
>>  src/gallium/drivers/nouveau/nvc0/nvc0_video_bsp.c  | 138 ++++++++++++++-------
>>  5 files changed, 150 insertions(+), 55 deletions(-)
>>
>> diff --git a/src/gallium/drivers/nouveau/nouveau_vp3_video.h b/src/gallium/drivers/nouveau/nouveau_vp3_video.h
>> index 70d8501..43b9556 100644
>> --- a/src/gallium/drivers/nouveau/nouveau_vp3_video.h
>> +++ b/src/gallium/drivers/nouveau/nouveau_vp3_video.h
>> @@ -118,6 +118,9 @@ struct nouveau_vp3_decoder {
>>     /* End of the bsp bo where new data should be appended between one begin/end
>>      * frame. */
>>     char *bsp_ptr;
>> +
>> +   /* Total data appended so far after last begin frame. */
>> +   unsigned bsp_size;
>>  };
>>
>>  struct comm {
>> diff --git a/src/gallium/drivers/nouveau/nouveau_vp3_video_bsp.c b/src/gallium/drivers/nouveau/nouveau_vp3_video_bsp.c
>> index b7045c9..c1d70ea 100644
>> --- a/src/gallium/drivers/nouveau/nouveau_vp3_video_bsp.c
>> +++ b/src/gallium/drivers/nouveau/nouveau_vp3_video_bsp.c
>> @@ -238,6 +238,7 @@ nouveau_vp3_bsp_begin(struct nouveau_vp3_decoder *dec)
>>     struct strparm_bsp *str_bsp = NULL;
>>
>>     dec->bsp_ptr = bsp_bo->map;
>> +   dec->bsp_size = NOUVEAU_VP3_BSP_RESERVED_SIZE;
>>
>>     dec->bsp_ptr += 0x100;
>>
>> @@ -329,6 +330,7 @@ nouveau_vp3_bsp_end(struct nouveau_vp3_decoder *dec, union pipe_desc desc)
>>     *(uint32_t *)dec->bsp_ptr = 0x00000000;
>>
>>     dec->bsp_ptr = NULL;
>> +   dec->bsp_size = 0;
>>
>>     return caps;
>>  }
>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_video.c b/src/gallium/drivers/nouveau/nvc0/nvc0_video.c
>> index 48ffac1..7c085e4 100644
>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_video.c
>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_video.c
>> @@ -26,6 +26,24 @@
>>  #include "util/u_format.h"
>>
>>  static void
>> +nvc0_decoder_begin_frame(struct pipe_video_codec *decoder,
>> +                         struct pipe_video_buffer *target,
>> +                         struct pipe_picture_desc *picture)
>> +{
>> +   struct nouveau_vp3_decoder *dec = (struct nouveau_vp3_decoder *)decoder;
>> +   uint32_t comm_seq = ++dec->fence_seq;
>> +   unsigned ret = 0;
>> +
>> +   assert(dec);
>> +   assert(target);
>> +   assert(target->base.buffer_format == PIPE_FORMAT_NV12);
>
> nvc0/nvc0_video.c: In function 'nvc0_decoder_begin_frame':
> nvc0/nvc0_video.c:39:17: error: 'struct pipe_video_buffer' has no
> member named 'base'
>     assert(target->base.buffer_format == PIPE_FORMAT_NV12);
>
> Oops? I'm guessing you didn't build this with --enable-debug -- don't
> worry about fixing this up for now, I'll probably have more comments
> later...

I don't know what you're messing up, but... it's something big. I just
tried playing the simpsons trailer, which is what I used for all my
initial RE of VP2 (pre-nv98 decoding engine) and the video is way
messed up when using vaapi as compared to vdpau. You can find the clip
here: http://www.h264info.com/clips.html (the 720p trailer, all the
way at the bottom). I tested using mpv --hwdec=vaapi as compared to
mpv --hwdec=vdpau (and mplayer -vo vdpau, which is what I normally
use), and the vaapi version was very corrupted.

I don't think it makes sense to merge these patches as-is given the
artifacts they show (in addition to the "usual" artifacts that we get
using h264 via vdpau which happen much more rarely and for
non-understood reasons).

  -ilia