[FFmpeg-devel] PixFmtInfo cleanup

[Baptiste Coudurier]

On 2/22/2009 6:06 PM, Michael Niedermayer wrote:
> On Sun, Feb 22, 2009 at 05:58:59PM -0800, Baptiste Coudurier wrote:
>> Hi,
>>
>> On 2/22/2009 5:26 PM, Michael Niedermayer wrote:
>>> [...]
>>>
>>>     uint8_t log2_chroma_w;      ///< chroma_width = -((-luma_width )>>log2_chroma_w)
>>>     uint8_t log2_chroma_h;      ///< chroma_height= -((-luma_height)>>log2_chroma_h)
>> Please don't try to do a contest on getting the most obfuscated API ever
>> designed. I'm all for this new API, and I'm also all for designing it to
>> make it obvious and simple.
>>
>> IMHO this is too much.
> 
> We already have these 2 fields in pixfmtinfo and we do need them
> above is a unambigous definition of their meaning.
> 
> I surely agree with you that this should be written in english first
> before this C code based definition in the comments 
> but its essential that chroma_width/height is calculated that way because
> otherwise your code will be exploitable.
> example YV12
> width=5
> chroma_width= -((-5)>>1) = 3
> if instead you do 5>>1 you get 2 which would allocate a image that is
> too small

I understand, I believe an explanation like this has its place in
documentation of the structure.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
checking for life_signs in -lkenny... no
FFmpeg maintainer                                  http://www.ffmpeg.org