[Mesa-dev] hardware xvmc video decoding with nouveau

[Maarten Lankhorst]

On 07/30/2011 12:23 AM, Younes Manton wrote:
> 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.
What create_buffer hook do you mean? If you mean
pipe_video_decoder->create_buffer, it creates a pipe_video_decode_buffer,
I need to create a pipe_video_buffer.
> 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.
>
Have to agree on that. :)

~Maarten