[VDPAU] [PATCH] Enable Lossless Decode Capabilities via VDPAU API

[Karthikeyan Sreenivasan]

Hello,

On 11/25/2014 10:42 AM, Andy Ritger wrote:
> On Tue, Nov 25, 2014 at 07:33:58PM +0300, Rémi Denis-Courmont wrote:
>>    Hello,
>>
>> Le 2014-11-25 18:21, Jose Soltren a écrit :
>>> 1. Name. Is there a preference for calling the new structure
>>> VdpPictureInfoH264Extended versus VdpPictureInfoH264Hi444PP ?
>>
>> If I recall the previous discussion on the list, the new parameter
>> can be appended to the existing structure.
> 
> Extending the existing VdpPictureInfoH264 structure will perturb its ABI.
> If different software in the system uses the structure in a way that
> relies on its size (e.g., an array of VdpPictureInfoH264s, or a structure
> with a VdpPictureInfoH264 member and other members afterwards), and
> those are passed between software components that were compiled with
> different VdpPictureInfoH264 definitions, bad things will happen.
> 
> For name: I'd recommend a name that describes the usage of the profile.
> E.g., VdpPictureInfoH264Predictive.  If we expect the structure to
> be useful for other profiles, then maybe a little more general; e.g.,
> VdpPictureInfoH264TransformBypass ?  But really, the bikeshed can be
> any color.

The names correspond to the structure as such:

1. VdpPictureInfoH264Extended, Will contain the current transform_bypass
field and a struct_version so as to add more fields in the future if
need arises.

2. VdpPictureInfoH264Predictive, will contain all the fields necessary for
VDP_DECODER_PROFILE_H264_HIGH_444_PREDICTIVE and the structure will not
change and will be used exclusively for Hi444PP [w/ or w/o lossless decode].

3. VdpPictureInfoH264TransformBypass would indicate that the structure
contains support for lossless flag and (assuming w/o version number)
cannot change and can be used by additional profiles as well. Hi444PP
being the last of the profiles this will fall back to (2) w/o the other
necessary fields for Hi444PP.

I see that people are leaning towards (2) (specifically no explicit
version number). Hence VdpPictureInfoH264Predictive would be a better
option assuming there will be no further additions to Hi444PP, but this
will also require adding more fields as mentioned in [5. Scope]

> 
>>> 2. Versioning. Is there a preference to make this a versioned
>>> structure or not? Aaron Plattner expressed a preference for not
>>> making
>>> this versioned; I'll let him explain why here.
>>
>> In practice, the decoder profile defines the type and layout of the
>> picture info structure. A version field does not seem to bring any
>> added value.
> 
> Agreed.
> 
>>> 3. Containment. Should VdpPictureInfoH264Extended contain an entire
>>> VdpPictureInfoH264, or simply a pointer to one?
>>
>> I would prefer the container approach over the pointer approach.
>> Then again, I would prefer extending the existing structure over the
>> container approach.
> 
> I agree a container makes more sense than a pointer: a pointer seems
> like it adds extra complexity for VDPAU clients to manage the memory.
> 
> Thanks,
> - Andy
> 
>>> 4. Flags. Are there any opinions on accepting values greater than 1
>>> for transform_bypass for internal use, or should we write these into
>>> the specification? NVIDIA video decoding hardware contains a mode to
>>> work around the recently-fixed ipred8x8 bug in x264; the NVIDIA
>>> driver
>>> would accept a value of 2 to enable that workaround.
>>
>> I don't know. It depends if there is demand for legacy compatibility
>> with the x264 bug.

It is safe to be legacy compatible.

>>
>>> 5. Scope. If this change really adds
>>> VDP_DECODER_PROFILE_H264_HIGH_444_PREDICTIVE, it will also need to
>>> add
>>> anything else related to Hi444PP, not just lossless decode. Namely it
>>> would need to introduce separate color plane coding, 4:4:4 chroma
>>> subsampling (a misnomer; not subsampling at all), and bit depths
>>> up to
>>> 14 bits per sample. If we choose to make VdpPictureInfoH264Extended
>>> versioned we can add those in a follow-on change. If not, we need to
>>> add those now.
>>
>> 4:4:4 surface chroma sampling is already defined by the API. Only
>> high depths are missing. So I think the patch is self-sufficient if
>> it sticks to 8-bits.
>>
>> It would be nice to mention which surface format are supported for
>> PutBits/GetBits, if any... presumably NV12 and YV12, as YUYV and
>> UYVY would not make sense.
>>
>> -- 
>> Rémi Denis-Courmont
>> _______________________________________________
>> VDPAU mailing list
>> VDPAU at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/vdpau
> _______________________________________________
> VDPAU mailing list
> VDPAU at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/vdpau
> 

-- 
Karthikeyan S
ksreenivasan at nvidia.com
(408)562-7102 Ext: 27102
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------