[Mesa-dev] hardware xvmc video decoding with nouveau

[Younes Manton]

On Fri, Jul 29, 2011 at 9:37 AM, Maarten Lankhorst
<m.b.lankhorst at gmail.com> wrote:
> Hi guys,
>
> With some help from the nouveau team I managed to get video acceleration
> working for my nv96 card. The video buffer api works well enough for nouveau,
> I added flags to vl_video_buffer_create_ex so I could force a linear surface
> with a nouveau specific resource flag, which I only specified when hardware
> that potentially supported hardware decoding was found. With the video
> buffer API, I only needed to specify that and I could get it to work.
> This made it easy for me, I only had to write code to talk to the decoder.
>
> The api for implementing the decoder I'm less happy about. I know this is
> because there is no real support yet for other decoders, but I think
> pipe_video_decode_buffer api is wrong right now. It assumes that the
> state tracker knows enough about how the decoder wants to interpret the
> macroblocks.
> The nouveau hardware decoder has to interpret it in it's own way, so that
> makes it need a different api. I think the best thing would be to pass
> information about the macroblock with a pointer to the data blocks,
> and then let the decoder buffer decide how to interpret it. Also is it the
> intention to only start decoding when XvMCPutSurface is called? If the
> reference surfaces are passed, I can start decoding in XvMCRenderSurface.
> I'd also like it if flush_buffer is removed, and instead the video buffers
> are passed to end_frame.
>
> Some of the methods to pipe_video_buffer also appear to be g3dvl specific,
> so could it be split out?
>
> I was thinking of something like this for pipe_video_decode_buffer, with
> flush_buffer in the decoder gone:
>
> struct pipe_video_decode_buffer
> {
>   struct pipe_video_decoder *decoder;
>
>   /* Should not leak even when begin_frame was called */
>   void (*destroy)(struct pipe_video_decode_buffer *decbuf);
>
>   void (*begin_frame)(struct pipe_video_decode_buffer *decbuf);
>
>   /* *ONLY* called on bitstream acceleration, makes no sense to
>    * call for XvMC, this allows it to be set to NULL */
>   void (*set_quant_matrix)(struct pipe_video_decode_buffer *decbuf,
>                            const uint8_t intra_matrix[64],
>                            const uint8_t non_intra_matrix[64]);
>   /* Same story here */
>   void (*decode_bitstream)(struct pipe_video_decode_buffer *decbuf,
>                            unsigned num_bytes, const void *data,
>                            struct pipe_picture_desc *picture,
>                            unsigned num_ycbcr_blocks[3]);
>
>   /* Can be NULL when bitstream acceleration is used.
>    * Append a single macroblock to the list for decoding */
>   void (*decode_macroblock)(struct pipe_video_decode_buffer *decbuf,
>                             struct pipe_video_macroblock *mb, short *datablocks);
>
>   /* If end frame is not set, it means more macroblocks may be
>    * queued after this, and this is just an intermediate render,
>    * if its beneficial to do so. Otherwise just return without
>    * doing anything.
>    */
>   void (*render_frame)(struct pipe_video_decode_buffer *decbuf,
>                        struct pipe_video_buffer *frames[3],
>                        bool end_frame);
> };
>
> Comments are welcome. The functions I removed should probably just be moved
> to a g3dvl specific struct vl_mpeg12_video_decode_buffer.
>
> If you feel like testing xvmc with a capable card, I put my tree at
> http://repo.or.cz/w/mesa/nouveau-pmpeg.git .
>
> I attached 2 patches, 1 is to clean up xvmc/test_rendering.c, the other allows me to
> specify a custom flag to force a linear surface. Should be mergeable right now.
>
> Special thanks to calim and mwk for their patience and help and to jb17bsome for the
> original code which I based this on, even though this code is significantly different
> from the original. :)

2nd patch isn't needed. You shouldn't call vl_video_buffer_create_ex,
you should override the create_buffer hook yourself and do what you
want. I'll push the 1st one later.

As for the changes required to support HW decoding, it was discussed
in [1-3]. I have some patches in the works for that that I'll clean
up, but the short story is that pipe_video_decode_buffer shouldn't
exist in the state tracker.

[1] http://lists.freedesktop.org/archives/mesa-dev/2011-July/009488.html
[2] http://lists.freedesktop.org/archives/mesa-dev/2011-July/009494.html
[3] http://lists.freedesktop.org/archives/mesa-dev/2011-July/009496.html