[Libva] [RFC] VA API extension proposals to support encode and post-processing

[Bian, Jonathan]

Hi Gwenole, 

Thanks for the feedbacks, and please see my comments (prefixed with >>) below.

Regards,
Jonathan

-----Original Message-----
From: Gwenole Beauchesne [mailto:gb.devel at gmail.com] 
Sent: Wednesday, September 07, 2011 3:12 PM
To: libva at lists.freedesktop.org
Cc: Bian, Jonathan; Xiang, Haihao
Subject: Re: [Libva] [RFC] VA API extension proposals to support encode and post-processing

Hi Jonathan,

2011/8/17 Bian, Jonathan <jonathan.bian at intel.com>:
> 3. Added "Ext" buffer types to extend encode parameters buffers without impacting compatibility with existing encode data structures 4. Added new buffer types for packed header to be sent by the app, and various buffers required for video post-processing 5. Added "Ext" version of sequence, picture and slice parameter buffer structures for H.264 to support High Profile encoding.

Can we think about a Cap mechanism similar to VPP's one?
Or should we add a note/comment that for high-profile encoding, the *Ext structures need to be used?

>> Yes we should add a comment to clarify the use of *Ext structures.

BTW, could we add an entropy_coding_mode_flag to the older API supporting Main profile? It seems the PVR driver can also support CABAC but there is no way to enable it through the API.

>> I don't believe that the existing PVR driver supports CABAC for encoding. Also the set of the tools used by Main profile is largely the same as High profile so it probably makes more sense to have *Ext structure cover both Main and High profiles.

> 6. Added new functions and data structures to support video post-processing (see va_vpp.h).

Can we push the latest vaapi-ext changes? It seems the current fd.o tree does not contain the latest additions and this split to va_vpp.h.

>> Haihao, could you check if the latest version have been pushed to the fd.o repository on the vaapi-ext branch.

Some extra comments:
- s/VA_ENC_INTERLACED_PIC_AFF/VA_ENC_INTERLACED_PAFF/  to align with the MBAFF definition?

>> Yes

- VAEncH264VUIBufferH264 is still present. Either the comment is no longer accurate or we need to drop it. Besides, the bitfields won't compile with an ISO C compliant compiler

>> Will look into this.

- VAEncSequenceParameterBufferH264Ext needs to be documented more:
  - target_usage is a little ambiguous. Is this specific to HW rate control? Or driver-based rate control?
  - rate_control_method needs correct enums, possibly not exposing Gen's values directly. :) I am wondering if we could also add a "custom" one, meaning app-controlled through additional API we will define.

>> Will have these documented/defined more clearly.

I also see that we have VAEnc*BufferExtTypes, why can't we alias Ext sequence, picture & slice parameter buffer types to the original types and let the driver determine the actual type through the size parameter?

>> Personally I would prefer not adding the *Ext* types. Will look at whether we can have these dropped.

I think we can drop the VA_ENUM_MAX definitions since this won't be useful in practise. What are they used for?

>> This is added so that private profiles, entrypoints, attributes, buffer types can be added without colliding with the public ones.

Thanks,
Gwenole.